What the developer wrote
Authored by Justin Ehrenhofer
Fix: Delete additional items
What changed, and why it matters
This commit fixes a bug in the wallet-deletion process. Previously, the app deleted wallet files while a background sync service still had them open in another process. That could leave deleted wallet data partially restored on disk, or cause the app to keep syncing a wallet the user asked to remove. The fix tells the sync service to stop first, then deletes the wallet through the core library's proper teardown path.
Treat as a security-relevant bug fix. Verify that `stopSyncAndDeleteWallets` reliably stops the foreground isolate and releases file handles before deletion, and that no other background tasks (e.g., Tor service, notification service) can reopen the wallet during deletion. Add tests covering deletion while a foreground sync is active. Review whether similar teardown ordering issues exist for wallet rebuild or logout paths.
Security signals we found
Deletion of sensitive files while another isolate/process still has them open
Possible resurrection/rewriting of wallet data after user-initiated deletion
Missing teardown ordering between foreground service and wallet manager
Fix explicitly references preventing a deleted wallet from continuing to sync and rewrite
Evidence from the diff
The patch changes deleteWallet() in lib/wallet_core_glue.dart from calling WalletManager.deleteAll() directly to calling a new stopSyncAndDeleteWallets() helper exported from wallet_background. It also exports stopSyncAndDeleteWallets in lib/services/foreground_sync_service.dart. The inline comment explains that the foreground sync isolate holds its own wallet2 instance open on the same files, so deleting them while it runs leaves that isolate syncing and rewriting a wallet the user just deleted. The fix ensures the foreground service is stopped before the wallet files are removed, preventing resurrection of deleted state and possible data-consistency issues.
Changed components
lib/services/foreground_sync_service.dartlib/wallet_core_glue.dartwallet_background foreground sync isolateWalletManager.deleteAll / stopSyncAndDeleteWalletsInspect captured patch +14 / −3
diff --git a/lib/services/foreground_sync_service.dart b/lib/services/foreground_sync_service.dart
index bf2ca3a..5337bde 100644
--- a/lib/services/foreground_sync_service.dart
+++ b/lib/services/foreground_sync_service.dart
@@ -6,7 +6,12 @@ import 'package:wallet_background/wallet_background.dart';
// The handler + service control live in wallet-core (`wallet_background`); the
// app keeps only the isolate entry point (see periodic_tasks.dart).
export 'package:wallet_background/wallet_background.dart'
- show startForegroundSync, stopForegroundSync, startForegroundSyncIfEnabled, isWalletFullySynced;
+ show
+ startForegroundSync,
+ stopForegroundSync,
+ startForegroundSyncIfEnabled,
+ isWalletFullySynced,
+ stopSyncAndDeleteWallets;
/// Foreground-service isolate entry: bootstrap this isolate, then hand off to
/// the shared handler. Top-level `@pragma` so it survives tree-shaking.
diff --git a/lib/wallet_core_glue.dart b/lib/wallet_core_glue.dart
index 8452dce..adc003e 100644
--- a/lib/wallet_core_glue.dart
+++ b/lib/wallet_core_glue.dart
@@ -9,7 +9,8 @@ import 'package:skylight_wallet/models/fiat_rate_model.dart';
import 'package:skylight_wallet/models/monero_wallet_adapter.dart';
import 'package:skylight_wallet/widgets/tx_details.dart' show TxDetailsDialog;
import 'package:skylight_wallet/periodic_tasks.dart' show backgroundDispatcher;
-import 'package:skylight_wallet/services/foreground_sync_service.dart' show foregroundSyncCallback;
+import 'package:skylight_wallet/services/foreground_sync_service.dart'
+ show foregroundSyncCallback, stopSyncAndDeleteWallets;
import 'package:skylight_wallet/services/notifications_service.dart';
import 'package:skylight_wallet/services/shared_preferences_service.dart';
import 'package:skylight_wallet/services/tor_service.dart';
@@ -230,10 +231,15 @@ Future<void> unlockWithPassword(BuildContext context, String password) async {
}
/// Deletes the wallet and everything derived from it.
+///
+/// Through wallet-core's teardown rather than a bare `deleteAll`: the
+/// foreground service holds its own wallet2 instance open on these files in its
+/// own isolate, and deleting them while it runs leaves it syncing -- and
+/// rewriting -- a wallet the user just deleted.
Future<void> deleteWallet(BuildContext context) async {
// TODO(wallet-core): pass skylight's own pref keys (contacts, pending tx,
// notification state) once the delete path is validated on device.
- await Provider.of<WalletManager>(context, listen: false).deleteAll();
+ await stopSyncAndDeleteWallets(Provider.of<WalletManager>(context, listen: false));
}
/// Rebuilds the wallet if the server kind (LWS↔node) changed, then resyncs.
Why this scored 57/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.