From e5b537bbdfbfb43ac7bd8e91958e4a2bbd2577ff Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 16 Jul 2024 14:11:13 -0400 Subject: [PATCH 1/5] rest: improve error when only header of a block is available. This avoids calling ReadRawBlockFromDisk() when the block is expected not to be available because we haven't downloaded it yet and only know the header. --- src/rest.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index c42bc8e40c2..4732922a156 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -309,8 +309,11 @@ static bool rest_block(const std::any& context, if (!pblockindex) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } - if (chainman.m_blockman.IsBlockPruned(*pblockindex)) { - return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not available (pruned data)"); + if (!(pblockindex->nStatus & BLOCK_HAVE_DATA)) { + if (chainman.m_blockman.IsBlockPruned(*pblockindex)) { + return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not available (pruned data)"); + } + return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not available (not fully downloaded)"); } pos = pblockindex->GetBlockPos(); } From 5290cbd58504dcbab97cb0944b3ae0a0f79c4502 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 8 Jul 2024 18:18:36 -0400 Subject: [PATCH 2/5] rpc: Improve getblock / getblockstats error when only header is available. This improves the error message of the getblock and getblockstats rpc and prevents calls to ReadRawBlockFromDisk(), which are unnecessary if we know from the header nStatus field that the block is not available. --- src/rpc/blockchain.cpp | 37 ++++++++++++--------- src/rpc/blockchain.h | 1 + test/functional/feature_assumeutxo.py | 6 ++-- test/functional/p2p_node_network_limited.py | 4 +-- test/functional/p2p_unrequested_blocks.py | 8 ++--- test/functional/rpc_getblockfrompeer.py | 2 +- 6 files changed, 33 insertions(+), 25 deletions(-) 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() From 69fc867ea19ba3bd8c38a18b5e3f0e366c46af5b Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 17 Jul 2024 11:50:56 -0400 Subject: [PATCH 3/5] test: add coverage to getblock and getblockstats also removes an unnecessary newline. Co-authored-by: tdb3 <106488469+tdb3@users.noreply.github.com> --- test/functional/rpc_blockchain.py | 13 +++++++++++++ test/functional/rpc_getblockstats.py | 13 ++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 98147237b10..5a91cab59e1 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -633,6 +633,19 @@ class BlockchainTest(BitcoinTestFramework): assert 'previousblockhash' not in node.getblock(node.getblockhash(0)) assert 'nextblockhash' not in node.getblock(node.getbestblockhash()) + self.log.info("Test getblock when only header is known") + current_height = node.getblock(node.getbestblockhash())['height'] + block_time = node.getblock(node.getbestblockhash())['time'] + 1 + block = create_block(int(blockhash, 16), create_coinbase(current_height + 1, nValue=100), block_time) + block.solve() + node.submitheader(block.serialize().hex()) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", lambda: node.getblock(block.hash)) + + self.log.info("Test getblock when block is missing") + move_block_file('blk00000.dat', 'blk00000.dat.bak') + assert_raises_rpc_error(-1, "Block not found on disk", node.getblock, blockhash) + move_block_file('blk00000.dat.bak', 'blk00000.dat') + if __name__ == '__main__': BlockchainTest(__file__).main() diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py index d1e4895eb6d..002763201a5 100755 --- a/test/functional/rpc_getblockstats.py +++ b/test/functional/rpc_getblockstats.py @@ -114,7 +114,7 @@ class GetblockstatsTest(BitcoinTestFramework): assert_equal(stats[self.max_stat_pos]['height'], self.start_height + self.max_stat_pos) for i in range(self.max_stat_pos+1): - self.log.info('Checking block %d\n' % (i)) + self.log.info('Checking block %d' % (i)) assert_equal(stats[i], self.expected_stats[i]) # Check selecting block by hash too @@ -182,5 +182,16 @@ class GetblockstatsTest(BitcoinTestFramework): assert_equal(tip_stats["utxo_increase_actual"], 4) assert_equal(tip_stats["utxo_size_inc_actual"], 300) + self.log.info("Test when only header is known") + block = self.generateblock(self.nodes[0], output="raw(55)", transactions=[], submit=False) + self.nodes[0].submitheader(block["hex"]) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", lambda: self.nodes[0].getblockstats(block['hash'])) + + self.log.info('Test when block is missing') + (self.nodes[0].blocks_path / 'blk00000.dat').rename(self.nodes[0].blocks_path / 'blk00000.dat.backup') + assert_raises_rpc_error(-1, 'Block not found on disk', self.nodes[0].getblockstats, hash_or_height=1) + (self.nodes[0].blocks_path / 'blk00000.dat.backup').rename(self.nodes[0].blocks_path / 'blk00000.dat') + + if __name__ == '__main__': GetblockstatsTest(__file__).main() From 6cbf2e5f8197e51b8f3d789ba9f5874a2fd7b93a Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 17 Jul 2024 13:23:52 -0400 Subject: [PATCH 4/5] rpc: Improve gettxoutproof error when only header is available. --- src/rpc/txoutproof.cpp | 5 +++++ test/functional/rpc_txoutproof.py | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp index 7958deb6770..40294fda064 100644 --- a/src/rpc/txoutproof.cpp +++ b/src/rpc/txoutproof.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -96,6 +97,10 @@ static RPCHelpMan gettxoutproof() } } + { + LOCK(cs_main); + CheckBlockDataAvailability(chainman.m_blockman, *pblockindex, /*check_for_undo=*/false); + } CBlock block; if (!chainman.m_blockman.ReadBlockFromDisk(block, *pblockindex)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk"); diff --git a/test/functional/rpc_txoutproof.py b/test/functional/rpc_txoutproof.py index 387132b6809..90572245d60 100755 --- a/test/functional/rpc_txoutproof.py +++ b/test/functional/rpc_txoutproof.py @@ -67,6 +67,10 @@ class MerkleBlockTest(BitcoinTestFramework): assert_equal(self.nodes[0].verifytxoutproof(self.nodes[0].gettxoutproof([txid_spent], blockhash)), [txid_spent]) # We can't get the proof if we specify a non-existent block assert_raises_rpc_error(-5, "Block not found", self.nodes[0].gettxoutproof, [txid_spent], "0000000000000000000000000000000000000000000000000000000000000000") + # We can't get the proof if we only have the header of the specified block + block = self.generateblock(self.nodes[0], output="raw(55)", transactions=[], submit=False) + self.nodes[0].submitheader(block["hex"]) + assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", self.nodes[0].gettxoutproof, [txid_spent], block['hash']) # We can get the proof if the transaction is unspent assert_equal(self.nodes[0].verifytxoutproof(self.nodes[0].gettxoutproof([txid_unspent])), [txid_unspent]) # We can get the proof if we provide a list of transactions and one of them is unspent. The ordering of the list should not matter. From 6a1aa510e31e8b77793341473aa5afc9d023a6e3 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 16 Jul 2024 16:30:02 -0400 Subject: [PATCH 5/5] rpc: check block index before reading block / undo data This avoids low-level log errors that are supposed to only occur when there is an actual problem with the block on disk missing unexpectedly, but not in the case where the block and/or undo data are expected not to be there. It changes behavior such that in the first case (block index indicates data is available but retrieving it fails) an error is thrown. It also adjusts a functional tests that tried to simulate not having undo data (but having block data) by deleting the undo file. This situation should occur reality because block and undo data are pruned together. Instead, test this situation with a block that hasn't been connected. --- src/rpc/blockchain.cpp | 6 ++-- src/rpc/rawtransaction.cpp | 9 ++++-- test/functional/rpc_blockchain.py | 48 +++++++++++++++++++------------ 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 0dc1dca89c0..a9cea44779d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -201,8 +201,10 @@ UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIn case TxVerbosity::SHOW_DETAILS_AND_PREVOUT: CBlockUndo blockUndo; const bool is_not_pruned{WITH_LOCK(::cs_main, return !blockman.IsBlockPruned(blockindex))}; - const bool have_undo{is_not_pruned && blockman.UndoReadFromDisk(blockUndo, blockindex)}; - + bool have_undo{is_not_pruned && WITH_LOCK(::cs_main, return blockindex.nStatus & BLOCK_HAVE_UNDO)}; + if (have_undo && !blockman.UndoReadFromDisk(blockUndo, blockindex)) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Undo data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event."); + } for (size_t i = 0; i < block.vtx.size(); ++i) { const CTransactionRef& tx = block.vtx.at(i); // coinbase transaction (i.e. i == 0) doesn't have undo data diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 21bc0e52f13..bb072370ce1 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -405,11 +405,16 @@ static RPCHelpMan getrawtransaction() CBlockUndo blockUndo; CBlock block; - if (tx->IsCoinBase() || !blockindex || WITH_LOCK(::cs_main, return chainman.m_blockman.IsBlockPruned(*blockindex)) || - !(chainman.m_blockman.UndoReadFromDisk(blockUndo, *blockindex) && chainman.m_blockman.ReadBlockFromDisk(block, *blockindex))) { + if (tx->IsCoinBase() || !blockindex || WITH_LOCK(::cs_main, return !(blockindex->nStatus & BLOCK_HAVE_MASK))) { TxToJSON(*tx, hash_block, result, chainman.ActiveChainstate()); return result; } + if (!chainman.m_blockman.UndoReadFromDisk(blockUndo, *blockindex)) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Undo data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event."); + } + if (!chainman.m_blockman.ReadBlockFromDisk(block, *blockindex)) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Block data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event."); + } CTxUndo* undoTX {nullptr}; auto it = std::find_if(block.vtx.begin(), block.vtx.end(), [tx](CTransactionRef t){ return *t == *tx; }); diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 5a91cab59e1..91e9975ad54 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -32,14 +32,16 @@ from test_framework.blocktools import ( TIME_GENESIS_BLOCK, create_block, create_coinbase, + create_tx_with_script, ) from test_framework.messages import ( CBlockHeader, + COIN, from_hex, msg_block, ) from test_framework.p2p import P2PInterface -from test_framework.script import hash256 +from test_framework.script import hash256, OP_TRUE from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -556,12 +558,12 @@ class BlockchainTest(BitcoinTestFramework): block = node.getblock(blockhash, verbosity) assert_equal(blockhash, hash256(bytes.fromhex(block[:160]))[::-1].hex()) - def assert_fee_not_in_block(verbosity): - block = node.getblock(blockhash, verbosity) + def assert_fee_not_in_block(hash, verbosity): + block = node.getblock(hash, verbosity) assert 'fee' not in block['tx'][1] - def assert_fee_in_block(verbosity): - block = node.getblock(blockhash, verbosity) + def assert_fee_in_block(hash, verbosity): + block = node.getblock(hash, verbosity) tx = block['tx'][1] assert 'fee' in tx assert_equal(tx['fee'], tx['vsize'] * fee_per_byte) @@ -580,8 +582,8 @@ class BlockchainTest(BitcoinTestFramework): total_vout += vout["value"] assert_equal(total_vin, total_vout + tx["fee"]) - def assert_vin_does_not_contain_prevout(verbosity): - block = node.getblock(blockhash, verbosity) + def assert_vin_does_not_contain_prevout(hash, verbosity): + block = node.getblock(hash, verbosity) tx = block["tx"][1] if isinstance(tx, str): # In verbosity level 1, only the transaction hashes are written @@ -595,16 +597,16 @@ class BlockchainTest(BitcoinTestFramework): assert_hexblock_hashes(False) self.log.info("Test that getblock with verbosity 1 doesn't include fee") - assert_fee_not_in_block(1) - assert_fee_not_in_block(True) + assert_fee_not_in_block(blockhash, 1) + assert_fee_not_in_block(blockhash, True) self.log.info('Test that getblock with verbosity 2 and 3 includes expected fee') - assert_fee_in_block(2) - assert_fee_in_block(3) + assert_fee_in_block(blockhash, 2) + assert_fee_in_block(blockhash, 3) self.log.info("Test that getblock with verbosity 1 and 2 does not include prevout") - assert_vin_does_not_contain_prevout(1) - assert_vin_does_not_contain_prevout(2) + assert_vin_does_not_contain_prevout(blockhash, 1) + assert_vin_does_not_contain_prevout(blockhash, 2) self.log.info("Test that getblock with verbosity 3 includes prevout") assert_vin_contains_prevout(3) @@ -612,7 +614,7 @@ class BlockchainTest(BitcoinTestFramework): self.log.info("Test getblock with invalid verbosity type returns proper error message") assert_raises_rpc_error(-3, "JSON value of type string is not of expected type number", node.getblock, blockhash, "2") - self.log.info("Test that getblock with verbosity 2 and 3 still works with pruned Undo data") + self.log.info("Test that getblock doesn't work with deleted Undo data") def move_block_file(old, new): old_path = self.nodes[0].blocks_path / old @@ -622,10 +624,8 @@ class BlockchainTest(BitcoinTestFramework): # Move instead of deleting so we can restore chain state afterwards move_block_file('rev00000.dat', 'rev_wrong') - assert_fee_not_in_block(2) - assert_fee_not_in_block(3) - assert_vin_does_not_contain_prevout(2) - assert_vin_does_not_contain_prevout(3) + assert_raises_rpc_error(-32603, "Undo data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event.", lambda: node.getblock(blockhash, 2)) + assert_raises_rpc_error(-32603, "Undo data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event.", lambda: node.getblock(blockhash, 3)) # Restore chain state move_block_file('rev_wrong', 'rev00000.dat') @@ -641,6 +641,18 @@ class BlockchainTest(BitcoinTestFramework): node.submitheader(block.serialize().hex()) assert_raises_rpc_error(-1, "Block not available (not fully downloaded)", lambda: node.getblock(block.hash)) + self.log.info("Test getblock when block data is available but undo data isn't") + # Submits a block building on the header-only block, so it can't be connected and has no undo data + tx = create_tx_with_script(block.vtx[0], 0, script_sig=bytes([OP_TRUE]), amount=50 * COIN) + block_noundo = create_block(block.sha256, create_coinbase(current_height + 2, nValue=100), block_time + 1, txlist=[tx]) + block_noundo.solve() + node.submitblock(block_noundo.serialize().hex()) + + assert_fee_not_in_block(block_noundo.hash, 2) + assert_fee_not_in_block(block_noundo.hash, 3) + assert_vin_does_not_contain_prevout(block_noundo.hash, 2) + assert_vin_does_not_contain_prevout(block_noundo.hash, 3) + self.log.info("Test getblock when block is missing") move_block_file('blk00000.dat', 'blk00000.dat.bak') assert_raises_rpc_error(-1, "Block not found on disk", node.getblock, blockhash)