Return correct error codes in blockchain.cpp.
RPCs in blockchain.cpp were returning misleading or incorrect error codes (for example getblock() returning RPC_INTERNAL_ERROR when the block had been pruned). 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. - RPC_METHOD_NOT_FOUND should not be returned in response to a JSON request for an existing method. Those error codes have been replaced with RPC_MISC_ERROR or RPC_INVALID_PARAMETER as appropriate.
This commit is contained in:
John Newbery
parent
6d07c62322
commit
c1190963b3
3 changed files with 31 additions and 47 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)")
|
||||
|
|
|
|||
|
|
@ -745,10 +745,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)
|
||||
{
|
||||
|
|
@ -830,7 +835,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);
|
||||
|
||||
|
|
@ -844,7 +849,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;
|
||||
}
|
||||
|
|
@ -852,7 +857,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) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue