feat(chain)!: make `CheckPoint` `data` field optional #2024

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

crates/core/src/checkpoint.rs

crates/chain/src/local_chain.rs

evanlinjin

.

crates/chain/src/local_chain.rs

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.

crates/core/src/checkpoint.rs

crates/core/tests/test_checkpoint.rs

crates/core/src/checkpoint.rs

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

crates/chain/src/local_chain.rs

        let new_tip = match base {
            Some(base) => base
                .extend(extension)
                .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

crates/chain/src/local_chain.rs

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

crates/chain/src/local_chain.rs

    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.

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

crates/core/src/checkpoint.rs Show resolved Hide resolved

crates/core/src/checkpoint.rs Show resolved Hide resolved

crates/core/src/checkpoint.rs Show resolved Hide resolved

evanlinjin reviewed Sep 10, 2025

View reviewed changes

crates/chain/src/local_chain.rs Outdated Show resolved Hide 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

crates/chain/src/local_chain.rs Outdated Show resolved Hide resolved

crates/chain/src/local_chain.rs Outdated Show resolved Hide 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

Open

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

6 tasks

evanlinjin requested changes Sep 25, 2025

View reviewed changes

crates/core/src/checkpoint.rs Show resolved Hide resolved

crates/core/src/checkpoint.rs Show resolved Hide 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

crates/core/src/checkpoint.rs Show resolved Hide resolved

oleonardolima reviewed Oct 2, 2025

View reviewed changes

crates/core/tests/test_checkpoint.rs Show resolved Hide 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

crates/core/src/checkpoint.rs Show resolved Hide 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

Open

evanlinjin mentioned this pull request Jan 8, 2026

feat(core): Add prev_blockhash to trait ToBlockHash #2091

Open

6 tasks

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

feat(chain)!: make CheckPoint data field optional #2024

Are you sure you want to change the base?

feat(chain)!: make CheckPoint data field optional #2024

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(chain)!: make `CheckPoint` `data` field optional #2024

feat(chain)!: make `CheckPoint` `data` field optional #2024

LagginTimes commented Sep 10, 2025 •

edited

Loading

evanlinjin Jan 8, 2026 •

edited

Loading