diff options
author | Martin Zumsande <mzumsande@gmail.com> | 2024-04-26 15:06:55 -0400 |
---|---|---|
committer | Martin Zumsande <mzumsande@gmail.com> | 2024-05-14 14:54:27 -0400 |
commit | d9e477c4dc39d9623ed66c35c06e28f94ae62ad5 (patch) | |
tree | ffdf694d028624320b245aa3745414f9771529b2 /src | |
parent | 064859bbad6984a6ec85c744064abdf757807c58 (diff) | |
download | bitcoin-d9e477c4dc39d9623ed66c35c06e28f94ae62ad5.tar.xz |
validation, blockstorage: Separate code paths for reindex and saving new blocks
By calling SaveBlockToDisk only when we actually want to save a new
block to disk. In the reindex case, we now call UpdateBlockInfo
directly from validation.
This commit doesn't change behavior.
Diffstat (limited to 'src')
-rw-r--r-- | src/bench/readblock.cpp | 2 | ||||
-rw-r--r-- | src/node/blockstorage.cpp | 30 | ||||
-rw-r--r-- | src/node/blockstorage.h | 4 | ||||
-rw-r--r-- | src/test/blockmanager_tests.cpp | 24 | ||||
-rw-r--r-- | src/validation.cpp | 16 |
5 files changed, 34 insertions, 42 deletions
diff --git a/src/bench/readblock.cpp b/src/bench/readblock.cpp index 0545c6b017..2b2bfe069e 100644 --- a/src/bench/readblock.cpp +++ b/src/bench/readblock.cpp @@ -18,7 +18,7 @@ static FlatFilePos WriteBlockToDisk(ChainstateManager& chainman) CBlock block; stream >> TX_WITH_WITNESS(block); - return chainman.m_blockman.SaveBlockToDisk(block, 0, nullptr); + return chainman.m_blockman.SaveBlockToDisk(block, 0); } static void ReadBlockFromDiskTest(benchmark::Bench& bench) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 0fe989d722..9d8c4701ef 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -1147,28 +1147,20 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF return true; } -FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, const FlatFilePos* dbp) +FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight) { unsigned int nBlockSize = ::GetSerializeSize(TX_WITH_WITNESS(block)); FlatFilePos blockPos; - const auto position_known {dbp != nullptr}; - if (position_known) { - blockPos = *dbp; - // position_known is set iff performing a reindex. In this case, no blocks need to be written, only the blockfile info database needs to be rebuilt. - UpdateBlockInfo(block, nHeight, *dbp); - } else { - // when known, blockPos.nPos points at the offset of the block data in the blk file. that already accounts for - // the serialization header present in the file (the 4 magic message start bytes + the 4 length bytes = 8 bytes = BLOCK_SERIALIZATION_HEADER_SIZE). - // we add BLOCK_SERIALIZATION_HEADER_SIZE only for new blocks since they will have the serialization header added when written to disk. - nBlockSize += static_cast<unsigned int>(BLOCK_SERIALIZATION_HEADER_SIZE); - if (!FindBlockPos(blockPos, nBlockSize, nHeight, block.GetBlockTime())) { - LogError("%s: FindBlockPos failed\n", __func__); - return FlatFilePos(); - } - if (!WriteBlockToDisk(block, blockPos)) { - m_opts.notifications.fatalError(_("Failed to write block.")); - return FlatFilePos(); - } + // Account for the 4 magic message start bytes + the 4 length bytes (8 bytes total, + // defined as BLOCK_SERIALIZATION_HEADER_SIZE) + nBlockSize += static_cast<unsigned int>(BLOCK_SERIALIZATION_HEADER_SIZE); + if (!FindBlockPos(blockPos, nBlockSize, nHeight, block.GetBlockTime())) { + LogError("%s: FindBlockPos failed\n", __func__); + return FlatFilePos(); + } + if (!WriteBlockToDisk(block, blockPos)) { + m_opts.notifications.fatalError(_("Failed to write block.")); + return FlatFilePos(); } return blockPos; } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index ff77a66b3b..ae47478779 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -328,8 +328,6 @@ public: EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Store block on disk and update block file statistics. - * If dbp is non-null, it means the block data is already stored, and dbp contains the file position. - * In this case, the block data will not be written, only the block file statistics will be updated. This case should only happen during reindexing. * * @param[in] block the block to be stored * @param[in] nHeight the height of the block @@ -337,7 +335,7 @@ public: * @returns in case of success, the position to which the block was written to * in case of an error, an empty FlatFilePos */ - FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight, const FlatFilePos* dbp); + FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight); /** Update blockfile info while processing a block during reindex. The block must be available on disk. * diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index d7ac0bf823..efe0983698 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -35,20 +35,20 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) }; BlockManager blockman{*Assert(m_node.shutdown), blockman_opts}; // simulate adding a genesis block normally - BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, nullptr).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); // simulate what happens during reindex // simulate a well-formed genesis block being found at offset 8 in the blk00000.dat file // the block is found at offset 8 because there is an 8 byte serialization header // consisting of 4 magic bytes + 4 length bytes before each block in a well-formed blk file. - FlatFilePos pos{0, BLOCK_SERIALIZATION_HEADER_SIZE}; - BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, &pos).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); + const FlatFilePos pos{0, BLOCK_SERIALIZATION_HEADER_SIZE}; + blockman.UpdateBlockInfo(params->GenesisBlock(), 0, pos); // now simulate what happens after reindex for the first new block processed // the actual block contents don't matter, just that it's a block. // verify that the write position is at offset 0x12d. // this is a check to make sure that https://github.com/bitcoin/bitcoin/issues/21379 does not recur // 8 bytes (for serialization header) + 285 (for serialized genesis block) = 293 // add another 8 bytes for the second block's serialization header and we get 293 + 8 = 301 - FlatFilePos actual{blockman.SaveBlockToDisk(params->GenesisBlock(), 1, nullptr)}; + FlatFilePos actual{blockman.SaveBlockToDisk(params->GenesisBlock(), 1)}; BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(TX_WITH_WITNESS(params->GenesisBlock())) + BLOCK_SERIALIZATION_HEADER_SIZE); } @@ -156,12 +156,11 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) // Blockstore is empty BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), 0); - // Write the first block; dbp=nullptr means this block doesn't already have a disk - // location, so allocate a free location and write it there. - FlatFilePos pos1{blockman.SaveBlockToDisk(block1, /*nHeight=*/1, /*dbp=*/nullptr)}; + // Write the first block to a new location. + FlatFilePos pos1{blockman.SaveBlockToDisk(block1, /*nHeight=*/1)}; // Write second block - FlatFilePos pos2{blockman.SaveBlockToDisk(block2, /*nHeight=*/2, /*dbp=*/nullptr)}; + FlatFilePos pos2{blockman.SaveBlockToDisk(block2, /*nHeight=*/2)}; // Two blocks in the file BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); @@ -181,22 +180,19 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) BOOST_CHECK_EQUAL(read_block.nVersion, 2); } - // When FlatFilePos* dbp is given, SaveBlockToDisk() will not write or - // overwrite anything to the flat file block storage. It will, however, - // update the blockfile metadata. This is to facilitate reindexing - // when the user has the blocks on disk but the metadata is being rebuilt. + // During reindex, the flat file block storage will not be written to. + // UpdateBlockInfo will, however, update the blockfile metadata. // Verify this behavior by attempting (and failing) to write block 3 data // to block 2 location. CBlockFileInfo* block_data = blockman.GetBlockFileInfo(0); BOOST_CHECK_EQUAL(block_data->nBlocks, 2); - BOOST_CHECK(blockman.SaveBlockToDisk(block3, /*nHeight=*/3, /*dbp=*/&pos2) == pos2); + blockman.UpdateBlockInfo(block3, /*nHeight=*/3, /*pos=*/pos2); // Metadata is updated... BOOST_CHECK_EQUAL(block_data->nBlocks, 3); // ...but there are still only two blocks in the file BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); // Block 2 was not overwritten: - // SaveBlockToDisk() did not call WriteBlockToDisk() because `FlatFilePos* dbp` was non-null blockman.ReadBlockFromDisk(read_block, pos2); BOOST_CHECK_EQUAL(read_block.nVersion, 2); } diff --git a/src/validation.cpp b/src/validation.cpp index 903f9caf13..2bdffdb1b0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4342,10 +4342,16 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, // Write block to history file if (fNewBlock) *fNewBlock = true; try { - FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, pindex->nHeight, dbp)}; - if (blockPos.IsNull()) { - state.Error(strprintf("%s: Failed to find position to write new block to disk", __func__)); - return false; + FlatFilePos blockPos{}; + if (dbp) { + blockPos = *dbp; + m_blockman.UpdateBlockInfo(block, pindex->nHeight, blockPos); + } else { + blockPos = m_blockman.SaveBlockToDisk(block, pindex->nHeight); + if (blockPos.IsNull()) { + state.Error(strprintf("%s: Failed to find position to write new block to disk", __func__)); + return false; + } } ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { @@ -4845,7 +4851,7 @@ bool Chainstate::LoadGenesisBlock() try { const CBlock& block = params.GenesisBlock(); - FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, 0, nullptr)}; + FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, 0)}; if (blockPos.IsNull()) { LogError("%s: writing genesis block to disk failed\n", __func__); return false; |