Skip to content

Commit c82ef91

Browse files
committed
make GetFirstStoredBlock assert that 'start_block' always has data
And transfer the responsibility of verifying whether 'start_block' has data or not to the caller. This is because the 'GetFirstStoredBlock' function responsibility is to return the first block containing data. And the current implementation can return 'start_block' when it has no data!. Which is misleading at least. Edge case behavior change: Previously, if the block tip lacked data but all preceding blocks contained data, there was no prune violation. And now, such scenario will result in a prune violation.
1 parent 430e702 commit c82ef91

File tree

5 files changed

+40
-6
lines changed

5 files changed

+40
-6
lines changed

src/index/base.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ bool BaseIndex::Init()
115115
if (!start_block) {
116116
// index is not built yet
117117
// make sure we have all block data back to the genesis
118-
prune_violation = m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis();
118+
bool has_tip_data = active_chain.Tip()->nStatus & BLOCK_HAVE_DATA;
119+
prune_violation = !has_tip_data || m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis();
119120
}
120121
// in case the index has a best block set and is not fully synced
121122
// check if we have the required blocks to continue building the index

src/node/blockstorage.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& start_bl
407407
{
408408
AssertLockHeld(::cs_main);
409409
const CBlockIndex* last_block = &start_block;
410+
assert(last_block->nStatus & BLOCK_HAVE_DATA);
410411
while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) {
411412
last_block = last_block->pprev;
412413
}

src/node/blockstorage.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,9 @@ class BlockManager
222222
//! Returns last CBlockIndex* that is a checkpoint
223223
const CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
224224

225-
//! Find the first block that is not pruned
225+
//! Find the first stored ancestor of start_block immediately after the last
226+
//! pruned ancestor. Return value will never be null. Caller is responsible
227+
//! for ensuring that start_block has data is not pruned.
226228
const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
227229

228230
/** True if any block files have ever been pruned. */

src/rpc/blockchain.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -812,9 +812,7 @@ static RPCHelpMan pruneblockchain()
812812

813813
PruneBlockFilesManual(active_chainstate, height);
814814
const CBlockIndex& block{*CHECK_NONFATAL(active_chain.Tip())};
815-
const CBlockIndex* last_block{active_chainstate.m_blockman.GetFirstStoredBlock(block)};
816-
817-
return static_cast<int64_t>(last_block->nHeight - 1);
815+
return block.nStatus & BLOCK_HAVE_DATA ? active_chainstate.m_blockman.GetFirstStoredBlock(block)->nHeight - 1 : block.nHeight;
818816
},
819817
};
820818
}
@@ -1267,7 +1265,8 @@ RPCHelpMan getblockchaininfo()
12671265
obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage());
12681266
obj.pushKV("pruned", chainman.m_blockman.IsPruneMode());
12691267
if (chainman.m_blockman.IsPruneMode()) {
1270-
obj.pushKV("pruneheight", chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight);
1268+
bool has_tip_data = tip.nStatus & BLOCK_HAVE_DATA;
1269+
obj.pushKV("pruneheight", has_tip_data ? chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight : tip.nHeight + 1);
12711270

12721271
const bool automatic_pruning{chainman.m_blockman.GetPruneTarget() != BlockManager::PRUNE_TARGET_MANUAL};
12731272
obj.pushKV("automatic_pruning", automatic_pruning);

src/test/blockmanager_tests.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,35 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain
9090
BOOST_CHECK(!AutoFile(blockman.OpenBlockFile(new_pos, true)).IsNull());
9191
}
9292

93+
BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup)
94+
{
95+
LOCK(::cs_main);
96+
auto& chainman = m_node.chainman;
97+
auto& blockman = chainman->m_blockman;
98+
const CBlockIndex& tip = *chainman->ActiveTip();
99+
100+
// Function to prune all blocks from 'last_pruned_block' down to the genesis block
101+
const auto& func_prune_blocks = [&](CBlockIndex* last_pruned_block)
102+
{
103+
LOCK(::cs_main);
104+
CBlockIndex* it = last_pruned_block;
105+
while (it != nullptr && it->nStatus & BLOCK_HAVE_DATA) {
106+
it->nStatus &= ~BLOCK_HAVE_DATA;
107+
it = it->pprev;
108+
}
109+
};
110+
111+
// 1) Return genesis block when all blocks are available
112+
BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), chainman->ActiveChain()[0]);
113+
114+
// Prune half of the blocks
115+
int height_to_prune = tip.nHeight / 2;
116+
CBlockIndex* first_available_block = chainman->ActiveChain()[height_to_prune + 1];
117+
CBlockIndex* last_pruned_block = first_available_block->pprev;
118+
func_prune_blocks(last_pruned_block);
119+
120+
// 2) The last block not pruned is in-between upper-block and the genesis block
121+
BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), first_available_block);
122+
}
123+
93124
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)