diff --git a/qa/rpc-tests/fundrawtransaction.py b/qa/rpc-tests/fundrawtransaction.py --- a/qa/rpc-tests/fundrawtransaction.py +++ b/qa/rpc-tests/fundrawtransaction.py @@ -185,11 +185,8 @@ 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 # @@ -202,13 +199,9 @@ 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 # @@ -222,13 +215,8 @@ 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']) @@ -342,11 +330,8 @@ 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 @@ -505,21 +490,15 @@ 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 --- a/qa/rpc-tests/importprunedfunds.py +++ b/qa/rpc-tests/importprunedfunds.py @@ -30,12 +30,12 @@ address1 = self.nodes[0].getnewaddress() # pubkey address2 = self.nodes[0].getnewaddress() - address2_pubkey = self.nodes[0].validateaddress( - address2)['pubkey'] # Using pubkey + # Using pubkey + address2_pubkey = self.nodes[0].validateaddress(address2)['pubkey'] # privkey address3 = self.nodes[0].getnewaddress() - address3_privkey = self.nodes[0].dumpprivkey( - address3) # Using privkey + # Using privkey + address3_privkey = self.nodes[0].dumpprivkey(address3) # Check only one address address_info = self.nodes[0].validateaddress(address1) @@ -78,12 +78,8 @@ 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)) @@ -114,12 +110,8 @@ 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 --- a/qa/rpc-tests/nodehandling.py +++ b/qa/rpc-tests/nodehandling.py @@ -35,19 +35,20 @@ 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") + # still only one banned ip because 127.0.0.1 is within the range of + # 127.0.0.0/24 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 --- a/qa/rpc-tests/p2p-acceptblock.py +++ b/qa/rpc-tests/p2p-acceptblock.py @@ -207,13 +207,8 @@ 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) @@ -238,30 +233,20 @@ 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 --- a/qa/rpc-tests/pruning.py +++ b/qa/rpc-tests/pruning.py @@ -172,10 +172,11 @@ calc_usage(self.prunedir)) def reorg_test(self): - # Node 1 will mine a 300 block chain starting 287 blocks back from Node 0 and Node 2's tip - # This will cause Node 2 to do a reorg requiring 288 blocks of undo data to the reorg_test chain - # Reboot node 1 to clear its mempool (hopefully make the invalidate faster) - # Lower the block max size so we don't keep mining all our big mempool + # Node 1 will mine a 300 block chain starting 287 blocks back from Node + # 0 and Node 2's tip. This will cause Node 2 to do a reorg requiring + # 288 blocks of undo data to the reorg_test chain. Reboot node 1 to + # clear its mempool (hopefully make the invalidate faster). Lower the + # block max size so we don't keep mining all our big mempool # transactions (from disconnected blocks) self.stop_node(1) self.nodes[1] = start_node(1, self.options.tmpdir, ["-debug", @@ -193,7 +194,7 @@ print("Invalidating block at height:", invalidheight, badhash) self.nodes[1].invalidateblock(badhash) - # We've now switched to our previously mined-24 block fork on node 1, but thats not what we want + # We've now switched to our previously mined-24 block fork on node 1, but thats not what we want. # So invalidate that fork as well, until we're on the same chain as # node 0/2 (but at an ancestor 288 blocks ago) mainchainhash = self.nodes[0].getblockhash(invalidheight - 1) @@ -232,8 +233,8 @@ for i in range(22): # This can be slow, so do this in multiple RPC calls to avoid # RPC timeouts. - self.nodes[0].generate( - 10) # node 0 has many large tx's in its mempool from the disconnects + # node 0 has many large tx's in its mempool from the disconnects + self.nodes[0].generate(10) sync_blocks(self.nodes[0:3], timeout=300) usage = calc_usage(self.prunedir) @@ -245,17 +246,14 @@ 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) - - # 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 - # and only its other 299 small and 220 large block are in the block files after it, - # its expected to still be retained + 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 and only its other 299 small and 220 large block are in + # the block files after it, its expected to still be retained. self.nodes[2].getblock(self.nodes[2].getblockhash(self.forkheight)) first_reorg_height = self.nodes[2].getblockcount() @@ -264,15 +262,16 @@ goalbestheight = self.mainchainheight goalbesthash = self.mainchainhash2 - # As of 0.10 the current block download logic is not able to reorg to the original chain created in - # create_chain_with_stale_blocks because it doesn't know of any peer thats on that chain from which to - # redownload its missing blocks. - # Invalidate the reorg_test chain in node 0 as well, it can successfully switch to the original chain - # because it has all the block data. - # However it must mine enough blocks to have a more work chain than the reorg_test chain in order - # to trigger node 2's block download logic. - # At this point node 2 is within 288 blocks of the fork point so it - # will preserve its ability to reorg + # As of 0.10 the current block download logic is not able to reorg to + # the original chain created in create_chain_with_stale_blocks because + # it doesn't know of any peer thats on that chain from which to + # redownload its missing blocks. Invalidate the reorg_test chain in + # node 0 as well, it can successfully switch to the original chain + # because it has all the block data. However it must mine enough blocks + # to have a more work chain than the reorg_test chain in order to + # trigger node 2's block download logic. At this point node 2 is within + # 288 blocks of the fork point so it will preserve its ability to + # reorg. if self.nodes[2].getblockcount() < self.mainchainheight: blocks_to_mine = first_reorg_height + 1 - self.mainchainheight print( @@ -301,8 +300,8 @@ node_number, self.options.tmpdir, ["-debug=0", "-blockmaxsize=1000000"], 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 @@ -337,16 +336,15 @@ # 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) @@ -405,15 +403,9 @@ 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", - "-blockmaxsize=1000000"]) - 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. @@ -421,14 +413,10 @@ 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", - "-blockmaxsize=1000000"]) - print ("Success") - except Exception as detail: - raise AssertionError("Wallet test: unable to re-start node5") + # Stop and start to trigger rescan + self.stop_node(5) + start_node(5, self.options.tmpdir, ["-debug=1", "-prune=550"]) + print ("Success") def run_test(self): print( diff --git a/qa/rpc-tests/rawtransactions.py b/qa/rpc-tests/rawtransactions.py --- a/qa/rpc-tests/rawtransactions.py +++ b/qa/rpc-tests/rawtransactions.py @@ -42,7 +42,6 @@ self.sync_all() def run_test(self): - # prepare some coins for multiple *rawtransaction commands self.nodes[2].generate(1) self.sync_all() @@ -66,12 +65,9 @@ rawtx = self.nodes[2].signrawtransaction( rawtx, None, None, "ALL|FORKID") - 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 # @@ -95,9 +91,9 @@ self.sync_all() self.nodes[0].generate(1) self.sync_all() + # node2 has both keys of the 2of2 ms addr., tx should affect the + # balance assert_equal(self.nodes[2].getbalance(), bal + Decimal('1.20000000')) - # node2 has both keys of the 2of2 ms addr., tx should - # affect the balance # 2of3 test from different nodes bal = self.nodes[2].getbalance() @@ -187,16 +183,16 @@ 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, "False") # 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}] @@ -205,17 +201,21 @@ 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}] diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -830,12 +830,15 @@ 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"); + // 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) { @@ -921,7 +924,7 @@ if (!fPruneMode) { throw JSONRPCError( - RPC_METHOD_NOT_FOUND, + RPC_MISC_ERROR, "Cannot prune blocks because node is not in prune mode."); } @@ -941,7 +944,7 @@ chainActive.FindEarliestAtLeast(heightParam - 7200); if (!pindex) { throw JSONRPCError( - RPC_INTERNAL_ERROR, + RPC_INVALID_PARAMETER, "Could not find block with at least the specified timestamp."); } heightParam = pindex->nHeight; @@ -950,7 +953,7 @@ unsigned int height = (unsigned int)heightParam; unsigned int chainHeight = (unsigned int)chainActive.Height(); if (chainHeight < Params().PruneAfterHeight()) { - throw JSONRPCError(RPC_INTERNAL_ERROR, + throw JSONRPCError(RPC_MISC_ERROR, "Blockchain is too short for pruning."); } else if (height > chainHeight) { throw JSONRPCError( diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -543,9 +543,12 @@ static UniValue setban(const Config &config, const JSONRPCRequest &request) { std::string strCommand; - if (request.params.size() >= 2) strCommand = request.params[1].get_str(); + if (request.params.size() >= 2) { + strCommand = request.params[1].get_str(); + } + if (request.fHelp || request.params.size() < 2 || - (strCommand != "add" && strCommand != "remove")) + (strCommand != "add" && strCommand != "remove")) { throw std::runtime_error( "setban \"subnet\" \"add|remove\" (bantime) (absolute)\n" "\nAttempts add or remove a IP/Subnet from the banned list.\n" @@ -565,50 +568,64 @@ HelpExampleCli("setban", "\"192.168.0.6\" \"add\" 86400") + HelpExampleCli("setban", "\"192.168.0.0/24\" \"add\"") + HelpExampleRpc("setban", "\"192.168.0.6\", \"add\", 86400")); - if (!g_connman) + } + + if (!g_connman) { throw JSONRPCError( RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + } CSubNet subNet; CNetAddr netAddr; bool isSubnet = false; - if (request.params[0].get_str().find("/") != std::string::npos) + if (request.params[0].get_str().find("/") != std::string::npos) { isSubnet = true; + } if (!isSubnet) { CNetAddr resolved; LookupHost(request.params[0].get_str().c_str(), resolved, false); netAddr = resolved; - } else + } else { LookupSubNet(request.params[0].get_str().c_str(), subNet); + } - if (!(isSubnet ? subNet.IsValid() : netAddr.IsValid())) - throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, + if (!(isSubnet ? subNet.IsValid() : netAddr.IsValid())) { + throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Invalid IP/Subnet"); + } if (strCommand == "add") { if (isSubnet ? g_connman->IsBanned(subNet) - : g_connman->IsBanned(netAddr)) + : g_connman->IsBanned(netAddr)) { throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: IP/Subnet already banned"); + } - int64_t banTime = 0; // use standard bantime if not specified - if (request.params.size() >= 3 && !request.params[2].isNull()) + // Use standard bantime if not specified. + int64_t banTime = 0; + if (request.params.size() >= 3 && !request.params[2].isNull()) { banTime = request.params[2].get_int64(); + } bool absolute = false; - if (request.params.size() == 4 && request.params[3].isTrue()) + if (request.params.size() == 4 && request.params[3].isTrue()) { absolute = true; + } isSubnet ? g_connman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute) : g_connman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute); } else if (strCommand == "remove") { - if (!(isSubnet ? g_connman->Unban(subNet) : g_connman->Unban(netAddr))) - throw JSONRPCError(RPC_MISC_ERROR, "Error: Unban failed"); + if (!(isSubnet ? g_connman->Unban(subNet) + : g_connman->Unban(netAddr))) { + 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 --- a/src/rpc/protocol.h +++ b/src/rpc/protocol.h @@ -29,9 +29,15 @@ //! Bitcoin RPC error codes 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 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -339,9 +339,11 @@ UniValue removeprunedfunds(const Config &config, const JSONRPCRequest &request) { - if (!EnsureWalletIsAvailable(request.fHelp)) return NullUniValue; + if (!EnsureWalletIsAvailable(request.fHelp)) { + return NullUniValue; + } - if (request.fHelp || request.params.size() != 1) + if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( "removeprunedfunds \"txid\"\n" "\nDeletes the specified transaction from the wallet. Meant for " @@ -358,6 +360,7 @@ HelpExampleRpc("removprunedfunds", "\"a8d0c0184dde994a09ec054286f1c" "e581bebf46446a512166eae7628734e" "a0a5\"")); + } LOCK2(cs_main, pwalletMain->cs_wallet); @@ -368,12 +371,12 @@ std::vector vHashOut; if (pwalletMain->ZapSelectTx(vHash, vHashOut) != DB_LOAD_OK) { - throw JSONRPCError(RPC_INTERNAL_ERROR, + throw JSONRPCError(RPC_WALLET_ERROR, "Could not properly delete the transaction."); } if (vHashOut.empty()) { - throw JSONRPCError(RPC_INTERNAL_ERROR, + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction does not exist in wallet."); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3251,7 +3251,7 @@ if (!address.IsValid()) { throw JSONRPCError( - RPC_INVALID_PARAMETER, + RPC_INVALID_ADDRESS_OR_KEY, "changeAddress must be a valid bitcoin address"); } @@ -3330,7 +3330,7 @@ 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);