feat(chain)!: make `CheckPoint` `data` field optional by LagginTimes · Pull Request #2024 · bitcoindevkit/bdk

LagginTimes · 2025-09-10T06:19:16Z

Resolves #2021.

Description

This PR makes the CheckPoint::data field optional. Gaps inferred from prev_blockhash are represented as placeholder checkpoints where data is None. Subsequent insert()s will provide data for the placeholders when a matching block arrives.

See: #2017 (comment)

Notes to the reviewers

WIP

Changelog notice

Checklists

All Submissions:

I followed the contribution guidelines

New Features:

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

Bugfixes:

This pull request breaks the existing API
I've added tests to reproduce the issue which are now passing
I'm linking the issue being fixed by this PR

evanlinjin

.

evanlinjin · 2025-09-11T04:43:20Z

Some recommended test scenarios:

Update chain that adds data to a placeholder checkpoint in the original chain.
Update chain which has a PoA with the original chain at a placeholder checkpoint (and vise-versa).

With the above tests:

Ensure the resultant chain has no "floating" checkpoints.

oleonardolima

cACK c24dd6a

It looking good code-wise, though we probably need further discussion on what's the best approach, as discussed by Evan and VM in discord.

evanlinjin · 2025-10-12T11:46:52Z

@LagginTimes please review the modifications I made to push and insert here: evanlinjin@1939dd2

This implementation builds upon and improves the original PR's approach to optional checkpoint data in several key ways:

Cleaner insert logic
The original PR had complex nested conditions for handling insertions. This version:
- Splits the chain into base and tail components upfront
- Uses a systematic rebuild approach that leverages push for validation
- Handles all edge cases (displacement, purging, placeholder filling) in a unified flow
Proper displacement handling
- Clear distinction: When a prev_blockhash conflicts, the checkpoint is displaced (converted to placeholder) rather than purged, preserving the chain structure
- Orphan management: Checkpoints that become orphaned after displacement are correctly purged
- The original PR struggled with maintaining consistency when conflicts occurred at different points in the chain
Placeholder cleanup in push
- Added logic to remove trailing placeholder checkpoints before pushing new data
- Prevents accumulation of unnecessary placeholders at the chain tip
- Maintains cleaner chain structure over time

* Base tip must always have data. * Update should be able to introduce data to a placeholder checkpoint in the original chain. * Remove unnecessary logic.

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 - Push creates placeholder for non-contiguous heights 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>

evanlinjin · 2026-01-08T08:01:24Z

@LagginTimes I've rebased on master. Would you be okay if I took over this PR?

Rework CheckPoint::insert to properly handle conflicts: - Split chain into base (below insert height) and tail (at/above) - Rebuild chain by pushing tail items back, handling conflicts - Displace checkpoints to placeholders when prev_blockhash conflicts - Purge orphaned checkpoints that can no longer connect to chain - Fix edge cases where placeholders weren't handled correctly Improve CheckPoint::push: - Remove trailing placeholder checkpoints before pushing - Maintain existing behavior of creating placeholders for gaps Documentation improvements: - Clarify displacement vs purging rules without complex if-else chains - Add concrete examples showing the behavior - Add inline comments explaining the complex rebuild logic Add comprehensive test coverage for displacement scenarios including inserting at new/existing heights with various conflict types.

evanlinjin · 2026-01-08T09:02:16Z

@@ -51,6 +54,7 @@
                .expect("extension is strictly greater than base"),


This expect is now wrong. A block in the extension may have a prev_blockhash that doesn't line up.

evanlinjin · 2026-01-08T09:04:04Z

There seems to be a lot of additional logic in LocalChain to manage ChangeSets. This is non-ideal as the mutating logic of CheckPoint is now more complex - so we really want to keep the complexity in CheckPoint only.

Instead of having apply_changeset_to_checkpoint, I'm thinking of using methods of CheckPoint directly and comparing the original and updated checkpoint chains to derive the checkpoint.

evanlinjin · 2026-01-08T09:26:21Z

    update_tip: CheckPoint<D>,
 ) -> Result<(CheckPoint<D>, ChangeSet<D>), CannotConnectError>
 where
    D: ToBlockHash + fmt::Debug + Copy,


