You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The wallet-level observed_spent_outpoints map introduced for #649 (#851) accumulates one entry per input of every transaction in every downloaded block and cannot prune any of it until the initial sync completes. Peak resident size scales with matched-block activity rather than with wallet activity.
This is a memory concern, not a correctness bug. Balance and the UTXO set stay correct, and the map collapses to its steady-state size once the first chainlock is applied.
Mechanism
Two independent facts combine:
Recording is un-narrowed by design.record_observed_spends sits above the relevance gate in key-wallet/src/transaction_checking/wallet_checker.rs, and key-wallet-manager/src/process_block.rs iterates all of block.txdata, so every input of every transaction in a matched block is recorded, including transactions belonging to strangers that merely shared a block with ours. This is deliberate: at spend time the wallet often cannot know whether the coin is its own, which is the whole point of bug: out-of-order block processing causes SPV wallet to miss UTXO spends #649.
Pruning cannot run during initial sync.prune_finalized_observed_spends early-returns while last_applied_chain_lock is None, and dash-spv/src/client/event_handler.rs (spawn_chainlock_wallet_dispatch) deliberately buffers every chainlock during sync cycle 0, calling apply_chain_lock only at SyncComplete { cycle: 0 }. So the update_synced_height prune call is a no-op for the entire historical scan regardless of how many batches commit.
Net: nothing prunes until initial sync finishes, then everything below the finality boundary is evicted at once.
Rough scale
Pessimal case, a CoinJoin-heavy mainnet wallet doing a full sync (assumptions stated, not measured): ~2 years of history at ~2.5 min blocks is ~420k blocks scanned; a large mixing address set pushes the match rate up (assume ~10%, so ~42k matched blocks); a blended ~60 inputs per matched block gives ~2.5M entries. A BTreeMap<OutPoint, CoreBlockHeight> entry is 40 bytes of payload plus node overhead, so budget 60-80 bytes resident: roughly 150-200 MB at peak, and it also serialises into the wallet file. On a memory-constrained mobile client that is a plausible termination rather than a nuisance. An ordinary wallet with a small match rate stays in the low single-digit MB.
MAX_OBSERVED_SPENT_OUTPOINTS = 10_000_000 does not help: it is enforced only on deserialization, so it sits above any realistic peak and never fires. Note the asymmetry, though — a wallet persisted with more entries than the cap would serialize fine and then fail to load.
Fix direction: narrow recording to our own spends, AFTER #866
The map only needs spends of coins the wallet could own. Dash has no segwit, so an input's prevout script is reconstructible from its scriptSig: a P2PKH scriptSig ends with the spender's public key (PublicKey::from_slice → pubkey_hash() → ScriptBuf::new_p2pkh), and a P2SH scriptSig ends with the redeem script (Script::script_hash() → ScriptBuf::new_p2sh). Testing the reconstructed candidates with the account's existing contains_script_pub_key would cut the map to roughly the wallet's own historical spend count — thousands rather than millions. Inputs that yield no derivable candidate (bare multisig, P2PK, non-standard) must fall back to unconditional recording.
This must not land before #866. The ownership test consults the derived script set, so a script not yet derived reads as "not ours" and its spend is skipped. That is reachable today:
Block 100 funds address A at a CoinJoin index beyond the derived window.
Block 200 spends it and is downloaded because it also touches an already-derived script of ours (a sibling denomination in the same mixing transaction).
An out-of-order rescan processes block 200 first. A is not derived, so the narrowing skips recording the spend.
Block 100 is then processed, derives A by gap-limit extension, and inserts the UTXO with nothing left to block it.
That is #649 reopened, in exactly the CoinJoin gap-limit scenario the fix targets. Today's unconditional recording is immune because it never asks whether the coin is ours. #866's committed-range rescan is what closes the hole: once A is derived, the spend block's filter contains A as a prevout element (Dash Core's BasicFilterElements, src/blockfilter.cpp:206-212, adds every prevout scriptPubKey), so the sweep re-delivers the spend block and it gets recorded then.
Alternative worth evaluating separately
Applying tip chainlocks during initial sync rather than deferring them to SyncComplete would make the existing pruning engage and keep the map bounded to the in-flight batch window, with no narrowing and no #866 dependency. It changes the deliberate single-writer promotion timing in spawn_chainlock_wallet_dispatch and would make historical blocks record as InChainLockedBlock earlier, so it needs its own analysis.
Summary
The wallet-level
observed_spent_outpointsmap introduced for #649 (#851) accumulates one entry per input of every transaction in every downloaded block and cannot prune any of it until the initial sync completes. Peak resident size scales with matched-block activity rather than with wallet activity.This is a memory concern, not a correctness bug. Balance and the UTXO set stay correct, and the map collapses to its steady-state size once the first chainlock is applied.
Mechanism
Two independent facts combine:
record_observed_spendssits above the relevance gate inkey-wallet/src/transaction_checking/wallet_checker.rs, andkey-wallet-manager/src/process_block.rsiterates all ofblock.txdata, so every input of every transaction in a matched block is recorded, including transactions belonging to strangers that merely shared a block with ours. This is deliberate: at spend time the wallet often cannot know whether the coin is its own, which is the whole point of bug: out-of-order block processing causes SPV wallet to miss UTXO spends #649.prune_finalized_observed_spendsearly-returns whilelast_applied_chain_lockisNone, anddash-spv/src/client/event_handler.rs(spawn_chainlock_wallet_dispatch) deliberately buffers every chainlock during sync cycle 0, callingapply_chain_lockonly atSyncComplete { cycle: 0 }. So theupdate_synced_heightprune call is a no-op for the entire historical scan regardless of how many batches commit.Net: nothing prunes until initial sync finishes, then everything below the finality boundary is evicted at once.
Rough scale
Pessimal case, a CoinJoin-heavy mainnet wallet doing a full sync (assumptions stated, not measured): ~2 years of history at ~2.5 min blocks is ~420k blocks scanned; a large mixing address set pushes the match rate up (assume ~10%, so ~42k matched blocks); a blended ~60 inputs per matched block gives ~2.5M entries. A
BTreeMap<OutPoint, CoreBlockHeight>entry is 40 bytes of payload plus node overhead, so budget 60-80 bytes resident: roughly 150-200 MB at peak, and it also serialises into the wallet file. On a memory-constrained mobile client that is a plausible termination rather than a nuisance. An ordinary wallet with a small match rate stays in the low single-digit MB.MAX_OBSERVED_SPENT_OUTPOINTS = 10_000_000does not help: it is enforced only on deserialization, so it sits above any realistic peak and never fires. Note the asymmetry, though — a wallet persisted with more entries than the cap would serialize fine and then fail to load.Fix direction: narrow recording to our own spends, AFTER #866
The map only needs spends of coins the wallet could own. Dash has no segwit, so an input's prevout script is reconstructible from its scriptSig: a P2PKH scriptSig ends with the spender's public key (
PublicKey::from_slice→pubkey_hash()→ScriptBuf::new_p2pkh), and a P2SH scriptSig ends with the redeem script (Script::script_hash()→ScriptBuf::new_p2sh). Testing the reconstructed candidates with the account's existingcontains_script_pub_keywould cut the map to roughly the wallet's own historical spend count — thousands rather than millions. Inputs that yield no derivable candidate (bare multisig, P2PK, non-standard) must fall back to unconditional recording.This must not land before #866. The ownership test consults the derived script set, so a script not yet derived reads as "not ours" and its spend is skipped. That is reachable today:
That is #649 reopened, in exactly the CoinJoin gap-limit scenario the fix targets. Today's unconditional recording is immune because it never asks whether the coin is ours. #866's committed-range rescan is what closes the hole: once A is derived, the spend block's filter contains A as a prevout element (Dash Core's
BasicFilterElements,src/blockfilter.cpp:206-212, adds every prevout scriptPubKey), so the sweep re-delivers the spend block and it gets recorded then.Alternative worth evaluating separately
Applying tip chainlocks during initial sync rather than deferring them to
SyncCompletewould make the existing pruning engage and keep the map bounded to the in-flight batch window, with no narrowing and no #866 dependency. It changes the deliberate single-writer promotion timing inspawn_chainlock_wallet_dispatchand would make historical blocks record asInChainLockedBlockearlier, so it needs its own analysis.References
key-wallet/src/transaction_checking/wallet_checker.rs,key-wallet-manager/src/process_block.rskey-wallet/src/wallet/managed_wallet_info/mod.rsdash-spv/src/client/event_handler.rs