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 @@ -8,6 +8,7 @@ class SignRawTransactionsTest(BitcoinTestFramework): + """Tests transaction signing via RPC command "signrawtransaction".""" def __init__(self): @@ -26,13 +27,16 @@ 1) The transaction has a complete set of signatures 2) No script verification error occurred""" - privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N', 'cVKpPfVKSJxKqVpE9awvXNWuLHCa5j5tiE7K6zbUSptFpTEtiFrA'] + privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N', + 'cVKpPfVKSJxKqVpE9awvXNWuLHCa5j5tiE7K6zbUSptFpTEtiFrA'] 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'}, ] @@ -48,22 +52,26 @@ # 2) No script verification error occurred assert 'errors' not in rawTxSigned - # Check that signrawtransaction doesn't blow up on garbage merge attempts - dummyTxInconsistent = self.nodes[0].createrawtransaction([inputs[0]], outputs) - rawTxUnsigned = self.nodes[0].signrawtransaction(rawTx + dummyTxInconsistent, inputs) + # Check that signrawtransaction doesn't blow up on garbage merge + # attempts + dummyTxInconsistent = self.nodes[ + 0].createrawtransaction([inputs[0]], outputs) + rawTxUnsigned = self.nodes[0].signrawtransaction( + rawTx + dummyTxInconsistent, inputs) assert 'complete' in rawTxUnsigned assert_equal(rawTxUnsigned['complete'], False) - # Check that signrawtransaction properly merges unsigned and signed txn, even with garbage in the middle - rawTxSigned2 = self.nodes[0].signrawtransaction(rawTxUnsigned["hex"] + dummyTxInconsistent + rawTxSigned["hex"], inputs) + # Check that signrawtransaction properly merges unsigned and signed + # txn, even with garbage in the middle + rawTxSigned2 = self.nodes[0].signrawtransaction( + rawTxUnsigned["hex"] + dummyTxInconsistent + rawTxSigned["hex"], inputs) assert 'complete' in rawTxSigned2 assert_equal(rawTxSigned2['complete'], True) assert 'errors' not in rawTxSigned2 - def script_verification_error_test(self): """Creates and signs a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script. @@ -77,19 +85,24 @@ inputs = [ # Valid pay-to-pubkey script - {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0}, + {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', + 'vout': 0, 'amount': 0}, # Invalid script - {'txid': '5b8673686910442c644b1f4993d8f7753c7c8fcb5c87ee40d56eaeef25204547', 'vout': 7}, + {'txid': '5b8673686910442c644b1f4993d8f7753c7c8fcb5c87ee40d56eaeef25204547', + 'vout': 7, 'amount': '1.1'}, # Missing scriptPubKey - {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 1}, + {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', + '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'} ] @@ -104,9 +117,11 @@ assert_equal(decodedRawTx["vin"][i]["vout"], inp["vout"]) # Make sure decoderawtransaction throws if there is extra data - assert_raises(JSONRPCException, self.nodes[0].decoderawtransaction, rawTx + "00") + assert_raises(JSONRPCException, self.nodes[ + 0].decoderawtransaction, rawTx + "00") - rawTxSigned = self.nodes[0].signrawtransaction(rawTx, scripts, privKeys) + rawTxSigned = self.nodes[0].signrawtransaction( + rawTx, scripts, privKeys) # 3) The transaction has no complete set of signatures assert 'complete' in rawTxSigned @@ -123,7 +138,8 @@ assert 'sequence' in rawTxSigned['errors'][0] assert 'error' in rawTxSigned['errors'][0] - # 6) The verification errors refer to the invalid (vin 1) and missing input (vin 2) + # 6) The verification errors refer to the invalid (vin 1) and missing + # input (vin 2) assert_equal(rawTxSigned['errors'][0]['txid'], inputs[1]['txid']) assert_equal(rawTxSigned['errors'][0]['vout'], inputs[1]['vout']) assert_equal(rawTxSigned['errors'][1]['txid'], inputs[2]['txid']) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -753,8 +753,8 @@ "key\n" " \"redeemScript\": \"hex\", (string, required for P2SH " "or P2WSH) redeem script\n" - " \"amount\": \"value\" (string, required) The " - "decimal amount spent\n" + " \"amount\": value (numeric, required) The " + "amount spent\n" " }\n" " ,...\n" " ]\n" @@ -906,7 +906,10 @@ {"txid", UniValueType(UniValue::VSTR)}, {"vout", UniValueType(UniValue::VNUM)}, {"scriptPubKey", UniValueType(UniValue::VSTR)}, - {"amount", 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"); @@ -940,13 +943,21 @@ 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. + throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing amount"); } - if (coins->vout[nOut].nValue <= 0LL) { - // 'amount' param is now required to be >0 in signed tx + if (!MoneyRange(coins->vout[nOut].nValue)) { + // 'amount' param is not a valid money range, so error throw JSONRPCError(RPC_INVALID_PARAMETER, - strprintf("Amount on previous output " - "%d must be >0", - nOut)); + strprintf("amount on prevtx #%d, " + "vout %d out of range", + int(idx), nOut)); } } @@ -960,7 +971,6 @@ {"vout", UniValueType(UniValue::VNUM)}, {"scriptPubKey", UniValueType(UniValue::VSTR)}, {"redeemScript", UniValueType(UniValue::VSTR)}, - {"amount", UniValueType(UniValue::VSTR)}, }); UniValue v = find_value(prevOut, "redeemScript"); if (!v.isNull()) { 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,7 +135,7 @@ "8f50977c8493f3\"," "\"vout\":1,\"scriptPubKey\":" "\"a914b10c9df5f7edf436c697f02f1efdba4cf399615187\"," - "\"amount\":\"3.14159\"," + "\"amount\":3.14159," "\"redeemScript\":" "\"512103debedc17b3df2badbcdd86d5feb4562b86fe182e5998" "abd8bcd4f122c6155b1b21027e940bb73ab8732bfdf7f9216ece"