Skip to content

Commit 81e67d9

Browse files
committed
Merge bitcoin#34179: refactor: Enable transparent lookup for setBlockIndexCandidates to remove const_cast
3bd98b4 refactor: use transparent comparator for setBlockIndexCandidates lookups (joaonevess) Pull request description: ### Rationale This PR improves code safety by removing a `const_cast` in `src/validation.cpp`. Currently, `setBlockIndexCandidates` stores mutable `CBlockIndex*`. However, validation logic (like `CVerifyDB`) often holds `const CBlockIndex*`. Previously, checking for existence in the set required casting away constness. While currently benign, this bypasses compiler safety checks and could mask accidental modifications in future refactors. ### Description 1. **Enable Heterogeneous Lookup:** Added `using is_transparent = void;` to `CBlockIndexWorkComparator` in `src/node/blockstorage.h`. This allows the `std::set` to natively accept `const CBlockIndex*` for lookup (utilizing C++14 heterogeneous lookup). 2. **Remove Cast:** Removed the now unnecessary `const_cast<CBlockIndex*>` in `src/validation.cpp`, allowing the compiler to strictly enforce const-correctness. ### Notes - **Refactoring only:** No behavioral change. - **Verification:** `validation_tests` and `blockmanager_tests` pass. ACKs for top commit: maflcko: review ACK 3bd98b4 🚪 frankomosh: ACK 3bd98b4. Good use of transparent comparator to eliminate `const_cast` in this specific code path. sedited: ACK 3bd98b4 Tree-SHA512: 0f76bdce2a54b759dfec99633afce1e95586e62f4057ecf1e82eed1a073eb8ecb2d659ccbf28a7a139f0aa09a30f058ac6966cafdfbf1f2ee878fa2d86b2c487
2 parents ec70bea + 3bd98b4 commit 81e67d9

File tree

2 files changed

+4
-3
lines changed

2 files changed

+4
-3
lines changed

src/node/blockstorage.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ using BlockMap = std::unordered_map<uint256, CBlockIndex, BlockHasher>;
136136

137137
struct CBlockIndexWorkComparator {
138138
bool operator()(const CBlockIndex* pa, const CBlockIndex* pb) const;
139+
using is_transparent = void;
139140
};
140141

141142
struct CBlockIndexHeightOnlyComparator {

src/validation.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5380,15 +5380,15 @@ void ChainstateManager::CheckBlockIndex() const
53805380
// needs to be added if it is an ancestor of the target
53815381
// block.
53825382
if (!c->TargetBlock() || c->TargetBlock()->GetAncestor(pindex->nHeight) == pindex) {
5383-
assert(c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex)));
5383+
assert(c->setBlockIndexCandidates.contains(pindex));
53845384
}
53855385
}
53865386
// If some parent is missing, then it could be that this block was in
53875387
// setBlockIndexCandidates but had to be removed because of the missing data.
53885388
// In this case it must be in m_blocks_unlinked -- see test below.
53895389
}
53905390
} else { // If this block sorts worse than the current tip or some ancestor's block has never been seen, it cannot be in setBlockIndexCandidates.
5391-
assert(!c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex)));
5391+
assert(!c->setBlockIndexCandidates.contains(pindex));
53925392
}
53935393
}
53945394
// Check whether this block is in m_blocks_unlinked.
@@ -5420,7 +5420,7 @@ void ChainstateManager::CheckBlockIndex() const
54205420
// So if this block is itself better than any m_chain.Tip() and it wasn't in
54215421
// setBlockIndexCandidates, then it must be in m_blocks_unlinked.
54225422
for (const auto& c : m_chainstates) {
5423-
if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && !c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex))) {
5423+
if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && !c->setBlockIndexCandidates.contains(pindex)) {
54245424
if (pindexFirstInvalid == nullptr) {
54255425
if (!c->TargetBlock() || c->TargetBlock()->GetAncestor(pindex->nHeight) == pindex) {
54265426
assert(foundInUnlinked);

0 commit comments

Comments
 (0)