Permalink
Browse files

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: dab804c
  • Loading branch information...
1 parent 4943d7a commit f5efe82a832a050d1e8f483904913d238dde2e93 @jnewbery jnewbery committed with luke-jr Feb 9, 2017
Showing with 16 additions and 47 deletions.
  1. +6 −34 qa/rpc-tests/fundrawtransaction.py
  2. +9 −12 qa/rpc-tests/rawtransactions.py
  3. +1 −1 src/wallet/rpcwallet.cpp
@@ -186,12 +186,7 @@ def run_test(self):
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 @@ def run_test(self):
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 @@ def run_test(self):
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 @@ def run_test(self):
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 @@ def run_test(self):
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()
@@ -61,13 +61,8 @@ def run_test(self):
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,27 +156,29 @@ def run_test(self):
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 }
rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
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 }
View
@@ -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)));

0 comments on commit f5efe82

Please sign in to comment.