diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5133,6 +5133,8 @@ // so any transaction can fit in the buffer. CBufferedFile blkdat(fileIn, 2 * MAX_TX_SIZE, MAX_TX_SIZE + 8, SER_DISK, CLIENT_VERSION); + // nRewind indicates where to resume scanning in case something goes + // wrong, such as a block fails to deserialize. uint64_t nRewind = blkdat.GetPos(); while (!blkdat.eof()) { if (ShutdownRequested()) { @@ -5163,35 +5165,44 @@ } } catch (const std::exception &) { // No valid block header found; don't complain. + // (this happens at the end of every blk.dat file) break; } try { - // read block - uint64_t nBlockPos = blkdat.GetPos(); + // read block header + const uint64_t nBlockPos{blkdat.GetPos()}; if (dbp) { dbp->nPos = nBlockPos; } blkdat.SetLimit(nBlockPos + nSize); - std::shared_ptr pblock = std::make_shared(); - CBlock &block = *pblock; - blkdat >> block; - nRewind = blkdat.GetPos(); + CBlockHeader header; + blkdat >> header; + const BlockHash hash{header.GetHash()}; + // Skip the rest of this block (this may read from disk + // into memory); position to the marker before the next block, + // but it's still possible to rewind to the start of the + // current block (without a disk read). + nRewind = nBlockPos + nSize; + blkdat.SkipTo(nRewind); + + // needs to remain available after the cs_main lock is released + // to avoid duplicate reads from disk + std::shared_ptr pblock{}; - const BlockHash hash = block.GetHash(); { LOCK(cs_main); // detect out of order blocks, and store them for later if (hash != params.GetConsensus().hashGenesisBlock && - !m_blockman.LookupBlockIndex(block.hashPrevBlock)) { + !m_blockman.LookupBlockIndex(header.hashPrevBlock)) { LogPrint( BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), - block.hashPrevBlock.ToString()); + header.hashPrevBlock.ToString()); if (dbp && blocks_with_unknown_parent) { blocks_with_unknown_parent->emplace( - block.hashPrevBlock, *dbp); + header.hashPrevBlock, *dbp); } continue; } @@ -5200,6 +5211,13 @@ const CBlockIndex *pindex = m_blockman.LookupBlockIndex(hash); if (!pindex || !pindex->nStatus.hasData()) { + // This block can be processed immediately; rewind to + // its start, read and deserialize it. + blkdat.SetPos(nBlockPos); + pblock = std::make_shared(); + blkdat >> *pblock; + nRewind = blkdat.GetPos(); + BlockValidationState state; if (AcceptBlock(pblock, state, true, dbp, nullptr, true)) { diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -6,8 +6,12 @@ - Start a single node and generate 3 blocks. - Stop the node and restart it with -reindex. Verify that the node has reindexed up to block 3. - Stop the node and restart it with -reindex-chainstate. Verify that the node has reindexed up to block 3. +- Verify that out-of-order blocks are correctly processed, see LoadExternalBlockFile() """ +import os + +from test_framework.p2p import DISK_MAGIC_BYTES from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal @@ -29,12 +33,62 @@ assert_equal(self.nodes[0].getblockcount(), blockcount) self.log.info("Success") + # Check that blocks can be processed out of order + def out_of_order(self): + # The previous test created 12 blocks + assert_equal(self.nodes[0].getblockcount(), 12) + self.stop_nodes() + + # In this test environment, blocks will always be in order (since + # we're generating them rather than getting them from peers), so to + # test out-of-order handling, swap blocks 1 and 2 on disk. + blk0 = os.path.join( + self.nodes[0].datadir, self.nodes[0].chain, "blocks", "blk00000.dat" + ) + with open(blk0, "r+b") as bf: + # Read at least the first few blocks (including genesis) + b = bf.read(2000) + + # Find the offsets of blocks 2, 3, and 4 (the first 3 blocks beyond genesis) + # by searching for the regtest marker bytes (see pchMessageStart). + def find_block(b, start): + return b.find(DISK_MAGIC_BYTES["regtest"], start) + 4 + + genesis_start = find_block(b, 0) + assert_equal(genesis_start, 4) + b2_start = find_block(b, genesis_start) + b3_start = find_block(b, b2_start) + b4_start = find_block(b, b3_start) + + # Blocks 2 and 3 should be the same size. + assert_equal(b3_start - b2_start, b4_start - b3_start) + + # Swap the second and third blocks (don't disturb the genesis block). + bf.seek(b2_start) + bf.write(b[b3_start:b4_start]) + bf.write(b[b2_start:b3_start]) + + # The reindexing code should detect and accommodate out of order blocks. + with self.nodes[0].assert_debug_log( + [ + "LoadExternalBlockFile: Out of order block", + "LoadExternalBlockFile: Processing out of order child", + ] + ): + extra_args = [["-reindex"]] + self.start_nodes(extra_args) + + # All blocks should be accepted and processed. + assert_equal(self.nodes[0].getblockcount(), 12) + def run_test(self): self.reindex(False) self.reindex(True) self.reindex(False) self.reindex(True) + self.out_of_order() + if __name__ == "__main__": ReindexTest().main() diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -132,12 +132,18 @@ b"version": msg_version, } -MAGIC_BYTES = { +NET_MAGIC_BYTES = { "mainnet": b"\xe3\xe1\xf3\xe8", "testnet3": b"\xf4\xe5\xf3\xf4", "regtest": b"\xda\xb5\xbf\xfa", } +DISK_MAGIC_BYTES = { + "mainnet": b"\xf9\xbe\xb4\xd9", + "testnet3": b"\x0b\x11\x09\x07", + "regtest": b"\xfa\xbf\xb5\xda", +} + class P2PConnection(asyncio.Protocol): """A low-level connection object to a node's P2P interface. @@ -171,7 +177,7 @@ self.on_connection_send_msg = None self.on_connection_send_msg_is_raw = False self.recvbuf = b"" - self.magic_bytes = MAGIC_BYTES[net] + self.magic_bytes = NET_MAGIC_BYTES[net] def peer_connect(self, dstaddr, dstport, *, net, timeout_factor): self.peer_connect_helper(dstaddr, dstport, net, timeout_factor)