Skip to content

fix(key-wallet): stop pruning spent CoinJoin addresses from the filter scan - #985

Draft
ZocoLini wants to merge 1 commit into
devfrom
fix/coinjoin-scan-keeps-spent-addresses
Draft

fix(key-wallet): stop pruning spent CoinJoin addresses from the filter scan#985
ZocoLini wants to merge 1 commit into
devfrom
fix/coinjoin-scan-keeps-spent-addresses

Conversation

@ZocoLini

Copy link
Copy Markdown
Collaborator

#949 dropped a CoinJoin address from the forward compact-filter query once it was used and held no unspent output, on the grounds that CoinJoin addresses are single-use by protocol, so nothing would ever pay one again.

Mainnet does not honour that. On a restore of a mixing-heavy wallet, 287 CoinJoin addresses were paid by more than one transaction — verified on the scriptPubKey, not on a derivation label — some of them nearly 100 000 blocks after being emptied. Eight of those fell in a window where the wallet had already stopped watching them, and the scan never saw the payments:

9 transactions missed between heights 2 170 285 and 2 170 440
5 outputs never recorded (80 000 sat)
9 spends never recorded (176 780 sat)
4 already-spent outputs left on the books as spendable
balance overstated by 96 780 sat

Reproduced identically across four full syncs and by two independent methods (an A/B diff of per-address dumps, and an end-of-sync re-match of the whole chain against still-unspent scripts, which named the same heights and the same addresses without being told what to look for).

The saving did not pay for that. Pruning shrinks the query sixfold at the tip (13 378 to 2 200 scripts), but over a full mainnet restore it moves the sync by 7.7% in blocks and 2.3% in time — inside the run-to-run spread, with the fastest of the four runs being an unpruned one.

ManagedWalletInfo therefore no longer overrides scan_script_pubkeys, so the scan query is the monitored set again. The plumbing stays: a narrower query is still worth having if someone can show which scripts are genuinely unpayable. ManagedCoreFundsAccount::unspent_or_unused_script_pubkeys also stays, now documented as a description of the single-use discipline rather than as something safe to scan from, and the benchmark keeps measuring the two query shapes against each other so the prize stays visible.

The regression test walks the whole shape: pay a CoinJoin address, spend the output, then build a real BIP158 filter over the block carrying a second payment to it and require the scan query to match. It fails on the pruned query.

@coderabbitai

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
auto_review:
drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecovBot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.19%. Comparing base (237f79a) to head (ec214ba).

Additional details and impacted files
@@ Coverage Diff @@## dev #985 +/- ##
==========================================
+ Coverage 77.14% 77.19% +0.05% 
==========================================
Files 329 329 Lines 82998 83056 +58 ==========================================
+ Hits 64026 64117 +91 + Misses 18972 18939 -33 
FlagCoverage Δ
core78.25% <ø> (ø)
ffi52.40% <ø> (ø)
rpc20.00% <ø> (ø)
spv92.14% <ø> (+0.14%)⬆️
wallet79.28% <100.00%> (+0.06%)⬆️
Files with missing linesCoverage Δ
dash-spv/src/sync/filters/manager.rs97.91% <ø> (ø)
key-wallet-manager/src/process_block.rs93.33% <100.00%> (+1.02%)⬆️
key-wallet-manager/src/wallet_interface.rs9.37% <ø> (ø)
.../src/managed_account/managed_core_funds_account.rs87.77% <ø> (ø)
...allet/managed_wallet_info/wallet_info_interface.rs80.70% <ø> (+0.23%)⬆️

... and 4 files with indirect coverage changes

…r scan
#949 dropped a CoinJoin address from the forward compact-filter query once it
was used and held no unspent output, on the grounds that CoinJoin addresses
are single-use by protocol, so nothing would ever pay one again.
Mainnet does not honour that. On a restore of a mixing-heavy wallet, 287
CoinJoin addresses were paid by more than one transaction — verified on the
scriptPubKey, not on a derivation label — some of them nearly 100 000 blocks
after being emptied. Eight of those fell in a window where the wallet had
already stopped watching them, and the scan never saw the payments:
9 transactions missed between heights 2 170 285 and 2 170 440
5 outputs never recorded (80 000 sat)
9 spends never recorded (176 780 sat)
4 already-spent outputs left on the books as spendable
balance overstated by 96 780 sat
Reproduced identically across four full syncs and by two independent methods
(an A/B diff of per-address dumps, and an end-of-sync re-match of the whole
chain against still-unspent scripts, which named the same heights and the
same addresses without being told what to look for).
The saving did not pay for that either. Pruning shrinks the query sixfold at
the tip (13 378 to 2 200 scripts), but over a full mainnet restore it moves
the sync by 7.7% in blocks and 2.3% in time — inside the run-to-run spread,
with the fastest of the four runs being an unpruned one.
So this is a full revert of the mechanism, not a switch left off.
ManagedWalletInfo no longer overrides scan_script_pubkeys, and
ManagedCoreFundsAccount::unspent_or_unused_script_pubkeys goes with it: after
the override, nothing in production called it. The seam in the trait stays,
documented with what happened here, so a future narrower query has to earn
its place rather than inherit one.
The benchmark keeps measuring the query the scan actually runs and no longer
measures the one it does not.
The regression test walks the whole shape: pay a CoinJoin address, spend the
output, then build a real BIP158 filter over the block carrying a second
payment to it and require the scan query to match. It fails on the pruned
query.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015NhHBDGiKfiGpy7FwooyfS
@ZocoLini
ZocoLiniforce-pushed the fix/coinjoin-scan-keeps-spent-addresses branch from ec214ba to aa2b6fbCompareAugust 26, 2026 15:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@ZocoLini