fix(key-wallet): prune observed spends during the initial sync by ZocoLini · Pull Request #1014 · dashpay/rust-dashcore

ZocoLini · 2026-09-12T12:27:00Z

observed_spent_outpoints records every input of every transaction in every block the wallet processes, false positives included, and is pruned only up to a chainlock the wallet has applied. The client defers chainlocks until SyncComplete, so nothing is pruned during an initial sync. On a mainnet restore of the bench wallet the map reached 1 487 967 entries (every input of 10 295 blocks, 505 946 transactions), above the 1 000 000 entries the serde adapter accepts on load, so a wallet persisted mid-sync could not be loaded back.

The dispatcher still defers applying chainlocks, but now passes the deferred chainlock's height on through note_chain_lock_height. The wallet uses it only as a finality boundary for pruning: entries at or below min(synced_height, highest chainlock applied or noted) are evicted. As before, only chain-locked spends are forgotten, and no record is promoted.

Pruning mid-sync opened one path: redelivering a funding transaction whose output sits in spent_before_funded, after its observed-spend entry was evicted, re-inserted the spent coin into utxos. update_utxos now keeps such an output held; the new test fails without that guard.

Validated with 5 consecutive mainnet restores at 100 Mbit / 100 ms

Closes #899

Summary by CodeRabbit

Bug Fixes
- Prevented previously spent wallet outputs from reappearing in the UTXO set during transaction reprocessing, rescans, or mempool-to-block transitions.
- Wallets now recognize newly observed chain locks earlier, allowing finalized spent-output records to be pruned sooner and more accurately.
Tests
- Added coverage for chain-lock-based pruning and ensuring spent outputs remain excluded after pruning.

PR Hygiene · `47f7edd`

Bots — coderabbitai ✓
Self-review — post /self-reviewed
Within your 5 open PRs
Build green
Approvals
- dash-spv (dash-spv/src/client/event_handler.rs) — QuantumExplorer or xdustinface
- key-wallet-manager (key-wallet-manager/src/process_block.rs, key-wallet-manager/src/wallet_interface.rs) — QuantumExplorer or xdustinface
- key-wallet (key-wallet/src/managed_account/managed_core_funds_account.rs, key-wallet/src/tests/observed_spent_outpoints_tests.rs, key-wallet/src/wallet/managed_wallet_info/mod.rs and 1 more) — QuantumExplorer or xdustinface

When every box is checked the PR Hygiene check passes and this can merge.

coderabbitai · 2026-09-12T12:27:10Z

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: dashpay/rust-dashcore/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d838b36a-18b8-4eef-965d-a9daaad7815e

📥 Commits

Reviewing files that changed from the base of the PR and between 3a427a4 and 47f7edd.

📒 Files selected for processing (82)

.github/CODEOWNERS
.github/workflows/pr-review-policy.yml
dash-spv/Cargo.toml
dash-spv/benches/storage.rs
dash-spv/src/client/event_handler.rs
dash-spv/src/network/addrv2.rs
dash-spv/src/network/discovery.rs
dash-spv/src/network/reputation.rs
dash-spv/src/sync/mempool/manager.rs
dash-spv/tests/dashd_sync/tests_restart.rs
dash-spv/tests/dashd_sync/tests_transaction.rs
dash/Cargo.toml
dash/examples/ecdsa-psbt.rs
dash/examples/handshake.rs
dash/examples/taproot-psbt.rs
dash/src/address.rs
dash/src/blockdata/script/borrowed.rs
dash/src/blockdata/script/builder.rs
dash/src/blockdata/script/owned.rs
dash/src/bloom/filter.rs
dash/src/consensus/encode.rs
dash/src/crypto/key.rs
dash/src/crypto/sighash.rs
dash/src/crypto/taproot.rs
dash/src/merkle_tree/block.rs
dash/src/sign_message.rs
dash/src/signer.rs
dash/src/taproot.rs
dash/src/test_utils/address.rs
key-wallet-ffi/Cargo.toml
key-wallet-ffi/src/derivation.rs
key-wallet-ffi/src/transaction.rs
key-wallet-ffi/src/tx_decode.rs
key-wallet-ffi/tests/test_valid_addr.rs
key-wallet-manager/src/process_block.rs
key-wallet-manager/src/wallet_interface.rs
key-wallet/Cargo.toml
key-wallet/examples/account_types.rs
key-wallet/examples/basic_usage.rs
key-wallet/src/account/account_collection_test.rs
key-wallet/src/account/mod.rs
key-wallet/src/bip32.rs
key-wallet/src/bip38.rs
key-wallet/src/bip38_tests.rs
key-wallet/src/derivation.rs
key-wallet/src/derivation_bls_bip32.rs
key-wallet/src/dip9.rs
key-wallet/src/gap_limit.rs
key-wallet/src/managed_account/address_pool.rs
key-wallet/src/managed_account/managed_account_collection.rs
key-wallet/src/managed_account/managed_account_trait.rs
key-wallet/src/managed_account/managed_account_type.rs
key-wallet/src/managed_account/managed_core_funds_account.rs
key-wallet/src/mnemonic.rs
key-wallet/src/psbt/mod.rs
key-wallet/src/psbt/serialize.rs
key-wallet/src/seed.rs
key-wallet/src/tests/account_tests.rs
key-wallet/src/tests/address_pool_tests.rs
key-wallet/src/tests/address_reservation_tests.rs
key-wallet/src/tests/dashpay_contact_gap_limit_tests.rs
key-wallet/src/tests/mod.rs
key-wallet/src/tests/observed_spent_outpoints_tests.rs
key-wallet/src/tests/performance_tests.rs
key-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/src/wallet/accounts.rs
key-wallet/src/wallet/bip38.rs
key-wallet/src/wallet/helper.rs
key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs
key-wallet/src/wallet/managed_wallet_info/mod.rs
key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
key-wallet/src/wallet/root_extended_keys.rs
key-wallet/tests/address_tests.rs
key-wallet/tests/bip32_tests.rs
key-wallet/tests/derivation_tests.rs
key-wallet/tests/psbt.rs
masternode-seeds-fetcher/Cargo.toml
masternode-seeds-fetcher/src/main.rs
rpc-client/src/client.rs
rpc-integration-test/src/main.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: bc569c1b-b009-45d0-b623-894c91a822a9

