feat(chain)!: add CheckPointEntry#2039
feat(chain)!: add CheckPointEntry#2039LagginTimes wants to merge 1 commit intobitcoindevkit:masterfrom
CheckPointEntry#2039Conversation
evanlinjin
left a comment
There was a problem hiding this comment.
The original implementation of merge_chains was much more efficient. You don't need to change it so drastically to take into account prevhashes. Have an enum CheckPointEntry where both variants have a CheckPoint and you can call next to get to the closest lower height.
173cb46 to
95b86f9
Compare
| } else if o.height() > u.height() { | ||
| // Original > Update: mark original as potentially invalidated. | ||
| potentially_invalidated_heights.push(o.height()); | ||
| prev_orig_was_invalidated = false; | ||
| prev_orig = curr_orig.take(); | ||
| is_update_height_superset_of_original = false; |
There was a problem hiding this comment.
question: isn't this already being covered by the lines L652-656 ?
|
Thanks for working on this.
Even looking at the unfinished state of this, I think I would much prefer #2024 (or the initial public With this implementation, we need alternative methods to figure out if a height "exists": a height with no checkpoint where the next adjacent checkpoint has a prev-blockhash. |
|
Closing this and keeping #2024 per feedback. |
Resolves #2021.
Description
Introduce
CheckPointEntryto represent either a realCheckPointor aprev_blockhashplaceholder.merge_chains()now handles updates containingprev_blockhashdata while preserving previous behavior when onlyCheckPoints are present.is_block_in_chain()can infer membership usingprev_blockhash.Notes to the reviewers
TODO: Add tests.
Changelog notice
Checklists
All Submissions:
New Features:
Bugfixes: