Summary
Follow-up from PR #851 (fix for #649). ManagedWalletInfo::observed_spent_outpoints permanently and one-way records every on-chain-observed spend to prevent #649-style out-of-order resurrection. As of PR #851, entries below min(chainlock_height, synced_height) are safely pruned — but entries above that boundary (i.e. spends observed in a block that isn't chain-locked yet) are still permanent, with no retraction path if that block gets reorged out and its spend never re-confirms.
Current failure mode: a spend observed in a non-chain-locked block that is later reorged out, and never re-confirms, permanently hides the funding coin. Balance under-reports silently, forever. Worse: this is currently unrecoverable even by a full rescan — the funding record's history was already destructively compensated, the UTXO is persisted-absent, and once the funding tx (re)appears its redelivery is short-circuited by the wallet's own finalization machinery. The only recovery today is deleting the wallet and re-importing from seed.
On healthy mainnet the exposure window is small (ChainLocks land within ~1 block), but it's unbounded during a ChainLock outage.
Why this needs a dash-spv change first
A chainlock-gated "trust tier" for observed_spent_outpoints (only permanently trust entries at/below the last chain-locked height; retract entries above it on reorg) is architecturally coherent — the boundary, persistence, and promotion machinery already exist in key-wallet. But retraction requires the wallet to be told a reorg happened, and today:
WalletInterface (key-wallet-manager/src/wallet_interface.rs) has no disconnect/reorg method — the wallet layer is never notified of a reorg.dash-spv has no operational reorg pipeline: a header-chain continuity break is a hard SyncError::Validation (sync/block_headers/manager.rs); ChainTipManager (chain/chain_tip.rs) is referenced only from test_utils; the storage-level truncate_above (storage/mod.rs) has no production caller.
Proposed follow-up stack (dependency order)
- dash-spv reorg pipeline (own design doc): fork detection in header sync, chainlock-gated fork acceptance (never reorg below the chain-locked boundary),
truncate_above for headers/filter-headers/filters, rewind of per-wallet synced_height, replay of the replacement branch.
WalletInterface::handle_chain_disconnect(fork_height) — new trait method (default no-op for compatibility), fanned out per wallet by key-wallet-manager.- key-wallet retraction:
ManagedWalletInfo::retract_observed_spends_above(fork_height) (debug-assert fork_height >= chainlock boundary — a chain-locked spend must never be retracted), plus reversible compensation (flag spent outputs instead of destructively deleting them, so retraction can undo it) and a provisional undo buffer for guard-removed UTXOs above the boundary.
Context
Full architecture analysis (call-site citations, safety proofs, why LRU/naive eviction was rejected for the related pruning fix) is in the PR #851 review trail — available on request if whoever picks this up wants the detailed writeup rather than re-deriving it.
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary
Follow-up from PR #851 (fix for #649).
ManagedWalletInfo::observed_spent_outpointspermanently and one-way records every on-chain-observed spend to prevent #649-style out-of-order resurrection. As of PR #851, entries belowmin(chainlock_height, synced_height)are safely pruned — but entries above that boundary (i.e. spends observed in a block that isn't chain-locked yet) are still permanent, with no retraction path if that block gets reorged out and its spend never re-confirms.Current failure mode: a spend observed in a non-chain-locked block that is later reorged out, and never re-confirms, permanently hides the funding coin. Balance under-reports silently, forever. Worse: this is currently unrecoverable even by a full rescan — the funding record's history was already destructively compensated, the UTXO is persisted-absent, and once the funding tx (re)appears its redelivery is short-circuited by the wallet's own finalization machinery. The only recovery today is deleting the wallet and re-importing from seed.
On healthy mainnet the exposure window is small (ChainLocks land within ~1 block), but it's unbounded during a ChainLock outage.
Why this needs a dash-spv change first
A chainlock-gated "trust tier" for
observed_spent_outpoints(only permanently trust entries at/below the last chain-locked height; retract entries above it on reorg) is architecturally coherent — the boundary, persistence, and promotion machinery already exist in key-wallet. But retraction requires the wallet to be told a reorg happened, and today:WalletInterface(key-wallet-manager/src/wallet_interface.rs) has no disconnect/reorg method — the wallet layer is never notified of a reorg.dash-spvhas no operational reorg pipeline: a header-chain continuity break is a hardSyncError::Validation(sync/block_headers/manager.rs);ChainTipManager(chain/chain_tip.rs) is referenced only fromtest_utils; the storage-leveltruncate_above(storage/mod.rs) has no production caller.Proposed follow-up stack (dependency order)
truncate_abovefor headers/filter-headers/filters, rewind of per-walletsynced_height, replay of the replacement branch.WalletInterface::handle_chain_disconnect(fork_height)— new trait method (default no-op for compatibility), fanned out per wallet bykey-wallet-manager.ManagedWalletInfo::retract_observed_spends_above(fork_height)(debug-assertfork_height >= chainlock boundary— a chain-locked spend must never be retracted), plus reversible compensation (flag spent outputs instead of destructively deleting them, so retraction can undo it) and a provisional undo buffer for guard-removed UTXOs above the boundary.Context
Full architecture analysis (call-site citations, safety proofs, why LRU/naive eviction was rejected for the related pruning fix) is in the PR #851 review trail — available on request if whoever picks this up wants the detailed writeup rather than re-deriving it.
🤖 Co-authored by Claudius the Magnificent AI Agent