📥 Commits

Reviewing files that changed from the base of the PR and between ed0df9d and 3a427a4.

📒 Files selected for processing (7)

dash-spv/src/client/event_handler.rs
key-wallet-manager/src/process_block.rs
key-wallet-manager/src/wallet_interface.rs
key-wallet/src/managed_account/managed_core_funds_account.rs
key-wallet/src/tests/observed_spent_outpoints_tests.rs
key-wallet/src/wallet/managed_wallet_info/mod.rs
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The change propagates buffered chainlock heights to wallets, enables observed-spend pruning before chainlock application, and prevents reprocessed spent outputs from returning to the UTXO set. Tests cover both behaviors.

Changes

Chainlock-aware wallet pruning

Layer / File(s)	Summary
Propagate buffered chainlock heights `dash-spv/src/client/event_handler.rs`, `key-wallet-manager/src/...`	Wallet interfaces now accept chainlock height notifications. The wallet manager forwards heights to all managed wallets. Buffered cycle-0 chainlocks notify wallets when a newer height arrives.
Track noted heights and prune spends `key-wallet/src/wallet/managed_wallet_info/...`	`ManagedWalletInfo` stores the highest noted chainlock height. Observed-spend pruning uses the higher of noted and applied chainlock heights, capped by `synced_height`.
Preserve spent-before-funded outputs `key-wallet/src/managed_account/...`, `key-wallet/src/tests/...`	`update_utxos` excludes outputs already recorded in `spent_before_funded`. Tests cover pruning from a noted chainlock and reprocessing a funding transaction.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 3a427

The available evidence identifies no confirmed behavior that would block merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the primary change: pruning observed spends during the initial key-wallet synchronization. It is concise and related to the changeset.
Linked Issues check	✅ Passed	The changes address issue `#899`. Buffered chainlocks now report their heights through `note_chain_lock_height`. `ManagedWalletInfo` prunes `observed_spent_outpoints` up to `min(synced_height, highest a…
Out of Scope Changes check	✅ Passed	The changes stay within issue `#899`. The wallet-interface and SPV changes deliver buffered chainlock heights to the wallet. The `update_utxos` guard and its test preserve UTXO correctness when pruning …

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/prune-observed-spends-during-sync

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.

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 · 2026-09-12T12:43:22Z

Codecov Report

❌ Patch coverage is 63.63636% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.26%. Comparing base (859f0ac) to head (47f7edd).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines	Patch %	Lines
key-wallet-manager/src/process_block.rs	0.00%	5 Missing ⚠️
key-wallet-manager/src/wallet_interface.rs	0.00%	1 Missing ⚠️
key-wallet/src/wallet/managed_wallet_info/mod.rs	83.33%	1 Missing ⚠️
...allet/managed_wallet_info/wallet_info_interface.rs	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1014      +/-   ##
==========================================
+ Coverage   76.02%   77.26%   +1.23%     
==========================================
  Files         255      329      +74     
  Lines       58413    83733   +25320     
==========================================
+ Hits        44410    64696   +20286     
- Misses      14003    19037    +5034

Flag	Coverage Δ
core	`78.24% <ø> (ø)`
ffi	`50.52% <ø> (ø)`
rpc	`20.00% <ø> (ø)`
spv	`92.24% <100.00%> (+0.10%)`	⬆️
wallet	`80.03% <61.90%> (?)`

Files with missing lines	Coverage Δ
dash-spv/src/client/event_handler.rs	`93.75% <100.00%> (-0.27%)`	⬇️
.../src/managed_account/managed_core_funds_account.rs	`87.56% <100.00%> (ø)`
key-wallet-manager/src/wallet_interface.rs	`9.09% <0.00%> (ø)`
key-wallet/src/wallet/managed_wallet_info/mod.rs	`74.24% <83.33%> (ø)`
...allet/managed_wallet_info/wallet_info_interface.rs	`80.82% <85.71%> (ø)`
key-wallet-manager/src/process_block.rs	`92.24% <0.00%> (ø)`

... and 74 files with indirect coverage changes

github-actions · 2026-09-20T14:24:03Z

Ready for review — needs QuantumExplorer or xdustinface.
Full checklist in the description.

ZocoLini · 2026-09-20T16:51:11Z

/self-reviewed

`observed_spent_outpoints` records every input of every transaction in every block the wallet processes, false positives included, and is pruned only up to a chainlock the wallet has applied. The client defers chainlocks until `SyncComplete`, so nothing is pruned during an initial sync. On a mainnet restore of the bench wallet the map reached 1 487 967 entries (every input of 10 295 blocks, 505 946 transactions), above the 1 000 000 entries the serde adapter accepts on load, so a wallet persisted mid-sync could not be loaded back. The dispatcher still defers applying chainlocks, but now passes the deferred chainlock's height on through `note_chain_lock_height`. The wallet uses it only as a finality boundary for pruning: entries at or below min(synced_height, highest chainlock applied or noted) are evicted. As before, only chain-locked spends are forgotten, and no record is promoted. Pruning mid-sync opened one path: redelivering a funding transaction whose output sits in `spent_before_funded`, after its observed-spend entry was evicted, re-inserted the spent coin into `utxos`. `update_utxos` now keeps such an output held; the new test fails without that guard. Validated with 5 consecutive mainnet restores at 100 Mbit / 100 ms, all ending at 7112 records, 14 114 383 sat and 13 389 addresses. Peak RSS was 1378–1724 MiB against 1488 MiB before, within run-to-run spread. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf

With the committed-height release gone, the segment cache itself has to bound the resident set. Ten 50 000-item segments per cache kept most of a restore in memory; two keep the peak below what the release achieved, without costing time. Mainnet restore, mainnet.100mbi.100ms, #1015/#1016/#1014 applied, jemalloc heap profiling, wallet identical in every run (14114383 sat, 7112 records, 13389 addresses): segments peak RSS time segment loads from disk (headers/filter headers/filters/blocks) 10 + #946 982 MiB 8.2 min - 10 1490 MiB 8.8 min 84 / 2 / 28 / 0 2 930 MiB 8.8 min 212 / 103 / 74 / 0 1 1000 MiB 7.7 min 427 / 742 / 156 / 33 Each cache served ~2.34M requests from memory in every run. One segment starts reloading block segments of up to 87 MB from disk, which raises the peak again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf

A dirty segment leaving the resident set used to wait in `evicted`, in memory, until the next 5 s storage tick wrote it. The resident limit was therefore not a limit: every segment evicted between two ticks stayed in memory. Now eviction writes the segment first and only then drops it; if the write fails the segment stays resident and the error is returned. The `evicted` map is gone, and the tick only persists the resident segments. Mainnet restore, mainnet.100mbi.100ms, #1015/#1016/#1014 applied, two resident segments: 7.7 and 7.9 min, peak RSS 943 and 948 MiB, wallet identical (14114383 sat, 7112 records, 13389 addresses). No cache misses served from `evicted` any more; header, filter header and filter segments are reloaded from disk about as often as they used to come back from `evicted` (~270 / ~150 / ~90), block segments never. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf

… one buffer `Segment::persist` encoded the whole segment into a `Vec` before handing it to `atomic_write`. For a block segment near the tip (up to 87 MB on disk) that `Vec` grew to 128 MiB, and since dirty segments are now written on eviction this happened in the middle of the sync. `atomic_write_items` encodes one item at a time through a 1 MiB `BufWriter` into the temporary file, then syncs and renames it as before. `atomic_write` keeps its behaviour and shares the temporary file and rename logic. Mainnet restore, mainnet.100mbi.100ms, #1015/#1016/#1014 applied, two resident segments, wallet identical in every run (14114383 sat, 7112 records, 13389 addresses): whole-segment buffer peak RSS 943 / 948 MiB 7.7 / 7.9 min streamed peak RSS 845 / 855 / 856 MiB 9.4 / 7.7 / 6.5 min The persist buffer, 68–133 MiB in the earlier heap snapshots, no longer shows up. The 9.4 min run lost time to peers in the header and filter header phases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf

Every segment cache held 50 000 items, whatever an item costs. Near the tip a block segment holds tens of MB of decoded blocks, while a header segment spanning the same heights is 5.6 MB and a filter header one 1.6 MB. `Persistable::ITEMS_PER_SEGMENT` lets each type choose: headers 10 000 (~1.1 MB per segment), filter headers 50 000 (~1.6 MB), filters 2 000 (~2 MB near the tip) and blocks 1 000. This changes the on-disk layout of the header, filter and block segments. Storage written with 50 000-item segments is misread by this layout and has to be deleted until a migration or a versioned folder lands. Mainnet restore, mainnet.100mbi.100ms, #1015/#1016/#1014 applied, two resident segments, jemalloc heap profiling, wallet identical in every run (14114383 sat, 7112 records, 13389 addresses): segment items time peak RSS segment caches 50 000 for every type 7.0 min 818 MiB 332 MiB 5 000 for every type 7.3-8.3 min 554-687 MiB 23-106 MiB 1 000 for every type 9.2-12.2 min 453-577 MiB 2-66 MiB per type (this commit) 7.5 min 538 MiB 38 MiB With 1 000 items everywhere, the header and filter header phases paid an fsync per evicted segment (105-141 s and 254-332 s instead of ~60 s and ~150 s). Blocks at 500 items saved ~18 MiB of cache but reloaded 50 % more block segments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf

github-actions · 2026-09-22T16:40:50Z

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

ZocoLini requested review from QuantumExplorer and xdustinface as code owners September 12, 2026 12:27

ZocoLini mentioned this pull request Sep 15, 2026

perf(dash-spv): reduce ram usage #1024

Draft

ZocoLini force-pushed the fix/prune-observed-spends-during-sync branch from 026fba0 to 3a427a4 Compare September 16, 2026 08:35

coderabbitai Bot approved these changes Sep 16, 2026

View reviewed changes

github-actions Bot added the ready-for-review CodeRabbit has approved this PR label Sep 16, 2026

github-actions Bot added the waiting-self-review Waiting for the author to post /self-reviewed label Sep 20, 2026

github-actions Bot added ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. and removed waiting-self-review Waiting for the author to post /self-reviewed labels Sep 20, 2026

ZocoLini force-pushed the fix/prune-observed-spends-during-sync branch from 3a427a4 to 47f7edd Compare September 22, 2026 16:05

github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. ready-for-review CodeRabbit has approved this PR labels Sep 22, 2026

ZocoLini closed this Sep 22, 2026

ZocoLini reopened this Sep 22, 2026

ZocoLini merged commit 6533320 into dev Sep 23, 2026
62 of 70 checks passed

ZocoLini deleted the fix/prune-observed-spends-during-sync branch September 23, 2026 17:16

Navigation Menu

fix(key-wallet): prune observed spends during the initial sync - #1014

fix(key-wallet): prune observed spends during the initial sync#1014
ZocoLini merged 1 commit into
devfrom
fix/prune-observed-spends-during-sync

ZocoLini commented Sep 12, 2026 •

edited by github-actions Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 12, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Sep 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 20, 2026 •

edited

Loading

Uh oh!

ZocoLini commented Sep 20, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ZocoLini commented Sep 12, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

PR Hygiene · 47f7edd

Uh oh!

coderabbitai Bot commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZocoLini commented Sep 20, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ZocoLini commented Sep 12, 2026 •

edited by github-actions Bot

Loading

PR Hygiene · `47f7edd`

coderabbitai Bot commented Sep 12, 2026 •

edited

Loading

codecov Bot commented Sep 12, 2026 •

edited

Loading

github-actions Bot commented Sep 20, 2026 •

edited

Loading

github-actions Bot commented Sep 22, 2026 •

edited

Loading