From 96b4facc912927305b06a233cb8b36e7e5964c08 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 17 Mar 2024 15:38:04 +0100 Subject: refactor, blockstorage: Generalize GetFirstStoredBlock GetFirstStoredBlock is generalized to check for any data status with a status mask that needs to be passed as a parameter. To reflect this the function is also renamed to GetFirstBlock. Co-authored-by: stickies-v --- src/node/blockstorage.cpp | 8 ++++---- src/node/blockstorage.h | 31 +++++++++++++++++++++++++++---- src/rpc/blockchain.cpp | 25 +++++++++++++++++++++---- src/rpc/blockchain.h | 4 ++++ src/test/blockchain_tests.cpp | 34 ++++++++++++++++++++++++++++++++++ src/test/blockmanager_tests.cpp | 5 +++-- 6 files changed, 93 insertions(+), 14 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 576c07a833..6dbc111eb6 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -595,12 +595,12 @@ bool BlockManager::IsBlockPruned(const CBlockIndex& block) return m_have_pruned && !(block.nStatus & BLOCK_HAVE_DATA) && (block.nTx > 0); } -const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& upper_block, const CBlockIndex* lower_block) +const CBlockIndex* BlockManager::GetFirstBlock(const CBlockIndex& upper_block, uint32_t status_mask, const CBlockIndex* lower_block) const { AssertLockHeld(::cs_main); const CBlockIndex* last_block = &upper_block; - assert(last_block->nStatus & BLOCK_HAVE_DATA); // 'upper_block' must have data - while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) { + assert((last_block->nStatus & status_mask) == status_mask); // 'upper_block' must satisfy the status mask + while (last_block->pprev && ((last_block->pprev->nStatus & status_mask) == status_mask)) { if (lower_block) { // Return if we reached the lower_block if (last_block == lower_block) return lower_block; @@ -617,7 +617,7 @@ const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& upper_bl bool BlockManager::CheckBlockDataAvailability(const CBlockIndex& upper_block, const CBlockIndex& lower_block) { if (!(upper_block.nStatus & BLOCK_HAVE_DATA)) return false; - return GetFirstStoredBlock(upper_block, &lower_block) == &lower_block; + return GetFirstBlock(upper_block, BLOCK_HAVE_DATA, &lower_block) == &lower_block; } // If we're using -prune with -reindex, then delete block files that will be ignored by the diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index ce514cc645..d1e46fde2b 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -335,10 +335,33 @@ public: //! (part of the same chain). bool CheckBlockDataAvailability(const CBlockIndex& upper_block LIFETIMEBOUND, const CBlockIndex& lower_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - //! Find the first stored ancestor of start_block immediately after the last - //! pruned ancestor. Return value will never be null. Caller is responsible - //! for ensuring that start_block has data is not pruned. - const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND, const CBlockIndex* lower_block=nullptr) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** + * @brief Returns the earliest block with specified `status_mask` flags set after + * the latest block _not_ having those flags. + * + * This function starts from `upper_block`, which must have all + * `status_mask` flags set, and iterates backwards through its ancestors. It + * continues as long as each block has all `status_mask` flags set, until + * reaching the oldest ancestor or `lower_block`. + * + * @pre `upper_block` must have all `status_mask` flags set. + * @pre `lower_block` must be null or an ancestor of `upper_block` + * + * @param upper_block The starting block for the search, which must have all + * `status_mask` flags set. + * @param status_mask Bitmask specifying required status flags. + * @param lower_block The earliest possible block to return. If null, the + * search can extend to the genesis block. + * + * @return A non-null pointer to the earliest block between `upper_block` + * and `lower_block`, inclusive, such that every block between the + * returned block and `upper_block` has `status_mask` flags set. + */ + const CBlockIndex* GetFirstBlock( + const CBlockIndex& upper_block LIFETIMEBOUND, + uint32_t status_mask, + const CBlockIndex* lower_block = nullptr + ) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** True if any block files have ever been pruned. */ bool m_have_pruned = false; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index a1135c27d4..822498c6d0 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -782,6 +782,24 @@ static RPCHelpMan getblock() }; } +//! Return height of highest block that has been pruned, or std::nullopt if no blocks have been pruned +std::optional GetPruneHeight(const BlockManager& blockman, const CChain& chain) { + AssertLockHeld(::cs_main); + + const CBlockIndex* chain_tip{chain.Tip()}; + if (!(chain_tip->nStatus & BLOCK_HAVE_DATA)) return chain_tip->nHeight; + + // Get first block with data, after the last block without data. + // This is the start of the unpruned range of blocks. + const auto& first_unpruned{*Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_DATA))}; + if (!first_unpruned.pprev) { + // No block before the first unpruned block means nothing is pruned. + return std::nullopt; + } + // Block before the first unpruned block is the last pruned block. + return Assert(first_unpruned.pprev)->nHeight; +} + static RPCHelpMan pruneblockchain() { return RPCHelpMan{"pruneblockchain", "", @@ -834,8 +852,7 @@ static RPCHelpMan pruneblockchain() } PruneBlockFilesManual(active_chainstate, height); - const CBlockIndex& block{*CHECK_NONFATAL(active_chain.Tip())}; - return block.nStatus & BLOCK_HAVE_DATA ? active_chainstate.m_blockman.GetFirstStoredBlock(block)->nHeight - 1 : block.nHeight; + return GetPruneHeight(chainman.m_blockman, active_chain).value_or(-1); }, }; } @@ -1288,8 +1305,8 @@ RPCHelpMan getblockchaininfo() obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage()); obj.pushKV("pruned", chainman.m_blockman.IsPruneMode()); if (chainman.m_blockman.IsPruneMode()) { - bool has_tip_data = tip.nStatus & BLOCK_HAVE_DATA; - obj.pushKV("pruneheight", has_tip_data ? chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight : tip.nHeight + 1); + const auto prune_height{GetPruneHeight(chainman.m_blockman, active_chainstate.m_chain)}; + obj.pushKV("pruneheight", prune_height ? prune_height.value() + 1 : 0); const bool automatic_pruning{chainman.m_blockman.GetPruneTarget() != BlockManager::PRUNE_TARGET_MANUAL}; obj.pushKV("automatic_pruning", automatic_pruning); diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index c2021c3608..f6a7fe236c 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -21,6 +21,7 @@ class CBlockIndex; class Chainstate; class UniValue; namespace node { +class BlockManager; struct NodeContext; } // namespace node @@ -57,4 +58,7 @@ UniValue CreateUTXOSnapshot( const fs::path& path, const fs::path& tmppath); +//! 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); + #endif // BITCOIN_RPC_BLOCKCHAIN_H diff --git a/src/test/blockchain_tests.cpp b/src/test/blockchain_tests.cpp index be515a9eac..2a63d09795 100644 --- a/src/test/blockchain_tests.cpp +++ b/src/test/blockchain_tests.cpp @@ -5,7 +5,9 @@ #include #include +#include #include +#include #include #include @@ -74,4 +76,36 @@ BOOST_AUTO_TEST_CASE(get_difficulty_for_very_high_target) TestDifficulty(0x12345678, 5913134931067755359633408.0); } +//! Prune chain from height down to genesis block and check that +//! GetPruneHeight returns the correct value +static void CheckGetPruneHeight(node::BlockManager& blockman, CChain& chain, int height) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +{ + AssertLockHeld(::cs_main); + + // Emulate pruning all blocks from `height` down to the genesis block + // by unsetting the `BLOCK_HAVE_DATA` flag from `nStatus` + for (CBlockIndex* it{chain[height]}; it != nullptr && it->nHeight > 0; it = it->pprev) { + it->nStatus &= ~BLOCK_HAVE_DATA; + } + + const auto prune_height{GetPruneHeight(blockman, chain)}; + BOOST_REQUIRE(prune_height.has_value()); + BOOST_CHECK_EQUAL(*prune_height, height); +} + +BOOST_FIXTURE_TEST_CASE(get_prune_height, TestChain100Setup) +{ + LOCK(::cs_main); + auto& chain = m_node.chainman->ActiveChain(); + auto& blockman = m_node.chainman->m_blockman; + + // Fresh chain of 100 blocks without any pruned blocks, so std::nullopt should be returned + BOOST_CHECK(!GetPruneHeight(blockman, chain).has_value()); + + // Start pruning + CheckGetPruneHeight(blockman, chain, 1); + CheckGetPruneHeight(blockman, chain, 99); + CheckGetPruneHeight(blockman, chain, 100); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index d7ac0bf823..12f3257700 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -113,7 +114,7 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) }; // 1) Return genesis block when all blocks are available - BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), chainman->ActiveChain()[0]); + BOOST_CHECK_EQUAL(blockman.GetFirstBlock(tip, BLOCK_HAVE_DATA), chainman->ActiveChain()[0]); BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *chainman->ActiveChain()[0])); // 2) Check lower_block when all blocks are available @@ -127,7 +128,7 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) func_prune_blocks(last_pruned_block); // 3) The last block not pruned is in-between upper-block and the genesis block - BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), first_available_block); + BOOST_CHECK_EQUAL(blockman.GetFirstBlock(tip, BLOCK_HAVE_DATA), first_available_block); BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *first_available_block)); BOOST_CHECK(!blockman.CheckBlockDataAvailability(tip, *last_pruned_block)); } -- cgit v1.2.3 From 4a1975008b602aeacdad9a74d1837a7455148074 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 17 Mar 2024 15:57:38 +0100 Subject: rpc: Make pruneheight also reflect undo data presence --- src/rpc/blockchain.cpp | 22 +++++++++++++++------- test/functional/feature_pruning.py | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 822498c6d0..fd1d1e4e87 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -786,16 +786,24 @@ static RPCHelpMan getblock() std::optional GetPruneHeight(const BlockManager& blockman, const CChain& chain) { AssertLockHeld(::cs_main); + // Search for the last block missing block data or undo data. Don't let the + // search consider the genesis block, because the genesis block does not + // have undo data, but should not be considered pruned. + const CBlockIndex* first_block{chain[1]}; const CBlockIndex* chain_tip{chain.Tip()}; - if (!(chain_tip->nStatus & BLOCK_HAVE_DATA)) return chain_tip->nHeight; - // Get first block with data, after the last block without data. - // This is the start of the unpruned range of blocks. - const auto& first_unpruned{*Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_DATA))}; - if (!first_unpruned.pprev) { - // No block before the first unpruned block means nothing is pruned. - return std::nullopt; + // If there are no blocks after the genesis block, or no blocks at all, nothing is pruned. + if (!first_block || !chain_tip) return std::nullopt; + + // If the chain tip is pruned, everything is pruned. + if (!((chain_tip->nStatus & BLOCK_HAVE_MASK) == BLOCK_HAVE_MASK)) return chain_tip->nHeight; + + const auto& first_unpruned{*Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))}; + if (&first_unpruned == first_block) { + // All blocks between first_block and chain_tip have data, so nothing is pruned. + return std::nullopt; } + // Block before the first unpruned block is the last pruned block. return Assert(first_unpruned.pprev)->nHeight; } diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index 4b548ef0f3..5f99b8dee8 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -25,6 +25,7 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_raises_rpc_error, + try_rpc, ) # Rescans start at the earliest block up to 2 hours before a key timestamp, so @@ -479,8 +480,12 @@ class PruneTest(BitcoinTestFramework): self.log.info("Test invalid pruning command line options") self.test_invalid_command_line_options() + self.log.info("Test scanblocks can not return pruned data") self.test_scanblocks_pruned() + self.log.info("Test pruneheight reflects the presence of block and undo data") + self.test_pruneheight_undo_presence() + self.log.info("Done") def test_scanblocks_pruned(self): @@ -494,5 +499,18 @@ class PruneTest(BitcoinTestFramework): assert_raises_rpc_error(-1, "Block not available (pruned data)", node.scanblocks, "start", [{"desc": f"raw({false_positive_spk.hex()})"}], 0, 0, "basic", {"filter_false_positives": True}) + def test_pruneheight_undo_presence(self): + node = self.nodes[2] + pruneheight = node.getblockchaininfo()["pruneheight"] + fetch_block = node.getblockhash(pruneheight - 1) + + self.connect_nodes(1, 2) + peers = node.getpeerinfo() + node.getblockfrompeer(fetch_block, peers[0]["id"]) + self.wait_until(lambda: not try_rpc(-1, "Block not available (pruned data)", node.getblock, fetch_block), timeout=5) + + new_pruneheight = node.getblockchaininfo()["pruneheight"] + assert_equal(pruneheight, new_pruneheight) + if __name__ == '__main__': PruneTest().main() -- cgit v1.2.3 From 8789dc8f315a9d9ad7142d831bc9412f780248e7 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 17 Mar 2024 17:41:17 +0100 Subject: doc: Add note to getblockfrompeer on missing undo data --- src/rpc/blockchain.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index fd1d1e4e87..9a838ab77d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -429,6 +429,7 @@ static RPCHelpMan getblockfrompeer() "getblockfrompeer", "Attempt to fetch block from a given peer.\n\n" "We must have the header for this block, e.g. using submitheader.\n" + "The block will not have any undo data which can limit the usage of the block data in a context where the undo data is needed.\n" "Subsequent calls for the same block may cause the response from the previous peer to be ignored.\n" "Peers generally ignore requests for a stale block that they never fully verified, or one that is more than a month old.\n" "When a peer does not respond with a block, we will disconnect.\n" -- cgit v1.2.3