Skip to content

chain 0.23.x: Fix panic in scan loop and update msrv and esplora-client#2148

Merged
oleonardolima merged 4 commits intobitcoindevkit:release/chain-0.23.xfrom
evanlinjin:feature/chain-0.23.x-esplora
Mar 11, 2026
Merged

chain 0.23.x: Fix panic in scan loop and update msrv and esplora-client#2148
oleonardolima merged 4 commits intobitcoindevkit:release/chain-0.23.xfrom
evanlinjin:feature/chain-0.23.x-esplora

Conversation

@evanlinjin
Copy link
Member

@evanlinjin evanlinjin commented Mar 11, 2026

Description

Backport of #2053, #2136. Also fixes CI MSRV issues.

Replaces #2147 - Purely MSRV fixes still resulted in CI failure due to clippy.
Replaces #2141 - Might as well include the whole PR backport.

Changelog notice

Fixed:
- Avoid a panic in the Esplora stop‑gap scan loop by tracking consecutive unused scripts to compute the gap boundary.
- Bump `esplora_client` to `0.12.3` so that the `.get_block_infos` method is always available.

Checklists

All Submissions:

@evanlinjin evanlinjin self-assigned this Mar 11, 2026
@evanlinjin evanlinjin added this to the Chain 0.23.3 milestone Mar 11, 2026
@evanlinjin evanlinjin changed the title Fix panic in scan loop and update msrv and esplora-client chain 0.23.x: Fix panic in scan loop and update msrv and esplora-client Mar 11, 2026
@evanlinjin evanlinjin mentioned this pull request Mar 11, 2026
1 task
@evanlinjin evanlinjin marked this pull request as ready for review March 11, 2026 11:04
@evanlinjin evanlinjin moved this to Needs Review in BDK Chain Mar 11, 2026
@evanlinjin
Copy link
Member Author

Code coverage is borked. Can merge without imo.

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 2ee48d7

Copy link
Collaborator

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

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

ACK 2ee48d7

@oleonardolima oleonardolima merged commit d8be40c into bitcoindevkit:release/chain-0.23.x Mar 11, 2026
17 of 19 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Chain Mar 11, 2026
evanlinjin added a commit that referenced this pull request Mar 12, 2026
aff800d fix(chain): correct unconfirmed `ChainPosition` `last_seen` tiebreaker (志宇)
9d2dedc fix(chain): correct ChainPosition ordering for wallet transaction display (志宇)

Pull request description:

  ## Description

  Back-ported from #2074
  Depends on #2148

  This PR fixes incorrect ordering of `ChainPosition` that affected how wallet transactions are displayed. Previously, unconfirmed transactions that were never seen in the mempool would incorrectly appear before transactions with mempool timestamps due to the derived `Ord` implementation treating `None` as less than `Some(_)`.

  ### Problem

  The derived `Ord` implementation for `ChainPosition::Unconfirmed` caused incorrect transaction ordering:
  - `Unconfirmed { first_seen: None, last_seen: None }` (never in mempool)
  - Would appear **before** `Unconfirmed { first_seen: Some(10), last_seen: Some(10) }` (seen in mempool)

  This resulted in a confusing wallet display where transactions never broadcast would appear before pending mempool transactions.

  ### Solution

  Implemented manual `Ord` and `PartialOrd` traits for `ChainPosition` with the correct ordering:

  1. **Confirmed transactions** (earliest first by block height)
  2. **Unconfirmed transactions seen in mempool** (ordered by `first_seen`)
  3. **Unconfirmed transactions never seen** (these come last)

  Additional ordering rules:
  - At the same confirmation height, transitive confirmations come before direct confirmations (as they may actually be confirmed earlier)
  - Tie-breaking uses transaction IDs for deterministic ordering

  ## Changelog notice

  ```md
  ### Fixed
  - Fixed `ChainPosition` ordering so unconfirmed transactions never seen in mempool appear last instead of first

  ### Changed
  - `ChainPosition`, `CanonicalTx`, and `FullTxOut` now require `A: Ord` for their `Ord` implementations
  - Simplified `FullTxOut` ordering to only use essential fields (chain_position, outpoint, spent_by)
  ```

  ## Checklists

  ### All Submissions:

  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)

  ### Bugfixes:

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

Top commit has no ACKs.

Tree-SHA512: eccdf30a47c7e8ef40312f1dd4bf72ce42a3e26a1dd1e6d1f7dc4776350c99029408474439730256414bd0f173e755977a3e3deb81617a930e40ca7962bd490b
evanlinjin added a commit that referenced this pull request Mar 12, 2026
…` in `Anchor` implementation for `&A`

00d7f62 fix(chain): forward `confirmation_height_upper_bound` in `Anchor` impl for `&A` (志宇)

Pull request description:

  ### Description

  Backport of #2120
  Depends on #2148

  The blanket `Anchor` impl for `&A` was missing the `confirmation_height_upper_bound` method, causing it to fall back to the default implementation instead of delegating to the inner type.

  ### Changelog notice

  ```md
  Fixed:

  - The `Anchor::confirmation_height_upper_bound` impl was missing for `&A`, causing it to fallback to the default impl.
  ```

  ### Checklists

  #### All Submissions:

  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)

ACKs for top commit:
  luisschwab:
    ACK 00d7f62
  notmandatory:
    ACK 00d7f62

Tree-SHA512: 919ecc07e82e4c902ba30b576a4617ff63afc17eb4176410dac9abdbec5425a0ba7139312768365386c1d59bd7eb6b733bbae45c8a25d628996135dd226f715f
evanlinjin added a commit that referenced this pull request Mar 12, 2026
…confirmed

43850b5 fix(chain): Tx assumed to be canonical will not be forced unconfimred (志宇)

Pull request description:

  ### Description

  Fixes #2088
  Depends on #2148
  Backport of #2110

  ### Changelog notice

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

  ### 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

  #### Bugfixes:

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

ACKs for top commit:
  luisschwab:
    ACK 43850b5
  notmandatory:
    ACK 43850b5

Tree-SHA512: 72fa42111a470e4ab52b92213b54d5f6a1521e4994258a41764b18c141fcbc47d4ff58d9e5cdb5cd4dfae151e80e26b64eefc8e735e6c71768e114cefcc3ef8c
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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants