Skip to content

Commit 3a2b32c

Browse files
lklimekclaude
andcommitted
refactor(spv): remove mid-session restart workaround (blocked upstream)
Removes the mid-session wallet import restart workaround introduced in this branch. The mechanism — restart_spv_for_new_wallet, SpvManager::restart, SpvManager::try_filter_committed_height, TaskError::SpvRestartFailed, and four covering tests — is blocked by rust-dashcore#651 (constraint #4): SegmentCache::first_valid_offset under-reports after a fresh DiskStorageManager reload, so get_start_height() returns a value near the chain tip, and FiltersManager re-scans only a single filter instead of re-covering all historical heights for the new wallet. register_wallet now unconditionally calls handle_wallet_unlocked on the SPV path, matching the fresh-start behaviour. Mid-session imports that need full history rescanning should use the "Clear SPV Data" button (clear_data_dir / full wipe + resync from checkpoint), which remains correct and unaffected. Closes #829. Blocked by rust-dashcore#651. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d596b4a commit 3a2b32c

4 files changed

Lines changed: 2 additions & 393 deletions

File tree

src/backend_task/error.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -667,19 +667,6 @@ pub enum TaskError {
667667
#[error("Could not broadcast the transaction. Please check your connection and retry.")]
668668
SpvBroadcastFailed { detail: String },
669669

670-
/// The SPV client could not be restarted after a mid-session wallet import.
671-
///
672-
/// Mid-session imports require a restart so the filter scan can re-cover
673-
/// historical heights below the current sync tip. If the restart fails the
674-
/// newly imported wallet may show zero balance until the app is restarted.
675-
#[error(
676-
"Could not restart background sync to discover this wallet's history. Please restart the application to pick up the new wallet."
677-
)]
678-
SpvRestartFailed {
679-
#[source]
680-
source: crate::spv::SpvError,
681-
},
682-
683670
// ──────────────────────────────────────────────────────────────────────────
684671
// UTXO / asset-lock transaction build errors
685672
// ──────────────────────────────────────────────────────────────────────────

src/context/wallet_lifecycle.rs

Lines changed: 1 addition & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -143,108 +143,12 @@ impl AppContext {
143143
// 3. Bootstrap any additional addresses and load into SPV
144144
self.bootstrap_wallet_addresses(&wallet_arc);
145145
if self.core_backend_mode() == CoreBackendMode::Spv {
146-
// Mid-session imports land here after the SPV loop has already
147-
// advanced `filter_committed_height` past any historical heights
148-
// where the new wallet may hold UTXOs. `FiltersManager` reads
149-
// `earliest_required_height` and `filter_committed_height` at
150-
// construction and never rewinds, so simply loading the wallet
151-
// into the running WalletManager would leave those heights
152-
// unscanned. Restart the SPV run instead — on-disk filters /
153-
// headers / blocks are preserved, only the in-memory filter
154-
// scanner is rebuilt against the new wallet set.
155-
//
156-
// Fresh app / first-import path: SPV hasn't scanned anything yet
157-
// (filter_committed_height == 0) — the normal wallet-load path is
158-
// correct and cheaper. Same for the case where SPV isn't running.
159-
// Check filter_committed_height non-blockingly. When the lock is
160-
// held (e.g. a concurrent wallet load is in flight) treat it as
161-
// "already scanned" to be safe: the cost of an unnecessary restart
162-
// is a short re-scan of persisted filters; the cost of skipping a
163-
// needed restart is zero balance until the user restarts.
164-
let filter_height = self
165-
.spv_manager
166-
.try_filter_committed_height()
167-
.unwrap_or(u32::MAX);
168-
let needs_restart = self.spv_manager.status().status.is_active() && filter_height > 0;
169-
if needs_restart {
170-
self.restart_spv_for_new_wallet();
171-
} else {
172-
self.handle_wallet_unlocked(&wallet_arc);
173-
}
146+
self.handle_wallet_unlocked(&wallet_arc);
174147
}
175148

176149
Ok((seed_hash, wallet_arc))
177150
}
178151

