Silent failure in KycEmailVerificationCubit._completeRegistration masks registerWallet errors

[Blume1977]

Summary

KycEmailVerificationCubit.checkEmailVerification (lib/screens/kyc/steps/email/cubits/email_verification/kyc_email_verification_cubit.dart:26-39) emits KycEmailVerificationSuccess unconditionally whenever the JWT account-id changes, even if the subsequent _completeRegistration call failed. Two distinct failure modes are silently swallowed: (a) realUnitUserDataDto returning null from getWalletStatus, and (b) registerWallet throwing. Both failures emit KycEmailVerificationRegistrationFailure first, but that state is immediately overwritten by the outer Success emit on the next line. The resulting symptom is a successful-looking merge confirmation with no EIP-712 registration signature actually produced, leaving the wallet in an inconsistent state.

This is documented by two tests in test/screens/kyc/steps/email/kyc_email_verification_cubit_test.dart (:131-148 and :150-170) which assert the current behaviour without questioning whether it is correct.

Code

// kyc_email_verification_cubit.dart:26-39
Future<void> checkEmailVerification() async {
  emit(const KycEmailVerificationLoading());

  final currentAccountId = await getAccountId();
  _dfxService.invalidateAuthToken();
  final newAccountId = await getAccountId();

  if (currentAccountId != newAccountId) {
    await _completeRegistration();
    emit(const KycEmailVerificationSuccess());   // ← unconditional
  } else {
    emit(const KycEmailVerificationFailure());
  }
}

// kyc_email_verification_cubit.dart:41-50
Future<void> _completeRegistration() async {
  try {
    final status = await _walletService.getWalletStatus();
    if (status.realUnitUserDataDto == null) throw Exception('No existing user data');
    await _registrationService.registerWallet(status.realUnitUserDataDto!);
  } catch (e) {
    developer.log(e.toString());
    emit(const KycEmailVerificationRegistrationFailure());  // ← overwritten by Success above
  }
}

The RegistrationFailure snackbar in kyc_email_verification_page.dart:47-56 fires for an instant before Success triggers context.pop(true) (:57-59). On a real device the snackbar may not be perceptible.

Failure scenarios

Scenario 1 — realUnitUserDataDto == null

If getWalletStatus returns RealUnitWalletStatusDto(realUnitUserDataDto: null, ...) for a merged user, _completeRegistration throws "No existing user data" before registerWallet is even called. No EIP-712 sign is produced. The user is popped back as if everything succeeded.

Possible causes for null realUnitUserDataDto immediately after invalidateAuthToken:

• Backend race: merge propagation in the user-data service lags the auth service, so getWalletStatus on the new account hits a window where the user data is still empty
• Stale cache: the response is cached or the request returns before the backend has finished associating data
• Genuine absence: the user has no DFX user-data record despite owning the account (edge case)

Verified by test kyc_email_verification_cubit_test.dart:131-148 titled 'changed account id but no userData → cubit settles on Success (RegistrationFailure is intermediate, overwritten by the outer Success emit)'.

Scenario 2 — registerWallet throws

If the backend rejects registerWallet (validation, signature mismatch, transient 5xx, network), the exception is caught and logged but otherwise swallowed.

Verified by test kyc_email_verification_cubit_test.dart:150-170 titled 'registerWallet throws: cubit still settles on Success (does not crash)'.

Downstream impact

For any user who hits scenario 1 or 2:

