Fix error codes from various RPCs

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
 =======
 

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,15 +209,15 @@ 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})
 
 
 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
 
 

qa/rpc-tests/fundrawtransaction.py

@@ -182,12 +182,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 #
@@ -200,12 +195,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 #
@@ -219,12 +209,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]
@@ -333,12 +318,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
@@ -494,21 +474,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()
 

qa/rpc-tests/importprunedfunds.py

@@ -76,12 +76,7 @@ def run_test(self):
         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 @@ def run_test(self):
         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'))

qa/rpc-tests/nodehandling.py

@@ -29,15 +29,13 @@ def run_test(self):
         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)

qa/rpc-tests/p2p-acceptblock.py

@@ -197,11 +197,8 @@ def run_test(self):
                 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 @@ def run_test(self):
                 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

qa/rpc-tests/pruning.py

@@ -184,11 +184,8 @@ def reorg_test(self):
 
     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 @@ def manual_test(self, node_number, use_timestamp):
         # 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 @@ def has_block(index):
             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,25 +315,19 @@ def has_block(index):
     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.
         print ("Syncing node 5 to test wallet")
         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)")