From f5efe82a832a050d1e8f483904913d238dde2e93 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 9 Feb 2017 15:48:39 -0500 Subject: [PATCH] Return correct error codes in fundrawtransaction(). The fundrawtransaction() RPC was returning misleading or incorrect error codes (for example RPC_INTERNAL_ERROR when funding the transaction failed). This commit fixes those error codes: - RPC_INTERNAL_ERROR should not be returned for application-level errors, only for genuine internal errors such as corrupted data. That error code has been replaced with RPC_WALLET_ERROR. This commit also updates the test cases to explicitly test the error code. Github-Pull: #9853 Rebased-From: dab804c18a427901684ebe936b2069a97e04a268 --- qa/rpc-tests/fundrawtransaction.py | 40 +++++------------------------- qa/rpc-tests/rawtransactions.py | 21 +++++++--------- src/wallet/rpcwallet.cpp | 2 +- 3 files changed, 16 insertions(+), 47 deletions(-) diff --git a/qa/rpc-tests/fundrawtransaction.py b/qa/rpc-tests/fundrawtransaction.py index cdea390e224..511faf740eb 100755 --- a/qa/rpc-tests/fundrawtransaction.py +++ b/qa/rpc-tests/fundrawtransaction.py @@ -186,12 +186,7 @@ class RawTransactionsTest(BitcoinTestFramework): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - try: - self.nodes[2].fundrawtransaction(rawtx, {'foo': 'bar'}) - raise AssertionError("Accepted invalid option foo") - except JSONRPCException as e: - assert("Unexpected key foo" in e.error['message']) - + assert_raises_jsonrpc(-3, "Unexpected key foo", self.nodes[2].fundrawtransaction, rawtx, {'foo':'bar'}) ############################################################ # test a fundrawtransaction with an invalid change address # @@ -204,12 +199,7 @@ class RawTransactionsTest(BitcoinTestFramework): dec_tx = self.nodes[2].decoderawtransaction(rawtx) assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) - try: - self.nodes[2].fundrawtransaction(rawtx, {'changeAddress': 'foobar'}) - raise AssertionError("Accepted invalid bitcoin address") - except JSONRPCException as e: - assert("changeAddress must be a valid bitcoin address" in e.error['message']) - + assert_raises_jsonrpc(-5, "changeAddress must be a valid bitcoin address", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':'foobar'}) ############################################################ # test a fundrawtransaction with a provided change address # @@ -223,12 +213,7 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(utx['txid'], dec_tx['vin'][0]['txid']) change = self.nodes[2].getnewaddress() - try: - rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {'changeAddress': change, 'changePosition': 2}) - except JSONRPCException as e: - assert('changePosition out of bounds' == e.error['message']) - else: - assert(False) + assert_raises_jsonrpc(-8, "changePosition out of bounds", self.nodes[2].fundrawtransaction, rawtx, {'changeAddress':change, 'changePosition':2}) rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {'changeAddress': change, 'changePosition': 0}) dec_tx = self.nodes[2].decoderawtransaction(rawtxfund['hex']) out = dec_tx['vout'][0] @@ -337,12 +322,7 @@ class RawTransactionsTest(BitcoinTestFramework): rawtx = self.nodes[2].createrawtransaction(inputs, outputs) dec_tx = self.nodes[2].decoderawtransaction(rawtx) - try: - rawtxfund = self.nodes[2].fundrawtransaction(rawtx) - raise AssertionError("Spent more than available") - except JSONRPCException as e: - assert("Insufficient" in e.error['message']) - + assert_raises_jsonrpc(-4, "Insufficient funds", self.nodes[2].fundrawtransaction, rawtx) ############################################################ #compare fee of a standard pubkeyhash transaction @@ -498,21 +478,13 @@ class RawTransactionsTest(BitcoinTestFramework): rawTx = self.nodes[1].createrawtransaction(inputs, outputs) # fund a transaction that requires a new key for the change output # creating the key must be impossible because the wallet is locked - try: - fundedTx = self.nodes[1].fundrawtransaction(rawTx) - raise AssertionError("Wallet unlocked without passphrase") - except JSONRPCException as e: - assert('Keypool ran out' in e.error['message']) + assert_raises_jsonrpc(-4, "Insufficient funds", self.nodes[1].fundrawtransaction, rawtx) #refill the keypool self.nodes[1].walletpassphrase("test", 100) self.nodes[1].walletlock() - try: - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.2) - raise AssertionError("Wallet unlocked without passphrase") - except JSONRPCException as e: - assert('walletpassphrase' in e.error['message']) + assert_raises_jsonrpc(-13, "walletpassphrase", self.nodes[1].sendtoaddress, self.nodes[0].getnewaddress(), 1.2) oldBalance = self.nodes[0].getbalance() diff --git a/qa/rpc-tests/rawtransactions.py b/qa/rpc-tests/rawtransactions.py index 33a6f2b0cca..2e27bbabb03 100755 --- a/qa/rpc-tests/rawtransactions.py +++ b/qa/rpc-tests/rawtransactions.py @@ -61,13 +61,8 @@ class RawTransactionsTest(BitcoinTestFramework): rawtx = self.nodes[2].createrawtransaction(inputs, outputs) rawtx = self.nodes[2].signrawtransaction(rawtx) - try: - rawtx = self.nodes[2].sendrawtransaction(rawtx['hex']) - except JSONRPCException as e: - assert("Missing inputs" in e.error['message']) - else: - assert(False) - + # This will raise an exception since there are missing inputs + assert_raises_jsonrpc(-25, "Missing inputs", self.nodes[2].sendrawtransaction, rawtx['hex']) ######################### # RAW TX MULTISIG TESTS # @@ -161,13 +156,13 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(self.nodes[0].getrawtransaction(txHash, True)["hex"], rawTxSigned['hex']) # 6. invalid parameters - supply txid and string "Flase" - assert_raises(JSONRPCException, self.nodes[0].getrawtransaction, txHash, "Flase") + assert_raises_jsonrpc(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, "Flase") # 7. invalid parameters - supply txid and empty array - assert_raises(JSONRPCException, self.nodes[0].getrawtransaction, txHash, []) + assert_raises_jsonrpc(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, []) # 8. invalid parameters - supply txid and empty dict - assert_raises(JSONRPCException, self.nodes[0].getrawtransaction, txHash, {}) + assert_raises_jsonrpc(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, {}) inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 1000}] outputs = { self.nodes[0].getnewaddress() : 1 } @@ -175,13 +170,15 @@ class RawTransactionsTest(BitcoinTestFramework): decrawtx= self.nodes[0].decoderawtransaction(rawtx) assert_equal(decrawtx['vin'][0]['sequence'], 1000) + # 9. invalid parameters - sequence number out of range inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : -1}] outputs = { self.nodes[0].getnewaddress() : 1 } - assert_raises(JSONRPCException, self.nodes[0].createrawtransaction, inputs, outputs) + assert_raises_jsonrpc(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs) + # 10. invalid parameters - sequence number out of range inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967296}] outputs = { self.nodes[0].getnewaddress() : 1 } - assert_raises(JSONRPCException, self.nodes[0].createrawtransaction, inputs, outputs) + assert_raises_jsonrpc(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs) inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967294}] outputs = { self.nodes[0].getnewaddress() : 1 } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c141c0ca159..7cc03201650 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2657,7 +2657,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) string strFailReason; if(!pwalletMain->FundTransaction(tx, nFeeOut, overrideEstimatedFeerate, feeRate, changePosition, strFailReason, includeWatching, lockUnspents, setSubtractFeeFromOutputs, reserveChangeKey, changeAddress)) - throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason); + throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); UniValue result(UniValue::VOBJ); result.push_back(Pair("hex", EncodeHexTx(tx)));