Skip to content

fix(chain): reject an occupied index in insert_spk - #2335

Open
h55n wants to merge 1 commit into
bitcoindevkit:masterfrom
h55n:fix-spk-txout-occupied-index
Open

h55n wants to merge 1 commit into
bitcoindevkit:masterfrom
h55n:fix-spk-txout-occupied-index

Conversation

@h55n

@h55n h55n commented Oct 1, 2026

Copy link
Copy Markdown

Description

Fixes #2279.

SpkTxOutIndex::insert_spk currently accepts a different script at an existing index. The forward lookup is overwritten while the old reverse lookup remains, so the two maps disagree.

This returns false without changing state when the index is already occupied, matching the existing behavior for an occupied script. The method documentation now covers both cases.

Notes to the reviewers

Added regressions for both lookup directions, unused scripts and previously scanned outputs. A rejected insertion preserves the old output and used status, and the rejected script is not tracked.

Testing on the refreshed master base:

  • Rust 1.85: bdk_chain with all features, 99 tests/doctests pass, 1 ignored.
  • Changed-crate clippy with all targets and features passes with warnings denied.
  • Workspace formatting check passes.
  • Full workspace pre-push suite has not been rerun on this base.

Changelog notice

Reject duplicate indices in SpkTxOutIndex::insert_spk without changing existing script or output state.

Copilot AI balanced review requested due to automatic review settings October 1, 2026 00:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.87%. Comparing base (0790f04) to head (da0dc65).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2335      +/-   ##
==========================================
+ Coverage   78.84%   78.87%   +0.02%     
==========================================
  Files          31       31              
  Lines        6060     6063       +3     
  Branches      288      289       +1     
==========================================
+ Hits         4778     4782       +4     
+ Misses       1203     1202       -1     
  Partials       79       79              
Flag Coverage Δ
rust 78.87% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@evanlinjin evanlinjin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the PR. Looks good. Just some suggestions for doc comments before ACK.

Comment thread crates/chain/src/indexer/spk_txout.rs Outdated
@evanlinjin

Copy link
Copy Markdown
Member

A final request - could you please squash the commits into one? Thanks

Reject an occupied index before changing either lookup map. Preserve existing script, scanned-output and used-status mappings on a rejected insertion, matching the existing behavior for an occupied script.
@h55n
h55n force-pushed the fix-spk-txout-occupied-index branch from 599b5fd to da0dc65 Compare October 2, 2026 00:18
@h55n

h55n commented Oct 2, 2026

Copy link
Copy Markdown
Author

Thanks, squashed into one commit and applied the doc suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

SpkTxOutIndex::insert_spk leaves stale mappings when reusing an index

3 participants