fix: close #485 scope gaps — _onHidden race + onboarding mnemonic

lib/packages/service/wallet_service.dart

@@ -48,9 +48,40 @@ class WalletService {
     this._appStore,
   );
 
-  Future<SoftwareWallet> createSeedWallet(String name) async {
+  /// Generates a fresh bip39 mnemonic and returns a [SoftwareWallet] that
+  /// is **not yet persisted** — `id` is the `0` sentinel and no row has
+  /// been written to `walletInfos`. Pair with [commitGeneratedWallet] once
+  /// the user has confirmed the seed (e.g. via the verify-seed quiz) so the
+  /// encrypted mnemonic only lands on disk for seeds the user has actually
+  /// kept. Prevents N+1 encrypted-seed rows from accumulating when the
+  /// onboarding cubit regenerates the mnemonic on every `hidden` cycle.
+  Future<SoftwareWallet> generateUncommittedSeedWallet(String name) async {
     final mnemonic = bip39.generateMnemonic();
-    return _persistSoftwareWallet(name, mnemonic);
+    return SoftwareWallet(0, name, mnemonic);
+  }
+
+  /// Persists a [draft] [SoftwareWallet] returned from
+  /// [generateUncommittedSeedWallet] into `walletInfos` (encrypted seed +
+  /// cached address) and returns a new [SoftwareWallet] carrying the
+  /// DB-assigned id. The draft is expected to carry the `0` sentinel id; a
+  /// different id indicates a misuse (commit called on an already-persisted
+  /// wallet) — surfaced via [assert] in dev and tolerated in release by
+  /// re-using the draft's seed.
+  Future<SoftwareWallet> commitGeneratedWallet(SoftwareWallet draft) async {
+    assert(draft.id == 0,
+        'commitGeneratedWallet expects an uncommitted draft (id == 0); '
+        'got id=${draft.id} — likely double-commit or wrong caller.');
+    return _persistSoftwareWallet(draft.name, draft.seed);
+  }
+
+  /// Generate-and-commit convenience for callers that persist immediately
+  /// (e.g. [restoreWallet]). Onboarding callers should NOT use this — they
+  /// must call [generateUncommittedSeedWallet] and defer [commitGeneratedWallet]
+  /// until the user has confirmed the seed, otherwise every regenerate on
+  /// `hidden` writes an undeletable encrypted-seed row to `walletInfos`.
+  Future<SoftwareWallet> createSeedWallet(String name) async {
+    final draft = await generateUncommittedSeedWallet(name);
+    return commitGeneratedWallet(draft);
   }
 
   Future<BitboxWallet> createBitboxWallet(String name) async {
@@ -60,6 +91,10 @@ class WalletService {
     return BitboxWallet(walletId, name, address, _bitboxService);
   }
 
+  /// Persists a user-supplied seed phrase immediately — the user typed an
+  /// existing mnemonic, so there is no verify-seed quiz to gate the write
+  /// behind. Deferring would not help: the seed is already known and the
+  /// user expects to land on the dashboard on `restore` success.
   Future<SoftwareWallet> restoreWallet(String name, String seed) async {
     final wallet = await _persistSoftwareWallet(name, seed);
     await _settingsRepository.saveCurrentWalletId(wallet.id);
@@ -146,23 +181,44 @@ class WalletService {
   /// don't tear the unlocked state out from under each other.
   Future<void> ensureCurrentWalletUnlocked() async {
     _activeUnlockHolders++;
+    // Tracks whether THIS call's unlock landed in [AppStore.wallet]. When
+    // the lock-during-flight invalidates the slot, the write is skipped and
+    // arming the post-unlock timer would just point at a [SoftwareViewWallet]
+    // — `_lockWalletInPlace` would safely no-op via `is! SoftwareWallet`, so
+    // it's dead work, not a correctness bug. Skip the arm in that case.
+    var landedInStore = false;
     if (_appStore.wallet is SoftwareViewWallet) {
       // Coalesce overlapping unlocks onto a single in-flight future so we
       // don't hammer the DB and re-run AES-GCM for every concurrent caller.
       final inFlight = _unlockInFlight ??= unlockCurrentWallet();
       try {
-        _appStore.wallet = await inFlight;
+        final unlocked = await inFlight;
+        // If [lockCurrentWallet] fired while this unlock was in flight, it
+        // invalidates the slot (`_unlockInFlight = null`). Skip the write
+        // so the mnemonic doesn't resurface in [AppStore.wallet] after the
+        // user has already covered the app — closes the `_onHidden` race.
+        if (identical(_unlockInFlight, inFlight)) {
+          _appStore.wallet = unlocked;
+          landedInStore = true;
+        }
       } finally {
         // Only the caller that started the unlock clears the slot; later
         // joiners observe the field already nulled and skip the clear.
         if (identical(_unlockInFlight, inFlight)) _unlockInFlight = null;
       }
+    } else {
+      // Wallet was already unlocked (a previous holder's write is still in
+      // place). Re-arming the safety net is the right call — extends the
+      // 60 s window for the joining holder.
+      landedInStore = true;
     }
     // Safety net: if a caller forgets to call lockCurrentWallet() after the
     // sign, drop the mnemonic anyway once the post-unlock window elapses. The
-    // reviewer flagged "user sells once then leaves the app foregrounded" —
-    // that path now caps at [_postUnlockLockTimeout].
-    _schedulePostUnlockLock();
+    // reviewer flagged "user signs once then leaves the app foregrounded" —
+    // that path now caps at [_postUnlockLockTimeout]. Skip the arm when the
+    // intervening lock invalidated our write: the timer would just fire
+    // against a view-wallet (no-op) and we'd be doing dead work.
+    if (landedInStore) _schedulePostUnlockLock();
   }
 
   /// Replaces the in-memory [SoftwareWallet] with its lock-screen-safe
@@ -184,6 +240,12 @@ class WalletService {
 
     if (_activeUnlockHolders > 0) _activeUnlockHolders--;
     if (_activeUnlockHolders > 0) return;
+    // Invalidate any in-flight unlock so its resolution doesn't write the
+    // unlocked [SoftwareWallet] back into [AppStore.wallet] after this lock —
+    // the race the 60s safety net used to catch as defence-in-depth, now
+    // closed at the source.
+    _unlockInFlight?.ignore();
+    _unlockInFlight = null;
     _postUnlockLockTimer?.cancel();
     _postUnlockLockTimer = null;
     _lockWalletInPlace();

lib/screens/create_wallet/bloc/create_wallet_cubit.dart

@@ -1,5 +1,6 @@
 import 'dart:async';
 
+import 'package:flutter/widgets.dart';
 import 'package:flutter_bloc/flutter_bloc.dart';
 import 'package:realunit_wallet/packages/service/dfx/dfx_auth_service.dart';
 import 'package:realunit_wallet/packages/service/wallet_service.dart';
@@ -8,27 +9,82 @@ import 'package:realunit_wallet/packages/wallet/wallet.dart';
 part 'create_wallet_state.dart';
 
 class CreateWalletCubit extends Cubit<CreateWalletState> {
-  CreateWalletCubit(this._service, this._authService) : super(const CreateWalletState());
+  CreateWalletCubit(this._service, this._authService) : super(const CreateWalletState()) {
+    // Onboarding-equivalent of `WalletService.lockCurrentWallet()` for the
+    // freshly generated mnemonic. While the user is on the create-wallet
+    // screen, the mnemonic lives in `CreateWalletState.wallet` — not in
+    // `AppStore.wallet` — so the service-level lock is a no-op for this
+    // path. Clearing the cubit state on `hidden` drops the seed before iOS
+    // suspends the isolate; the user returning is sent back to the start
+    // of the create flow, which is the safe restart point.
+    _lifecycleListener = AppLifecycleListener(onStateChange: _onLifecycleState);
+  }
 
   final WalletService _service;
   final DFXAuthService _authService;
+  late final AppLifecycleListener _lifecycleListener;
 
   void createWallet() async {
-    final wallet = await _service.createSeedWallet('Obi-Wallet-Kenobi');
-    // Fire-and-forget the auth-signature capture — the lazy path in
-    // DFXAuthService.getSignature is the safety net, and a 20 s HTTP timeout
-    // shouldn't gate the "creating wallet" UI.
+    // Defer the DB write to `VerifySeedCubit.verify()` via
+    // `WalletService.commitGeneratedWallet`. Writing on every regenerate
+    // would persist a fresh encrypted-seed row on each `_dropMnemonic`
+    // cycle (N+1 rows per onboarding session with N hide-cycles), and
+    // `WalletStorage.deleteWallet` only touches `walletAccountInfos` —
+    // those `walletInfos` rows would accumulate undeletable. The draft
+    // carries the `0` sentinel id until committed.
+    final wallet = await _service.generateUncommittedSeedWallet('Obi-Wallet-Kenobi');
+    // Fire-and-forget the auth-signature capture. The signature is derived
+    // from the primary address, which is deterministic from the mnemonic
+    // — valid for the same wallet once it's committed. The lazy path in
+    // DFXAuthService.getSignature is the safety net, and a 20 s HTTP
+    // timeout shouldn't gate the "creating wallet" UI.
     unawaited(
       warmAuthSignature(
         _authService,
         wallet.currentAccount,
         loggerName: '$CreateWalletCubit',
       ),
     );
+    // Async-tail guard: with the `_dropMnemonic` re-fire on `hidden`, the
+    // user can return to foreground and immediately pop the screen before
+    // the regenerated `generateUncommittedSeedWallet` resolves — the
+    // AppBar back closes the cubit, and a post-close `emit` would throw
+    // `StateError`. Matches the `connect_bitbox_cubit` / `kyc_cubit`
+    // pattern.
+    if (isClosed) return;
     emit(state.copyWith(wallet: wallet));
   }
 
   void toggleShowSeed() {
     emit(state.copyWith(hideSeed: !state.hideSeed));
   }
+
+  void _onLifecycleState(AppLifecycleState state) {
+    if (state == AppLifecycleState.hidden) {
+      _dropMnemonic();
+    }
+  }
+
+  void _dropMnemonic() {
+    // Reset to the initial state — drops `wallet` (and its mnemonic) and
+    // restores the default `hideSeed: true`. `copyWith` would carry the
+    // existing wallet through, so we emit a fresh state explicitly.
+    if (state.wallet == null) return;
+    emit(const CreateWalletState());
+    // The cubit is built once via `BlocProvider.create` (`..createWallet()`
+    // fires exactly once at construction), so without re-firing here the
+    // user would resume to a `state.wallet == null` and the view's
+    // `BlocBuilder` would render `CupertinoActivityIndicator` indefinitely
+    // — escapable only via the AppBar back button. Re-issue a fresh
+    // generation so the next emission replaces the cleared state; the
+    // screen briefly flashes the loading indicator, then re-renders with
+    // the new mnemonic. The prior in-memory seed is already gone.
+    createWallet();
+  }
+
+  @override
+  Future<void> close() {
+    _lifecycleListener.dispose();
+    return super.close();
+  }
 }

lib/screens/verify_seed/cubit/verify_seed_cubit.dart

@@ -17,7 +17,11 @@ class VerifySeedCubit extends Cubit<VerifySeedState> {
     _initVerification();
   }
 
-  final SoftwareWallet _wallet;
+  /// The draft wallet handed in by `CreateWalletCubit`. Until [verify]
+  /// succeeds and `WalletService.commitGeneratedWallet` lands the row, the
+  /// id is the `0` sentinel — it must NOT be passed to
+  /// `setCurrentWallet` directly; commit first, use the returned id.
+  SoftwareWallet _wallet;
   final WalletService _walletService;
 
   void _initVerification() {
@@ -63,7 +67,17 @@ class VerifySeedCubit extends Cubit<VerifySeedState> {
       }
     }
 
-    await _walletService.setCurrentWallet(_wallet.id);
+    // Commit the draft mnemonic to disk BEFORE marking it current — the
+    // wallet handed in by `CreateWalletCubit` is the in-memory-only draft
+    // produced by `WalletService.generateUncommittedSeedWallet` (id == 0).
+    // Persisting at confirm time means a regenerate triggered by an
+    // app-hidden cycle in the create flow never leaves an orphan row in
+    // `walletInfos`. The user only reaches this branch by typing the four
+    // requested words correctly, so the seed they kept is the seed we
+    // store.
+    final committed = await _walletService.commitGeneratedWallet(_wallet);
+    _wallet = committed;
+    await _walletService.setCurrentWallet(committed.id);
     emit(state.copyWith(isVerified: true));
     return true;
   }

test/packages/service/wallet_service_test.dart

@@ -58,8 +58,86 @@ void main() {
   });
 
   group('$WalletService', () {
+    group('generateUncommittedSeedWallet', () {
+      test('returns an in-memory SoftwareWallet with the id=0 sentinel and a valid bip39 mnemonic',
+          () async {
+        final draft = await service.generateUncommittedSeedWallet('Main');
+
+        expect(draft, isA<SoftwareWallet>());
+        expect(draft.id, 0,
+            reason: 'uncommitted drafts use the 0 sentinel until commitGeneratedWallet lands the row');
+        expect(draft.name, 'Main');
+        expect(service.validateSeed(draft.seed), isTrue);
+      });
+
+      test('does NOT write to the repository — the encrypted seed must not land on disk', () async {
+        await service.generateUncommittedSeedWallet('Main');
+
+        // Pin the disk-side guarantee: nothing flows into `walletInfos` until
+        // a separate `commitGeneratedWallet` call. Without this, every
+        // `_dropMnemonic` regenerate in `CreateWalletCubit` would persist a
+        // fresh encrypted-seed row, and `WalletStorage.deleteWallet` only
+        // touches `walletAccountInfos`, so those rows would accumulate
+        // undeletable.
+        verifyNever(() => repo.createWallet(any(), any(), any(), any()));
+        verifyNever(() => settings.saveCurrentWalletId(any()));
+      });
+
+      test('two consecutive calls produce distinct mnemonics (entropy not pinned by the API)',
+          () async {
+        final a = await service.generateUncommittedSeedWallet('Main');
+        final b = await service.generateUncommittedSeedWallet('Main');
+
+        expect(a.seed, isNot(equals(b.seed)),
+            reason: 'each call must produce a fresh mnemonic — pinning entropy would '
+                'silently break the "regenerate on hidden" contract');
+      });
+    });
+
+    group('commitGeneratedWallet', () {
+      test('persists the draft seed and returns a SoftwareWallet carrying the DB-assigned id',
+          () async {
+        when(() => repo.createWallet(any(), any(), any(), any())).thenAnswer((_) async => 42);
+
+        final draft = await service.generateUncommittedSeedWallet('Main');
+        final committed = await service.commitGeneratedWallet(draft);
+
+        expect(committed.id, 42);
+        expect(committed.name, 'Main');
+        expect(committed.seed, draft.seed,
+            reason: 'commit must preserve the draft mnemonic — no silent re-generation');
+        final expectedAddress = committed.currentAccount.primaryAddress.address.hexEip55;
+        verify(
+          () => repo.createWallet('Main', WalletType.software, draft.seed, expectedAddress),
+        ).called(1);
+      });
+
+      test('writes exactly one row per call (no implicit dedup at this layer)', () async {
+        // Pin the disk-side contract: each commit call is one row. The dedup
+        // lives at the cubit layer (`VerifySeedCubit.verify` is invoked once
+        // per successful quiz). Surfaces a regression where commit silently
+        // dedups and a follow-up caller assumes idempotence.
+        when(() => repo.createWallet(any(), any(), any(), any())).thenAnswer((_) async => 1);
+
+        final draft = await service.generateUncommittedSeedWallet('Main');
+        await service.commitGeneratedWallet(draft);
+
+        verify(() => repo.createWallet(any(), any(), any(), any())).called(1);
+      });
+
+      test('does not set the wallet as current (caller is responsible)', () async {
+        when(() => repo.createWallet(any(), any(), any(), any())).thenAnswer((_) async => 7);
+
+        final draft = await service.generateUncommittedSeedWallet('Main');
+        await service.commitGeneratedWallet(draft);
+
+        verifyNever(() => settings.saveCurrentWalletId(any()));
+      });
+    });
+
     group('createSeedWallet', () {
-      test('returns a SoftwareWallet with the generated mnemonic and address persisted', () async {
+      test('generate+commit convenience — persists a freshly generated mnemonic in one call',
+          () async {
         when(() => repo.createWallet(any(), any(), any(), any())).thenAnswer((_) async => 42);
 
         final wallet = await service.createSeedWallet('Main');
@@ -503,6 +581,62 @@ void main() {
             reason: 'final holder release flips back to view wallet');
       });
 
+      // The `_onHidden` race: a single sign-flow ensure is still mid-unlock
+      // when `lockCurrentWallet` fires from the app-lifecycle hidden hook.
+      // Without invalidating the in-flight unlock, its resolution would
+      // write the unlocked [SoftwareWallet] back to [AppStore.wallet]
+      // AFTER the lock — resurfacing the mnemonic in memory until either
+      // the 60s safety net or the sign-flow `finally lock` clears it
+      // again. The 60s window is best-effort under iOS isolate suspension
+      // (the gap #485 set out to close in the first place), so the fix
+      // closes it at the source: the lock invalidates `_unlockInFlight`
+      // and the ensure skips its write.
+      test('lock during a single in-flight unlock does not resurface the mnemonic',
+          () async {
+        final stored = <AWallet>[SoftwareViewWallet(7, 'Main', _debugAddress)];
+        when(() => appStore.wallet).thenAnswer((_) => stored.last);
+        when(() => appStore.wallet = any(that: isA<AWallet>())).thenAnswer((inv) {
+          final newWallet = inv.positionalArguments.single as AWallet;
+          stored.add(newWallet);
+          return newWallet;
+        });
+        when(() => settings.currentWalletId).thenReturn(7);
+
+        // Pin the unlock mid-flight so we can fire `lockCurrentWallet`
+        // exactly between the ensure starting and its DB read resolving.
+        final gate = Completer<WalletInfo>();
+        when(() => repo.getUnlockedWalletById(7)).thenAnswer((_) => gate.future);
+
+        // Sign-flow ensure starts, counter=1, blocks on gated read.
+        final ensure = service.ensureCurrentWalletUnlocked();
+
+        // App-lifecycle hidden fires — counter goes to 0, lock would
+        // normally no-op (wallet still SoftwareViewWallet) and let the
+        // pending unlock leak through.
+        await service.lockCurrentWallet();
+        expect(stored.last, isA<SoftwareViewWallet>(),
+            reason: 'lock observed the still-view wallet — nothing to flip');
+
+        // Release the gated DB read so the in-flight ensure resolves.
+        gate.complete(
+          _info(id: 7, name: 'Main', seed: _testMnemonic, type: WalletType.software),
+        );
+        await ensure;
+
+        // The fix: the post-resolve write is gated on the in-flight token
+        // still matching, which the lock invalidated. So the mnemonic
+        // never lands in [AppStore.wallet] after the user covered the app.
+        expect(stored.last, isA<SoftwareViewWallet>(),
+            reason: 'in-flight unlock invalidated by intervening lock must not '
+                'resurface the mnemonic');
+        // Pin the mechanism, not just the outcome: the `_unlockInFlight`
+        // gate must suppress the post-resolve write — never let a future
+        // refactor pass this test by tolerating the write and clearing it
+        // again from somewhere else (which would still expose the mnemonic
+        // to any code path observing `AppStore.wallet` between the writes).
+        verifyNever(() => appStore.wallet = any(that: isA<SoftwareWallet>()));
+      });
+
       // Two overlapping ensures must coalesce onto a single DB read +
       // AES-GCM decrypt, not trigger the repository twice. Functionally
       // both versions would land on the same SoftwareWallet, but the