fix: BitBox pairing flow shows code in app and survives Home auth#301
Conversation
joshuakrueger-dfx
changed the title
joshuakrueger-dfx
force-pushed
the
fix/bitbox-pairing-code-sync
branch
from
07a9609 to
1be44cf
Compare
joshuakrueger-dfx
changed the title
|
Funktional ist der PR eine deutliche Verbesserung — das eigentliche UX-Problem (Pairing-Code auf BitBox sichtbar, App spinnt) ist sauber gelöst, und die Timeouts sind insgesamt besser als das vorherige unbegrenzte Zwei Punkte, die ich vor dem Merge testen würde: 1. Stale-Channel-Hash beim Re-Pair (real, neu durch diesen PR) Original:
Test-Szenario das im Plan fehlt:
Falls das reproduzierbar ist, wäre die saubere Lösung: vor dem Polling-Start den initialen Hash-Wert merken und warten, bis er sich ändert (statt nur 2. Wenn Kleinere Nits (kein Blocker):
|
|
Habe einen Folge-Commit auf den Branch gepusht ( 1. Stale-Channel-Hash beim Re-Pair 2. Polish nebenbei:
Den Punkt „Future.timeout cancelt |
|
Aufgeräumt: zwei Folge-Commits gepusht, die meine erste Runde an Polish + den Original-Diff zurückbauen.
Ergebnis: Cubit-Diff jetzt 69 Zeilen statt vorher 78 (Joshuas Original) bzw. 92 (mit meinem ersten Bloat). Stale-Hash-Schutz drin, sonst keine unnötigen Änderungen vs. Original. CI-Run für |
There was a problem hiding this comment.
close() doesn't clean up _pendingInit
If the user navigates away while init() is still in flight, the future keeps running and could emit on a closed cubit. _waitForChannelHash would also keep polling. Worth guarding against — e.g. checking isClosed in the polling loop and nulling out _pendingInit in close().
|
@davidleomay Guter Fang — habe den
|
Test-Plan vor MergeGeräte: iOS Device + BitBox02 Plus mit USB-C Adapter. App im Debug-Build laufen lassen, Konsole offen für 1. Happy Path — frische erste Kopplung
Failure-Mode wenn Bug: Code erscheint nur auf BitBox, App zeigt unendlich Spinner. 2. Cancel im BitboxCheckHash-State
3. Wegnavigieren während BitboxConnecting (vor Code-Anzeige)
Hintergrund: Dies testet Davids Review-Punkt — 4. Wegnavigieren während BitboxPairing (nach Bestätigen, vor Device-Confirm)
5. Decline auf BitBox (User confirmt nicht auf Device)
6. Re-Pair in derselben App-Session (priorHash-Schutz)Das ist der kritische Test für meinen Stale-Hash-Fix.
Wenn das nicht reproduzierbar ist (weil 7. USB-Disconnect mitten im PairingA) Während BitboxConnecting (vor Code):
B) Während BitboxPairing:
8. App-Background während Pairing
9. Konsolen-CheckWährend aller Tests in der Konsole achten auf:
Smoke-Test nach Merge auf develop
Wenn 1, 2, 3, 4 und 6 grün sind, ist der PR aus meiner Sicht prod-ready. 5, 7, 8 sind nice-to-have-Robustheit-Checks. |
joshuakrueger-dfx
force-pushed
the
fix/bitbox-pairing-code-sync
branch
5 times, most recently
from
493fafa to
51ba50c
Compare
joshuakrueger-dfx
changed the title
Two related problems are addressed: 1. The connect screen never displayed the pairing code. bitbox02-api-go's pair() sets device.channelHash before issuing opICanHasPairinVerificashun, which blocks until the user confirms the pairing on the device. The cubit awaited init() so the BitBox displayed the code while the app stayed on its spinner. Run init() in the background and poll ChannelHash() in parallel so the same code shows in the app for visual comparison. confirmPairing() awaits the same future before calling channelHashVerify(), preventing the host-side verify (and the createBitboxWallet that follows) from running before the device-side verify has landed. 2. Once paired, HomeBloc._setupFiatService asks the wallet to sign a challenge for the DFX auth endpoint. For BitBox-backed wallets this goes through bitbox_flutter, whose ETHSignMessage panics on a NACK from the device — Go panics in gomobile bindings cannot be caught from Dart, so the engine dies. Skip the automatic auth call for BitBox wallets until the SDK returns the error gracefully. DE/EN copy on the connecting screen reworded to set the right expectation.
joshuakrueger-dfx
force-pushed
the
fix/bitbox-pairing-code-sync
branch
from
51ba50c to
a524de1
Compare
## Summary - Add `isClosed` guards to all async methods in `ConnectBitboxCubit` to prevent resource leaks on disposal - Align `close()` with the project convention (`return super.close()` instead of `async` + bare `super.close()`) ## Problem `ConnectBitboxCubit` lives inside a `showModalBottomSheet` without `isDismissible: false`. During the `BitboxConnecting` state there is no cancel button — the only way to abort is swiping the modal down. When the user swipes during the 90-second polling loop introduced in #301: 1. `BlocProvider` removes the widget → `close()` runs → `_checkForTimer` is cancelled 2. But `connectToBitbox()` is an in-flight async method — it keeps running 3. When the loop eventually times out or `init` fails, the `catch` block creates a **new `Timer.periodic`** that is never cancelled (because `close()` already ran) 4. This timer holds a reference to the cubit, preventing garbage collection, and calls `checkForBitbox()` every 500ms indefinitely The same issue exists in the `confirmPairing()` catch block. ## Fix - `checkForBitbox()`: `if (isClosed) return` after `await getAllUsbDevices()` — prevents starting a connection flow on a disposed cubit - `connectToBitbox()` `while` condition: `&& !isClosed` — stops polling immediately on dispose - `connectToBitbox()` after `await Future.delayed`: `if (isClosed) return` — catches the edge case where close happens during the delay - `connectToBitbox()` after loop exit: `if (isClosed) return` — prevents `emit` on a closed cubit - Both `catch` blocks: `if (isClosed) return` — prevents creating orphaned timers - `close()`: `_pendingInit = null` — drops the future reference - `close()`: aligned with project convention — non-async with `return super.close()` ## Test plan - [x] `flutter analyze` — 0 errors - [x] `flutter test` — 182/182 passed - [ ] Manual: open BitBox modal → swipe down during "connecting" spinner → verify no timer leak in DevTools
## Summary - Move the BitBox short-circuit out of `HomeBloc._setupFiatService` and into `DFXAuthService.getAuthToken()` so every caller is covered - `getAuthToken()` returns `null` for `BitboxCredentials` wallets, which the existing callers already treat as \"not authenticated\" ## Why `bitbox_flutter`'s `ETHSignMessage` panics on a NACK from the device and crashes the Flutter engine (panics in gomobile bindings cannot be caught from Dart). #301 added a guard inside `HomeBloc._setupFiatService` so the auto-auth on Home would not crash a BitBox-backed wallet, but the same `getAuthToken()` path is reachable from `DfxKycService`, `DfxSupportService`, `DfxWidgetService`, and every future `DFXAuthService` consumer — each one would still crash the app the moment the user touched that feature. Putting the check at the source means the workaround can stay in one place until the SDK returns the error gracefully (tracked in DFXswiss/bitbox_flutter). ## Changes - `lib/packages/service/dfx/dfx_auth_service.dart`: `getAuthToken()` short-circuits to `null` when the active credentials are `BitboxCredentials` - `lib/screens/home/bloc/home_bloc.dart`: drop the now-redundant guard and the `bitbox_credentials` import ## Test plan - [ ] Pair a BitBox, land on Home — no crash, fiat banner stays disabled (same as #301) - [ ] Open Settings → Contact / KYC entry while BitBox-paired — no crash, the affected screens treat the user as unauthenticated - [ ] Software wallet on Home still authenticates against DFX (regression check)
## Summary - Snapshot `getChannelHash()` before starting `init()` and require the polled hash to differ from it - Drop the `_pendingInit ?? Future.value(true)` fallback in `confirmPairing` and use the bang assertion the state guard already guarantees ## Why ### Stale hash on re-pair `BitboxService` is registered as a DI singleton (`lib/setup/di.dart:121`), and the `BitboxManager` instance inside it persists across pairings. The connect modal is reachable from at least two entry points (`welcome_page.dart`, `kyc_registration_page.dart`), and any second pairing attempt within the same app session goes through that same SDK instance. Before this PR, the polling loop accepted any non-empty hash. If `bitboxManager.getChannelHash()` still holds the previous session's hash when polling starts, the app shows the *old* hash while the BitBox displays the *new* one — codes don't match, user is stuck. The 500 ms first-iteration delay introduced in #305 (`...so the SDK can finish setting up its Go-side device pointer...`) confirms there is a real timing gap during which the SDK is not yet up-to-date for the new session, which is precisely the window where stale hashes surface. The fix records the hash once before `init()` runs and ignores any polled hash that matches the snapshot. ### `??` fallback in confirmPairing `_pendingInit ?? Future.value(true)` silently substitutes "init succeeded" if the future is missing. The `is! BitboxCheckHash` state guard at the top of `confirmPairing` already guarantees that `connectToBitbox` set `_pendingInit`. The fallback only triggers in an impossible state, and if that state ever became reachable, returning `true` would let `channelHashVerify()` run on an unverified channel — exactly the crash #301 set out to prevent. Replacing it with the bang assertion makes the precondition explicit and matches the project's bang usage in similar state-guarded code paths. ## Changes - `lib/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart`: add `priorHash` snapshot + `hash != priorHash` check in polling; replace `??` fallback with bang on `_pendingInit` ## Test plan - [ ] Happy path on iOS BitBox02 Plus: open Welcome → pair → wallet creates (regression check, no behavioural change expected on first pair) - [ ] **Re-pair in same session:** pair successfully → trigger a second pairing without restarting the app (e.g. via KYC retry flow, or by opening the connect modal again). Verify the code shown in app matches the code on the BitBox — not the previous session's code - [ ] Cancel during `BitboxConnecting` (swipe modal down before code appears): no `StateError`, no orphaned timer (regression check for #303)
3 participants
Summary
BitboxService.init()in the background and pollgetChannelHash()in parallel so the app shows the pairing code at the same time as the BitBox doesconfirmPairing()awaits the in-flightinit()future before callingchannelHashVerify(), so the host-side verify (andcreateBitboxWalletafter it) cannot run before the device-side verify has landedisClosedand clear_pendingInitinclose()— dismissing the modal during pairing no longer leaks the re-scan timer or emits on a disposed cubit (incorporates the dispose-leak hardening from fix: prevent orphaned timer leak on BitBox modal dismiss #303)HomeBloc._setupFiatServicefor BitBox-backed wallets —bitbox_flutter'sETHSignMessagepanics on a NACK and crashes the engine, which previously killed the app right after pairingWhy
Pairing code never shown in app.
bitbox02-api-go'spair()setsdevice.channelHashbefore issuingopICanHasPairinVerificashun, which blocks until the user confirms the pairing on the device. The previous flowawait init()first, so the BitBox displayed the pairing code while the app stayed on its spinner indefinitely — there was no way to compare the codes. CallingchannelHashVerify()beforeinit()returned also crashedcreateBitboxWalletbecause the noise channel was not yet considered verified on the device side.bitbox.ChannelHash()is a plain field read on the Go-sideDevice, so polling it concurrently with the still-runninginit()is safe.Modal-dismiss timer leak. The cubit lives inside a
showModalBottomSheet. Dismissing the sheet during the 90 s polling loop ranclose()whileconnectToBitbox()was still in flight; thecatchblock then armed a freshTimer.periodicon the disposed cubit and emitted onto a closed bloc. TheisClosedguards stop the loop on disposal, prevent the catch path from arming a new timer, and letclose()drop the_pendingInitreference cleanly.Engine crash on first Home load. Right after a fresh pairing,
HomeBloc._setupFiatServiceasks the wallet to sign a challenge so the DFX backend can verify ownership. For BitBox-backed wallets this goes throughbitbox_flutter, and the SDK'sETHSignMessagepanics withunexpected NACK responsewhen the device rejects the request. Go panics in gomobile bindings cannot be caught from Dart, so the engine dies. Skipping the automatic call for BitBox wallets keeps the app alive — signing is then triggered explicitly by user-initiated flows that can present a clearer error.Changes
lib/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart: parallel-poll for the channel hash, gateconfirmPairing()on the init future, isClosed guards on every async path, drop_pendingInitinclose()lib/screens/home/bloc/home_bloc.dart: skip auto-auth when the active credentials areBitboxCredentialsassets/languages/strings_de.arb,assets/languages/strings_en.arb: rewordedconnectBitboxConnectingTest plan
Devices: iPhone with USB-C cable, BitBox02 Plus, debug-mode build, console open for
developer.logoutput.1. Happy path — fresh first pairing
2. Cancel from
BitboxCheckHash3. Modal-dismiss during
BitboxConnectingCannot emit new states after calling closeand no orphan timer in DevTools4. Decline / timeout on the device
5. Reach Home with a BitBox wallet