From: merge-script <hello@evanlinjin.me>
Date: Sun, 18 Oct 2026 03:34:48 +0000 (+1100)
Subject: Merge bitcoindevkit/bdk#2265: fix(chain): sanitize and bound-check derivation indices... 
X-Git-Url: http://internal-gitweb-vhost/-client-interface/%22https:/bdk/keys/struct.CheckedHrpstring.html?a=commitdiff_plain;p=bdk

Merge bitcoindevkit/bdk#2265: fix(chain): sanitize and bound-check derivation indices near BIP32_MAX_INDEX

1ca8d29854d41269982a1f47b03603a98a221d99 fix(chain): fix integer overflow in lookahead_to_target (busayo-OD)
de43ac6941bd7196f1421bac3646e5dafea83290 fix(chain): fix off-by-one in replenish_inner_index stop bound (busayo-OD)
803fd968d7999bda911eb2b968cd54a585fac8b4 fix(chain): clamp derivation index before apply_changeset (busayo-OD)

Pull request description:

  Fixes [bdk_wallet/issues/60](https://github.com/bitcoindevkit/bdk_wallet/issues/60)

  ### Description

  `KeychainTxOutIndex::apply_changeset` accepted `ChangeSet::last_revealed` values with no upper bound, allowing an index greater than `BIP32_MAX_INDEX` (2^31 - 1) to be stored, violating an invariant relied on by `next_index()`.

  The changes in this PR address boundary issues around derivation indices:

  1. **`apply_changeset`**: clamps `last_revealed` to `BIP32_MAX_INDEX` before storing it, instead of storing the raw value.
  2. **`replenish_inner_index`**: fixes an off-by-one in the `stop_index` calculation that caused a panic (`"we just inserted it"`) when a keychain's derivation index was revealed exactly up to `BIP32_MAX_INDEX`. The exclusive upper bound was clamped to `BIP32_MAX_INDEX` instead of `BIP32_MAX_INDEX + 1`, silently excluding the boundary value itself.
  3. **`lookahead_to_target`**: fixes an integer overflow panic when called with `target_index == u32::MAX`, and changes out of range targets to clamp and replenish up to `BIP32_MAX_INDEX` instead of silently doing nothing.

  ### Notes to the reviewers

  This revisits the earlier implementation attempt (#1792) and implements the clamp-in-`apply_changeset` approach from that review discussion.

  The second and third fixes were found while adding the boundary tests requested in review of an earlier version of this PR.

  Also added a test documenting current saturation behavior: `reveal_next_spk`/`next_unused_spk` return the last revealed script with an empty `ChangeSet`, not `None`, once the index is saturated. This matches `reveal_next_spk`'s existing doc comment.

  Went through the other index-taking methods (`spk_at_index`, `is_used`, `mark_used`, `unmark_used`) as well. These are plain lookups with no boundary-sensitive logic, so no dedicated tests were added there.

  ### Changelog notice

  - Clamped `last_revealed` to `BIP32_MAX_INDEX` in `KeychainTxOutIndex::apply_changeset` to prevent it accepting an out-of-range derivation index.
  - Fixed an off-by-one in `replenish_inner_index` that could panic when revealing up to `BIP32_MAX_INDEX`.
  - Fixed an integer overflow in `lookahead_to_target` when called with `target_index == u32::MAX`, and changed out of range targets to clamp to `BIP32_MAX_INDEX` instead of silently doing nothing.

  ### Checklists

  **All Submissions**

  - [x] I've signed all my commits
  - [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  - [x]  I ran `just p` before committing

  **Bugfixes**

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

ACKs for top commit:
  evanlinjin:
    ACK 1ca8d29854d41269982a1f47b03603a98a221d99
  notmandatory:
    ACK 1ca8d29854d41269982a1f47b03603a98a221d99

Tree-SHA512: 2e2f86b24772424fe716453f76b5628d55121a70f7fe493f2bed7ca42a662ee705e94d79488f6e71c1d442570f81d863b4f59081bda1cddfbef59435483167e1
---

b13d07acf75b65e8905ffb7d3f37b6c8290428b7