I wonder if the + Copy here will be problematic.

evanlinjin · 2026-02-07T10:53:27Z

Replaced by #2115

c486ba7 docs: address review feedback on `prev_blockhash` validation (志宇) 031b30f docs(core): address review feedback on docs and tests (志宇) ef35b19 fix(chain)!: make genesis immutable in `merge_chains` (志宇) d8cb41f test(core): address review feedback on checkpoint tests (志宇) 9c0453c docs(core): Add module-level docs for `checkpoint_entry` (志宇) 3ef6ed8 feat(chain)!: Add `ApplyBlockError` for `prev_blockhash` validation (志宇) bf6541b feat(chain)!: Relax the generic parameter for `LocalChain<D>` (志宇) 19b9006 test(core): add tests for CheckPoint::push and insert methods (志宇) 9971fda test(chain): Test `apply_update` with a single `CheckPoint<Header>` (valued mammal) 3df0609 test(chain): make `TestLocalChain` generic and add `prev_blockhash` test (志宇) c314b25 fix(chain): `merge_chains` now takes account of `prev_blockhash`es (志宇) f10ba0e fix(core): `Checkpoint::insert` now evicts on `prev_blockhash` mismatch (志宇) 046a771 fix(core): `push` now errors on `prev_blockhash` mismatch (志宇) 68d1ef4 feat(core): Initial work on `CheckPointEntry` (志宇) d4bdff0 feat(core): Add `prev_blockhash` method to `ToBlockHash` trait (志宇) Pull request description: 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: * #2024 - where we made the `CheckPoint::data` optional - however this resulted in internal complexity and an API with annoying edge cases. The tests from this PR were still useful. * #2091 - This second attempt had some good ideas, but was distracted from the goal of #2021. I mostly reused the `CheckPoint::insert` implementation of that PR. ### Changelog notice ```md 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` - `ApplyBlockError` - this is a new error type with two variants; `MissingGenesis` and `PrevBlockhashMismatch`. The second variant is a new error case introduced by `prev_blockhash` awareness. 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 - `LocalChain<D>` generic parameter - relaxed constraint to `D: Clone` instead of `D: Copy`. Fixed: - `merge_chains` no longer replaces the genesis block. ``` ### Checklists #### All Submissions: * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) #### New Features: * [x] I've added tests for the new feature * [x] I've added docs for the new feature ACKs for top commit: evanlinjin: self-ACK c486ba7 Tree-SHA512: 5622a8a418034443931c8b5938e708c2222c42c05efbdac71d47a4e0ce1b4f4b0b7bcd2e1e14e2f295cc056e12c03e5750b40b1a1313cdbdf59a009d81d0b8a1

github-project-automation Bot added this to BDK Chain Sep 10, 2025

LagginTimes self-assigned this Sep 10, 2025

LagginTimes marked this pull request as draft September 10, 2025 06:19

LagginTimes closed this Sep 10, 2025

github-project-automation Bot moved this to Done in BDK Chain Sep 10, 2025

LagginTimes reopened this Sep 10, 2025

LagginTimes moved this from Done to In Progress in BDK Chain Sep 10, 2025

LagginTimes added this to the Wallet 3.0.0 milestone Sep 10, 2025

evanlinjin requested changes Sep 10, 2025

View reviewed changes

Comment thread crates/core/src/checkpoint.rs Resolved

Comment thread crates/core/src/checkpoint.rs Resolved

Comment thread crates/core/src/checkpoint.rs Resolved

evanlinjin reviewed Sep 10, 2025

View reviewed changes

Comment thread crates/chain/src/local_chain.rs Resolved

LagginTimes mentioned this pull request Sep 10, 2025

feat!(chain): implement CheckPointEntry WIP #2017

Closed

6 tasks

LagginTimes force-pushed the optional_data branch 2 times, most recently from cf98f58 to eddadd4 Compare September 10, 2025 17:26

evanlinjin reviewed Sep 11, 2025

View reviewed changes

