diff --git a/dash-spv/src/client/event_handler.rs b/dash-spv/src/client/event_handler.rs index 1e4b57860..4ea1c53b8 100644 --- a/dash-spv/src/client/event_handler.rs +++ b/dash-spv/src/client/event_handler.rs @@ -151,7 +151,8 @@ pub(crate) fn spawn_progress_monitor( /// [`SyncEvent::SyncComplete`], serializing all `apply_chain_lock` /// calls on a single task. During the initial sync cycle (cycle 0) /// chainlock arrivals are buffered as the latest-seen height rather -/// than applied. At `SyncComplete { cycle: 0 }` the buffered height +/// than applied; only that height is passed on +/// (`note_chain_lock_height`). At `SyncComplete { cycle: 0 }` the buffered height /// is applied once, then every subsequent validated chainlock is /// applied directly. Only validated chainlocks advance the wallet. /// @@ -185,6 +186,7 @@ where .as_ref() .is_none_or(|buffered| chain_lock.block_height > buffered.block_height) { + wallet.write().await.note_chain_lock_height(chain_lock.block_height); deferred_chain_lock = Some(chain_lock); } } diff --git a/key-wallet-manager/src/process_block.rs b/key-wallet-manager/src/process_block.rs index cc442df16..de39fad84 100644 --- a/key-wallet-manager/src/process_block.rs +++ b/key-wallet-manager/src/process_block.rs @@ -414,6 +414,12 @@ impl WalletInterface for WalletM } } + fn note_chain_lock_height(&mut self, height: CoreBlockHeight) { + for info in self.wallet_infos.values_mut() { + info.note_chain_lock_height(height); + } + } + fn process_instant_send_lock(&mut self, instant_lock: InstantLock) { let txid = instant_lock.txid; diff --git a/key-wallet-manager/src/wallet_interface.rs b/key-wallet-manager/src/wallet_interface.rs index 6f43d99de..4e15faa55 100644 --- a/key-wallet-manager/src/wallet_interface.rs +++ b/key-wallet-manager/src/wallet_interface.rs @@ -218,6 +218,8 @@ pub trait WalletInterface: Send + Sync + 'static { /// in-flight block processing. fn apply_chain_lock(&mut self, chain_lock: ChainLock); + fn note_chain_lock_height(&mut self, _height: CoreBlockHeight) {} + /// Provide a human-readable description of the wallet implementation. /// /// Implementations are encouraged to include high-level state such as the diff --git a/key-wallet/src/managed_account/managed_core_funds_account.rs b/key-wallet/src/managed_account/managed_core_funds_account.rs index 491b15507..e075f977c 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -336,7 +336,9 @@ impl ManagedCoreFundsAccount { // earlier-processed block, so this output is genuinely spent // on-chain even though this account has never seen it before — // never insert it, so the record built below is born correct. - if observed_spent.contains_key(&outpoint) { + if observed_spent.contains_key(&outpoint) + || self.spent_before_funded.contains_key(&outpoint) + { tracing::debug!( outpoint = %outpoint, "Skipping UTXO already observed spent in an earlier-processed block (#649)" diff --git a/key-wallet/src/tests/observed_spent_outpoints_tests.rs b/key-wallet/src/tests/observed_spent_outpoints_tests.rs index 70a7e68ed..4f89fd701 100644 --- a/key-wallet/src/tests/observed_spent_outpoints_tests.rs +++ b/key-wallet/src/tests/observed_spent_outpoints_tests.rs @@ -131,6 +131,24 @@ fn prune_finalized_observed_spends_respects_finality_boundary() { assert_eq!(remaining.len(), 1); } +#[test] +fn a_noted_chain_lock_lets_the_prune_run_before_any_is_applied() { + use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + let mut info = ManagedWalletInfo::dummy(9); + let op_low = OutPoint::new(Txid::from([0x01; 32]), 0); + let op_high = OutPoint::new(Txid::from([0x03; 32]), 0); + info.record_observed_spends(&spending_tx(&[op_low]), 50); + info.record_observed_spends(&spending_tx(&[op_high]), 150); + info.metadata.synced_height = 100; + + info.note_chain_lock_height(200); + + assert!(info.metadata.last_applied_chain_lock.is_none()); + assert!(!info.observed_spent_outpoints().contains_key(&op_low)); + assert!(info.observed_spent_outpoints().contains_key(&op_high)); +} + /// Adding a standalone (from-xpub) account rewinds the sync checkpoint below /// wallet birth, so the new account's coins get filter coverage before pruning /// can consume the certificate (dashpay/rust-dashcore#649). A wallet still in @@ -428,6 +446,18 @@ async fn spend_seen_before_its_funding_is_recorded_on_redelivery() { ); } +#[tokio::test] +async fn a_held_output_stays_out_of_utxos_once_its_observed_spend_is_pruned() { + let (mut ctx, funding, _spend) = spend_first_context(in_block(100, 1)).await; + ctx.managed_wallet.observed_spent_outpoints.clear(); + + ctx.check_transaction(&funding, in_block(100, 1)).await; + + let account = ctx.managed_wallet.first_bip44_managed_account().expect("BIP44 account"); + assert!(account.utxos.is_empty()); + assert_eq!(ctx.managed_wallet.balance.total(), 0); +} + /// Abandoning the funding transaction takes its held output with it: the coin /// was never ours, so a spend of it must stop being recognisable. #[tokio::test] diff --git a/key-wallet/src/wallet/managed_wallet_info/mod.rs b/key-wallet/src/wallet/managed_wallet_info/mod.rs index 6bfece722..0d288e35f 100644 --- a/key-wallet/src/wallet/managed_wallet_info/mod.rs +++ b/key-wallet/src/wallet/managed_wallet_info/mod.rs @@ -83,16 +83,15 @@ pub struct ManagedWalletInfo { /// /// # Bounded permanence /// - /// An entry `(outpoint, height)` is removed only when - /// `height <= min(last_applied_chain_lock.block_height, synced_height)` — - /// the finality boundary. At that boundary the spend is chain-locked (it can - /// never be reorged out) and any funding transaction for the outpoint has - /// been delivered (BIP158 filters have no false negatives below - /// `synced_height`) and finalized (promoted into `finalized_txids`, or kept - /// as a chainlocked record), so every redelivery path short-circuits before - /// a coin could be re-inserted — in both `keep-finalized-transactions` - /// configurations and across a reload. No other removal path may be added - /// without a deliberate decision. + /// An entry `(outpoint, height)` is removed only when `height` is at or + /// below both `synced_height` and the highest chainlock applied or noted + /// (`note_chain_lock_height`) — the finality boundary. At that boundary the + /// spend is chain-locked (it can never be reorged out) and any funding + /// transaction for the outpoint has been delivered (BIP158 filters have no + /// false negatives below `synced_height`), so a redelivery finds the coin + /// already spent, or held in `spent_before_funded`, before it could be + /// re-inserted. No other removal path may be added without a deliberate + /// decision. /// /// Eviction is event-driven (chainlock application, sync-checkpoint commit), /// never age- or recency-based: during an out-of-order rescan `synced_height` @@ -140,6 +139,8 @@ pub struct ManagedWalletInfo { /// fresh process restarts both sides at 0. #[cfg_attr(feature = "serde", serde(skip))] pub(crate) account_generation: u64, + #[cfg_attr(feature = "serde", serde(skip))] + pub(crate) noted_chain_lock_height: Option, } /// Serde adapter for [`ManagedWalletInfo::observed_spent_outpoints`] that @@ -245,6 +246,7 @@ impl ManagedWalletInfo { instant_send_locks: HashSet::new(), observed_spent_outpoints: BTreeMap::new(), account_generation: 0, + noted_chain_lock_height: None, } } @@ -261,6 +263,7 @@ impl ManagedWalletInfo { instant_send_locks: HashSet::new(), observed_spent_outpoints: BTreeMap::new(), account_generation: 0, + noted_chain_lock_height: None, } } @@ -287,6 +290,7 @@ impl ManagedWalletInfo { instant_send_locks: HashSet::new(), observed_spent_outpoints: BTreeMap::new(), account_generation: 0, + noted_chain_lock_height: None, } } @@ -358,22 +362,23 @@ impl ManagedWalletInfo { } /// Evict [`Self::observed_spent_outpoints`] entries at or below the finality - /// boundary `min(last_applied_chain_lock.block_height, synced_height)`. + /// boundary: `synced_height`, capped by the highest chainlock applied or noted. /// /// An entry `(outpoint, height)` with `height <= boundary` is safe to /// forget: the spend at that height is chain-locked (never reorged out) and - /// any funding transaction for the outpoint has been delivered and finalized, - /// so no redelivery path can re-insert the coin (dashpay/rust-dashcore#649). - /// No-op until a chainlock has been applied (`last_applied_chain_lock` is - /// `None`) — without a finality boundary nothing can be proven final. + /// any funding transaction for the outpoint has been delivered, so no + /// redelivery path can re-insert the coin (dashpay/rust-dashcore#649). + /// No-op until a chainlock has been applied or noted — without a finality + /// boundary nothing can be proven final. /// - /// Called on chainlock application and sync-checkpoint commit; not age- or - /// recency-based. + /// Called on chainlock application or noting and on sync-checkpoint commit; + /// not age- or recency-based. pub(crate) fn prune_finalized_observed_spends(&mut self) { - let Some(chain_lock) = self.metadata.last_applied_chain_lock.as_ref() else { + let applied = self.metadata.last_applied_chain_lock.as_ref().map(|cl| cl.block_height); + let Some(final_height) = applied.max(self.noted_chain_lock_height) else { return; }; - let boundary = chain_lock.block_height.min(self.metadata.synced_height); + let boundary = final_height.min(self.metadata.synced_height); self.observed_spent_outpoints.retain(|_, height| *height > boundary); } diff --git a/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs b/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs index 585effb53..102b70c55 100644 --- a/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs +++ b/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs @@ -246,6 +246,8 @@ pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccount ApplyChainLockOutcome::default() } + fn note_chain_lock_height(&mut self, _height: CoreBlockHeight) {} + /// Update chain state and process any matured transactions /// This should be called when the chain tip advances to a new height fn update_last_processed_height(&mut self, current_height: u32); @@ -386,6 +388,13 @@ impl WalletInfoInterface for ManagedWalletInfo { } } + fn note_chain_lock_height(&mut self, height: CoreBlockHeight) { + if self.noted_chain_lock_height.is_none_or(|noted| height > noted) { + self.noted_chain_lock_height = Some(height); + self.prune_finalized_observed_spends(); + } + } + fn update_last_synced(&mut self, timestamp: u64) { self.metadata.last_synced = Some(timestamp); }