diff options
-rw-r--r-- | doc/release-notes.md | 49 | ||||
-rwxr-xr-x | qa/rpc-tests/bumpfee.py | 18 | ||||
-rwxr-xr-x | qa/rpc-tests/fundrawtransaction.py | 40 | ||||
-rwxr-xr-x | qa/rpc-tests/importprunedfunds.py | 14 | ||||
-rwxr-xr-x | qa/rpc-tests/nodehandling.py | 14 | ||||
-rwxr-xr-x | qa/rpc-tests/p2p-acceptblock.py | 30 | ||||
-rwxr-xr-x | qa/rpc-tests/pruning.py | 31 | ||||
-rwxr-xr-x | qa/rpc-tests/rawtransactions.py | 21 | ||||
-rw-r--r-- | src/rpc/blockchain.cpp | 17 | ||||
-rw-r--r-- | src/rpc/net.cpp | 4 | ||||
-rw-r--r-- | src/rpc/protocol.h | 6 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 4 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 26 |
13 files changed, 134 insertions, 140 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index eaa0b330eb..af792118d6 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -33,6 +33,55 @@ frequently tested on them. Notable changes =============== +Low-level RPC changes +--------------------- + +- Error codes have been updated to be more accurate for the following error cases: + - `getblock` now returns RPC_MISC_ERROR if the block can't be found on disk (for + example if the block has been pruned). Previously returned RPC_INTERNAL_ERROR. + - `pruneblockchain` now returns RPC_MISC_ERROR if the blocks cannot be pruned + because the node is not in pruned mode. Previously returned RPC_METHOD_NOT_FOUND. + - `pruneblockchain` now returns RPC_INVALID_PARAMETER if the blocks cannot be pruned + because the supplied timestamp is too late. Previously returned RPC_INTERNAL_ERROR. + - `pruneblockchain` now returns RPC_MISC_ERROR if the blocks cannot be pruned + because the blockchain is too short. Previously returned RPC_INTERNAL_ERROR. + - `setban` now returns RPC_CLIENT_INVALID_IP_OR_SUBNET if the supplied IP address + or subnet is invalid. Previously returned RPC_CLIENT_NODE_ALREADY_ADDED. + - `setban` now returns RPC_CLIENT_INVALID_IP_OR_SUBNET if the user tries to unban + a node that has not previously been banned. Previously returned RPC_MISC_ERROR. + - `removeprunedfunds` now returns RPC_WALLET_ERROR if bitcoind is unable to remove + the transaction. Previously returned RPC_INTERNAL_ERROR. + - `removeprunedfunds` now returns RPC_INVALID_PARAMETER if the transaction does not + exist in the wallet. Previously returned RPC_INTERNAL_ERROR. + - `fundrawtransaction` now returns RPC_INVALID_ADDRESS_OR_KEY if an invalid change + address is provided. Previously returned RPC_INVALID_PARAMETER. + - `fundrawtransaction` now returns RPC_WALLET_ERROR if bitcoind is unable to create + the transaction. The error message provides further details. Previously returned + RPC_INTERNAL_ERROR. + - `bumpfee` now returns RPC_INVALID_PARAMETER if the provided transaction has + descendants in the wallet. Previously returned RPC_MISC_ERROR. + - `bumpfee` now returns RPC_INVALID_PARAMETER if the provided transaction has + descendants in the mempool. Previously returned RPC_MISC_ERROR. + - `bumpfee` now returns RPC_WALLET_ERROR if the provided transaction has + has been mined or conflicts with a mined transaction. Previously returned + RPC_INVALID_ADDRESS_OR_KEY. + - `bumpfee` now returns RPC_WALLET_ERROR if the provided transaction is not + BIP 125 replaceable. Previously returned RPC_INVALID_ADDRESS_OR_KEY. + - `bumpfee` now returns RPC_WALLET_ERROR if the provided transaction has already + been bumped by a different transaction. Previously returned RPC_INVALID_REQUEST. + - `bumpfee` now returns RPC_WALLET_ERROR if the provided transaction contains + inputs which don't belong to this wallet. Previously returned RPC_INVALID_ADDRESS_OR_KEY. + - `bumpfee` now returns RPC_WALLET_ERROR if the provided transaction has multiple change + outputs. Previously returned RPC_MISC_ERROR. + - `bumpfee` now returns RPC_WALLET_ERROR if the provided transaction has no change + output. Previously returned RPC_MISC_ERROR. + - `bumpfee` now returns RPC_WALLET_ERROR if the fee is too high. Previously returned + RPC_MISC_ERROR. + - `bumpfee` now returns RPC_WALLET_ERROR if the fee is too low. Previously returned + RPC_MISC_ERROR. + - `bumpfee` now returns RPC_WALLET_ERROR if the change output is too small to bump the + fee. Previously returned RPC_MISC_ERROR. + Credits ======= diff --git a/qa/rpc-tests/bumpfee.py b/qa/rpc-tests/bumpfee.py index cc897a32c7..68e9808ea5 100755 --- a/qa/rpc-tests/bumpfee.py +++ b/qa/rpc-tests/bumpfee.py @@ -128,7 +128,7 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address): def test_nonrbf_bumpfee_fails(peer_node, dest_address): # cannot replace a non RBF transaction (from node which did not enable RBF) not_rbfid = create_fund_sign_send(peer_node, {dest_address: 0.00090000}) - assert_raises_message(JSONRPCException, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid) + assert_raises_jsonrpc(-4, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid) def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address): @@ -148,7 +148,7 @@ def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address): signedtx = rbf_node.signrawtransaction(rawtx) signedtx = peer_node.signrawtransaction(signedtx["hex"]) rbfid = rbf_node.sendrawtransaction(signedtx["hex"]) - assert_raises_message(JSONRPCException, "Transaction contains inputs that don't belong to this wallet", + assert_raises_jsonrpc(-4, "Transaction contains inputs that don't belong to this wallet", rbf_node.bumpfee, rbfid) @@ -159,7 +159,7 @@ def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address) tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 0.00020000}) tx = rbf_node.signrawtransaction(tx) txid = rbf_node.sendrawtransaction(tx["hex"]) - assert_raises_message(JSONRPCException, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id) + assert_raises_jsonrpc(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id) def test_small_output_fails(rbf_node, dest_address): @@ -174,7 +174,7 @@ def test_small_output_fails(rbf_node, dest_address): Decimal("0.00100000"), {dest_address: 0.00080000, get_change_address(rbf_node): Decimal("0.00010000")}) - assert_raises_message(JSONRPCException, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 20001}) + assert_raises_jsonrpc(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 20001}) def test_dust_to_fee(rbf_node, dest_address): @@ -209,7 +209,7 @@ def test_rebumping(rbf_node, dest_address): rbf_node.settxfee(Decimal("0.00001000")) rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000}) bumped = rbf_node.bumpfee(rbfid, {"totalFee": 1000}) - assert_raises_message(JSONRPCException, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 2000}) + assert_raises_jsonrpc(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 2000}) rbf_node.bumpfee(bumped["txid"], {"totalFee": 2000}) @@ -217,7 +217,7 @@ def test_rebumping_not_replaceable(rbf_node, dest_address): # check that re-bumping a non-replaceable bump tx fails rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000}) bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False}) - assert_raises_message(JSONRPCException, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], + assert_raises_jsonrpc(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"], {"totalFee": 20000}) @@ -268,7 +268,7 @@ def test_bumpfee_metadata(rbf_node, dest_address): def test_locked_wallet_fails(rbf_node, dest_address): rbfid = create_fund_sign_send(rbf_node, {dest_address: 0.00090000}) rbf_node.walletlock() - assert_raises_message(JSONRPCException, "Please enter the wallet passphrase with walletpassphrase first.", + assert_raises_jsonrpc(-13, "Please enter the wallet passphrase with walletpassphrase first.", rbf_node.bumpfee, rbfid) @@ -315,9 +315,7 @@ def submit_block_with_tx(node, tx): block.rehash() block.hashMerkleRoot = block.calc_merkle_root() block.solve() - error = node.submitblock(bytes_to_hex_str(block.serialize(True))) - if error is not None: - raise Exception(error) + node.submitblock(bytes_to_hex_str(block.serialize(True))) return block diff --git a/qa/rpc-tests/fundrawtransaction.py b/qa/rpc-tests/fundrawtransaction.py index 7892e85e22..1f86581297 100755 --- a/qa/rpc-tests/fundrawtransaction.py +++ b/qa/rpc-tests/fundrawtransaction.py @@ -182,12 +182,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 # @@ -200,12 +195,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 # @@ -219,12 +209,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] @@ -333,12 +318,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 @@ -494,21 +474,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/importprunedfunds.py b/qa/rpc-tests/importprunedfunds.py index a941124656..4e529951ed 100755 --- a/qa/rpc-tests/importprunedfunds.py +++ b/qa/rpc-tests/importprunedfunds.py @@ -76,12 +76,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework): self.sync_all() #Import with no affiliated address - try: - self.nodes[1].importprunedfunds(rawtxn1, proof1) - except JSONRPCException as e: - assert('No addresses' in e.error['message']) - else: - assert(False) + assert_raises_jsonrpc(-5, "No addresses", self.nodes[1].importprunedfunds, rawtxn1, proof1) balance1 = self.nodes[1].getbalance("", 0, True) assert_equal(balance1, Decimal(0)) @@ -112,12 +107,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework): assert_equal(address_info['ismine'], True) #Remove transactions - try: - self.nodes[1].removeprunedfunds(txnid1) - except JSONRPCException as e: - assert('does not exist' in e.error['message']) - else: - assert(False) + assert_raises_jsonrpc(-8, "Transaction does not exist in wallet.", self.nodes[1].removeprunedfunds, txnid1) balance1 = self.nodes[1].getbalance("*", 0, True) assert_equal(balance1, Decimal('0.075')) diff --git a/qa/rpc-tests/nodehandling.py b/qa/rpc-tests/nodehandling.py index 89059d2760..a6b10a0d83 100755 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -29,15 +29,13 @@ class NodeHandlingTest (BitcoinTestFramework): assert_equal(len(self.nodes[2].listbanned()), 0) self.nodes[2].setban("127.0.0.0/24", "add") assert_equal(len(self.nodes[2].listbanned()), 1) - try: - self.nodes[2].setban("127.0.0.1", "add") #throws exception because 127.0.0.1 is within range 127.0.0.0/24 - except: - pass + # This will throw an exception because 127.0.0.1 is within range 127.0.0.0/24 + assert_raises_jsonrpc(-23, "IP/Subnet already banned", self.nodes[2].setban, "127.0.0.1", "add") + # This will throw an exception because 127.0.0.1/42 is not a real subnet + assert_raises_jsonrpc(-30, "Error: Invalid IP/Subnet", self.nodes[2].setban, "127.0.0.1/42", "add") assert_equal(len(self.nodes[2].listbanned()), 1) #still only one banned ip because 127.0.0.1 is within the range of 127.0.0.0/24 - try: - self.nodes[2].setban("127.0.0.1", "remove") - except: - pass + # This will throw an exception because 127.0.0.1 was not added above + assert_raises_jsonrpc(-30, "Error: Unban failed", self.nodes[2].setban, "127.0.0.1", "remove") assert_equal(len(self.nodes[2].listbanned()), 1) self.nodes[2].setban("127.0.0.0/24", "remove") assert_equal(len(self.nodes[2].listbanned()), 0) diff --git a/qa/rpc-tests/p2p-acceptblock.py b/qa/rpc-tests/p2p-acceptblock.py index 2f21fa149a..4767dd8fe2 100755 --- a/qa/rpc-tests/p2p-acceptblock.py +++ b/qa/rpc-tests/p2p-acceptblock.py @@ -197,11 +197,8 @@ class AcceptBlockTest(BitcoinTestFramework): assert_equal(x['status'], "headers-only") # But this block should be accepted by node0 since it has more work. - try: - self.nodes[0].getblock(blocks_h3[0].hash) - print("Unrequested more-work block accepted from non-whitelisted peer") - except: - raise AssertionError("Unrequested more work block was not processed") + self.nodes[0].getblock(blocks_h3[0].hash) + print("Unrequested more-work block accepted from non-whitelisted peer") # Node1 should have accepted and reorged. assert_equal(self.nodes[1].getblockcount(), 3) @@ -225,26 +222,17 @@ class AcceptBlockTest(BitcoinTestFramework): tips[j] = next_block time.sleep(2) - for x in all_blocks: - try: - self.nodes[0].getblock(x.hash) - if x == all_blocks[287]: - raise AssertionError("Unrequested block too far-ahead should have been ignored") - except: - if x == all_blocks[287]: - print("Unrequested block too far-ahead not processed") - else: - raise AssertionError("Unrequested block with more work should have been accepted") + # Blocks 1-287 should be accepted, block 288 should be ignored because it's too far ahead + for x in all_blocks[:-1]: + self.nodes[0].getblock(x.hash) + assert_raises_jsonrpc(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[-1].hash) headers_message.headers.pop() # Ensure the last block is unrequested white_node.send_message(headers_message) # Send headers leading to tip white_node.send_message(msg_block(tips[1])) # Now deliver the tip - try: - white_node.sync_with_ping() - self.nodes[1].getblock(tips[1].hash) - print("Unrequested block far ahead of tip accepted from whitelisted peer") - except: - raise AssertionError("Unrequested block from whitelisted peer not accepted") + white_node.sync_with_ping() + self.nodes[1].getblock(tips[1].hash) + print("Unrequested block far ahead of tip accepted from whitelisted peer") # 5. Test handling of unrequested block on the node that didn't process # Should still not be processed (even though it has a child that has more diff --git a/qa/rpc-tests/pruning.py b/qa/rpc-tests/pruning.py index d4924d058c..6d778cfc5e 100755 --- a/qa/rpc-tests/pruning.py +++ b/qa/rpc-tests/pruning.py @@ -184,11 +184,8 @@ class PruneTest(BitcoinTestFramework): def reorg_back(self): # Verify that a block on the old main chain fork has been pruned away - try: - self.nodes[2].getblock(self.forkhash) - raise AssertionError("Old block wasn't pruned so can't test redownload") - except JSONRPCException as e: - print("Will need to redownload block",self.forkheight) + assert_raises_jsonrpc(-1, "Block not available (pruned data)", self.nodes[2].getblock, self.forkhash) + print("Will need to redownload block",self.forkheight) # Verify that we have enough history to reorg back to the fork point # Although this is more than 288 blocks, because this chain was written more recently @@ -233,7 +230,7 @@ class PruneTest(BitcoinTestFramework): # at this point, node has 995 blocks and has not yet run in prune mode node = self.nodes[node_number] = start_node(node_number, self.options.tmpdir, ["-debug=0"], timewait=900) assert_equal(node.getblockcount(), 995) - assert_raises_message(JSONRPCException, "not in prune mode", node.pruneblockchain, 500) + assert_raises_jsonrpc(-1, "not in prune mode", node.pruneblockchain, 500) self.stop_node(node_number) # now re-start in manual pruning mode @@ -265,14 +262,14 @@ class PruneTest(BitcoinTestFramework): return os.path.isfile(self.options.tmpdir + "/node{}/regtest/blocks/blk{:05}.dat".format(node_number, index)) # should not prune because chain tip of node 3 (995) < PruneAfterHeight (1000) - assert_raises_message(JSONRPCException, "Blockchain is too short for pruning", node.pruneblockchain, height(500)) + assert_raises_jsonrpc(-1, "Blockchain is too short for pruning", node.pruneblockchain, height(500)) # mine 6 blocks so we are at height 1001 (i.e., above PruneAfterHeight) node.generate(6) assert_equal(node.getblockchaininfo()["blocks"], 1001) # negative heights should raise an exception - assert_raises_message(JSONRPCException, "Negative", node.pruneblockchain, -10) + assert_raises_jsonrpc(-8, "Negative", node.pruneblockchain, -10) # height=100 too low to prune first block file so this is a no-op prune(100) @@ -318,12 +315,9 @@ class PruneTest(BitcoinTestFramework): def wallet_test(self): # check that the pruning node's wallet is still in good shape print("Stop and start pruning node to trigger wallet rescan") - try: - self.stop_node(2) - start_node(2, self.options.tmpdir, ["-debug=1","-prune=550"]) - print("Success") - except Exception as detail: - raise AssertionError("Wallet test: unable to re-start the pruning node") + self.stop_node(2) + start_node(2, self.options.tmpdir, ["-debug=1","-prune=550"]) + print("Success") # check that wallet loads loads successfully when restarting a pruned node after IBD. # this was reported to fail in #7494. @@ -331,12 +325,9 @@ class PruneTest(BitcoinTestFramework): connect_nodes(self.nodes[0], 5) nds = [self.nodes[0], self.nodes[5]] sync_blocks(nds, wait=5, timeout=300) - try: - self.stop_node(5) #stop and start to trigger rescan - start_node(5, self.options.tmpdir, ["-debug=1","-prune=550"]) - print ("Success") - except Exception as detail: - raise AssertionError("Wallet test: unable to re-start node5") + self.stop_node(5) #stop and start to trigger rescan + start_node(5, self.options.tmpdir, ["-debug=1","-prune=550"]) + print ("Success") def run_test(self): print("Warning! This test requires 4GB of disk space and takes over 30 mins (up to 2 hours)") diff --git a/qa/rpc-tests/rawtransactions.py b/qa/rpc-tests/rawtransactions.py index 5adef31207..0374d8984a 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/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f38b8f88aa..96254a8cb9 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -744,10 +744,15 @@ UniValue getblock(const JSONRPCRequest& request) CBlockIndex* pblockindex = mapBlockIndex[hash]; if (fHavePruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0) - throw JSONRPCError(RPC_INTERNAL_ERROR, "Block not available (pruned data)"); + throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); - if(!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) - throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk"); + if (!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) + // Block not found on disk. This could be because we have the block + // header in our index but don't have the block (for example if a + // non-whitelisted node sends us an unrequested long chain of valid + // blocks, we add the headers to our index, but don't accept the + // block). + throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); if (!fVerbose) { @@ -829,7 +834,7 @@ UniValue pruneblockchain(const JSONRPCRequest& request) + HelpExampleRpc("pruneblockchain", "1000")); if (!fPruneMode) - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Cannot prune blocks because node is not in prune mode."); + throw JSONRPCError(RPC_MISC_ERROR, "Cannot prune blocks because node is not in prune mode."); LOCK(cs_main); @@ -843,7 +848,7 @@ UniValue pruneblockchain(const JSONRPCRequest& request) // Add a 2 hour buffer to include blocks which might have had old timestamps CBlockIndex* pindex = chainActive.FindEarliestAtLeast(heightParam - TIMESTAMP_WINDOW); if (!pindex) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "Could not find block with at least the specified timestamp."); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Could not find block with at least the specified timestamp."); } heightParam = pindex->nHeight; } @@ -851,7 +856,7 @@ UniValue pruneblockchain(const JSONRPCRequest& request) unsigned int height = (unsigned int) heightParam; unsigned int chainHeight = (unsigned int) chainActive.Height(); if (chainHeight < Params().PruneAfterHeight()) - throw JSONRPCError(RPC_INTERNAL_ERROR, "Blockchain is too short for pruning."); + throw JSONRPCError(RPC_MISC_ERROR, "Blockchain is too short for pruning."); else if (height > chainHeight) throw JSONRPCError(RPC_INVALID_PARAMETER, "Blockchain is shorter than the attempted prune height."); else if (height > chainHeight - MIN_BLOCKS_TO_KEEP) { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index b946a64ec6..44c6e6d308 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -504,7 +504,7 @@ UniValue setban(const JSONRPCRequest& request) LookupSubNet(request.params[0].get_str().c_str(), subNet); if (! (isSubnet ? subNet.IsValid() : netAddr.IsValid()) ) - throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: Invalid IP/Subnet"); + throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Invalid IP/Subnet"); if (strCommand == "add") { @@ -524,7 +524,7 @@ UniValue setban(const JSONRPCRequest& request) else if(strCommand == "remove") { if (!( isSubnet ? g_connman->Unban(subNet) : g_connman->Unban(netAddr) )) - throw JSONRPCError(RPC_MISC_ERROR, "Error: Unban failed"); + throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned."); } return NullUniValue; } diff --git a/src/rpc/protocol.h b/src/rpc/protocol.h index 47e56e712b..eafb8d9e67 100644 --- a/src/rpc/protocol.h +++ b/src/rpc/protocol.h @@ -31,9 +31,15 @@ enum HTTPStatusCode enum RPCErrorCode { //! Standard JSON-RPC 2.0 errors + // RPC_INVALID_REQUEST is internally mapped to HTTP_BAD_REQUEST (400). + // It should not be used for application-layer errors. RPC_INVALID_REQUEST = -32600, + // RPC_METHOD_NOT_FOUND is internally mapped to HTTP_NOT_FOUND (404). + // It should not be used for application-layer errors. RPC_METHOD_NOT_FOUND = -32601, RPC_INVALID_PARAMS = -32602, + // RPC_INTERNAL_ERROR should only be used for genuine errors in bitcoind + // (for exampled datadir corruption). RPC_INTERNAL_ERROR = -32603, RPC_PARSE_ERROR = -32700, diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 5006dbc477..9554f0541e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -348,11 +348,11 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) std::vector<uint256> vHashOut; if (pwallet->ZapSelectTx(vHash, vHashOut) != DB_LOAD_OK) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "Could not properly delete the transaction."); + throw JSONRPCError(RPC_WALLET_ERROR, "Could not properly delete the transaction."); } if(vHashOut.empty()) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction does not exist in wallet."); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction does not exist in wallet."); } return NullUniValue; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f38b639bec..7d5cb930a9 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2705,7 +2705,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) CBitcoinAddress address(options["changeAddress"].get_str()); if (!address.IsValid()) - throw JSONRPCError(RPC_INVALID_PARAMETER, "changeAddress must be a valid bitcoin address"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "changeAddress must be a valid bitcoin address"); changeAddress = address.Get(); } @@ -2759,7 +2759,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) std::string strFailReason; if (!pwallet->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); @@ -2860,33 +2860,33 @@ UniValue bumpfee(const JSONRPCRequest& request) CWalletTx& wtx = pwallet->mapWallet[hash]; if (pwallet->HasWalletSpend(hash)) { - throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the wallet"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the wallet"); } { LOCK(mempool.cs); auto it = mempool.mapTx.find(hash); if (it != mempool.mapTx.end() && it->GetCountWithDescendants() > 1) { - throw JSONRPCError(RPC_MISC_ERROR, "Transaction has descendants in the mempool"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction has descendants in the mempool"); } } if (wtx.GetDepthInMainChain() != 0) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction has been mined, or is conflicted with a mined transaction"); + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has been mined, or is conflicted with a mined transaction"); } if (!SignalsOptInRBF(wtx)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction is not BIP 125 replaceable"); + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction is not BIP 125 replaceable"); } if (wtx.mapValue.count("replaced_by_txid")) { - throw JSONRPCError(RPC_INVALID_REQUEST, strprintf("Cannot bump transaction %s which was already bumped by transaction %s", hash.ToString(), wtx.mapValue.at("replaced_by_txid"))); + throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Cannot bump transaction %s which was already bumped by transaction %s", hash.ToString(), wtx.mapValue.at("replaced_by_txid"))); } // check that original tx consists entirely of our inputs // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee) if (!pwallet->IsAllFromMe(wtx, ISMINE_SPENDABLE)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction contains inputs that don't belong to this wallet"); + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction contains inputs that don't belong to this wallet"); } // figure out which output was change @@ -2895,13 +2895,13 @@ UniValue bumpfee(const JSONRPCRequest& request) for (size_t i = 0; i < wtx.tx->vout.size(); ++i) { if (pwallet->IsChange(wtx.tx->vout[i])) { if (nOutput != -1) { - throw JSONRPCError(RPC_MISC_ERROR, "Transaction has multiple change outputs"); + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction has multiple change outputs"); } nOutput = i; } } if (nOutput == -1) { - throw JSONRPCError(RPC_MISC_ERROR, "Transaction does not have a change output"); + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction does not have a change output"); } // Calculate the expected size of the new transaction. @@ -2992,7 +2992,7 @@ UniValue bumpfee(const JSONRPCRequest& request) // Check that in all cases the new fee doesn't violate maxTxFee if (nNewFee > maxTxFee) { - throw JSONRPCError(RPC_MISC_ERROR, + throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)", FormatMoney(nNewFee), FormatMoney(maxTxFee))); } @@ -3004,7 +3004,7 @@ UniValue bumpfee(const JSONRPCRequest& request) // moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment. CFeeRate minMempoolFeeRate = mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) { - throw JSONRPCError(RPC_MISC_ERROR, strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK()))); + throw JSONRPCError(RPC_WALLET_ERROR, strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK()))); } // Now modify the output to increase the fee. @@ -3014,7 +3014,7 @@ UniValue bumpfee(const JSONRPCRequest& request) CMutableTransaction tx(*(wtx.tx)); CTxOut* poutput = &(tx.vout[nOutput]); if (poutput->nValue < nDelta) { - throw JSONRPCError(RPC_MISC_ERROR, "Change output is too small to bump the fee"); + throw JSONRPCError(RPC_WALLET_ERROR, "Change output is too small to bump the fee"); } // If the output would become dust, discard it (converting the dust to fee) |