Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

index: Fix backwards search for bestblock #23365

Merged
merged 2 commits into from
Dec 13, 2021
Merged

index: Fix backwards search for bestblock #23365

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
698c524
index: Fix backwards search for bestblock
mzumsande Oct 26, 2021
9600ea0
test: Add edge case of pruning up to index height
mzumsande Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
index: Fix backwards search for bestblock 
This allows filters to be reconstructed when the best known block is
the Genesis block without needing to reindex.
It fixes Init errors seen in #23289.
  • Loading branch information
@mzumsande
mzumsande committed Nov 26, 2021
commit 698c524698c33595a4d555eaa9e21bc19b4d3e93
3 changes: 2 additions & 1 deletion src/index/base.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,12 @@ bool BaseIndex::Init()
const CBlockIndex* block = active_chain.Tip();
prune_violation = true;
// check backwards from the tip if we have all block data until we reach the indexes bestblock
while (block_to_test && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
while (block_to_test && block && (block->nStatus & BLOCK_HAVE_DATA)) {
if (block_to_test == block) {
prune_violation = false;
break;
}
assert(block->pprev);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth adding a comment here on why block->pprev must exist (because we're walking backwards from the active chain tip to a block index in the active chain. If a block index that we encounter doesn't have a backwards pointer, then the chain is malformed).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually a correct explanation (the genesis block is in the active chain and doesn't have pprev set). The reason this assertion will hold is because of how block and block_to_test are chosen; the conditional above this line is guaranteed to break at-or-higher-than genesis because of the few lines of logic above (which, if summarized in a comment, would just end up repeating the fairly-straightforward code above).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're saying the same thing as I am. Because of the way block and block_to_test are chosen, if we walk backwards from block, we're guaranteed to encounter block_to_test (either at or before the genesis block).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't get to this earlier, would have added a comment, do people think that this warrants a follow-up?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mzumsande sounds like it could be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in #23777

block = block->pprev;
}
}
Expand Down