Skip to content

fix(bitbox): explain an unseeded device instead of looping#724

Draft
TaprootFreak wants to merge 4 commits into
stagingfrom
fix/bitbox-unseeded-device
Draft

fix(bitbox): explain an unseeded device instead of looping#724
TaprootFreak wants to merge 4 commits into
stagingfrom
fix/bitbox-unseeded-device

Conversation

@TaprootFreak

Copy link
Copy Markdown
Contributor

Problem

Connecting a brand-new BitBox that has no wallet set up (no seed) left the user stuck. Pairing succeeds, but the device has no seed to derive an ETH address from, so getETHAddress comes back empty. That empty read failed as a generic error → BitboxNotConnected → a SnackBar "something went wrong" → and because the re-scan timer is re-armed, the device is immediately found again and the user is walked through the pairing code → fail → SnackBar loop, with no hint that the real problem is simply an un-set-up device.

(The earlier fix #710 stopped the empty address from being persisted — no more grey screen — but did not distinguish "no seed" from a transient empty read.)

What changed

After channel-hash verify, read the device's firmware status via the new bitbox_flutter getDeviceStatus() (cached read, no device round-trip). When it reports uninitialized, emit a dedicated BitboxNotInitialized state that explains the user must set up / restore a wallet on the device first.

  • The state offers a retry (recheckDeviceStatus) that re-reads the status — if the user has since set up a wallet, the connection continues without re-pairing.
  • It deliberately does not arm the re-scan timer, so the silent re-pair loop is gone.
  • Only uninitialized is treated as "no wallet". Other non-ready statuses (e.g. firmware-upgrade-required) intentionally keep the existing failure path rather than being mislabelled.

The address-derivation/observe/sign tail of confirmPairing was extracted into _acquireWalletAndConnect() so the initial flow and the retry share one path.

Dependency

Requires bitbox_flutter getDeviceStatus() from DFXswiss/bitbox_flutter#29. This PR temporarily pins the plugin to that fix branch; it will be moved to the v0.0.9 tag once #29 is merged and tagged. Kept as Draft until then.

Test plan

  • flutter analyze — clean (only the pre-existing generated-i18n.dart warning)
  • flutter test — bitbox cubit / service / view suites all green
  • Cubit: unseeded → BitboxNotInitialized, no wallet created, no re-scan loop (state stays stable); retry continues once seeded; retry stays while still unseeded; no-op off-state
  • Service: getDeviceStatus pass-through via the simulator
  • Widget: BitboxNotInitialized renders retry + cancel; retry calls recheckDeviceStatus
  • i18n: new keys in both de/en ARBs (case-sensitive ASCII order), regenerated
  • On-device: pair an un-set-up BitBox → lands on the explanatory screen; set up a wallet + retry → continues to the dashboard

A BitBox paired without a wallet set up (no seed) cannot derive an ETH address.
The empty address read failed as a generic error and bounced the user into the
silent re-scan loop with no explanation, so a brand-new device was effectively
unusable with no hint why.

After pairing, read the device's firmware status via the new bitbox_flutter
getDeviceStatus and, when it reports `uninitialized`, emit a dedicated
BitboxNotInitialized state that tells the user to set up or restore a wallet on
the device first. The state offers a retry that re-checks the status without
re-pairing and, crucially, does not arm the re-scan timer, so the device is no
longer picked up and re-paired in a loop. Only `uninitialized` is treated as
"no wallet"; other non-ready statuses keep the existing path.

Temporarily pins bitbox_flutter to the fix branch; moves to the v0.0.9 tag once
the plugin PR is merged and tagged.
Resolves the pubspec conflict from #723 (bump to v0.0.9, 16 KB alignment). Keeps
the dependency pinned to the bitbox_flutter fix branch — which now carries BOTH
the 16 KB alignment (merged from #30) and the new getDeviceStatus — at the
updated commit. Moves to the v0.0.10 tag once the plugin PR lands.
Harden the new unseeded-device check so it can only ever ADD the dedicated
"not set up" path, never break a device that would otherwise pair. The status
read is wrapped so any failure or unexpected value falls through to the normal
acquire path (the exact pre-change behaviour); only a clean, explicit
`uninitialized` read diverts to BitboxNotInitialized. Adds a regression test
asserting a throwing status read still reaches BitboxConnected.
@TaprootFreak

Copy link
Copy Markdown
Contributor Author

Held as Draft pending: (1) plugin #29 merge + v0.0.10 tag, after which the pin moves from the fix branch to v0.0.10; (2) an on-device smoke test with an already set-up BitBox to confirm the normal pairing flow is unaffected. The status gate is fail-open (commit 98dcfb6): any failed/unexpected status read falls through to today's behaviour — only a clean uninitialized diverts.

The status read is a local cached lookup, but it was the only device-facing call
in the pairing path without a timeout. Cap it at 5s; on timeout the read is
treated as "not uninitialized" (fail-open via the existing catch), so a
hypothetical stall can never hang an otherwise-working pairing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant