diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7ffd03e71af..0dc1dca89c0 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -597,20 +597,32 @@ static RPCHelpMan getblockheader() }; } +void CheckBlockDataAvailability(BlockManager& blockman, const CBlockIndex& blockindex, bool check_for_undo) +{ + AssertLockHeld(cs_main); + uint32_t flag = check_for_undo ? BLOCK_HAVE_UNDO : BLOCK_HAVE_DATA; + if (!(blockindex.nStatus & flag)) { + if (blockman.IsBlockPruned(blockindex)) { + throw JSONRPCError(RPC_MISC_ERROR, strprintf("%s not available (pruned data)", check_for_undo ? "Undo data" : "Block")); + } + if (check_for_undo) { + throw JSONRPCError(RPC_MISC_ERROR, "Undo data not available"); + } + throw JSONRPCError(RPC_MISC_ERROR, "Block not available (not fully downloaded)"); + } +} + static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex& blockindex) { CBlock block; { LOCK(cs_main); - if (blockman.IsBlockPruned(blockindex)) { - throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); - } + CheckBlockDataAvailability(blockman, blockindex, /*check_for_undo=*/false); } if (!blockman.ReadBlockFromDisk(block, blockindex)) { - // Block not found on disk. This could be because we have the block - // header in our index but not yet have the block or did not accept the - // block. Or if the block was pruned right after we released the lock above. + // Block not found on disk. This shouldn't normally happen unless the block was + // pruned right after we released the lock above. throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); } @@ -623,16 +635,13 @@ static std::vector GetRawBlockChecked(BlockManager& blockman, const CBl FlatFilePos pos{}; { LOCK(cs_main); - if (blockman.IsBlockPruned(blockindex)) { - throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); - } + CheckBlockDataAvailability(blockman, blockindex, /*check_for_undo=*/false); pos = blockindex.GetBlockPos(); } if (!blockman.ReadRawBlockFromDisk(data, pos)) { - // Block not found on disk. This could be because we have the block - // header in our index but not yet have the block or did not accept the - // block. Or if the block was pruned right after we released the lock above. + // Block not found on disk. This shouldn't normally happen unless the block was + // pruned right after we released the lock above. throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); } @@ -648,9 +657,7 @@ static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex& bloc { LOCK(cs_main); - if (blockman.IsBlockPruned(blockindex)) { - throw JSONRPCError(RPC_MISC_ERROR, "Undo data not available (pruned data)"); - } + CheckBlockDataAvailability(blockman, blockindex, /*check_for_undo=*/true); } if (!blockman.UndoReadFromDisk(blockUndo, blockindex)) { diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index b5a0382da16..a23f5cf024e 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -60,5 +60,6 @@ UniValue CreateUTXOSnapshot( //! Return height of highest block that has been pruned, or std::nullopt if no blocks have been pruned std::optional GetPruneHeight(const node::BlockManager& blockman, const CChain& chain) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); +void CheckBlockDataAvailability(node::BlockManager& blockman, const CBlockIndex& blockindex, bool check_for_undo) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); #endif // BITCOIN_RPC_BLOCKCHAIN_H diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index c91f254f113..2995ece42fa 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -313,7 +313,7 @@ class AssumeutxoTest(BitcoinTestFramework): self.connect_nodes(snapshot_node.index, miner.index) self.sync_blocks(nodes=(miner, snapshot_node)) # Check the base snapshot block was stored and ensure node signals full-node service support - self.wait_until(lambda: not try_rpc(-1, "Block not found", snapshot_node.getblock, snapshot_block_hash)) + self.wait_until(lambda: not try_rpc(-1, "Block not available (not fully downloaded)", snapshot_node.getblock, snapshot_block_hash)) self.wait_until(lambda: 'NETWORK' in snapshot_node.getnetworkinfo()['localservicesnames']) # Now that the snapshot_node is synced, verify the ibd_node can sync from it @@ -485,7 +485,7 @@ class AssumeutxoTest(BitcoinTestFramework): # find coinbase output at snapshot height on node0 and scan for it on node1, # where the block is not available, but the snapshot was loaded successfully coinbase_tx = n0.getblock(snapshot_hash, verbosity=2)['tx'][0] - assert_raises_rpc_error(-1, "Block not found on disk", n1.getblock, snapshot_hash) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", n1.getblock, snapshot_hash) coinbase_output_descriptor = coinbase_tx['vout'][0]['scriptPubKey']['desc'] scan_result = n1.scantxoutset('start', [coinbase_output_descriptor]) assert_equal(scan_result['success'], True) @@ -557,7 +557,7 @@ class AssumeutxoTest(BitcoinTestFramework): self.log.info("Submit a spending transaction for a snapshot chainstate coin to the mempool") # spend the coinbase output of the first block that is not available on node1 spend_coin_blockhash = n1.getblockhash(START_HEIGHT + 1) - assert_raises_rpc_error(-1, "Block not found on disk", n1.getblock, spend_coin_blockhash) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", n1.getblock, spend_coin_blockhash) prev_tx = n0.getblock(spend_coin_blockhash, 3)['tx'][0] prevout = {"txid": prev_tx['txid'], "vout": 0, "scriptPubKey": prev_tx['vout'][0]['scriptPubKey']['hex']} privkey = n0.get_deterministic_priv_key().key diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index df6e6a2e28a..7788be6adb1 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -102,10 +102,10 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): tip_height = pruned_node.getblockcount() limit_buffer = 2 # Prevent races by waiting for the tip to arrive first - self.wait_until(lambda: not try_rpc(-1, "Block not found", full_node.getblock, pruned_node.getbestblockhash())) + self.wait_until(lambda: not try_rpc(-1, "Block not available (not fully downloaded)", full_node.getblock, pruned_node.getbestblockhash())) for height in range(start_height_full_node + 1, tip_height + 1): if height <= tip_height - (NODE_NETWORK_LIMITED_MIN_BLOCKS - limit_buffer): - assert_raises_rpc_error(-1, "Block not found on disk", full_node.getblock, pruned_node.getblockhash(height)) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", full_node.getblock, pruned_node.getblockhash(height)) else: full_node.getblock(pruned_node.getblockhash(height)) # just assert it does not throw an exception diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py index 835ecbf184f..1430131a97c 100755 --- a/test/functional/p2p_unrequested_blocks.py +++ b/test/functional/p2p_unrequested_blocks.py @@ -119,7 +119,7 @@ class AcceptBlockTest(BitcoinTestFramework): assert_equal(x['status'], "headers-only") tip_entry_found = True assert tip_entry_found - assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, block_h1f.hash) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", self.nodes[0].getblock, block_h1f.hash) # 4. Send another two block that build on the fork. block_h2f = create_block(block_h1f.sha256, create_coinbase(2), block_time) @@ -191,7 +191,7 @@ class AcceptBlockTest(BitcoinTestFramework): # 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_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[-1].hash) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", self.nodes[0].getblock, all_blocks[-1].hash) # 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 @@ -230,7 +230,7 @@ class AcceptBlockTest(BitcoinTestFramework): assert_equal(self.nodes[0].getblockcount(), 290) self.nodes[0].getblock(all_blocks[286].hash) assert_equal(self.nodes[0].getbestblockhash(), all_blocks[286].hash) - assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[287].hash) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", self.nodes[0].getblock, all_blocks[287].hash) self.log.info("Successfully reorged to longer chain") # 8. Create a chain which is invalid at a height longer than the @@ -260,7 +260,7 @@ class AcceptBlockTest(BitcoinTestFramework): assert_equal(x['status'], "headers-only") tip_entry_found = True assert tip_entry_found - assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, block_292.hash) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", self.nodes[0].getblock, block_292.hash) test_node.send_message(msg_block(block_289f)) test_node.send_and_ping(msg_block(block_290f)) diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index e309018516c..62b3d664e0f 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -58,7 +58,7 @@ class GetBlockFromPeerTest(BitcoinTestFramework): self.log.info("Node 0 should only have the header for node 1's block 3") x = next(filter(lambda x: x['hash'] == short_tip, self.nodes[0].getchaintips())) assert_equal(x['status'], "headers-only") - assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, short_tip) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", self.nodes[0].getblock, short_tip) self.log.info("Fetch block from node 1") peers = self.nodes[0].getpeerinfo()