Skip to content

Split PR 578: KYC registration flow#610

Open
joshuakrueger-dfx wants to merge 2 commits into
RealUnitCH:stagingfrom
joshuakrueger-dfx:split/pr578-05-kyc-flow
Open

Split PR 578: KYC registration flow#610
joshuakrueger-dfx wants to merge 2 commits into
RealUnitCH:stagingfrom
joshuakrueger-dfx:split/pr578-05-kyc-flow

Conversation

@joshuakrueger-dfx
Copy link
Copy Markdown
Collaborator

Draft split from #578 after TaprootFreak review.

Scope:

Notes:

  • Kept this branch independent from the sign-pipeline split by removing typed sign-pipeline-only exception usage.
  • Kept ARB additions scoped to KYC/Buy strings only.

Validation:

  • flutter pub get
  • dart run tool/generate_localization.dart
  • dart run tool/generate_release_info.dart
  • flutter pub run build_runner build --delete-conflicting-outputs
  • flutter analyze --no-pub
  • flutter test --reporter compact test/packages/service/dfx/models/user/dto/user_dto_test.dart test/screens/buy/buy_page_test.dart test/screens/buy/cubits/buy_payment_info/buy_payment_info_state_test.dart test/screens/buy/cubits/buy_payment_info_cubit_test.dart test/screens/kyc/cubits/kyc/kyc_cubit_test.dart test/screens/kyc/steps/email/kyc_email_verification_cubit_test.dart test/screens/kyc_bitbox_create_wallet_states_test.dart test/goldens/screens/kyc/kyc_registration_address_step_golden_test.dart

@joshuakrueger-dfx joshuakrueger-dfx marked this pull request as ready for review May 29, 2026 08:34
@TaprootFreak
Copy link
Copy Markdown
Contributor

Re API as Decision Authority (CONTRIBUTING.md:23–67):

Wins ✅

  • BuyPaymentInfoFailure.message now carries the API's actual error text through to the UI (BuyPaymentInfoCubit). Replaces the previous collapse into a bare enum — this is literally "call the API, render its error".
  • The new 403/UNKNOWN regression test asserts the app no longer force-maps generic 403s onto registrationRequired. Good.
  • DTO mirroring of userCapabilities is consistent with Wave 3 (PR refactor(app): consume UserCapabilities + ALREADY_REGISTERED registration status #497).

Concern — KycEmailVerificationCubit post-merge retry loop
The new constructor params:

int registrationInfoRetries = 4,
Duration registrationInfoRetryDelay = const Duration(seconds: 2),

with the inline attempt < _registrationInfoRetries - 1 retry loop, are app-side scheduling for what the cubit comment itself describes as:

Bounded auto-retry for the post-merge user-data propagation race.

That's the app deciding "Backend hasn't propagated yet → I'll try again in 2s". The rule (CONTRIBUTING.md:55) names this case explicitly:

When the API doesn't yet expose what we need: Extend the API, then change the app. Do not add app-side workarounds. … Temporary local logic is technical debt that stays — every shortcut accumulates as another place the app diverges from the API.

The correct landing place for this is on the API side: either (a) getRegistrationInfo blocks-or-resolves synchronously after merge, or (b) it returns a definitive status like MergeProcessing so the app renders a waiting screen instead of polling on a hardcoded backoff. The 21-minute cron-lag this works around is the same root cause as the post-merge "Fehler beim Laden" tracked on the main bug issue — that one needs an API fix too (cron → event-driven status follow-up), and the same fix removes the need for client-side retries here.

The rest of the PR is in the correct layer (rendering API state, surfacing API errors). Only the retry loop pulls a server-side responsibility into the client.

Suggested follow-up (consistent with pair-PR convention): open an issue in DFXswiss/api for "non-blocking getRegistrationInfo after merge + explicit MergeProcessing status", land the API change, then drop registrationInfoRetries and have the cubit render the API's status 1:1.

@TaprootFreak TaprootFreak changed the base branch from develop to staging June 1, 2026 14:35
F1 (HIGH) BL-006 — BitBox reconnect dead-end: the disconnect-mid-sign
handler used to reset the merge latch, so the post-reconnect retry re-ran
the one-shot JWT account-id check. By then getAuthToken returns the merged
account on both reads, so the same-account-id guard dead-ended on
"email not confirmed" and the user could never finish. Keep _mergeDetected
set across the BitboxNotConnected path so the retry re-attempts the
registration SIGN. Removed the now-dead _initialMergeDetected mirror field.
Rewrote the cubit test that pinned the buggy reset to assert the retry
re-signs and succeeds (auth-side check runs exactly once).

F2 (MED) — KycInitial flashed the diagnostic catch-all ("Unhandled KYC
state: KycInitial") on the first frame. Route the pre-checkKyc() seed
state to KycLoadingPage. Added a KycViewManager test.

F3 (LOW) — corrected the misleading KycUnsupportedStepFailure docstring
(it described a re-entrant email-page route that does not happen; it is a
terminal failure page).

F4 (LOW) — BL-002 was untested: added a behavioral test for the Swiss
tax-residence auto-tick (CH → ticked, non-CH → cleared) and the
user-override latch (manual toggle stops the country listener overriding).

All red→green verified (each fix proven to fail when reverted/mutated);
flutter analyze clean; full KYC suite (204 tests) green.
@TaprootFreak TaprootFreak force-pushed the split/pr578-05-kyc-flow branch from 9bb2fdc to 0eeaba1 Compare June 2, 2026 08:24
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.

2 participants