[Chain] Prune entire orphaned chains. #962
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the index contains forks other than the active fork, then removing one from an early part of the chain can cause startup to assert `pindexWalk->pprev` on the remaining blocks in the fork which no longer have parents in the index. (Issue Veil-Project#692) This is made likely by the pruning process that only prunes orphans based on height far enough from the best block. Solution: After finding out which index entries are old enough to be pruned, search the index for all entries that descend from any of those, and add them to be pruned. Additionally, CBlockIndex pointers are passed all over the codebase and are never deleted, but their hash values are pointers to the mapBlockIndex's keys. Normally this isn't a problem (unordered_map), until the data is erased and the memory reused.
codeofalltrades
added
Tag: Chainstate
|
utACK 2054946 |
CaveSpectre11
approved these changes
Jul 30, 2021
CaveSpectre11
added
Code Review: Passed
Dev Status: Merged
This was referenced May 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Pruned blocks sometimes cause other blocks in the index (mostly descendants of orphans) to have invalid parents, which may trip an assert
pindexWalk->pprev(#692) on startup. Additionally, because block indexes don't own their own hash values, removing the index frommapBlockIndexwhere the hash is allocated may lead to a use-after-free corrupting the hash of a block written to disk (may be the cause of #930 (comment)).Solution
Select pruning candidates in two rounds: first, based on height. Then, add all descendants of the blocks chosen in the first round.
For the memory issue, add a
shared_ptrfield to CBlockIndex that provides the hash value when the block is removed from mapBlockIndex. This will manage the memory of the hash for the lifetime of the object (which is forever anyway) and allows for efficient sharing and copying as usual (unique_ptr does not).Testing
Via regtest. See #692 (comment)