• The local in-memory state KycCubit._bitboxConfirmed is not updated by the merge path (see Existing DFX customer is forced to re-enter personal data after successful merge #464), so the user is misrouted to the registration form
• Server-side, the wallet is associated with the merged user account (JWT shows the new account) but the per-wallet EIP-712 registration signature was never written
• Depending on backend strictness, the user may:
  • Get away with it (buy / sell flows pull DFX data via a different path that does not require the per-wallet signature) — this is what the original reporter on Existing DFX customer is forced to re-enter personal data after successful merge #464 experienced
  • Hit "wallet not registered" errors on the first sensitive operation downstream
  • Cause silent data divergence if they later fill out the registration form (because the form's submit calls completeRegistration, which produces a fresh signature with user-typed data — potentially overwriting authoritative DFX data with a transcription mistake)

Relation to #464

#464 is the routing-level fix: mark _bitboxConfirmed = true in the email merge confirmation success path. That fix is correct for users whose _completeRegistration genuinely succeeded (no signature, no symptom).

This issue is the underlying robustness gap: even with #464 fixed, users who hit Scenario 1 or 2 will pass through silently and may end up in a degraded state without the app being able to tell. The two issues should be fixed together — fixing only #464 would mask the silent failure further.

Fix proposals

A. Stop swallowing failures (recommended)

In checkEmailVerification, only emit Success if _completeRegistration actually succeeded:

if (currentAccountId != newAccountId) {
  final ok = await _completeRegistration();
  if (ok) {
    emit(const KycEmailVerificationSuccess());
  }
  // else: RegistrationFailure was already emitted from inside _completeRegistration
} else {
  emit(const KycEmailVerificationFailure());
}

Change _completeRegistration to return a bool (or use a Result/Either type) and re-throw a typed exception caught by the caller. Update the two tests at kyc_email_verification_cubit_test.dart:131-148 and :150-170 to assert RegistrationFailure as the final state and adjust the page listener at kyc_email_verification_page.dart:47-56 accordingly so the snackbar is actually seen.

B. Retry getWalletStatus for the null-userData case

If Scenario 1 is caused by a backend race (auth service updates faster than user-data service), a bounded retry with exponential backoff might bridge the window without exposing the failure to the user. Combine with A so that genuine null data still surfaces as Failure.

C. Have the backend wait for full propagation before returning the new JWT

The cleanest fix is upstream: the merge endpoint should not return a new JWT until the user-data record is ready to be read by the user-data service. Out of scope for this repo but worth flagging to the API team.

References

• lib/screens/kyc/steps/email/cubits/email_verification/kyc_email_verification_cubit.dart:26-50 — the cubit
• lib/screens/kyc/steps/email/subpages/kyc_email_verification_page.dart:37-60 — the listener that consumes the states
• test/screens/kyc/steps/email/kyc_email_verification_cubit_test.dart:131-170 — tests documenting current behaviour
• Related: Existing DFX customer is forced to re-enter personal data after successful merge #464 — routing-level bug that surfaces the same symptom via a different code path

Status of this analysis

• Verified by code reading and tests on develop (HEAD 59a2010)
• Not yet reproduced on a device. To attribute a specific user-reported symptom to Scenario 1 vs Scenario 2 vs Existing DFX customer is forced to re-enter personal data after successful merge #464 routing-only, a network capture of getWalletStatus immediately after invalidateAuthToken and the response of registerWallet would be conclusive

Environment

• Reporter: jana.ruettimann@dfx.swiss
• Discovered while investigating Existing DFX customer is forced to re-enter personal data after successful merge #464
• Branch analysed: develop (HEAD 59a2010)

[Blume1977]

Addressed alongside #464 in PR #466 — specifically commit d728979 (fix(kyc-email-verification): surface _completeRegistration failures).

Two failure modes that previously silently emitted Success now correctly leave the cubit in RegistrationFailure:

1. getWalletStatus returning null realUnitUserDataDto (backend propagation race)
2. registerWallet throwing

The existing tests at kyc_email_verification_cubit_test.dart:131-170 that asserted the silent-Success behaviour have been updated to assert the new, correct behaviour: failures surface to the verification page so the user can retry.

CI on #466 is action_required — needs maintainer approval to run.

[Blume1977]

Fixed by PR #466 — commit e71f71e (fix(kyc-email-verification): surface failures, add generation guard + retry path), merged today via 9ade5342.

_completeRegistration now returns Future<bool> and checkEmailVerification only emits Success on actual success. Both failure modes (null realUnitUserDataDto, registerWallet throws) now leave the cubit in RegistrationFailure so the verification page stays open and the user can retry. Tests at kyc_email_verification_cubit_test.dart:131-235 lock the new behaviour in.

Closing — should have been tagged Closes #465 in the original commit body instead of Refs.