diff --git a/dash-spv-ffi/tests/dashd_sync/callbacks.rs b/dash-spv-ffi/tests/dashd_sync/callbacks.rs index c9e58b108..6fcb53f6a 100644 --- a/dash-spv-ffi/tests/dashd_sync/callbacks.rs +++ b/dash-spv-ffi/tests/dashd_sync/callbacks.rs @@ -3,7 +3,7 @@ use std::ffi::CStr; use std::os::raw::{c_char, c_void}; use std::slice; -use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -42,8 +42,13 @@ pub(super) struct CallbackTracker { pub(super) block_processed_wallet_count: AtomicU32, pub(super) block_processed_wallet_record_count: AtomicU32, pub(super) synced_height_updated_count: AtomicU32, - /// Highest synced-height value observed from any `SyncedHeightUpdated`. + /// The most recent synced-height value from `SyncedHeightUpdated`. Not + /// monotonic: backward coverage rewinds a wallet's checkpoint and reports + /// the lower value through this same callback. pub(super) last_synced_height: AtomicU32, + /// Set the first time a `SyncedHeightUpdated` reports a height BELOW one + /// already reported — the observable signature of that rewind. + pub(super) synced_height_rewound: AtomicBool, // Data from callbacks pub(super) last_header_tip: AtomicU32, @@ -110,6 +115,11 @@ pub(super) struct CallbackTracker { // Completion tracking pub(super) last_sync_cycle: AtomicU32, + /// Cycle number of the FIRST `on_sync_complete`. A wallet that derives + /// scripts while scanning is followed by a backward-coverage re-walk, + /// which completes as a further cycle, so `last_sync_cycle` is not the + /// initial one for such wallets. + pub(super) first_sync_cycle: AtomicU32, // Baseline for `wait_for_sync`: captured before the client starts so that // a SyncComplete firing between client start and `wait_for_sync` entry is @@ -346,7 +356,9 @@ extern "C" fn on_sync_complete(header_tip: u32, cycle: u32, user_data: *mut c_vo tracker.last_sync_cycle.store(cycle, Ordering::SeqCst); let seq = tracker.sequence_counter.fetch_add(1, Ordering::SeqCst); tracker.sync_complete_seq.store(seq, Ordering::SeqCst); - tracker.sync_complete_count.fetch_add(1, Ordering::SeqCst); + if tracker.sync_complete_count.fetch_add(1, Ordering::SeqCst) == 0 { + tracker.first_sync_cycle.store(cycle, Ordering::SeqCst); + } tracing::info!("on_sync_complete: header_tip={}, cycle={}, seq={}", header_tip, cycle, seq); } @@ -567,7 +579,10 @@ extern "C" fn on_sync_height_advanced( // Store the height before bumping the counter so a test that waits on the // counter and then reads `last_synced_height` is guaranteed to observe the // height for the same callback invocation. - tracker.last_synced_height.store(height, Ordering::SeqCst); + let previous = tracker.last_synced_height.swap(height, Ordering::SeqCst); + if height < previous { + tracker.synced_height_rewound.store(true, Ordering::SeqCst); + } tracker.synced_height_updated_count.fetch_add(1, Ordering::SeqCst); let wallet_str = unsafe { cstr_or_unknown(wallet_id) }; tracing::info!("on_sync_height_advanced: wallet={}, height={}", wallet_str, height); diff --git a/dash-spv-ffi/tests/dashd_sync/tests_callback.rs b/dash-spv-ffi/tests/dashd_sync/tests_callback.rs index 4acd4b53c..0c141eafa 100644 --- a/dash-spv-ffi/tests/dashd_sync/tests_callback.rs +++ b/dash-spv-ffi/tests/dashd_sync/tests_callback.rs @@ -146,21 +146,59 @@ fn test_all_callbacks_during_sync() { // so observing block-processed records does not guarantee it has fired yet. tracker.wait_for_callback(&tracker.synced_height_updated_count, 0, "synced_height_updated"); let synced_height_fired = tracker.synced_height_updated_count.load(Ordering::SeqCst); - let last_synced_height = tracker.last_synced_height.load(Ordering::SeqCst); assert!( synced_height_fired > 0, "on_synced_height_updated should fire at least once during sync" ); - assert!( - last_synced_height >= dashd.initial_height, - "last_synced_height ({}) should be at least initial_height ({}) after sync", - last_synced_height, - dashd.initial_height + // The callback is not monotonic: scripts derived during the scan are + // covered by rewinding the wallet's checkpoint and re-walking + // committed history (backward coverage), and the rewind is reported + // through this same callback. Wait for the re-walk to bring the + // reported height back to the tip instead of sampling it once. + // Two conditions, not one: the checkpoint must be seen going DOWN + // (the rewind itself) and then coming back to the tip (the re-walk + // that follows it). Waiting only for the tip would be satisfied by + // the value already stored before the rewind ever happened, so the + // whole backward-coverage path could be dead and this test would + // still pass. + let synced_deadline = std::time::Instant::now() + Duration::from_secs(60); + let last_synced_height = loop { + let h = tracker.last_synced_height.load(Ordering::SeqCst); + let rewound = tracker.synced_height_rewound.load(Ordering::SeqCst); + if rewound && h >= dashd.initial_height { + break h; + } + assert!( + std::time::Instant::now() < synced_deadline, + "backward coverage did not complete within 60s: rewind observed={}, \ + last_synced_height={} (initial_height={})", + rewound, + h, + dashd.initial_height + ); + std::thread::sleep(Duration::from_millis(100)); + }; + tracing::info!( + "SyncedHeightUpdated: fired {} time(s), last {}", + synced_height_fired, + last_synced_height ); - // Validate sync cycle (initial sync is cycle 0) - let last_sync_cycle = tracker.last_sync_cycle.load(Ordering::SeqCst); - assert_eq!(last_sync_cycle, 0, "Initial sync should be cycle 0"); + // Validate sync cycle (initial sync is cycle 0). This wallet has + // transactions, so the scan derives scripts and a backward-coverage + // re-walk follows, completing as a later cycle — check the first + // completion, not the last. + assert!( + tracker.sync_complete_count.load(Ordering::SeqCst) > 0, + "on_sync_complete should have fired" + ); + let first_sync_cycle = tracker.first_sync_cycle.load(Ordering::SeqCst); + assert_eq!(first_sync_cycle, 0, "Initial sync should be cycle 0"); + tracing::info!( + "Sync cycles: first={}, last={}", + first_sync_cycle, + tracker.last_sync_cycle.load(Ordering::SeqCst) + ); // Validate callback lifecycle ordering let sync_start_seq = tracker.sync_start_seq.load(Ordering::SeqCst); diff --git a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs index bf064fd97..6a82bac2f 100644 --- a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs +++ b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs @@ -50,6 +50,7 @@ use key_wallet::account::ManagedAccountTrait; use key_wallet::gap_limit::DEFAULT_COINJOIN_GAP_LIMIT; use key_wallet::managed_account::address_pool::{AddressPool, AddressPoolType, KeySource}; use key_wallet::wallet::initialization::WalletAccountCreationOptions; +use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet_manager::WalletManager; use tokio::sync::mpsc::unbounded_channel; @@ -201,11 +202,14 @@ async fn drive_to_quiescence( wallet: &Arc>>, blocks: &HashMap, initial_events: Vec, -) { +) -> Vec { let (tx, _rx) = unbounded_channel(); let requests = RequestSender::new(tx); let mut events = initial_events; + // Everything the run emitted that was not a block request — the caller's + // window into completion, which is otherwise consumed here. + let mut observed = Vec::new(); for _round in 0..64 { let mut pending: BTreeMap<(u32, BlockHash), BTreeSet> = BTreeMap::new(); for event in events.drain(..) { @@ -216,10 +220,12 @@ async fn drive_to_quiescence( for (key, wallets) in needed { pending.entry((key.height(), *key.hash())).or_default().extend(wallets); } + } else { + observed.push(event); } } if pending.is_empty() { - return; + return observed; } let mut next_events = Vec::new(); @@ -325,27 +331,25 @@ async fn coinjoin_gap_limit_inversion_within_batch_recovers() { ); } -/// Gap-window outputs in an already-COMMITTED batch (#846). +/// Backward coverage across a committed batch is a durable rewind, not an +/// in-memory sweep. /// -/// Same funding shape as the within-batch inversion test, but the early -/// block (indices G+10..=G+21, height 10) sits in batch 0..=99 while the -/// in-window block (indices 0..=29) sits at height 110 in batch 100..=199. -/// Batch 0 scans clean (nothing watched matches) and commits. Processing the -/// height-110 block extends the window past G+21, and those scripts DO match -/// block 10's filter — but `rescan_batch` only reaches `active_batches`, and -/// committed batches are gone (`try_commit_batches` removes them; the -/// tracker prunes at-or-below the committed height). Indices G+10..=G+21 — -/// squarely inside the BIP-44/CoinJoin gap-limit recovery contract -/// (G+21 < 29 + 1 + G) — used to stay invisible forever, along with their -/// funds; a fresh re-sync from genesis hit the same wall deterministically. +/// Block A (height 10, batch 0) funds CoinJoin External indices G+10..=G+21, +/// beyond the initial gap window; block B (height 110, batch 1) funds +/// 0..=29 and its processing derives the scripts that would have matched +/// block A — after batch 0 has already committed. At the forward drain the +/// manager must not sweep the committed range in memory (an iOS suspension +/// drops such a sweep whole) but rewind the wallet's `synced_height` so the +/// sync-manager tick re-walks committed history in persisted batches, and +/// it must NOT declare the filters complete while that re-walk is pending: +/// one "synced" cycle with the walk still to run is exactly what the host +/// would mistake for a caught-up wallet. Once the wallet is back at the +/// committed frontier, completion is emitted. /// -/// GREEN since `rescan_committed_range`: newly derived scripts are re-tested -/// against the persisted filters below the committing batch (BIP-158 filters -/// are address-independent, so re-matching needs no re-download), and hits -/// flow through the `track_for_new_scripts` re-download path to the same -/// commit-time fixpoint. `highest_used` reaches G+21. +/// The tick itself does not run in this harness, so the re-walk is +/// represented by advancing the wallet's checkpoint by hand. #[tokio::test] -async fn coinjoin_gap_limit_stall_across_committed_batch() { +async fn backward_coverage_rewinds_and_holds_completion_until_rewalked() { let (mut manager, wallet, wallet_id) = setup().await; let addresses = coinjoin_external_addresses(&wallet, &wallet_id, (G + 22) as u32).await; @@ -362,9 +366,11 @@ async fn coinjoin_gap_limit_stall_across_committed_batch() { { let mut header_storage = manager.header_storage.write().await; let mut filter_storage = manager.filter_storage.write().await; - for height in 0..=99u32 { + for height in 0..=199u32 { let (header, filter_bytes) = if height == 10 { (block_a.header, filter_a.content.clone()) + } else if height == 110 { + (block_b.header, filter_b.content.clone()) } else { let filler = Block::dummy(height, vec![]); let filter = BlockFilter::dummy(&filler); @@ -388,33 +394,97 @@ async fn coinjoin_gap_limit_stall_across_committed_batch() { batch_1.mark_verified(); manager.active_batches.insert(100, batch_1); manager.progress.update_stored_height(199); + // The re-walk below re-enters `start_download`, which scans against the + // filter-header frontier rather than the injected batches. Without a tip + // it takes the "nothing to download" early return and the rescan is a + // silent no-op — which is precisely the failure this test must not miss. + manager.progress.update_filter_header_tip_height(199); + manager.progress.update_target_height(199); let initial_events = manager.try_process_batch().await.unwrap(); drive_to_quiescence(&mut manager, &wallet, &blocks, initial_events).await; - let (highest_used, highest_generated, used_count) = - coinjoin_pool_state(&wallet, &wallet_id).await; - // Sanity: the in-window block was found and the gap window extended past - // index G+21, so the missed indices ARE inside the watched range by now. + let (_, highest_generated, _) = coinjoin_pool_state(&wallet, &wallet_id).await; assert!( highest_generated >= Some((G + 21) as u32), "gap maintenance must have extended the watch window past index G+21 \ (got {highest_generated:?})" ); + + // The drain rewound the wallet to its own floor instead of sweeping. + let (synced_height, birth_height) = { + let reader = wallet.read().await; + let info = reader.get_wallet_info(&wallet_id).expect("wallet info"); + (info.synced_height(), info.birth_height()) + }; + assert_eq!( + synced_height, + birth_height.saturating_sub(1), + "wallet synced_height must be rewound to birth_height - 1 for the durable re-walk" + ); + assert!(manager.rewalk_pending().await, "a re-walk must be pending after the rewind"); + assert_eq!( + manager.state(), + SyncState::Syncing, + "filters must not be declared complete while a rewound wallet is below the frontier" + ); + + // Block A's outputs are still missing at this point — the rewind exists + // to recover them, so the re-walk below has real work to do. + let (highest_used_before, _, _) = coinjoin_pool_state(&wallet, &wallet_id).await; + assert!( + highest_used_before < Some((G + 10) as u32), + "block A's beyond-window outputs must still be unapplied before the re-walk \ + (highest_used={highest_used_before:?})" + ); + + // Drive the real re-walk, not a stand-in: the tick is what notices a + // wallet below the committed frontier, restarts the scan at its rewound + // checkpoint, and re-requests the blocks whose filters match the scripts + // derived since. Feeding those blocks back through the wallet is the + // blocks-manager's job, which `drive_to_quiescence` performs. + let (tx, _rx) = unbounded_channel(); + let requests = RequestSender::new(tx); + let mut sync_complete_seen = false; + for _round in 0..64 { + let events = manager.tick(&requests).await.expect("tick"); + let quiesced = events.is_empty(); + let observed = drive_to_quiescence(&mut manager, &wallet, &blocks, events).await; + for event in observed { + if let SyncEvent::FiltersSyncComplete { + .. + } = event + { + // Completion is only honest once the re-walk has applied what + // it was rewound to find. + let (highest_used, _, _) = coinjoin_pool_state(&wallet, &wallet_id).await; + assert_eq!( + highest_used, + Some((G + 21) as u32), + "FiltersSyncComplete was emitted before the re-walk recovered block A" + ); + sync_complete_seen = true; + } + } + if !manager.rewalk_pending().await && quiesced { + break; + } + } + + let (highest_used, _, used_count) = coinjoin_pool_state(&wallet, &wallet_id).await; assert_eq!( highest_used, Some((G + 21) as u32), - "CoinJoin External indices G+10..=G+21 were funded at height 10 in a batch that \ - committed before their scripts were derived, and the new-script rescan never \ - looks below the committed boundary (rescan_batch only reaches active_batches; \ - BlockMatchTracker/commit pruning drops the range). The addresses are within \ - the gap-limit recovery contract and are watched now (highest_generated = \ - {highest_generated:?}), yet their outputs stay invisible: highest_used stalls \ - at {highest_used:?}, used_count={used_count}. Fix direction: key re-scan \ - suppression by (wallet, address/script) instead of block/commit progress, or \ - trigger a below-committed-height rescan for a wallet whose gap maintenance \ - derives scripts mid-sync." + "the re-walk must recover block A's beyond-window outputs (used_count={used_count})" + ); + assert_eq!(used_count, 30 + 12, "indices 0..=29 and G+10..=G+21 must all be marked used"); + + assert!(!manager.rewalk_pending().await, "the re-walk must have completed"); + assert!( + sync_complete_seen, + "FiltersSyncComplete must be emitted once the rewound wallet has been re-walked" ); + assert_eq!(manager.state(), SyncState::Synced); } /// Committed-range sweeps coalesce across batch commits. @@ -436,6 +506,12 @@ async fn coinjoin_gap_limit_stall_across_committed_batch() { /// applied. `committed_range_sweeps` counts sweeps that reach the chunk walk /// in `rescan_committed_range`. #[tokio::test] +#[ignore = "backward coverage no longer sweeps the committed range in the \ +manager; it rewinds the wallet and the sync-manager tick re-walks it. The \ +per-commit coalescing this test measured has no counterpart now — see \ +backward_coverage_rewinds_and_holds_completion_until_rewalked, which drives \ +that re-walk through the tick and asserts the same recovery. Remove together \ +with rescan_committed_range."] async fn committed_range_sweep_coalesces_across_batch_commits() { let (mut manager, wallet, wallet_id) = setup().await; let addresses = coinjoin_external_addresses(&wallet, &wallet_id, (G + 22) as u32).await; diff --git a/dash-spv/src/sync/filters/manager.rs b/dash-spv/src/sync/filters/manager.rs index 9e7834e48..d30dad772 100644 --- a/dash-spv/src/sync/filters/manager.rs +++ b/dash-spv/src/sync/filters/manager.rs @@ -561,7 +561,18 @@ impl= self.progress.filter_header_tip_height() && self.progress.committed_height() >= self.progress.target_height() @@ -590,10 +601,40 @@ impl bool { + let committed = self.progress.committed_height(); + let wallet_read = self.wallet.read().await; + let behind = wallet_read.wallets_behind(committed); + let stale_min_synced = behind.iter().map(|id| wallet_read.wallet_synced_height(id)).min(); + let birth_height = wallet_read.earliest_required_height().await; + drop(wallet_read); + let Some(stale_min_synced) = stale_min_synced else { + return false; + }; + let scan_floor = birth_height + .max(self.header_storage.read().await.get_start_height().await.unwrap_or(0)); + stale_min_synced.saturating_add(1).max(scan_floor) <= committed + } + /// Commit completed batches in order (lowest batch_start first). async fn try_commit_batches(&mut self) -> SyncResult> { let mut events = Vec::new(); + // Wallets whose synced_height was rewound this pass for backward + // coverage (see the forward-drained branch). Their commit-time + // advance below is skipped: advancing them to the batch end would + // both undo the in-memory rewind and race the persisted checkpoint + // back up to tip, losing the re-walk across a restart. + let mut rewound_wallets: std::collections::HashSet = + std::collections::HashSet::new(); + // Lowest height any scan can ever reach. Read once, before the wallet // write lock below, so header storage is never locked underneath it. let scan_floor = self.header_storage.read().await.get_start_height().await.unwrap_or(0); @@ -670,15 +711,77 @@ impl 0 { - // Found more blocks, can't commit yet - break; + // LOCAL BUILD: durable backward coverage via a persisted + // checkpoint rewind, replacing the in-memory monolithic + // sweep (`rescan_committed_range`). + // + // The sweep held three losing properties on a large + // restored wallet: it ran minutes of silent compute + // inside this task, it charged tens of thousands of + // BIP-158 false-positive downloads to this batch's + // commit gate, and every bit of it lived in memory — an + // iOS suspension eight seconds after "synced" was + // observed to drop 28,616 queued blocks irrecoverably. + // + // The scripts are already in the wallet's watch set + // (deriving them is what put them in `backward_scripts`), + // so backward coverage is exactly "re-walk committed + // history with the enlarged set". The existing + // wallet-behind restart does that durably: rewinding the + // wallet's synced_height makes the next tick restart the + // batch scan, which re-commits (and re-persists) progress + // every BATCH_PROCESSING_SIZE filters. The persisted + // checkpoint applies verbatim on the app side, so a + // suspension mid-walk resumes from the last committed + // batch instead of losing the debt. Already-stored blocks + // are served from storage on the re-walk, so completed + // work is not re-downloaded. + let script_count: usize = + backward_scripts.values().map(|scripts| scripts.len()).sum(); + // One floor for every wallet: the earliest height any of + // them requires. The restart tick clamps its actual + // resume point to the birth/storage floor anyway, so a + // conservative target here only ever means "re-walk from + // the beginning of what is locally scannable". + let wallet_base = self.wallet.read().await.earliest_required_height().await; + let target = wallet_base.saturating_sub(1); + let mut wallet = self.wallet.write().await; + for wallet_id in backward_scripts.keys() { + let before = wallet.wallet_synced_height(wallet_id); + if target >= before { + continue; } + wallet.rewind_wallet_synced_height(wallet_id, target); + // Read the checkpoint back instead of assuming the + // rewind landed. `rewind_wallet_synced_height` returns + // nothing and defaults to a no-op, so an implementation + // that predates backward coverage would otherwise have + // its commit-time advance skipped below on the strength + // of a call that did nothing — stranding this batch's + // certified coverage with no re-walk to replace it. A + // rewind clamped up to the wallet's own floor still + // sits under `before`, so it counts as what it is. + if wallet.wallet_synced_height(wallet_id) < before { + rewound_wallets.insert(*wallet_id); + } else { + tracing::warn!( + "Backward coverage: wallet {} did not honor the rewind to {} (still at {}); \ + committing its coverage forward instead — scripts derived after this range \ + committed stay untested against it", + hex::encode(wallet_id), + target, + before + ); + } + } + drop(wallet); + if !rewound_wallets.is_empty() { + tracing::info!( + "Backward coverage: {} new script(s) across {} wallet(s) — rewound their synced_height for a durable re-walk of committed history", + script_count, + rewound_wallets.len() + ); } } // Mark rescan as complete @@ -729,6 +832,15 @@ impl= batch_start { wallet.update_wallet_synced_height(wallet_id, end); // A committed batch certifies the whole range for @@ -1182,6 +1294,11 @@ impl= w1_synced_at_add, - "W1 synced_height regressed during mid-flight rescan: {} -> {}", - w1_synced_at_add, + w1_synced_now + 1 >= w1_birth_height, + "W1 synced_height rewound below its own birth height: {} (birth {})", w1_synced_now, + w1_birth_height, ); assert!( w1_processed_now >= w1_processed_at_add, diff --git a/key-wallet-manager/src/event_tests.rs b/key-wallet-manager/src/event_tests.rs index afccc35ae..be6e1a69c 100644 --- a/key-wallet-manager/src/event_tests.rs +++ b/key-wallet-manager/src/event_tests.rs @@ -1270,6 +1270,81 @@ async fn test_update_wallet_synced_height_does_not_re_emit_when_unchanged() { ); } +#[tokio::test] +async fn test_rewind_wallet_synced_height_lowers_and_emits() { + let (mut manager, wallet_id, _addr) = setup_manager_with_wallet(); + let mut rx = manager.subscribe_events(); + manager.update_wallet_synced_height(&wallet_id, 2000); + drain_events(&mut rx); + + // A rewind lowers the checkpoint and is reported through the same event + // an advance emits, so persisters store it verbatim and the rewind + // survives a restart. + manager.rewind_wallet_synced_height(&wallet_id, 1200); + assert_eq!(manager.wallet_synced_height(&wallet_id), 1200); + let synced_events: Vec<_> = drain_events(&mut rx) + .into_iter() + .filter_map(|e| match e { + WalletEvent::SyncHeightAdvanced { + wallet_id, + height, + } => Some((wallet_id, height)), + _ => None, + }) + .collect(); + assert_eq!(synced_events, vec![(wallet_id, 1200)]); + + // At or above the current checkpoint is ignored — no change, no event. + manager.rewind_wallet_synced_height(&wallet_id, 1200); + manager.rewind_wallet_synced_height(&wallet_id, 5000); + assert_eq!(manager.wallet_synced_height(&wallet_id), 1200); + let events = drain_events(&mut rx); + assert!( + !events.iter().any(|e| matches!(e, WalletEvent::SyncHeightAdvanced { .. })), + "no SyncHeightAdvanced for a non-lowering rewind, got {:?}", + events + ); + + // An unknown wallet is a no-op. + manager.rewind_wallet_synced_height(&[9u8; 32], 0); + assert!(drain_events(&mut rx).is_empty()); +} + +#[tokio::test] +async fn test_rewind_wallet_synced_height_clamps_to_own_birth_height() { + // The caller passes one floor for every wallet it rewinds; a wallet must + // never be dragged below its own start by another wallet's lower birth. + let mut manager = WalletManager::::new(Network::Testnet); + let wallet_id = manager + .create_wallet_from_mnemonic( + TEST_MNEMONIC, + 500, + key_wallet::wallet::initialization::WalletAccountCreationOptions::Default, + ) + .unwrap(); + manager.update_wallet_synced_height(&wallet_id, 3000); + let mut rx = manager.subscribe_events(); + + manager.rewind_wallet_synced_height(&wallet_id, 0); + assert_eq!(manager.wallet_synced_height(&wallet_id), 499); + let synced_events: Vec<_> = drain_events(&mut rx) + .into_iter() + .filter_map(|e| match e { + WalletEvent::SyncHeightAdvanced { + height, + .. + } => Some(height), + _ => None, + }) + .collect(); + assert_eq!(synced_events, vec![499]); + + // Already at the floor: a second rewind changes nothing. + manager.rewind_wallet_synced_height(&wallet_id, 0); + assert_eq!(manager.wallet_synced_height(&wallet_id), 499); + assert!(drain_events(&mut rx).is_empty()); +} + // --------------------------------------------------------------------------- // Dry run and irrelevant paths // --------------------------------------------------------------------------- diff --git a/key-wallet-manager/src/process_block.rs b/key-wallet-manager/src/process_block.rs index cc442df16..86a6e901b 100644 --- a/key-wallet-manager/src/process_block.rs +++ b/key-wallet-manager/src/process_block.rs @@ -363,6 +363,33 @@ impl WalletInterface for WalletM } } + fn rewind_wallet_synced_height(&mut self, wallet_id: &WalletId, height: CoreBlockHeight) { + if let Some(info) = self.wallet_infos.get_mut(wallet_id) { + // Never below this wallet's own start: the caller passes one + // floor for every wallet it rewinds (the earliest height any of + // them requires), and a wallet added at runtime with a lower + // birth height must not drag an older wallet's checkpoint under + // its own birth — that re-walks history the wallet cannot have + // touched, and on a persisted store it looks like the wallet + // was reset. + let height = height.max(info.birth_height().saturating_sub(1)); + if height < info.synced_height() { + info.update_synced_height(height); + // Deliberately the same event an advance emits: the + // persisters apply `synced_height` verbatim (no monotonic + // clamp at the row), so this is what makes the rewind + // durable across a restart. Only the in-batch changeset + // merge is monotonic-max; the caller keeps the rewind out + // of a batch that also carries a higher advance by not + // advancing a rewound wallet at the same commit. + self.emit_event(WalletEvent::SyncHeightAdvanced { + wallet_id: *wallet_id, + height, + }); + } + } + } + fn update_wallet_last_processed_height( &mut self, wallet_id: &WalletId, diff --git a/key-wallet-manager/src/test_utils/mock_wallet.rs b/key-wallet-manager/src/test_utils/mock_wallet.rs index 63e02b487..2110395b4 100644 --- a/key-wallet-manager/src/test_utils/mock_wallet.rs +++ b/key-wallet-manager/src/test_utils/mock_wallet.rs @@ -223,6 +223,12 @@ impl WalletInterface for MockWallet { } } + fn rewind_wallet_synced_height(&mut self, wallet_id: &WalletId, height: CoreBlockHeight) { + if wallet_id == &self.wallet_id && height < self.synced_height { + self.synced_height = height; + } + } + fn update_wallet_last_processed_height( &mut self, wallet_id: &WalletId, @@ -351,6 +357,12 @@ impl WalletInterface for NonMatchingMockWallet { } } + fn rewind_wallet_synced_height(&mut self, wallet_id: &WalletId, height: CoreBlockHeight) { + if wallet_id == &self.wallet_id && height < self.synced_height { + self.synced_height = height; + } + } + fn update_wallet_last_processed_height( &mut self, wallet_id: &WalletId, @@ -526,6 +538,14 @@ impl WalletInterface for MultiMockWallet { } } + fn rewind_wallet_synced_height(&mut self, wallet_id: &WalletId, height: CoreBlockHeight) { + if let Some(state) = self.wallets.get_mut(wallet_id) { + if height < state.synced_height { + state.synced_height = height; + } + } + } + fn update_wallet_last_processed_height( &mut self, wallet_id: &WalletId, diff --git a/key-wallet-manager/src/wallet_interface.rs b/key-wallet-manager/src/wallet_interface.rs index 6f43d99de..a587b57df 100644 --- a/key-wallet-manager/src/wallet_interface.rs +++ b/key-wallet-manager/src/wallet_interface.rs @@ -165,6 +165,24 @@ pub trait WalletInterface: Send + Sync + 'static { /// only advance forward (a value below the current is silently ignored). fn update_wallet_synced_height(&mut self, wallet_id: &WalletId, height: CoreBlockHeight); + /// Rewind one wallet's committed sync checkpoint below its current value, + /// so the filter sync re-walks committed history — used when scripts + /// derived after a range committed must still be tested against it. + /// Implementations must only lower (a value at or above the current is + /// silently ignored), must clamp `height` to the wallet's own earliest + /// required height so a floor computed across several wallets never + /// drags one below its birth, and must emit the same persistence signal + /// an advance emits, so the rewound checkpoint survives a restart and the + /// re-walk resumes from its own committed progress. + /// + /// The default is a no-op, for implementations that predate backward + /// coverage. Opting out that way costs backward coverage itself, not + /// correctness of the forward scan: the caller reads the checkpoint back + /// and, seeing it unmoved, commits the range forward as it always did + /// (and warns), rather than skipping the advance for a re-walk that will + /// never happen. + fn rewind_wallet_synced_height(&mut self, _wallet_id: &WalletId, _height: CoreBlockHeight) {} + /// Advance one wallet's last-processed height after a block has been applied /// to its state. Implementations must only advance forward. fn update_wallet_last_processed_height(