Comment thread crates/chain/src/local_chain.rs Resolved

Comment thread crates/chain/src/local_chain.rs Resolved

LagginTimes force-pushed the optional_data branch from e805152 to 86968bb Compare September 12, 2025 06:57

LagginTimes mentioned this pull request Sep 12, 2025

Ensure CheckPoint chain methods validate and link via previous blockhash #2021

Closed

1 of 3 tasks

LagginTimes force-pushed the optional_data branch 3 times, most recently from e1166b6 to 46bd424 Compare September 16, 2025 04:30

LagginTimes marked this pull request as ready for review September 16, 2025 06:22

LagginTimes requested a review from evanlinjin September 16, 2025 06:22

oleonardolima reviewed Sep 16, 2025

View reviewed changes

evanlinjin mentioned this pull request Sep 25, 2025

feat(chain)!: add CheckPointEntry #2039

Closed

4 of 6 tasks

evanlinjin requested changes Sep 25, 2025

View reviewed changes

Comment thread crates/core/src/checkpoint.rs Resolved

Comment thread crates/core/src/checkpoint.rs Resolved

LagginTimes force-pushed the optional_data branch from ba55b7e to a216494 Compare September 28, 2025 20:15

LagginTimes requested a review from evanlinjin September 28, 2025 20:22

evanlinjin reviewed Sep 29, 2025

View reviewed changes

Comment thread crates/core/src/checkpoint.rs Resolved

oleonardolima reviewed Oct 2, 2025

View reviewed changes

Comment thread crates/core/tests/test_checkpoint.rs Resolved

LagginTimes force-pushed the optional_data branch from a216494 to ae040c2 Compare October 3, 2025 23:54

LagginTimes force-pushed the optional_data branch from ae040c2 to 460bc46 Compare October 3, 2025 23:55

evanlinjin reviewed Oct 11, 2025

View reviewed changes

Comment thread crates/core/src/checkpoint.rs Resolved

LagginTimes force-pushed the optional_data branch 2 times, most recently from b301866 to 7ce4d91 Compare October 29, 2025 05:49

ValuedMammal mentioned this pull request Nov 24, 2025

CheckPoint design issue #2076

Closed

2 of 4 tasks

evanlinjin mentioned this pull request Jan 8, 2026

feat(chain)!: Add prev_blockhash to ToBlockHash trait #2091

Closed

6 tasks done

LagginTimes and others added 6 commits January 8, 2026 04:47

feat(chain)!: make CheckPoint data field optional

6dabf92

fix(chain): Fix merge_chains logic

214365f

* Base tip must always have data. * Update should be able to introduce data to a placeholder checkpoint in the original chain. * Remove unnecessary logic.

test(chain): add test for merge_chains

1f1304c

docs(chain): update CheckPoint and LocalChain docs

ad322d2

fix(core): fix CheckPoint::insert and CheckPoint::push logic

7feb47c

evanlinjin force-pushed the optional_data branch 2 times, most recently from 1939dd2 to 1f62476 Compare January 8, 2026 07:54

evanlinjin force-pushed the optional_data branch from 1f62476 to bd7a0db Compare January 8, 2026 08:02

evanlinjin reviewed Jan 8, 2026

View reviewed changes

evanlinjin mentioned this pull request Feb 5, 2026

Add prev_blockhash validation to CheckPoint #2115

Merged

3 tasks done

evanlinjin closed this Feb 7, 2026

github-project-automation Bot moved this from In Progress to Done in BDK Chain Feb 7, 2026

notmandatory removed this from BDK Chain Sep 29, 2026

		@@ -51,6 +54,7 @@
		.expect("extension is strictly greater than base"),

Conversation

LagginTimes commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

evanlinjin commented Sep 11, 2025

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evanlinjin commented Oct 12, 2025

Uh oh!

evanlinjin commented Jan 8, 2026

Uh oh!

evanlinjin Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

evanlinjin Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evanlinjin Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

evanlinjin commented Feb 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LagginTimes commented Sep 10, 2025 •

edited

Loading

evanlinjin Jan 8, 2026 •

edited

Loading