polish: PR #472 follow-ups — real race test + DRY + visibility + in-flight dedup#483
Merged
Conversation
Issue #468 follow-ups #461 — items A (routing) and F (architecture). Restores AppStore as a pure state container by giving the locking lifecycle to WalletService directly: - AppStore: drop _unlocker / attachUnlocker / ensureUnlocked. - WalletService: take AppStore as a constructor dep, add ensureCurrentWalletUnlocked() (1.3) and lockCurrentWallet() (4.2 prep). - WalletService now a lazySingleton (6.2). - DFXAuthService and all 12 subclasses thread WalletService through their constructors; getSignature() unlocks → signs → locks again. - SellPaymentInfoService.confirmPayment / RegistrationService.completeRegistration / registerWallet lock the wallet in a finally block so a mid-sign throw doesn't leave the mnemonic resident. - SettingsSeedCubit takes WalletService directly; locks again on close(). SellButton routes PaymentInfoError.bitboxDisconnected through the showBitboxReconnectSheet helper (1.1) — matches the buy / sell-bitbox / user-data flows. _LockedCredentials / SoftwareViewWalletAccount drop the typed WalletLockedException (1.2): every signing path now goes through ensureCurrentWalletUnlocked() first, so the locked sign call is a programmer error — surfaced via assert + StateError instead of an exception nothing catches. WalletLockedException class removed. SellBitboxCubit drops the redundant credentials.isConnected pre-check in confirmSwap / confirmDeposit (6.3) — the existing on BitboxNotConnectedException catch covers it. Page tests for CreateWallet / RestoreWallet now register DfxKycService alongside WalletService so the cubit's DFXAuthService dep resolves (5.1). SettingsSeedCubit._loadSeed uses copyWith so a toggleShowSeed that races ahead of the unlock isn't dropped when the seed lands.
Issue #468 follow-ups #461 — items 2.1 / 2.2 (UX regressions). ConnectBitboxCubit / CreateWalletCubit / RestoreWalletCubit now wrap the DFXAuthService.ensureSignatureFor call in unawaited(), so a slow 20 s HTTP timeout no longer freezes the onboarding UI between create/restore/pair and the success state. The lazy sign in DFXAuthService.getSignature still recovers on failure. ConnectBitboxCubit additionally: - starts the connection-status observer before the unawaited warm-up so a disconnect mid-warm-up is detected. - raises the warm-up failure to SEVERE so it surfaces in support logs — the failure mode it papers over (extra BitBox prompt on first auth) is exactly the regression #461 was fixing. i18n: new `connectBitboxSignInHint` key (en + de, alphabetically inserted between connectBitboxFailed and connectBitboxTitle) shown on the channel-hash verify screen, so the user knows a second BitBox confirmation is coming after the code match.
Issue #468 follow-ups #461 — items 3.1 through 3.5. - _runOrThrowDisconnect narrows to `on Exception catch` so a TypeError / assert from a sign-internal bug stays its own error instead of being silently re-labelled as a BitboxNotConnectedException. The original error + stack are logged before the rewrap. (3.1) - Every sign method snapshots `bitboxManager` + `derivationPath` into locals up-front, so the disconnect observer nulling them between the null-check and the call can no longer NoSuchMethod the bang-operator path. (3.2) - BitboxService tracks an in-flight `_pendingDisconnect` Future and awaits it at the top of `init()`, so a rapid disconnect-then-reconnect on the same singleton manager no longer races. The disconnect path also moves to `on Exception catch` to match the rest of the file. (3.3) - `BitboxCredentials.setBitbox(connection, [path])` falls back to the existing `derivationPath` when no path is passed, and `clearBitbox()` no longer wipes the path. Default-path-today, but multi-account would silently revert on every reconnect otherwise. (3.4) - `BitboxService._credentialsByAddress` (a Map) replaces the single `_credentials` field, so a wallet switch that hands out fresh credentials no longer orphans the previous wallet's reference. The reconnect observer + clear paths iterate the map. (3.5)
Issue #468 follow-ups #461 — items 4.1 and 4.2. - SecureStorage._pinHashIterations: 100_000 → 250_000. Roughly halves the offline brute-force budget for an attacker who has already broken the Keystore/Keychain boundary while staying sub-second on a mid-range phone. 100_000 hashes are now accepted once via [_legacyIterationCandidates] and transparently rehashed up to the new target on the next successful unlock. (4.1) - WalletService.ensureCurrentWalletUnlocked() arms a 60 s idle timer that calls lockCurrentWallet() if the caller forgets. Covers the "user sells once then leaves the app foregrounded" path the reviewer flagged — the mnemonic can't sit on the heap for an hour just because one sign path skipped the explicit lock. lockCurrentWallet() cancels the timer. (4.2) - RealUnitRegistrationService.completeRegistration / registerWallet now use try/finally so the explicit lock fires on the throw path too.
Issue #468 follow-ups #461 — items 5.2 and 5.3. - $SoftwareViewWallet: assert every sign entry point (signMessage, signToSignature, signPersonalMessage, signPersonalMessageToUint8List, signToEcSignature) surfaces as an Error subtype — AssertionError in debug, StateError in release — so a caller that bypasses WalletService.ensureCurrentWalletUnlocked() can't quietly read a wrong- type signature. - $WalletService.ensureCurrentWalletUnlocked: promotes a view wallet to a SoftwareWallet via getUnlockedWalletById, no-op for already-unlocked wallets. - $WalletService.lockCurrentWallet: writes back a SoftwareViewWallet, no-op for non-software wallets. - $BitboxCredentials race tests: - disconnect-mid-sign: manager.devices returns empty during sign → BitboxNotConnectedException (not the underlying Exception leaking). - cleared credentials: a sign called after clearBitbox() throws BitboxNotConnectedException — exercises the snapshot-on-entry pattern so a NoSuchMethodError can't slip through the bang-operator path.
Senior review follow-ups on PR #472 — four small correctness fixes around the post-unlock locking contract introduced in #461. - DFXAuthService.getSignature now wraps the sign + saveSignature in a try/finally so a thrown SigningCancelledException / TimeoutException / BitBox disconnect / cache-write failure can't skip lockCurrentWallet() and leave the mnemonic resident for the full 60 s window. Matches the pattern already used in RealUnitSellPaymentInfoService.confirmPayment and RealUnitRegistrationService.completeRegistration / registerWallet. - WalletService._scheduleIdleLock callback no longer double-nulls the timer field. lockCurrentWallet() owns the cancel + null itself, so the callback now binds directly to the bound method (Timer(timeout, lock)) and the two pieces of code stop sharing the invariant. - WalletService._idleLockTimeout / _idleLockTimer renamed to _postUnlockLockTimeout / _postUnlockLockTimer. The timer isn't reset on user activity — it caps the lifetime of the in-memory mnemonic after each explicit unlock — so "idle" was the wrong noun. Doc comment updated to say so. - BitboxService._credentialsByAddress now goes through a private _key() normaliser (lowercase) so a future EIP-55-vs-raw-hex address handoff can't fork the map and leak / orphan credentials. Lowercase invariant documented at the field declaration.
…e class PR #478 landed an exception surface test that enumerates every typed exception. PR #472 removed WalletLockedException in favour of assert + StateError (locked-sign is now a programmer error, not a runtime exception). Remove the obsolete entry + import so the test compiles against the post-#472 surface.
PR #473 pinned the pre-#472 contract: each getCredentials() returned a fresh instance. PR #472's robustness pass replaced the single _credentials slot with a Map<address, BitboxCredentials> + a per-entry clear loop in the disconnect observer. Under the new contract every caller for a given address must see the *same* canonical instance — otherwise the observer would clear an orphaned reference while the caller keeps holding the older one. Flip the assertion + rewrite the defensive-pin comment to match.
…can't trip MnemonicReadOnlyField's length assert #472 left SettingsSeedView wide open to a first-frame crash whenever the app boots with a SoftwareViewWallet — the default production state after #461. The cubit's initial state has an empty seed, the view used to hand that straight to SeedBlurCard → MnemonicReadOnlyField([]) and the `length == 12` assert killed the screen on every "Wallet sichern" open in debug, with a RangeError taking its place in release. The existing settings_seed_page_test couldn't catch it because the mocked appStore.wallet was a SoftwareWallet, not a SoftwareViewWallet — the seed was already in the initial state and the path that crashes in production never ran. Show a CupertinoActivityIndicator inside the BlocBuilder until the seed actually has 12 words. The early-seed optimisation for the already-unlocked case stays intact (state.seed is filled in the constructor), so re-opening the screen after one successful unlock still renders the SeedBlurCard on frame 0. Regression test added: drives a real SettingsSeedCubit against a SoftwareViewWallet fixture, holds the unlock future on a Completer, asserts the first frame shows the spinner with no SeedBlurCard, then completes the unlock and asserts the SeedBlurCard appears.
…nlock await `_loadSeed` awaits `ensureCurrentWalletUnlocked` and then casts + emits. If the user navigates away while the DB decrypt is in flight the cubit closes mid-await — the post-await emit then throws `StateError: Cannot emit after close` as an unhandled async error. Add an `if (isClosed) return;` between the await and the cast so the cubit closing during the unlock window is a quiet no-op instead of an uncaught error.
…lows can't tear the unlocked wallet out from under each other #472's ensure-then-lock pair was racey for two concurrent sign flows: flow A unlocks → signs → finally → locks; flow B between A's ensure and A's lock saw no-op (already unlocked) and proceeded mid-sign. A then locks, B reads `appStore.wallet.currentAccount.primaryAddress` → `_LockedCredentials` → `Eip712Signer` throws `UnsupportedError` because the locked sentinel has no signing surface. Add an `_activeUnlockHolders` counter: every ensure increments, every lock decrements and only flips the wallet back when the count hits 0. The 60s safety net can't respect the counter — a stuck holder would keep the mnemonic resident past the window — so it now runs through a dedicated `_forceLock` that zeroes the counter and unconditionally locks. The explicit `lockCurrentWallet` path still owns the holder contract. Test: two parallel ensures + one lock leaves the wallet unlocked (second holder still active); the second lock flips it back to a SoftwareViewWallet.
…oss paths can't race two disconnect() calls The periodic callback's body is async and straddles the `getAllUsbDevices()` await. On a slow probe two ticks can both reach the `devices.isEmpty` branch, both run `_pendingDisconnect = _disconnectAndForget()`, and the second assignment clobbers the first — now two `disconnect()` calls race on the SDK manager singleton with nothing left to await on. Guard the branch with `if (!_isConnected) return;` *before* flipping `_isConnected = false`. The first tick's flag flip is the single-writer fence: any second tick that arrived between the two ticks sees the already-cleared flag and bails before touching credentials, the observer cancel, or _pendingDisconnect.
…oarding flows log at the same SEVERE level
Three near-duplicate `_warmAuthSignature` helpers existed in
CreateWalletCubit, RestoreWalletCubit and ConnectBitboxCubit. Only the
BitBox one logged at `level: 1000` (SEVERE) — the other two used
default-level developer.log, which doesn't reach support telemetry.
PR body claimed SEVERE everywhere; only one of three actually was.
Move the helper to a top-level `warmAuthSignature(authService,
account, {required loggerName})` in dfx_auth_service.dart. All three
cubits call it; all log at SEVERE; the BitBox-specific copy in the
"next call triggers a fresh confirmation" wording is generalised to
"next authenticated call will trigger a fresh signature request".
create_wallet_page_test: the cubit now reads `wallet.currentAccount`
synchronously before handing it to the unawaited helper, so the
MockWallet has to surface a real account. Stub it (was: relied on the
old helper's try/catch swallowing the unstubbed-null cast).
…ethod to ensureCurrentWalletUnlocked Three callsites in the test tree still referenced the old name: - settings_seed_cubit_test L33 test name + L36 comment - real_unit_sell_payment_info_service_confirm_test L110 comment Rename so a future reader searching for the actual method finds the test, not the drift.
…a shared constant
…entials in wallet.dart
Contributor
BitBox02 simulator checkHost: Firmware:
Summary: 15 total · 15 passed · 0 failed |
Blume1977
approved these changes
May 21, 2026
Contributor
Blume1977
left a comment
Blume1977
left a comment
There was a problem hiding this comment.
✅ Approve.
Vollständige Verifikation gegen den Code abgeschlossen, alle 10 Commits durchgegangen, Logik selbst durchtraced, Tests grün (1412/1412), analyze clean.
Was die PR effektiv liefert
Der Titel sagt "polish", aber inhaltlich:
- 5 echte Bugfixes, davon einer 🔴 critical
- 1 Perf-Optimierung
- 3 Code-Hygiene-Commits
- 1 Test-Rename
Severity-Verteilung
| Severity | Commit | Was |
|---|---|---|
| 🔴 Critical | b0649 |
Concurrent-Sign-Race in #472: Flow A unlock → B sieht unlocked → A locks finally → B liest LockedCredentials → UnsupportedError. Counter-basiertes Holder-Tracking + _forceLock als 60s-Safety-Net das den Counter bypasst. Test deckt die Race via gated Completer<WalletInfo>. |
| 🔴 High | 60604 |
SettingsSeed-Crash beim ersten Öffnen nach #461. MnemonicReadOnlyField.length == 12 assert killte den Screen weil cubit mit empty seed startet. Mein Test mockte SoftwareWallet (statt SoftwareViewWallet) → production-Pfad nie getroffen. Spinner-Fix + Regression-Test mit echtem ViewWallet. |
| 🟠 Bug+DRY | 1841f |
warmAuthSignature SEVERE-Inkonsistenz: nur die BitBox-Variante loggte auf level 1000, die anderen zwei nicht — entgegen dem was #472 PR-Body behauptete. Top-level Helper, alle drei loggen jetzt SEVERE. |
| 🟡 Medium | f2149 |
isClosed-bail in SettingsSeedCubit gegen emit() after close() während DB-Decrypt. |
| 🟡 Medium | 9a94f |
BitBox-Observer single-writer-flag (if (!_isConnected) return VOR _isConnected = false) gegen Tick-Overlap-Race. |
| ✅ Perf | ea945 |
_unlockInFlight coalesce: 2 konkurrente Unlocks → 1 DB-Read. identical()-Check in finally für korrektes Clear-Lifecycle. |
| ✅ Polish | 8f539 |
Echte Concurrency-Test mit Completer-Gate — viel stärker als sequentieller Counter-Test. |
| ✅ Polish | 0e915 |
5× duplizierte 4-Zeilen-Comments → ein _viewWalletErrorRationale const. |
| ✅ Polish | 6df5e |
SoftwareViewWalletAccount zu wallet.dart migriert, parallel zum DebugWalletAccount-Pattern. |
| ⚪ Trivial | 7ef22 |
3 stale ensureUnlocked Test-References umbenannt. |
Logik selbst durchtraced
- Holder-Counter (b0649): 4 Szenarien (sequentiell, gated-concurrent, lock-before-resume, force-lock-during-flight) — alle korrekt.
- In-Flight Dedup (ea945):
_unlockInFlight ??=atomar,identical()-Check verhindert dass spätere Joiner den Slot clearen — wasserdicht. - Observer Fence (9a94f): Single-writer-flag VOR State-Mutation — Lehrbuch-korrekt.
- Force-Lock + Counter Interaktion: stuck holder kommt nicht über das 60s-Window hinaus, BitBox-Pfade sind no-ops (nicht SoftwareWallet), SW-Signs sind synchron und brauchen <60s — Trade-off bewusst.
Was ich beim eigenen #472-Self-Review übersehen habe
Drei der hier gefixten Issues habe ich beim "vollständigen kritischen Check" verpasst:
- Concurrent-Sign-Race (b0649) — meine "trace-through-the-scenarios"-Methode hatte keine Concurrency-Szenarien
- SettingsSeed-Crash (60604) — habe das pre-existing failing test als "Mock-Artifact" abgetan, war ein echter Production-Bug
- SEVERE-Inkonsistenz (1841f) — drei Duplikate, zwei davon subtil anders
Genau die Art Findings für die ein zweites Augenpaar da ist.
Minor non-blocking
- Zwei
setUpAll-Blocks increate_wallet_page_test.dart(33+62) — flutter_test erlaubt das, aber merge-able. Pedantisch. - PR-Description undersells den Inhalt — "4 polish items" sagt nicht dass 5 echte Bugs gefixt sind. Lass das den Maintainer im Merge-Commit zusammenfassen.
Merge-Empfehlung
Option 1 (Vorzug): #483 statt #472 mergen. PR #483 enthält alle 8 #472-Commits unverändert plus die 10 Follow-ups. Ein Merge-Cycle gespart, kritische Race-Fix landet sofort.
Option 2: Erst #472 mergen, dann #483 auf develop rebasen — funktioniert auch, ein zusätzlicher Roundtrip.
Mein orthogonaler Stacked-PR (Blume1977 #1: Lifecycle-Hidden-Lock) ist kompatibel — die Holder-Counter-Semantik akzeptiert lockCurrentWallet mit counter=0 als no-op.
9 tasks
This was referenced May 21, 2026
TaprootFreak
pushed a commit
that referenced
this pull request
May 21, 2026
Supersedes Blume1977#1 (closed) — same intent, rebased onto post-#483 develop, addresses TaprootFreak's two review points by pushing the precondition into `WalletService` instead of catching it at the call site. ## Why PR #472 / #483 added an explicit `WalletService.lockCurrentWallet()` after every sign and a 60 s post-unlock safety-net timer. Reviewer (#468) flagged the residual gap in the second-pass review: > 🟡 iOS suspendiert Dart-`Timer`s im Background — 60s-Cap ist Best-Effort. App-Lifecycle ruft `lockCurrentWallet` nicht explizit auf Hide auf — könnte ein eigenes Follow-up-Issue werden. This closes that gap. ## What changed Three atomic commits: 1. **`feat(app-store): isWalletLoaded getter`** — non-throwing predicate so services can early-return defensively. Named distinctly from `WalletService.hasWallet()` (which is persisted state, not in-memory load state) so the two predicates can't be confused at the call site. 2. **`fix(wallet-service): no-op lockCurrentWallet when no wallet is loaded`** — Onboarding-path guard inside `WalletService` itself. An alternative would have been a `try/catch` at the call site, which would silently mask any future regression in `lockCurrentWallet`. Pushing the precondition into the service means the call site stays clean and future extensions (DB write, audit log) won't get their errors quietly caught. Two regression tests added: the `!isWalletLoaded` no-op, and a lifecycle-hidden-during-active-sign scenario that pins the counter-underflow guard. 3. **`feat(lifecycle): lock the wallet on hidden`** — `LifecycleInitializer._onHidden()` fires `unawaited(getIt<WalletService>().lockCurrentWallet())`. No try/catch, no helper method — the service is defensive on its own. Tests cover happy path + the two no-call states (paused/resumed). ## Why `hidden` (not `paused`) - Fires **earlier than `paused`** on iOS (covers multitasking switcher + notification drag-down). - Android raises it via the unified lifecycle pipeline. - `PinAuthCubit` already uses `_onHidden` for its lockout-time stamp — semantically aligned. ## Scope gap (deliberately deferred) This PR drops the mnemonic stored in `AppStore.wallet`. It does **not** clear the mnemonic temporarily held in `CreateWalletState.wallet` (a `Cubit` state inside the create-wallet flow) — that copy lives independent of `AppStore` and `lockCurrentWallet` is a no-op for it. Backgrounding during onboarding still leaves the just-generated mnemonic in cubit memory until the cubit closes. Pre-existing condition, not introduced here. Worth a follow-up if the threat model demands it. ## Why no test for "lifecycle doesn't `catchError` the unawaited future" The architecture decision — _the lifecycle caller deliberately does not attach `.catchError` or wrap in try/catch_ — is locked in by the inline comment in `_onHidden`, not by a test. Every variant I tried (`thenThrow`, `thenAnswer((_) async => throw …)`, `Future.error(...)`) routed the failure through the testWidgets framework's synchronous catch path rather than the Zone uncaught-error sink that `tester.takeException()` reads from. The routing differs between Flutter 3.41 (CI) and 3.44 (local), so a test that's green locally would flake on CI. Brittle false-positives are worse than the source comment for catching a future regression at the call site — every code reviewer reads `_onHidden`. ## Reviewer feedback (TaprootFreak) Iteration after the first review: - **Naming-Kollision (Blocker 1)** — renamed `hasWallet` to `isWalletLoaded` on `AppStore` to avoid colliding with `WalletService.hasWallet()` which has different semantics. - **Holder-Counter test gap (Blocker 2)** — added a test that simulates `ensure → unpaired lifecycle lock → finally lock`, verifying the underflow guard prevents counter drift and the next ensure cycle still works. - **Inline comment too long (R3)** — condensed from 15 lines to 7; rationale moved to commit message + this PR body. - **Microtask ordering note (R4)** — added a one-line note in the lifecycle hook for any future contributor who introduces `ensureCurrentWalletUnlocked()` in `_onResumed`. - **Test setup drift (R5)** — `isWalletLoaded = true` moved out of the global `setUp` into per-group `setUp` so unrelated groups don't carry the stub and the negative path isn't masked. - **Doc accuracy (R7)** — clarified that both `LoadCurrentWalletEvent` and `LoadWalletEvent` populate `AppStore.wallet`. Why the architecture diverges from the original draft (Blume1977#1): previous incarnation wrapped `lockCurrentWallet()` in a `try { } on Exception catch (e)` helper in the lifecycle file. Reviewer pushed back that this would silently swallow future regressions (e.g. DB-write failures from a logging extension). Single Responsibility: the "is wallet loaded" precondition belongs to `WalletService`, not its callers. ## UX impact None visible. The next sign re-decrypts the mnemonic via the OS-keystore-wrapped key in sub-100 ms. ## Test plan - [x] \`flutter analyze\` — clean - [x] \`flutter test --coverage\` — 1424 / 1424 green (1417 develop baseline + 7 new: 2 isWalletLoaded, 1 no-wallet lock, 1 unpaired-lock counter race, 3 lifecycle states) - [x] Local CI parity run: \`flutter pub get\` → \`dart run tool/generate_localization.dart\` → \`dart run tool/generate_release_info.dart\` → \`flutter pub run build_runner build\` → \`flutter analyze\` → \`flutter test --coverage\` → \`lcov\` filter — all green; \`bitbox-audit\` 0 findings - [ ] **iOS manual**: sell → swipe up to multitasking → return → next sign re-decrypts in <100 ms, no PIN re-prompt within PinAuthCubit's lockoutDuration - [ ] **iOS manual**: sell → home button → wait 10 min → return → PIN re-prompt as before (unchanged) - [ ] **Android manual**: sell → recent-apps switcher → return → same as iOS path 1 - [ ] **Onboarding manual**: fresh install → reach onboarding screen → background app → return → no crash, no unhandled async error
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This collects the iteration-4 audit's "nice-to-have" polish items on top of #472 (
fix/pr461-followups). The 4 polish items are each their own commit and live on top of7ef2224(current tip of #472's branch).Polish items
1. Real reentrancy race test
test/packages/service/wallet_service_test.dartThe existing reentrancy test sequentially
awaits bothensureCurrentWalletUnlocked()calls — that exercises the counter, not a true race. Added aCompleter<WalletInfo>-gated test that holds twoensureCurrentWalletUnlocked()calls mid-decrypt, fires alockCurrentWallet()between them, then completes the gate. Asserts the wallet stays unlocked (lock did not shadow the in-flight unlock) and locks back only after the remaining holders are drained. The earlier mechanical test stays in place — they're complementary.2. DRY the
_LockedCredentialssign-method rationaletest/packages/wallet/wallet_test.dartFive identical ~4-line comments explaining the
assert(false)+StateErrorcontract were extracted into a top-level_viewWalletErrorRationaleconstant. Each test now passes that string as thereason:argument (first occurrence) or references the constant by name in a one-line comment. No test bodies changed.3. Visibility consistency: colocate
SoftwareViewWalletAccountwith_LockedCredentialslib/packages/wallet/wallet.dart+lib/packages/wallet/wallet_account.dartDebugWallet's analogs (_DebugCredentials+DebugWalletAccount) live in the same file (wallet.dart). The view-wallet analogs were split —_LockedCredentialsinwallet.dart,SoftwareViewWalletAccountinwallet_account.dart. MovedSoftwareViewWalletAccountnext to_LockedCredentialsinwallet.dartto match the existing pattern. Safe becauseSoftwareViewWalletAccountis only referenced fromwallet.dart(production) and one test file that already imports both files. No import changes needed elsewhere.4. Track in-flight unlock to dedupe DB decryption
lib/packages/service/wallet_service.dartTwo overlapping
ensureCurrentWalletUnlocked()calls on aSoftwareViewWalletpreviously each triggeredunlockCurrentWallet()(DB read + AES-GCM decrypt). Functionally identical results, but wasteful. Added aFuture<SoftwareWallet>? _unlockInFlightfield; the first caller starts the unlock and stores the future, subsequent overlapping callers await the same future. Cleared infinally(only by the caller that started it, viaidenticalcheck) so the next post-lock ensure runs a fresh unlock. New test verifies that two parallel ensures only invokerepository.getUnlockedWalletByIdonce.Test plan
flutter analyze— only pre-existinglib/generated/issues (33), no new lint warningsflutter test --coverage— 1412 tests pass (1410 baseline + 2 new race / dedup tests)StateErrorfrom_LockedCredentialsand the in-memory mnemonic still drops on the 60s safety net