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.
This commit is contained in:
furszy 2023-06-16 17:32:21 -03:00
parent 430e7027a1
commit c82ef91eae
No known key found for this signature in database
GPG Key ID: 5DD23CCC686AA623
5 changed files with 40 additions and 6 deletions

View File

@ -115,7 +115,8 @@ bool BaseIndex::Init()
if (!start_block) {
// index is not built yet
// make sure we have all block data back to the genesis
prune_violation = m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis();
bool has_tip_data = active_chain.Tip()->nStatus & BLOCK_HAVE_DATA;
prune_violation = !has_tip_data || m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis();
}
// in case the index has a best block set and is not fully synced
// check if we have the required blocks to continue building the index

View File

@ -407,6 +407,7 @@ const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& start_bl
{
AssertLockHeld(::cs_main);
const CBlockIndex* last_block = &start_block;
assert(last_block->nStatus & BLOCK_HAVE_DATA);
while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) {
last_block = last_block->pprev;
}

View File

@ -222,7 +222,9 @@ public:
//! Returns last CBlockIndex* that is a checkpoint
const CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
//! Find the first block that is not pruned
//! 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) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
/** True if any block files have ever been pruned. */

View File

@ -812,9 +812,7 @@ static RPCHelpMan pruneblockchain()
PruneBlockFilesManual(active_chainstate, height);
const CBlockIndex& block{*CHECK_NONFATAL(active_chain.Tip())};
const CBlockIndex* last_block{active_chainstate.m_blockman.GetFirstStoredBlock(block)};
return static_cast<int64_t>(last_block->nHeight - 1);
return block.nStatus & BLOCK_HAVE_DATA ? active_chainstate.m_blockman.GetFirstStoredBlock(block)->nHeight - 1 : block.nHeight;
},
};
}
@ -1267,7 +1265,8 @@ RPCHelpMan getblockchaininfo()
obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage());
obj.pushKV("pruned", chainman.m_blockman.IsPruneMode());
if (chainman.m_blockman.IsPruneMode()) {
obj.pushKV("pruneheight", chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight);
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 bool automatic_pruning{chainman.m_blockman.GetPruneTarget() != BlockManager::PRUNE_TARGET_MANUAL};
obj.pushKV("automatic_pruning", automatic_pruning);

View File

@ -90,4 +90,35 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain
BOOST_CHECK(!AutoFile(blockman.OpenBlockFile(new_pos, true)).IsNull());
}
BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup)
{
LOCK(::cs_main);
auto& chainman = m_node.chainman;
auto& blockman = chainman->m_blockman;
const CBlockIndex& tip = *chainman->ActiveTip();
// Function to prune all blocks from 'last_pruned_block' down to the genesis block
const auto& func_prune_blocks = [&](CBlockIndex* last_pruned_block)
{
LOCK(::cs_main);
CBlockIndex* it = last_pruned_block;
while (it != nullptr && it->nStatus & BLOCK_HAVE_DATA) {
it->nStatus &= ~BLOCK_HAVE_DATA;
it = it->pprev;
}
};
// 1) Return genesis block when all blocks are available
BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), chainman->ActiveChain()[0]);
// Prune half of the blocks
int height_to_prune = tip.nHeight / 2;
CBlockIndex* first_available_block = chainman->ActiveChain()[height_to_prune + 1];
CBlockIndex* last_pruned_block = first_available_block->pprev;
func_prune_blocks(last_pruned_block);
// 2) The last block not pruned is in-between upper-block and the genesis block
BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), first_available_block);
}
BOOST_AUTO_TEST_SUITE_END()