Skip to content

chain 0.23.x: Fix assumed canonical tx always being unconfirmed#2150

Merged
evanlinjin merged 1 commit intobitcoindevkit:release/chain-0.23.xfrom
evanlinjin:fix/assumed-canonical-check-anchor
Mar 12, 2026
Merged

chain 0.23.x: Fix assumed canonical tx always being unconfirmed#2150
evanlinjin merged 1 commit intobitcoindevkit:release/chain-0.23.xfrom
evanlinjin:fix/assumed-canonical-check-anchor

Conversation

@evanlinjin
Copy link
Member

@evanlinjin evanlinjin commented Mar 11, 2026

Description

Fixes #2088
Depends on #2148
Backport of #2110

Changelog notice

Fixed:
- Previously, assumed-canonical transactions always became unconfirmed even though the transaction may be anchored in the best chain.

Checklists

All Submissions:

New Features:

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

Bugfixes:

  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

Previously there was a bug where txs added in
`CanonicalizationParams::assume_canonical` were forced to always be
unconfirmed (even if it's anchored in the best chain). This commit fixes
this.

`test_assumed_canonical_with_anchor_is_confirmed` is added to ensure
expected behavior.

Co-authored-by: AayushGupta56 <aayushgupt56@gmail.com>
@oleonardolima oleonardolima force-pushed the fix/assumed-canonical-check-anchor branch from 8d65899 to 43850b5 Compare March 11, 2026 17:19
@oleonardolima oleonardolima added the backport A bug fix or security patch to be ported to a previous release label Mar 11, 2026
Copy link
Member

@luisschwab luisschwab left a comment

Choose a reason for hiding this comment

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

ACK 43850b5

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 43850b5

I ran this through my LLM 🤖 and no problems were found. The only small issue flagged is that CanonicalReason::Assumed.descendant isn't now not used or needed. But since this is a non-api-breaking back-port it makes perfect sense to leave it as is. Also that type is removed in #2038 as part of the refactoring there.

@evanlinjin evanlinjin merged commit b5173b1 into bitcoindevkit:release/chain-0.23.x Mar 12, 2026
17 of 18 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Chain Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport A bug fix or security patch to be ported to a previous release bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants