refactor: return reference instead of pointer
What changed, and why it matters
This is a small internal code cleanup in Bitcoin Core. A helper function that finds the earliest block matching certain criteria used to return a raw pointer; now it returns a reference. The change is purely stylistic/safety-related because the function was already guaranteed to return a non-null result (it contains an assertion). No behavior changes, no bug fixes, and no security issue is present.
No action required. This is a benign refactor with no security relevance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors BlockManager::GetFirstBlock to return const CBlockIndex& instead of const CBlockIndex*. Call sites are updated to take the address of the returned reference where pointer comparison is needed. The function already asserted that the result is non-null, so the change only makes the non-null contract explicit in the type system. It does not alter control flow, logic, or runtime behavior.
Changed components
src/node/blockstorage.cppsrc/node/blockstorage.hsrc/rpc/blockchain.cppsrc/test/blockmanager_tests.cppInspect captured patch +11 / −11
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index a9b9b9a6..b6e1b8b0 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -588,7 +588,7 @@ bool BlockManager::IsBlockPruned(const CBlockIndex& block) const
return m_have_pruned && !(block.nStatus & BLOCK_HAVE_DATA) && (block.nTx > 0);
}
-const CBlockIndex* BlockManager::GetFirstBlock(const CBlockIndex& upper_block, uint32_t status_mask, const CBlockIndex* lower_block) const
+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;
@@ -596,7 +596,7 @@ const CBlockIndex* BlockManager::GetFirstBlock(const CBlockIndex& upper_block, u
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;
+ if (last_block == lower_block) return *lower_block;
// if range was surpassed, means that 'lower_block' is not part of the 'upper_block' chain
// and so far this is not allowed.
assert(last_block->nHeight >= lower_block->nHeight);
@@ -604,13 +604,13 @@ const CBlockIndex* BlockManager::GetFirstBlock(const CBlockIndex& upper_block, u
last_block = last_block->pprev;
}
assert(last_block != nullptr);
- return last_block;
+ return *last_block;
}
bool BlockManager::CheckBlockDataAvailability(const CBlockIndex& upper_block, const CBlockIndex& lower_block)
{
if (!(upper_block.nStatus & BLOCK_HAVE_DATA)) return false;
- return GetFirstBlock(upper_block, BLOCK_HAVE_DATA, &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 4ee75f07..9ba7873c 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -422,11 +422,11 @@ public:
* @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`
+ * @return A reference 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& GetFirstBlock(
const CBlockIndex& upper_block LIFETIMEBOUND,
uint32_t status_mask,
const CBlockIndex* lower_block LIFETIMEBOUND = nullptr
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 4ef7bc96..ff8cb119 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -870,7 +870,7 @@ std::optional<int> GetPruneHeight(const BlockManager& blockman, const CChain& ch
// 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{*CHECK_NONFATAL(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))};
+ const auto& first_unpruned{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;
@@ -3116,8 +3116,8 @@ static RPCHelpMan dumptxoutset()
if (node.chainman->m_blockman.IsPruneMode()) {
LOCK(node.chainman->GetMutex());
const CBlockIndex* current_tip{node.chainman->ActiveChain().Tip()};
- const CBlockIndex* first_block{node.chainman->m_blockman.GetFirstBlock(*current_tip, /*status_mask=*/BLOCK_HAVE_MASK)};
- if (first_block->nHeight > target_index->nHeight) {
+ const CBlockIndex& first_block{node.chainman->m_blockman.GetFirstBlock(*current_tip, /*status_mask=*/BLOCK_HAVE_MASK)};
+ if (first_block.nHeight > target_index->nHeight) {
throw JSONRPCError(RPC_MISC_ERROR, "Could not roll back to requested height since necessary block data is already pruned.");
}
}
diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp
index a3504da6..a1551994 100644
--- a/src/test/blockmanager_tests.cpp
+++ b/src/test/blockmanager_tests.cpp
@@ -119,7 +119,7 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup)
};
// 1) Return genesis block when all blocks are available
- BOOST_CHECK_EQUAL(blockman.GetFirstBlock(tip, BLOCK_HAVE_DATA), 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
@@ -133,7 +133,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.GetFirstBlock(tip, BLOCK_HAVE_DATA), 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));
}
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.