diff --git a/qa/rpc-tests/signrawtransactions.py b/qa/rpc-tests/signrawtransactions.py --- a/qa/rpc-tests/signrawtransactions.py +++ b/qa/rpc-tests/signrawtransactions.py @@ -32,9 +32,11 @@ inputs = [ # Valid pay-to-pubkey scripts - {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0, + {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', + 'vout': 0, 'amount': 3.14159, 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'}, - {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0, + {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', + 'vout': 0, 'amount': '123.456', 'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'}, ] @@ -84,21 +86,23 @@ inputs = [ # Valid pay-to-pubkey script {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', - 'vout': 0}, + 'vout': 0, 'amount': 0}, # Invalid script {'txid': '5b8673686910442c644b1f4993d8f7753c7c8fcb5c87ee40d56eaeef25204547', - 'vout': 7}, + 'vout': 7, 'amount': '1.1'}, # Missing scriptPubKey {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', - 'vout': 1}, + 'vout': 1, 'amount': 2.0}, ] scripts = [ # Valid pay-to-pubkey script - {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0, + {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', + 'vout': 0, 'amount': 0, 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'}, # Invalid script - {'txid': '5b8673686910442c644b1f4993d8f7753c7c8fcb5c87ee40d56eaeef25204547', 'vout': 7, + {'txid': '5b8673686910442c644b1f4993d8f7753c7c8fcb5c87ee40d56eaeef25204547', + 'vout': 7, 'amount': '1.1', 'scriptPubKey': 'badbadbadbad'} ] diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -906,6 +906,10 @@ {"txid", UniValueType(UniValue::VSTR)}, {"vout", UniValueType(UniValue::VNUM)}, {"scriptPubKey", UniValueType(UniValue::VSTR)}, + // "amount" is also required but check is done + // below due to UniValue::VNUM erroneously + // not accepting quoted numerics + // (which are valid JSON) }); uint256 txid = ParseHashO(prevOut, "txid"); @@ -939,6 +943,17 @@ if (prevOut.exists("amount")) { coins->vout[nOut].nValue = AmountFromValue(find_value(prevOut, "amount")); + } else { + // amount param is required in replay-protected txs. + // Note that we must check for its presence here rather + // than use RPCTypeCheckObj() above, since UniValue::VNUM + // parser incorrectly parses numerics with quotes, eg + // "3.12" as a string when JSON allows it to also parse + // as numeric. And we have to accept numerics with quotes + // because our own dogfood (our rpc results) always + // produces decimal numbers that are quoted + // eg getbalance returns "3.14152" rather than 3.14152 + throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing amount"); } } diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -135,6 +135,7 @@ "8f50977c8493f3\"," "\"vout\":1,\"scriptPubKey\":" "\"a914b10c9df5f7edf436c697f02f1efdba4cf399615187\"," + "\"amount\":3.14159," "\"redeemScript\":" "\"512103debedc17b3df2badbcdd86d5feb4562b86fe182e5998" "abd8bcd4f122c6155b1b21027e940bb73ab8732bfdf7f9216ece" @@ -154,6 +155,45 @@ BOOST_CHECK(find_value(r.get_obj(), "complete").get_bool() == true); } +BOOST_AUTO_TEST_CASE(rpc_rawsign_missing_amount) { + // Old format, missing amount parameter for prevout should generate + // an RPC error. This is because of new replay-protected tx's require + // nonzero amount present in signed tx. + // See: https://github.com/Bitcoin-ABC/bitcoin-abc/issues/63 + // (We will re-use the tx + keys from the above rpc_rawsign test for + // simplicity.) + UniValue r; + std::string prevout = "[{\"txid\":" + "\"b4cc287e58f87cdae59417329f710f3ecd75a4ee1d2872b724" + "8f50977c8493f3\"," + "\"vout\":1,\"scriptPubKey\":" + "\"a914b10c9df5f7edf436c697f02f1efdba4cf399615187\"," + "\"redeemScript\":" + "\"512103debedc17b3df2badbcdd86d5feb4562b86fe182e5998" + "abd8bcd4f122c6155b1b21027e940bb73ab8732bfdf7f9216ece" + "fca5b94d6df834e77e108f68e66f126044c052ae\"}]"; + r = CallRPC(std::string("createrawtransaction ") + prevout + " " + + "{\"3HqAe9LtNBjnsfM4CyYaWTnvCaUYT7v4oZ\":11}"); + std::string notsigned = r.get_str(); + std::string privkey1 = + "\"KzsXybp9jX64P5ekX1KUxRQ79Jht9uzW7LorgwE65i5rWACL6LQe\""; + std::string privkey2 = + "\"Kyhdf5LuKTRx4ge69ybABsiUAWjVRK4XGxAKk2FQLp2HjGMy87Z4\""; + bool exceptionThrownDueToMissingAmount = false, + errorWasMissingAmount = false; + try { + r = CallRPC(std::string("signrawtransaction ") + notsigned + " " + + prevout + " " + "[" + privkey1 + "," + privkey2 + "]"); + } catch (const std::runtime_error &e) { + exceptionThrownDueToMissingAmount = true; + if (std::string(e.what()).find("amount") != std::string::npos) { + errorWasMissingAmount = true; + } + } + BOOST_CHECK(exceptionThrownDueToMissingAmount == true); + BOOST_CHECK(errorWasMissingAmount == true); +} + BOOST_AUTO_TEST_CASE(rpc_createraw_op_return) { BOOST_CHECK_NO_THROW( CallRPC("createrawtransaction "