⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Add prev_blockhash validation to CheckPoint#2115

Draft
evanlinjin wants to merge 8 commits intobitcoindevkit:masterfrom
evanlinjin:cp_entry
Draft

Add prev_blockhash validation to CheckPoint#2115
evanlinjin wants to merge 8 commits intobitcoindevkit:masterfrom
evanlinjin:cp_entry

Conversation

@evanlinjin
Copy link
Member

@evanlinjin evanlinjin commented Feb 5, 2026

Closes #2021
Related to #2076
Replaces #2024
Replaces #2091

Description

This PR adds prev_blockhash awareness to CheckPoint, enabling proper chain validation when merging checkpoint chains that store block headers or similar data with previous block hash information.

Notes to the reviewers

This PR replaces some prior attempts:

Changelog notice

Added:
- `ToBlockHash::prev_blockhash()` - optional method to expose previous block hash
- `CheckPointEntry` - new type for iterating with `prev_blockhash` awareness, yielding "placeholder" entries for heights inferred from `prev_blockhash`

Changed:
- `CheckPoint::push` - now errors when `prev_blockhash` conflicts with current tip (contiguous heights)
- `CheckPoint::insert` - now evicts/displaces checkpoints on `prev_blockhash` conflict
- `merge_chains` - now validates `prev_blockhash` consistency when merging

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

evanlinjin and others added 8 commits February 5, 2026 08:37
Additionally, `insert` now panics if the genesis block gets displaced (if
it existed in the first place).

Co-authored-by: valued mammal <valuedmammal@protonmail.com>
Make `TestLocalChain` and `ExpectedResult` generic over checkpoint data
type `D`, allowing the same test infrastructure to work with both
`BlockHash` and `TestBlock` types.

Add `merge_chains_with_prev_blockhash` test to verify that `prev_blockhash`
correctly invalidates conflicting blocks and connects disjoint chains.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add comprehensive tests for CheckPoint::push error cases:
- Push fails when height is not greater than current
- Push fails when prev_blockhash conflicts with self
- Push succeeds when prev_blockhash matches

Include tests for CheckPoint::insert conflict handling:
- Insert with conflicting prev_blockhash
- Insert purges conflicting tail
- Insert between conflicting checkpoints

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: valued mammal <valuedmammal@protonmail.com>
@evanlinjin evanlinjin self-assigned this Feb 5, 2026
@evanlinjin evanlinjin added the bug Something isn't working label Feb 5, 2026
@evanlinjin evanlinjin moved this to Needs Review in BDK Chain Feb 5, 2026
// OPTIMIZATION 2 -- if we have the same underlying pointer at this point, we
// can guarantee that no older blocks are introduced.
if o.eq_ptr(u) {
// TODO: Ensure this is correct!
Copy link
Member Author

Choose a reason for hiding this comment

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

This is correct! We can remove this comment!

Copy link
Member Author

@evanlinjin evanlinjin Feb 5, 2026

Choose a reason for hiding this comment

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

As highlighted in f7eedf3, we should do something about apply_changeset_to_checkpoint.

@evanlinjin evanlinjin marked this pull request as draft February 5, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

Ensure CheckPoint chain methods validate and link via previous blockhash

2 participants