feat(dash-spv): prune spent single-use CoinJoin addresses from the filter scan query - #949
Merged
Merged
Conversation
…lter scan query During filter sync, check_compact_filters_for_elements re-hashes and re-sorts the whole query set per filter (BIP158 keys SipHashes off the block hash), so per-filter cost grows with the monitored script count. For CoinJoin wallets that count grows monotonically through the scan — every mixing round pays a fresh single-use address — so late-scan filters cost several times more than early ones, concentrated in the wallet's dense activity region. CoinJoin addresses are single-use by protocol (reuse would link mixing rounds): once an address is used and holds no unspent output, nothing ever pays it again, so it contributes nothing to a forward scan. Drop such addresses from the scan query, keeping it roughly bounded by active UTXOs + gap lookahead instead of total historical addresses. - key-wallet: ManagedCoreFundsAccount::unspent_or_unused_script_pubkeys plus WalletInfoInterface::scan_script_pubkeys (default = full monitored set; ManagedWalletInfo prunes CoinJoin accounts only) - key-wallet-manager: WalletInterface::scan_script_pubkeys_for (default = monitored_script_pubkeys_for) - dash-spv: scan_batch queries the scan set; rescan_batch (freshly derived scripts) and block processing keep the full monitored set Fixes #948 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds wallet-specific compact-filter scan queries. Used, empty CoinJoin scripts are excluded. Unused and funded scripts remain. Full monitored scripts remain available for block processing. ChangesScan query pruning
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FilterManager
participant WalletInterface
participant ManagedWalletInfo
FilterManager->>WalletInterface: scan_script_pubkeys_for(wallet_id)
WalletInterface->>ManagedWalletInfo: scan_script_pubkeys()
ManagedWalletInfo-->>WalletInterface: unspent or unused scripts
WalletInterface-->>FilterManager: scan script set
FilterManager->>FilterManager: match compact filters
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
QuantumExplorer
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs`:
- Around line 93-105: Update the pull request title to use the supported
Conventional Commit prefix `feat:`, resulting in `feat: pruned compact-filter
scan queries`.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0792ff1e-ddaf-4768-a117-ae4752808fe5
📒 Files selected for processing (8)
dash-spv/src/sync/filters/manager.rskey-wallet-manager/src/process_block.rskey-wallet-manager/src/test_utils/mock_wallet.rskey-wallet-manager/src/wallet_interface.rskey-wallet/src/managed_account/managed_core_funds_account.rskey-wallet/src/tests/mod.rskey-wallet/src/tests/scan_script_pubkeys_tests.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #949 +/- ##
==========================================
+ Coverage 75.40% 75.42% +0.02%
==========================================
Files 328 328
Lines 78541 78630 +89
==========================================
+ Hits 59222 59305 +83
- Misses 19319 19325 +6
|
…CoinJoin wallets Criterion bench that mimics a wallet mid-recovery after many CoinJoin rounds: `used` spent single-use addresses on the external branch, 200 still-funded denominations, and the default gap lookahead. One 512-filter scan batch is matched with the full monitored query (pre-#948) and the pruned scan query. Measured on Apple Silicon (single-threaded, default features): used=500 monitored 855 scripts 5.21ms | pruned 555 scripts 3.33ms (1.6x) used=2000 monitored 2355 scripts 15.2ms | pruned 555 scripts 3.54ms (4.3x) used=6000 monitored 6355 scripts 45.8ms | pruned 555 scripts 3.30ms (13.9x) The pruned query stays flat as mixing history grows, while the monitored query's per-batch cost scales super-linearly with total historical addresses — the effect profiled in #948. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Added a criterion benchmark ( Results on Apple Silicon (single-threaded, default features):
The pruned query stays flat as mixing history grows, while the monitored query's per-batch cost scales super-linearly with total historical addresses (per-filter SipHash re-key + re-sort) — the effect profiled in #948. Run it with: 🤖 Generated with Claude Code |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@key-wallet-manager/benches/filter_scan.rs`:
- Around line 37-39: Remove the hardcoded MNEMONIC constant and update the
benchmark setup in the surrounding filter-scan benchmark to generate mnemonic
entropy with a cryptographically secure random number generator before b.iter
begins. Preserve the benchmark’s wallet derivation and timed matching path while
ensuring each setup generates fresh secure entropy instead of deterministic
keys.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 86420851-0af5-4f76-8c52-37cdb61e7a54
📒 Files selected for processing (2)
key-wallet-manager/Cargo.tomlkey-wallet-manager/benches/filter_scan.rs
… mnemonic Drop the hardcoded BIP39 test mnemonic from the filter-scan bench; the workload is defined by pool/UTXO counts, not key material, so a fresh random mnemonic per run keeps timings comparable while following the no-hardcoded-keys guideline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xdustinface
approved these changes
Aug 11, 2026
ZocoLini
pushed a commit
that referenced
this pull request
Aug 11, 2026
…lter scan query (#949) * perf(dash-spv): prune spent single-use CoinJoin addresses from the filter scan query During filter sync, check_compact_filters_for_elements re-hashes and re-sorts the whole query set per filter (BIP158 keys SipHashes off the block hash), so per-filter cost grows with the monitored script count. For CoinJoin wallets that count grows monotonically through the scan — every mixing round pays a fresh single-use address — so late-scan filters cost several times more than early ones, concentrated in the wallet's dense activity region. CoinJoin addresses are single-use by protocol (reuse would link mixing rounds): once an address is used and holds no unspent output, nothing ever pays it again, so it contributes nothing to a forward scan. Drop such addresses from the scan query, keeping it roughly bounded by active UTXOs + gap lookahead instead of total historical addresses. - key-wallet: ManagedCoreFundsAccount::unspent_or_unused_script_pubkeys plus WalletInfoInterface::scan_script_pubkeys (default = full monitored set; ManagedWalletInfo prunes CoinJoin accounts only) - key-wallet-manager: WalletInterface::scan_script_pubkeys_for (default = monitored_script_pubkeys_for) - dash-spv: scan_batch queries the scan set; rescan_batch (freshly derived scripts) and block processing keep the full monitored set Fixes #948 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(key-wallet-manager): benchmark filter matching for mixing-heavy CoinJoin wallets Criterion bench that mimics a wallet mid-recovery after many CoinJoin rounds: `used` spent single-use addresses on the external branch, 200 still-funded denominations, and the default gap lookahead. One 512-filter scan batch is matched with the full monitored query (pre-#948) and the pruned scan query. Measured on Apple Silicon (single-threaded, default features): used=500 monitored 855 scripts 5.21ms | pruned 555 scripts 3.33ms (1.6x) used=2000 monitored 2355 scripts 15.2ms | pruned 555 scripts 3.54ms (4.3x) used=6000 monitored 6355 scripts 45.8ms | pruned 555 scripts 3.30ms (13.9x) The pruned query stays flat as mixing history grows, while the monitored query's per-batch cost scales super-linearly with total historical addresses — the effect profiled in #948. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(key-wallet-manager): generate the benchmark wallet from a random mnemonic Drop the hardcoded BIP39 test mnemonic from the filter-scan bench; the workload is defined by pool/UTXO counts, not key material, so a fresh random mnemonic per run keeps timings comparable while following the no-hardcoded-keys guideline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 13, 2026
ZocoLini
added a commit
that referenced
this pull request
Aug 26, 2026
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #948.
Problem
check_compact_filters_for_elementsmatches the union of every monitored script against every filter, and BIP158 keys each filter's SipHashes off the block hash, so the whole query set is re-hashed and re-sorted per filter —O(M·hash + M log M)with nothing cacheable across filters. For CoinJoin walletsMgrows monotonically through the scan (every mixing round pays a fresh single-use address), so late-scan filters cost several times more than early ones, exactly in the wallet's dense activity region. Profiling in the issue shows the filters phase is matching-bound (~88% of on-CPU samples), not block-bound.Change
CoinJoin addresses are single-use by protocol — reuse would link mixing rounds — so once an address is used and all its outputs are spent, nothing ever pays it again in practice. This PR drops such spent-and-empty CoinJoin addresses from the forward-scan filter query, keeping the query roughly
O(active UTXOs + gap lookahead)instead ofO(total historical addresses). This is the conservative variant proposed in the issue: only CoinJoin-account scripts are pruned, since their single-use property is protocol-driven; every other account type keeps its full monitored set.Layering:
ManagedCoreFundsAccount::unspent_or_unused_script_pubkeys()(keep a script if its address is unused — gap window, including reserved — or still holds a UTXO), andWalletInfoInterface::scan_script_pubkeys()with the full monitored set as default; theManagedWalletInfooverride applies the pruning to CoinJoin accounts only.WalletInterface::scan_script_pubkeys_for(wallet_id), defaulting tomonitored_script_pubkeys_for, so existing implementations are unaffected.scan_batchbuilds its union query and per-wallet attribution queries from the scan set.rescan_batch(freshly derived scripts, never dead), block processing, gap-limit maintenance, and the bloom/mempool paths all keep using the full monitored set — pruning only narrows which blocks the filter scan downloads.The theoretical "someone paid an old CoinJoin address after it was emptied" case is deliberately accepted per the issue: it does not occur under the CoinJoin protocol, and only the filter-scan query is affected.
Testing
scan_batchtest proving a block paying only a pruned address is not downloaded while the monitored set still contains it.cargo test -p dash-spvwith dashd regtest integration tests (sync, restart, disconnect, transaction) — all pass.cargo test -p dash-spv-ffi --test dashd_sync— 7/7 pass.contrib/run_clippy.py),cargo fmt, and pre-commit hooks all clean.Related (not addressed here)
The issue's rayon dispatch overhead (~32%) and per-filter query re-sort (~12%) observations are separate follow-ups.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Performance