179-
/// Restart the SPV run so the filter scanner is rebuilt with the current
180-
/// wallet set, then re-queue every wallet load through the normal
181-
/// bootstrap path. Called when a new wallet is imported mid-session and
182-
/// the existing SPV run has already scanned past historical heights where
183-
/// the new wallet may hold UTXOs.
184-
///
185-
/// Spawned as a subtask so the caller (sync `register_wallet`) returns
186-
/// immediately — the UI continues to render while SPV winds down and
187-
/// starts again.
188-
pub(crate) fn restart_spv_for_new_wallet(self: &Arc<Self>) {
189-
let expected_wallets = self
190-
.wallets
191-
.read()
192-
.map(|guard| {
193-
guard
194-
.values()
195-
.filter(|w| {
196-
w.read()
197-
.ok()
198-
.is_some_and(|g| g.is_open() && g.seed_bytes().is_ok())
199-
})
200-
.count()
201-
})
202-
.unwrap_or(0);
203-
204-
let progress_banner = crate::ui::components::message_banner::MessageBanner::set_global(
205-
self.egui_ctx(),
206-
"Restarting background sync to discover this wallet's history.",
207-
crate::ui::MessageType::Info,
208-
);
209-
210-
let ctx = Arc::clone(self);
211-
self.subtasks
212-
.spawn_sync("spv_restart_for_import", async move {
213-
let outcome = ctx.spv_manager.restart(expected_wallets).await;
214-
// Always clear the progress banner — both paths replace it
215-
// with a concrete result message.
216-
progress_banner.clear();
217-
match outcome {
218-
Ok(()) => {
219-
tracing::info!(
220-
expected_wallets,
221-
"SPV restart completed after wallet import; re-bootstrapping wallets"
222-
);
223-
ctx.connection_status
224-
.set_spv_status(ctx.spv_manager.status().status);
225-
ctx.connection_status.refresh_state();
226-
// `bootstrap_loaded_wallets` re-queues every open wallet via
227-
// `handle_wallet_unlocked -> queue_spv_wallet_load`, which is
228-
// exactly what the restarted SPV's wait loop is blocking on.
229-
ctx.bootstrap_loaded_wallets();
230-
}
231-
Err(err) => {
232-
let task_err = TaskError::SpvRestartFailed { source: err };
233-
tracing::error!(
234-
error = %task_err,
235-
"SPV restart after wallet import failed; newly imported wallet may show a zero balance until the application is restarted"
236-
);
237-
crate::ui::components::message_banner::MessageBanner::set_global(
238-
ctx.egui_ctx(),
239-
&task_err,
240-
crate::ui::MessageType::Error,
241-
)
242-
.with_details(&task_err);
243-
}
244-
}
245-
});
246-
}
247-
248152
pub fn bootstrap_wallet_addresses(&self, wallet: &Arc<RwLock<Wallet>>) {
249153
if let Ok(mut guard) = wallet.write() {
250154
// Bootstrap when no addresses exist (fresh wallet) or when

src/spv/manager.rs

Lines changed: 1 addition & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use dash_sdk::dash_spv::client::config::MempoolStrategy;
77
use dash_sdk::dash_spv::client::event_handler::EventHandler;
88
use dash_sdk::dash_spv::network::NetworkEvent;
99
use dash_sdk::dash_spv::network::PeerNetworkManager;
10-
use dash_sdk::dash_spv::storage::{BlockHeaderStorage, DiskStorageManager};
10+
use dash_sdk::dash_spv::storage::DiskStorageManager;
1111
use dash_sdk::dash_spv::sync::SyncEvent;
1212
use dash_sdk::dash_spv::sync::SyncProgress as SpvSyncProgress;
1313
use dash_sdk::dash_spv::sync::SyncState;
@@ -711,120 +711,6 @@ impl SpvManager {
711711
wm.filter_committed_height()
712712
}
713713

714-
/// Non-blocking variant of `filter_committed_height`. Returns `None` when
715-
/// the WalletManager lock is currently held elsewhere (e.g. a wallet load
716-
/// is in flight). Callers on sync paths should treat `None` conservatively.
717-
pub fn try_filter_committed_height(&self) -> Option<u32> {
718-
self.wallet
719-
.try_read()
720-
.ok()
721-
.map(|wm| wm.filter_committed_height())
722-
}
723-
724-
/// Stop the running SPV loop, wait for it to exit cleanly, then start it
725-
/// again with the given expected wallet count.
726-
///
727-
/// The on-disk state (headers, filters, blocks) survives the restart — only
728-
/// the in-memory `WalletManager`/`FiltersManager` derived state is rebuilt.
729-
/// The filter scan re-reads persisted filters against the fresh wallet set
730-
/// from genesis (or the earliest birth height), which is the whole point of
731-
/// the mid-session import workaround.
732-
///
733-
/// Returns once `start()` has been invoked; sync itself runs in the
734-
/// background via the main loop task. The caller is expected to re-queue
735-
/// wallet loads (e.g. via `bootstrap_loaded_wallets`) after this returns.
736-
///
737-
/// Timing: `stop()` cancels the loop asynchronously — this method polls
738-
/// the `stop_token` guard until the main loop clears it (see bottom of
739-
/// `start()`'s spawned task). If the loop fails to exit within the timeout
740-
/// we still try to start again; stale state is cleared by the normal
741-
/// `run_spv_loop` setup path.
742-
pub async fn restart(self: &Arc<Self>, expected_wallet_count: usize) -> SpvResult<()> {
743-
const STOP_WAIT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
744-
const STOP_POLL_INTERVAL: std::time::Duration = std::time::Duration::from_millis(50);
745-
746-
tracing::info!(
747-
expected_wallet_count,
748-
"SpvManager::restart() — stopping current run"
749-
);
750-
self.stop();
751-
752-
let deadline = tokio::time::Instant::now() + STOP_WAIT_TIMEOUT;
753-
loop {
754-
let still_running = self
755-
.stop_token
756-
.lock()
757-
.map(|g| g.is_some())
758-
.map_err(|_| SpvError::LockPoisoned("stop_token".into()))?;
759-
if !still_running {
760-
break;
761-
}
762-
if tokio::time::Instant::now() >= deadline {
763-
tracing::warn!(
764-
"SpvManager::restart(): timed out waiting for previous run to exit; starting anyway"
765-
);
766-
// Clear the stop_token so the next start() isn't a no-op.
767-
if let Ok(mut guard) = self.stop_token.lock() {
768-
*guard = None;
769-
}
770-
break;
771-
}
772-
tokio::time::sleep(STOP_POLL_INTERVAL).await;
773-
}
774-
775-
// Determine the rescan floor: the earliest block header height stored on
776-
// disk. FiltersManager can only iterate heights that have stored headers,
777-
// so setting synced_height below this floor triggers missing-header panics.
778-
// After stop() the storage Arc is still alive here (cleared in start() setup).
779-
let rescan_floor: u32 = {
780-
let storage_opt = self
781-
.storage
782-
.lock()
783-
.ok()
784-
.and_then(|guard| guard.as_ref().map(Arc::clone));
785-
if let Some(storage_arc) = storage_opt {
786-
let storage = storage_arc.lock().await;
787-
storage.get_start_height().await.unwrap_or(0)
788-
} else {
789-
0
790-
}
791-
};
792-
tracing::info!(
793-
rescan_floor,
794-
"SpvManager::restart() — rescan floor determined"
795-
);
796-
797-
// Reset WalletManager's filter_committed_height to the rescan floor so the
798-
// next FiltersManager starts scanning from there instead of reading the stale
799-
// committed height and declaring itself "already synced".
800-
// We reset filter_committed_height (not synced_height) because at
801-
// rust-dashcore 309fac8 these became independent fields — FiltersManager::new()
802-
// reads filter_committed_height() for its "already synced" guard.
803-
// Using get_start_height() as the floor avoids missing-header panics that occur
804-
// when iterating below the checkpoint (storage starts there).
805-
{
806-
let mut wm = self.wallet.write().await;
807-
wm.update_filter_committed_height(rescan_floor);
808-
tracing::info!(
809-
rescan_floor,
810-
"SpvManager::restart() — reset WalletManager filter_committed_height to rescan_floor"
811-
);
812-
}
813-
814-
// Clear the seed→wallet_id map so bootstrap_loaded_wallets re-imports
815-
// cleanly instead of hitting the "wallet already exists" branch.
816-
if let Ok(mut wallet_map) = self.det_wallets.write() {
817-
wallet_map.clear();
818-
}
819-
820-
tracing::info!(
821-
expected_wallet_count,
822-
"SpvManager::restart() — starting fresh SPV run"
823-
);
824-
self.start(expected_wallet_count)
825-
.map_err(SpvError::SyncFailed)
826-
}
827-
828714
pub fn wallet(&self) -> Arc<AsyncRwLock<WalletManager<ManagedWalletInfo>>> {
829715
Arc::clone(&self.wallet)
830716
}

0 commit comments

Comments
 (0)