fix(bitbox): harden multi-page sign and KYC routing#332
Merged
TaprootFreak merged 10 commits intoMay 15, 2026
Merged
Conversation
3 tasks
joshuakrueger-dfx
force-pushed
the
joshua/bitbox-sign-hardening
branch
from
cd02562 to
1190dd5
Compare
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
…oc (+22 tests) (#333) ## Summary Stage 9 of the coverage push. Adds 22 unit tests for the remaining dashboard cubits/bloc plus the global settings bloc. | Cubit / Bloc under test | Test file | Cases | | --- | --- | --- | | \`dashboard/bloc/price_chart/price_chart_cubit.dart\` | \`test/screens/dashboard/price_chart_cubit_test.dart\` | 4 | | \`dashboard/bloc/portfolio_chart/portfolio_chart_cubit.dart\` | \`test/screens/dashboard/portfolio_chart_cubit_test.dart\` | 5 | | \`dashboard/bloc/dashboard_transaction_history_cubit.dart\` | \`test/screens/dashboard/dashboard_transaction_history_cubit_test.dart\` | 4 | | \`dashboard/bloc/dashboard_bloc.dart\` | \`test/screens/dashboard/dashboard_bloc_test.dart\` | 3 | | \`settings/bloc/settings_bloc.dart\` | \`test/screens/settings/settings_bloc_test.dart\` | 6 | ## What each file covers - **price_chart_cubit:** empty-input zero-window state, all-period spots scaled by 100 + 10% Y-padding, \`selectPeriod\`-same is a no-op (no emit), \`oneWeek\` filter narrows to recent points. - **portfolio_chart_cubit:** empty-input zero-window state, all-period scaling + 6 horizontal-line values, flat-value series spreads via the 5% floor (no Y-collapse — pins the \`average * 0.05\` lower bound and the rounding to nice numbers \`{1,2,5,10}\`), \`selectPeriod\`-same no-op, \`oneWeek\` narrows visibleSpots. - **dashboard_transaction_history_cubit:** initial empty list, subscribes to \`watchTransactionsOfAssets\` with limit \`3\`, forwards every stream emission into state, \`close()\` cancels the subscription. - **dashboard_bloc:** initial state carries the supplied currency, the constructor \`refresh()\` populates price + priceChart + portfolioHistory via the services, \`CurrencyChangedEvent\` updates state and re-fetches all three datasets in the new currency. - **settings_bloc:** initial state reads through the repo, \`SetLanguageEvent\` writes \`'de'\` + emits, \`SetCurrencyEvent\` writes \`'EUR'\` + emits, \`SetNetworkModeEvent\` writes + calls \`getNewAuthToken\` + emits, \`ToggleHideAmountEvent\` flips both ways, a single toggle sets \`hideAmounts=true\`. ## Notes - The \`dashboard_bloc\` CurrencyChangedEvent test attaches a listener BEFORE adding the event because \`Bloc.stream\` is broadcast (no replay) and the event-driven re-fetch can complete before a follow-on \`firstWhere\` subscribes — same constraint we've now hit a few times in this push. ## Excluded (and why) - \`dashboard_bloc\` refresh after a service throw — non-trivial to test cleanly because the bloc lets the exception propagate out of the handler (which then surfaces as an unhandled bloc error in tests). Leaving as a follow-up. - \`transaction_history_receipt_cubit\`, \`transaction_history_multi_receipt_cubit\`, \`settings_tax_report_cubit\` — all use \`getTemporaryDirectory()\` + real \`File\` IO. - Buy / sell / sell_bitbox / hardware_connect_bitbox cubits — touched by the still-open PRs (#321 dashboard buy actions, #332 bitbox sign hardening); held back to avoid review conflicts. - \`settings_user_data_cubit\` — coordinates 3 services + country-lookup branches; deserves its own focused PR. ## Test plan - [x] \`flutter analyze\` on all five new files — clean - [x] \`flutter test\` — 22 / 22 passing locally - [ ] CI green
This was referenced May 15, 2026
…tall surfaces as BitboxNotConnected
…noise nonce in order
…o the BitBox firmware accepts the values
…d step is not skipped at high level
Pulls in the BLE init-frame dedup reorder and GetDevice panic recovery released in bitbox_flutter v0.0.5 — without these the multi-page EIP-712 sign that this PR's sign queue serializes still aborts on the page-1 -> page-2 transition with a noise decryption failure.
TaprootFreak
force-pushed
the
joshua/bitbox-sign-hardening
branch
from
1190dd5 to
c4fc9dd
Compare
Contributor
|
Rebased onto current Open questions from review still standing:
|
This was referenced May 15, 2026
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary Stage 10 of the coverage push. Adds 21 unit tests for the two remaining easy-to-mock DFX backend services. | Service under test | Test file | Cases | | --- | --- | --- | | \`lib/packages/service/dfx/dfx_support_service.dart\` | \`test/packages/service/dfx/dfx_support_service_test.dart\` | 9 | | \`lib/packages/service/dfx/dfx_brokerbot_service.dart\` | \`test/packages/service/dfx/dfx_brokerbot_service_test.dart\` | 12 | ## What each file covers - **dfx_support_service:** \`getTickets\` GET shape (path, Bearer JWT) + ApiException on non-200; \`getTicket\` by uid + ApiException; \`createTicket\` POST body (type/reason/name + optional message) + omits message when null + requires status \`201\` (200 is rejected as ApiException); \`sendMessage\` POST shape + ApiException on non-201. \`getAuthToken\` is short-circuited by pre-populating \`sessionCache.authToken\` so the signing flow stays out of these unit tests. - **dfx_brokerbot_service:** \`getBuyPrice\` GET + currency-code query + invalid-input guards (non-numeric, zero, negative, non-200); \`getBuyShares\` GET + currency + invalid-input guards; \`getSellPrice\` with Bearer JWT + ApiException on non-200 + the invalid-input case skips the HTTP call entirely; same matrix for \`getSellShares\`. ## Notes - Same mocktail + \`http.testing.MockClient\` pattern as the previous DFX-service PRs (#326 / #328 / #329). - The "invalid input never reaches HTTP" assertions are a small but meaningful contract: callers can rely on these methods to fail fast before any network round-trip. ## Excluded (deferred) - \`settings_user_data_cubit\` was on the original Stage-10 plan but coordinates 3 services + Country lookups + multi-branch KYC-step-status handling. It deserves its own focused PR rather than tagging it onto these two service tests. - \`dfx_kyc_service\` — held back to avoid review conflicts with open PR #332 (KYC routing / bitbox sign hardening). - Buy / sell / Bitbox cubits — held back while #321 and #332 are open. ## Test plan - [x] \`flutter analyze\` on the two new files — clean - [x] \`flutter test\` — 21 / 21 passing locally - [ ] CI green
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
…ts) (#336) ## Summary Stage 12 of the coverage push. Covers the previously-deferred \`settings_user_data_cubit\` (3-service coordination + Country lookups) and the read-side of \`transaction_history_service\`. | File under test | Test file | Cases | | --- | --- | --- | | \`lib/screens/settings_user_data/cubit/settings_user_data_cubit.dart\` | \`test/screens/settings_user_data/settings_user_data_cubit_test.dart\` | 6 | | \`lib/packages/service/transaction_history_service.dart\` (\`fetchPendingTransactions\` only) | \`test/packages/service/transaction_history_service_test.dart\` | 6 | ## What each file covers - **settings_user_data_cubit:** the cubit fans out to \`RealUnitWalletService.getWalletStatus\` and \`DfxKycService.getKycStatus\` in parallel, then either falls back to \`getUser\` (when userData is missing) or runs two more \`getCountryBySymbol\` lookups. Tests pin: - Full Success when userData is present (with nationality + addressCountry country lookups resolved to distinct \`Country\` instances). - \`pendingSteps\` only contains the three change steps (name/address/phone) that are in \`inReview\` — other inReview steps (e.g. \`contactData\`) are ignored. - \`userData == null\` + \`getUser\` returns mail → \`Success(email)\`, no country lookups happen. - \`userData == null\` + \`getUser\` throws → \`Success()\` with both userData and email null. - Failure when \`getWalletStatus\` throws (the \`Future.wait\` propagation). - Failure when \`getCountryBySymbol\` throws on a userData with a country code. - **transaction_history_service.fetchPendingTransactions:** no auth token short-circuits to \`[]\` without any HTTP call (verified via a MockClient that records calls); GET shape with the Bearer JWT to \`/v1/transaction/detail\`; non-200 returns \`[]\` (does not throw — intentional UX); filters out \`Completed\`/\`Failed\`/\`Returned\` (\`isPending=false\` per the enum extension); filters out transactions whose \`sourceAccount\` / \`targetAccount\` don't match the current wallet; wallet-match is **case-insensitive** (covers the lower-casing on both sides). ## Notes - \`RealUnitUserDataDto.type\` must use \`'HUMAN'\` / \`'CORPORATION'\` (the jsonName values of \`RegistrationUserType\`), not a friendly label — \`fromName\` throws \`StateError\` otherwise. - Mocktail's \`stream.firstWhere\` pattern is reused for the cubit (constructor fires \`getUserData\` synchronously, so we can't use \`blocTest\`'s sequence model reliably here — same as #329, #330, #333). ## Excluded (and why) - \`transaction_history_service.apiBasedSync\` — writes through \`TransactionRepository.insertDfxTransaction\`/\`updateTransaction\` and depends on \`AppStore.primaryAddress\` + \`apiConfig.asset.chainId\`. Doable but adds repository mock plumbing for a method that's almost entirely orchestration; will be its own focused PR. - Buy / sell cubits — still held while #321 (dashboard buy actions) and #332 (bitbox sign hardening) are open. ## Test plan - [x] \`flutter analyze\` on the two new files — clean - [x] \`flutter test\` — 12 / 12 passing locally - [ ] CI green
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary Stage 13 of the coverage push. Covers four cubits in the sell and buy flows whose source files are NOT touched by the currently-open #321 (dashboard buy actions, sell_page) or #332 (bitbox sign). | Cubit under test | Test file | Cases | | --- | --- | --- | | \`sell/cubits/sell_selected_bank_account/sell_selected_bank_account_cubit.dart\` | \`test/screens/sell/cubits/sell_selected_bank_account_cubit_test.dart\` | 3 | | \`sell/cubits/sell_balance/sell_balance_cubit.dart\` | \`test/screens/sell/cubits/sell_balance_cubit_test.dart\` | 4 | | \`sell/cubits/sell_bank_accounts/sell_bank_accounts_cubit.dart\` | \`test/screens/sell/cubits/sell_bank_accounts_cubit_test.dart\` | 6 | | \`buy/cubits/buy_converter/buy_converter_cubit.dart\` | \`test/screens/buy/cubits/buy_converter_cubit_test.dart\` | 9 | ## What each file covers - **sell_selected_bank_account_cubit:** initial null, \`selectBankAccount\` emits the provided account, \`selectBankAccount(null)\` clears the selection. - **sell_balance_cubit:** initial zero-balance derived from \`AppStore.apiConfig.asset\` + \`primaryAddress\`; subscribes to \`BalanceRepository.watchBalance\` on init; emits each pushed balance; \`close()\` cancels the subscription. - **sell_bank_accounts_cubit:** Success with DTO → BankAccount mapping on init; LoadFailure on \`getBankAccounts\` throw; \`add()\` calls \`createBankAccount\` + re-fetches; \`AddFailure\` preserves prior accounts + carries error message; \`deactivate()\` calls \`updateBankAccount(isActive=false)\` + re-fetches; \`UpdateFailure\` preserves prior accounts. - **buy_converter_cubit:** initial empty state; \`onFiatChanged\` debounces (100ms) and writes the converted shares; multiple rapid keystrokes only fire the LAST service call (pins the debounce contract); service error keeps state stable; \`onSharesChanged\` matches the output's fractional digits to the input (\`'5'\` → 2 digits, \`'5.000'\` → 3 digits — pins \`_fractionDigits\` behaviour); \`onCurrencyChanged\` re-fetches in the new currency and emits both fields; currency still flips on service error; \`close()\` cancels pending debounce timers so no service call happens after close. ## Excluded (and why) - \`sell_payment_info\`, \`sell_confirm\`, \`sell_converter\` (the parallel cubit to buy_converter), \`buy_confirm\`, \`buy_payment_info\` — each pulls in \`real_unit_buy_payment_info_service\` / \`real_unit_sell_payment_info_service\` / signing flows that are likely on PR #321's path. Held back to avoid review conflicts. - \`sell_bitbox_cubit\`, \`hardware_connect_bitbox_cubit\` — both directly touch BitBox; held while #332 is open. ## Test plan - [x] \`flutter analyze\` on the four new files — clean - [x] \`flutter test\` — 22 / 22 passing locally - [ ] CI green
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary Stage 14 of the coverage push. Adds 14 unit tests for the two confirm/converter cubits where the source files are not on PR #321's or #332's path. | Cubit | Test file | Cases | | --- | --- | --- | | \`buy/cubits/buy_confirm/buy_confirm_cubit.dart\` | \`test/screens/buy/cubits/buy_confirm_cubit_test.dart\` | 5 | | \`sell/cubits/sell_converter/sell_converter_cubit.dart\` | \`test/screens/sell/cubits/sell_converter_cubit_test.dart\` | 9 | ## What each file covers - **buy_confirm_cubit:** initial \`BuyConfirmInitial\`; happy path emits \`BuyConfirmSuccess(reference)\`; \`ApiException(statusCode: 503)\` → \`BuyConfirmFailure(BuyConfirmError.aktionariat)\` (pins the Aktionariat-down branch); other \`ApiException\` → \`BuyConfirmFailure(BuyConfirmError.unknown)\`; generic exception → \`unknown\`. - **sell_converter_cubit:** initial empty + CHF; \`onFiatChanged\` debounces (100ms) and writes shares from \`getSellShares\`; respects an explicit \`currency\` argument; debounce keeps only the last value (pins the per-keystroke contract); state stable on service error; \`onSharesChanged\` writes \`estimatedAmount\` with matching fractional digits (\`'10.000'\` → 3 digits, \`'10'\` → 2 digits); \`onCurrencyChanged\` calls \`getBuyPrice\` — NOT \`getSellPrice\` — with the current \`sharesText\` (pins the intentional buy-side estimation on currency switch); currency still flips even when \`getBuyPrice\` throws; \`close()\` cancels pending debounce timers so no service call after close. ## Notes - The \`sell_converter\` "currency switch uses BUY price" pin documents a non-obvious behaviour in the production code — leaving it untested would let a future refactor silently switch to \`getSellPrice\` and lose the no-fee preview. ## Excluded (still deferred) - \`buy_payment_info_cubit\`, \`sell_payment_info_cubit\`, \`sell_confirm_cubit\` — all touch \`real_unit_buy_payment_info_service\` / \`real_unit_sell_payment_info_service\` more deeply; #321 modifies these services + their tests, so I'm holding them back to avoid review conflicts. - \`sell_bitbox_cubit\`, \`hardware_connect_bitbox_cubit\` — BitBox-coupled; held while #332 is open. ## Test plan - [x] \`flutter analyze\` on the two new files — clean - [x] \`flutter test\` — 14 / 14 passing locally - [ ] CI green
6 tasks
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary Stage 21 of the coverage push. Closes the \`dfx_kyc_service\` surface added in #343. | Method | Cases | | --- | --- | | \`continueKyc\` | 2 | | \`startStep\` | 2 | | \`setData\` | 2 | | \`getFinancialData\` | 3 | | \`setFinancialData\` | 2 | ## What each method pins - **continueKyc:** PUT \`/v2/kyc\` with the \`x-kyc-code\` header from \`getUser().kyc.hash\`; parses \`KycSessionDto\`; \`ApiException\` on non-2xx. - **startStep:** GET \`/v2/kyc/<stepName.value>\` (the path encodes the step enum's wire string); parses \`KycSessionDto\`; \`ApiException\` on non-2xx. - **setData:** PUTs the caller-provided session URL (NOT the host) with the JSON body + \`x-kyc-code\` header; \`ApiException\` on non-2xx. - **getFinancialData:** GET \`<url>?lang=<lang.code>\`; defaults the language to \`Language.de\` (\`'de'\`); \`x-kyc-code\` header; \`ApiException\` on non-2xx. - **setFinancialData:** PUTs \`{ "responses": [...] }\` to the caller-provided URL with \`x-kyc-code\` header; serialises each \`KycFinancialResponse\` to \`{ key, value }\`; \`ApiException\` on non-2xx. ## Conflict avoidance Same as #343 — \`dfx_kyc_service.dart\` is NOT in PR #332's diff. The service surface is stable on that branch. ## Test plan - [x] \`flutter analyze\` clean - [x] \`flutter test\` — 11 / 11 passing locally - [ ] CI green
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary
Stage 20 of the coverage push. Covers four core methods of the
previously-deferred \`dfx_kyc_service\`.
| Method | Cases |
| --- | --- |
| \`getUser\` | 3 |
| \`updateUser\` | 3 |
| \`request2FaCode\` | 2 |
| \`verify2FaCode\` | 2 |
## What each method's tests pin
- **getUser:** GET \`/v2/user\` with Bearer JWT + parses \`UserDto\`;
201 also accepted (in addition to 200); throws \`ApiException\` on a
non-2xx non-401 (the 401-refresh-and-retry path is large enough to
deserve its own test, deferred).
- **updateUser:** PUT \`/v2/user\` with the JSON body; 201 accepted;
\`ApiException\` on non-2xx.
- **request2FaCode:** POST \`/v2/kyc/2fa?level=Strict\` with
\`x-kyc-code\` header set from \`getUser().kyc.hash\` (pins the kyc-hash
propagation); \`ApiException\` on non-2xx.
- **verify2FaCode:** POST \`/v2/kyc/2fa/verify\` with \`{ token: code
}\` body and the same \`x-kyc-code\` header; \`ApiException\` on
non-2xx.
## Conflict avoidance
PR #332 (bitbox sign + KYC routing) touches
\`lib/screens/kyc/cubits/kyc/kyc_cubit.dart\` and
\`real_unit_registration_service.dart\` — NOT this service.
\`dfx_kyc_service.dart\` itself is unchanged on that branch, so this PR
is conflict-free.
## Notes
- Tests use a tiny \`_StubWallet\` so \`DFXAuthService.wallet\` resolves
without plumbing a real \`SoftwareWallet\` through.
- The wire format for \`kyc.level\` is the numeric value (e.g. \`20\` →
\`KycLevel.level20\`), pinned in the test fixture so a future enum / DTO
refactor surfaces immediately.
## Excluded (deferred)
- \`getUser\`'s 401 → \`refreshAuthToken\` → retry path needs a stateful
MockClient + the auth-service signing flow; deserves its own focused
test.
- \`continueKyc\`, \`startStep\`, \`setData\`, \`getFinancialData\`,
\`setFinancialData\` — more elaborate bodies + headers; will follow up
if/when needed.
## Test plan
- [x] \`flutter analyze\` clean
- [x] \`flutter test\` — 10 / 10 passing locally
- [ ] CI green
…c/kyc_cubit_test.dart The new KycCubit-routing tests this PR added landed under `test/screens/kyc/cubits/kyc_cubit_test.dart` (no `kyc/` subdir), parallel to the existing `test/screens/kyc/cubits/kyc/kyc_cubit_test.dart` from RealUnitCH#319. Both files would be picked up by `flutter test` but the parallel layout is confusing, doesn't mirror the production path (`lib/screens/kyc/cubits/kyc/kyc_cubit.dart`), and silently duplicates the cubit's setup helpers. Move the four new tests — KycCompleted with all steps completed, the high-level-but-failed-step regression guard, the continueKyc routing when ident is failed, and KycPending when ident is in review — into a new group `step-status routing (regression for RealUnitCH#332)` inside the existing file. Mocks and helpers (`_user`, `_step`, `_kycStatus`, `_session`) are reused from the file; mock class names are aligned to the `_Mock*` private convention documented in `docs/testing.md`. `flutter test test/screens/kyc/cubits/kyc/kyc_cubit_test.dart` — 23 / 23 passing locally; `flutter analyze test/screens/kyc` clean.
Four small but visible polish items surfaced while reviewing the four fixes in this PR: - Add `characters` as a direct dev_dependency (`^1.4.0`) instead of relying on a transitive Flutter constraint with a `// ignore: depend_on_referenced_packages`. The pubspec entry now documents the dep explicitly. - Make the 120s `init()` outer timeout in `ConnectBitboxCubit` injectable as `pairingPinTimeout` (default `_defaultPairingPinTimeout = 120s`) for consistency with the other two timeouts. The constant comment explains why 120s (SDK pairing-confirm budget + margin). - Treat `KycStepStatus.onHold` like `inReview` when routing required steps. Backend semantics are identical (user has no actionable next step), and at level >= required the previous routing would have fallen through to `KycCompleted` for an `onHold` step — exactly the regression this PR fixes for `failed/outdated`. New test case in `kyc_cubit_test.dart` pins it. - Rename `MockBitboxManager` → `_MockBitboxManager` in `bitbox_credentials_test.dart` and `eip712_signer_bitbox_test.dart` to match the file-private mock convention documented in `docs/testing.md` and used in the cubit tests. `flutter test test/screens/kyc/cubits/kyc/kyc_cubit_test.dart test/packages/hardware_wallet test/screens/hardware_connect_bitbox test/packages/utils test/packages/wallet/eip712_signer_bitbox_test.dart` — 98 / 98 passing; `flutter analyze` clean on all touched paths.
Contributor
|
Decided to keep the unconditional
On the second open question ( |
…erage Second pass on the review-style polish: - Align the mock class names in `connect_bitbox_cubit_test.dart` with the private-mock convention (`_MockBitboxService`, `_MockWalletService`, `_MockBitboxWallet`, `_FakeBitboxDevice`) used in this PR's other test files and documented in `docs/testing.md`. - Add a regression test for the new `pairingPinTimeout` outer cap on `_pendingInit`: when the user never presses the device-side pairing button, `init()` stays pending, and the cubit must bounce to `BitboxNotConnected` without ever reaching `service.confirmPairing()`. `verifyNever(() => service.confirmPairing())` pins that. `flutter test test/screens/hardware_connect_bitbox` — 8 / 8 passing.
3 tasks
… merge Two professional cleanup items found while reviewing the PR end-to-end: - `toBitboxSafeAsciiOrNull` was defined and tested but never called from production code — `git grep` shows 0 callsites outside its own spec. Removed the 2-line wrapper and its dedicated test group. If a nullable variant is ever needed, callers can use `?.let(toBitboxSafeAscii)` or the helper can be re-added when there is an actual consumer. - `test/screens/home/home_bloc_test.dart` survived the merge with `origin/develop` (commit ba1a968) carrying its pre-RealUnitCH#346 shape: it still referenced the removed `DfxWidgetService` and passed 7 positional args to a 6-arg `HomeBloc(...)`. CI did not flag it on this branch but any local `flutter analyze` reports 4 errors. Aligned the file with the develop tip so the type-mismatch is gone.
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary Stage 36 of the coverage push. Pure DTO tests for the RealUnit buy + sell wire surface, plus the \`PaymentInfoError\` enum contract. ## Cases | Target | Cases | | --- | --- | | \`RealUnitBuyDto.toJson\` | 2 — defaults currency to \`CHF\`; honours an override | | \`RealUnitBuyConfirmDto.fromJson\` | 1 — reference field | | \`RealUnitBuyPaymentInfoDto.fromJson\` | 2 — happy path; optional \`minVolume\` / \`maxVolume\` / \`paymentRequest\` / \`remittanceInfo\` as null | | \`RealUnitSellDto.toJson\` | 4 — amount-only; targetAmount-only; currency override; assertion that exactly one of \`amount\` / \`targetAmount\` is set | | \`BeneficiaryDto.fromJson\` | 2 — name + iban; name null | | \`PaymentInfoError\` | 1 — four documented variants pinned | ## What's pinned - \`RealUnitSellDto\` enforces XOR-style mutual exclusion on \`amount\` / \`targetAmount\` via assertion — both negative cases (neither / both) are covered. - The optional-field handling on \`RealUnitBuyPaymentInfoDto\` is byte-exact: \`minVolume\` / \`maxVolume\` / \`paymentRequest\` / \`remittanceInfo\` survive an explicit \`null\` on the wire. - \`Currency\` round-trips through \`.code\` on serialisation and \`Currency.fromCode\` on parse. - \`PaymentInfoError\` is locked at four variants so a sneaky addition doesn't slip past without an intentional bump. ## Excluded (and why) - \`RealUnitSellPaymentInfoDto.fromJson\` — entangles \`Eip7702Data\`, \`DfxFeesData\` and \`BeneficiaryDto\` parsing in one shot; better covered via the \`real_unit_sell_payment_info_service\` integration tests once PR #332 is in. ## Test plan - [x] \`flutter test test/packages/service/dfx/models/payment/buy_sell_dtos_test.dart\` — 12 pass - [x] \`flutter analyze\` clean on the new file - [ ] CI green
TaprootFreak
approved these changes
May 15, 2026
This was referenced May 15, 2026
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary Stage 54 of the coverage push. First cubit-level coverage of the BitBox sell flow now that PR #332 has landed (BitBox sign hardening + \`FakeBitboxCredentials\`). ## Cases | Target | Cases | | --- | --- | | constructor / \`_checkEthBalance\` | 4 — disconnected BitBox → \`SellBitboxBitboxRequired\`; \`ethBalance >= requiredGasEth\` → \`SellBitboxEthReady\`; \`ethBalance < required\` + faucet success → \`WaitingForEth\` (\`faucet.requestFaucet\` called once); faucet throws → \`SellBitboxError\` | | \`proceedToSwap\` | 2 — success: \`Preparing\` → \`AwaitingSwapConfirm\` with both raw txs; failure: \`Error\` | | \`confirmSwap\` | 2 — no-op outside \`AwaitingSwapConfirm\`; non-Bitbox credentials in \`AwaitingSwapConfirm\` → \`Error\` ("BitBox wallet not connected") | | \`confirmDeposit\` | 1 — no-op outside \`AwaitingDepositConfirm\` | | \`retryDeposit\` | 1 — no-op outside \`DepositRetry\` | ## What's pinned - The constructor schedules \`_checkEthBalance\` via \`scheduleMicrotask\`, so all initial-state assertions are made after \`stream.firstWhere((s) => s is! SellBitboxCheckingEth)\`. The fleeting intermediate states (\`CheckingEth\`, \`RequestingFaucet\`) are not asserted but the terminal state is pinned. - The BitBox-required branch reuses \`FakeBitboxCredentials\` from PR #332 with \`bitboxManager = null\` to flip \`isConnected\` to false — same fake that production sign code special-cases. - The non-Bitbox credentials branch in \`confirmSwap\` is the safety net that protects users on a software wallet from accidentally entering the BitBox sign ceremony. - Sign / broadcast / retry happy paths require a real \`MsgSignature\` round-trip — covered separately by the BitBox signer tests in #332. This file pins the cubit's state machine wiring, not the crypto. ## Excluded (and why) - Sign-and-broadcast happy paths (\`confirmDeposit\` success → \`SellBitboxSuccess\`, \`retryDeposit\` success, broadcast-on-deposit failure → \`DepositRetry\`) — these need an actual BitBox sign result threaded through the AppStore wallet chain and a working \`sellService.broadcastTransaction\` mock; deferred to a follow-up stage if needed. ## Test plan - [x] \`flutter test test/screens/sell_bitbox/sell_bitbox_cubit_test.dart\` — 10 pass - [x] \`flutter analyze\` clean on the new file - [ ] CI green
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
) ## Summary Stage 55 of the coverage push. First service-level coverage of the registration HTTP service now that PR #332's BitBox sign hardening + \`FakeBitboxCredentials\` are in develop. ## Cases | Target | Cases | | --- | --- | | \`registerEmail\` | 3 — happy path lowercases the email + carries the bearer token; 202 Accepted treated as success; 4xx throws \`ApiException\` | | \`completeRegistration\` | 1 — disconnected BitBox throws \`BitboxNotConnectedException\` before the signing ceremony runs | | \`registerWallet\` | 1 — disconnected BitBox throws \`BitboxNotConnectedException\` before the signing ceremony runs | ## What's pinned - Email is **lowercased** before going on the wire — pinned because the backend treats e-mails case-insensitively and a refactor that drops \`.toLowerCase()\` would create duplicate-account ghosts. - Both \`completeRegistration\` and \`registerWallet\` short-circuit with a typed \`BitboxNotConnectedException\` BEFORE they touch the signer. This is the contract the UI relies on to surface a "connect your BitBox" prompt without a wasted sign attempt. - Bearer-token plumbing comes from \`sessionCache.authToken\` — pinned via header assertion in the happy path. ## Excluded (and why) - Sign-and-post happy paths for \`completeRegistration\` / \`registerWallet\` — would need an end-to-end EIP-712 roundtrip with \`FakeBitboxCredentials(success)\` + a stub server that validates the recovered signer. Deferred to a follow-up stage; the BitBox sign code itself is covered by PR #332's \`eip712_signer_bitbox_test.dart\`. ## Test plan - [x] \`flutter test test/packages/service/dfx/real_unit_registration_service_test.dart\` — 5 pass - [x] \`flutter analyze\` clean on the new file - [ ] CI green
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
## Summary Stage 56 of the coverage push. HTTP-surface coverage of the sell payment-info service now that PR #332 has landed. Skips sign-heavy \`confirmPayment\` in favour of the routes the BitBox flow uses. ## Cases | Target | Cases | | --- | --- | | \`getPaymentInfo\` | 3 — 200 → parsed \`SellPaymentInfo\` + body carries \`amount\` + \`iban\` + \`currency\` code; 403 → \`ApiException\`; 500 → \`ApiException\` | | \`createUnsignedTransactions\` | 2 — 200 → parsed swap + deposit; path is \`/v1/realunit/sell/<id>/unsigned-transactions\`; 500 → \`ApiException\` | | \`broadcastTransaction\` | 2 — 201 → returns \`txHash\`; path is \`/v1/realunit/sell/<id>/broadcast\`; 500 → \`ApiException\` | | \`confirmPaymentWithTxHash\` | 1 — PUTs \`/confirm\` with ONLY the \`txHash\` payload (no \`eip7702\` envelope) | ## What's pinned - All three id-based endpoints embed the id in the URL — pinned via path assertions so a refactor to a query-param shape surfaces here. - \`getPaymentInfo\` carries \`amount\` + \`iban\` + \`Currency.code\` on the wire — the BitBox SellBitboxCubit relies on this contract. - \`confirmPaymentWithTxHash\` is the BitBox-flow shortcut: it sends ONLY the txHash branch of \`RealUnitSellConfirmDto\`, no EIP-7702 envelope. Pinned via \`body.containsKey('eip7702')\` negative. ## Excluded (and why) - \`confirmPayment\` (EIP-712 + EIP-7702 sign roundtrip) — would need a full \`FakeBitboxCredentials(success)\` sign flow with both \`signDelegation\` and \`signAuthorization\`. Deferred; signer correctness itself is covered by the EIP-7702 signer tests in develop. ## Test plan - [x] \`flutter test test/packages/service/dfx/real_unit_sell_payment_info_service_test.dart\` — 8 pass - [x] \`flutter analyze\` clean on the new file - [ ] CI green
TaprootFreak
added a commit
that referenced
this pull request
May 15, 2026
…ests) (#381) ## Summary Stage 57 of the coverage push. Drives the \`SellBitboxCubit\` through the real \`FakeBitboxCredentials(success)\` sign ceremony to cover the post-#332 \`confirmSwap\` / \`confirmDeposit\` / \`retryDeposit\` happy paths that the cubit-test in PR #378 had deferred. ## Cases | Target | Cases | | --- | --- | | \`confirmSwap\` | 2 — signs swap tx → \`AwaitingDepositConfirm\` carries the signed envelope with the raw tx byte-for-byte + 32-byte r/s pair, \`signCallCount=1\`; strips optional \`0x\` prefix before hex-decoding the raw tx | | \`confirmDeposit\` | 2 — signs deposit, broadcasts swap then deposit, calls \`confirmPaymentWithTxHash\` with the deposit txHash, \`signCallCount=2\`; deposit-broadcast failure → \`SellBitboxDepositRetry\` carrying both signed envelopes + the error message | | \`retryDeposit\` | 2 — successful retry emits \`SellBitboxSuccess\` (broadcast count reaches 3); a retry that throws again stays in \`DepositRetry\` with the new \`errorMessage\` | ## What's pinned - The cubit hex-decodes the raw transaction whether it starts with \`0x\` or not — pinned via the mixed-prefix test so a regression to a strict prefix check surfaces here. - \`confirmDeposit\` issues TWO broadcasts (the already-signed swap + the freshly-signed deposit) before calling \`confirmPaymentWithTxHash\`. The first broadcast's failure is **not** retried — pinned by the call ordering. - \`DepositRetry\` carries both signed envelopes verbatim so \`retryDeposit\` can re-broadcast without re-signing — pinned by checking that \`signCallCount\` stays at 2 across the retry. - The signed (r, s) pair always comes back as a 0x-prefixed 32-byte hex string (66 chars including the prefix) — pinned via length assertion, which catches an off-by-one padding regression. ## Test plan - [x] \`flutter test test/screens/sell_bitbox/sell_bitbox_cubit_happy_paths_test.dart\` — 6 pass - [x] \`flutter analyze\` clean on the new file - [ ] CI green
Collaborator
Author
|
/bitbox-simulator |
This was referenced May 23, 2026
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.
Summary
Four fixes that together let the BitBox-gated KYC registration run all the way through on iOS BLE without the user falling into a dead end:
fix(bitbox)WrapconfirmPairingandcreateBitboxWalletin 75s/30s timeouts so a silent BLE stall surfaces asBitboxNotConnectedinstead of an endless spinner.ConnectBitboxCubitgains injectable timeouts for unit tests.fix(bitbox)Serialize all sign calls (signToSignature,signPersonalMessage,signTypedDataV4) through a static_signQueue. The bitbox02-api-go SDK keeps a single noise cipher per device, so two concurrent signs would advance the nonce out of order and break decryption permanently.fix(bitbox)Transliterate every string field that goes into the EIP-712 envelope (and its matching DTO copy) to printable ASCII viatoBitboxSafeAscii— covers German umlauts, French/Spanish/Portuguese accents, Polish/Czech letters, Nordic æ/ø/å, Romanian/Turkish. BitBox firmware rejects any non-ASCII byte instring-typed values withErrInvalidInput (101). KYC personal-data fields keep the original spelling so ID-verification still sees the legal name with diacritics.fix(kyc)KycCubitnow routes based on the status of the required steps, not just the numeric level. A high aggregate level withident=failedorfinancialData=outdatedno longer short-circuits toKycCompleted— the user is sent back through the unfinished steps via_continueKyc.Test plan
flutter test(446 passing, +24 new acrossconnect_bitbox_cubit_test.dart,bitbox_credentials_test.dart,kyc_cubit_test.dart,eip712_signer_bitbox_test.dart,ascii_transliterate_test.dart)flutter analyzeclean