diff --git a/lib/main.dart b/lib/main.dart index 4efe297c8..ae15497bc 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,3 +1,5 @@ +import 'dart:developer' as developer; + import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; @@ -14,11 +16,14 @@ import 'package:realunit_wallet/setup/routing/router_config.dart'; import 'package:realunit_wallet/setup/routing/routes/app_routes.dart'; import 'package:realunit_wallet/setup/routing/routes/onboarding_routes.dart'; import 'package:realunit_wallet/setup/routing/routes/pin_routes.dart'; +import 'package:realunit_wallet/styles/colors.dart'; import 'package:realunit_wallet/styles/themes.dart'; Future main() async { final widgetsBinding = WidgetsFlutterBinding.ensureInitialized(); + _installErrorHandlers(); + // only preserve splash screen for 3 seconds for release version if (kReleaseMode) { FlutterNativeSplash.preserve(widgetsBinding: widgetsBinding); @@ -38,6 +43,25 @@ Future _initialize() async { ); } +/// Defense-in-depth against uncaught build/paint exceptions. Without this, a +/// single throw inside a widget's `build` (e.g. the empty-BitBox-address +/// `EthereumAddress.fromHex("")` crash) surfaces in release as a bare grey +/// [ErrorWidget] with no branding and no signal to the user. We log every such +/// error and replace the grey box with a minimal on-brand surface. +void _installErrorHandlers() { + final defaultOnError = FlutterError.onError; + FlutterError.onError = (details) { + developer.log( + 'uncaught Flutter error: ${details.exceptionAsString()}', + name: 'WalletApp', + error: details.exception, + stackTrace: details.stack, + ); + defaultOnError?.call(details); + }; + ErrorWidget.builder = (details) => _RealUnitErrorView(details: details); +} + Future _initializeWithSplashDuration() async { await Future.wait([ _initialize(), @@ -127,6 +151,11 @@ class _WalletAppState extends State { targetRoute = PinRoutes.setup; } else if (!pinState.isPinVerified) { targetRoute = PinRoutes.verify; + } else if (homeState.bitboxAddressRecoveryNeeded) { + // A BitBox wallet was persisted with an empty/invalid address — divert to + // the re-pairing recovery flow instead of loading it into the dashboard + // (which would crash the build via `EthereumAddress.fromHex("")`). + targetRoute = AppRoutes.bitboxAddressRecovery; } else if (homeState.openWallet == null) { // Wallet not loaded yet — trigger load and wait for HomeBloc update _loadWalletIfNeeded(); @@ -138,3 +167,47 @@ class _WalletAppState extends State { routerConfig.goNamed(targetRoute); } } + +/// Minimal on-brand replacement for the default grey [ErrorWidget]. Rendered by +/// [ErrorWidget.builder] for any uncaught build/paint exception, so it lives +/// outside the [MaterialApp] localization scope — the copy is deliberately +/// hardcoded (no `S.of(context)`) because this is a last-resort surface that +/// must render even when localization is unavailable. In debug it also shows +/// the exception text to keep diagnosis fast; release shows only the friendly +/// line. +class _RealUnitErrorView extends StatelessWidget { + const _RealUnitErrorView({required this.details}); + + final FlutterErrorDetails details; + + @override + Widget build(BuildContext context) => Directionality( + textDirection: TextDirection.ltr, + child: Container( + color: RealUnitColors.neutral50, + alignment: Alignment.center, + padding: const EdgeInsets.all(24), + child: Column( + mainAxisSize: MainAxisSize.min, + spacing: 12, + children: [ + Icon(Icons.error_outline, color: RealUnitColors.status.red600, size: 48), + const Text( + 'Something went wrong. Please restart the app.', + textAlign: TextAlign.center, + style: TextStyle( + color: RealUnitColors.neutral900, + fontWeight: FontWeight.w600, + ), + ), + if (kDebugMode) + Text( + details.exceptionAsString(), + textAlign: TextAlign.center, + style: const TextStyle(color: RealUnitColors.neutral500), + ), + ], + ), + ), + ); +} diff --git a/lib/packages/hardware_wallet/bitbox.dart b/lib/packages/hardware_wallet/bitbox.dart index 23a5600d4..1e5caa6cd 100644 --- a/lib/packages/hardware_wallet/bitbox.dart +++ b/lib/packages/hardware_wallet/bitbox.dart @@ -3,8 +3,14 @@ import 'dart:developer' as developer; import 'package:bitbox_flutter/bitbox_flutter.dart'; import 'package:realunit_wallet/packages/hardware_wallet/bitbox_credentials.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart'; class BitboxService { + // ETH's canonical BIP-44 derivation path. Centralised here so the create + // and heal flows share one literal — a drift between them would quote two + // different addresses for the same device. + static const _ethDerivationPath = "m/44'/60'/0'/0/0"; + // Observer poll period is widened in production and tightened in tests so // device-loss-recovery behaviour can be exercised in real time without // five-second sleeps. @@ -122,4 +128,27 @@ class BitboxService { final didVerify = await bitboxManager.channelHashVerify(); if (!didVerify) throw Exception('Failed to verify'); } + + /// Derives the wallet's ETH address from the device, retrying transient empty + /// reads before giving up. + /// + /// The SDK coerces a native `null` into `""` at the transport boundary + /// (`bitbox_usb_method_channel.dart`'s `return result ?? ''`), so a device + /// that isn't fully ready (e.g. a BLE stall right after channel-hash verify) + /// resolves `getETHAddress` with an empty string instead of throwing. This is + /// the single boundary through which create + heal fetch the address, so an + /// empty read can never be persisted: a transient stall self-recovers across + /// [attempts], and a persistent one throws [BitboxAddressUnavailableException] + /// instead of handing back `""`. + Future getEthAddress({ + int attempts = 3, + Duration retryDelay = const Duration(milliseconds: 200), + }) async { + for (var attempt = 0; attempt < attempts; attempt++) { + final address = await bitboxManager.getETHAddress(1, _ethDerivationPath); + if (address.isNotEmpty) return address; + if (attempt < attempts - 1) await Future.delayed(retryDelay); + } + throw const BitboxAddressUnavailableException(); + } } diff --git a/lib/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart b/lib/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart new file mode 100644 index 000000000..0cf5627c0 --- /dev/null +++ b/lib/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart @@ -0,0 +1,16 @@ +/// Raised when a BitBox device fails to hand back a usable ETH address — either +/// at pairing time ([WalletService.createBitboxWallet]) or while self-healing a +/// wallet that was persisted with an empty/invalid address +/// ([WalletService.healCurrentBitboxAddress]). +/// +/// Persisting an empty address used to crash the dashboard on the next launch: +/// `EthereumAddress.fromHex("")` throws an uncaught `ArgumentError` inside the +/// build phase, which surfaces as a grey [ErrorWidget] in release. Surfacing a +/// typed exception lets the pairing / recovery flow fall back to its retry path +/// instead. +class BitboxAddressUnavailableException implements Exception { + const BitboxAddressUnavailableException(); + + @override + String toString() => 'BitBox did not return a valid wallet address'; +} diff --git a/lib/packages/service/wallet_service.dart b/lib/packages/service/wallet_service.dart index a0a0fa707..83b32e5fc 100644 --- a/lib/packages/service/wallet_service.dart +++ b/lib/packages/service/wallet_service.dart @@ -5,7 +5,9 @@ import 'package:realunit_wallet/packages/hardware_wallet/bitbox.dart'; import 'package:realunit_wallet/packages/repository/settings_repository.dart'; import 'package:realunit_wallet/packages/repository/wallet_repository.dart'; import 'package:realunit_wallet/packages/service/app_store.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart'; import 'package:realunit_wallet/packages/wallet/wallet.dart'; +import 'package:web3dart/web3dart.dart'; class WalletService { final WalletRepository _repository; @@ -85,12 +87,73 @@ class WalletService { } Future createBitboxWallet(String name) async { - final address = await _bitboxService.bitboxManager.getETHAddress(1, "m/44'/60'/0'/0/0"); + // [BitboxService.getEthAddress] already retries the transient empty read + // the SDK produces when it coerces a native `null` into `""` (its + // `return result ?? ''` transport boundary) and throws on a persistent + // one. The format guard below is defence-in-depth — it also rejects the + // rarer non-empty-but-malformed read before `EthereumAddress.fromHex` + // would crash the dashboard build on the next launch. + final address = await _bitboxService.getEthAddress(); + if (!_isValidEthAddress(address)) { + throw const BitboxAddressUnavailableException(); + } final walletId = await _repository.createViewWallet(name, WalletType.bitbox, address); await setCurrentWallet(walletId); return BitboxWallet(walletId, name, address, _bitboxService); } + /// True when [address] parses as a canonical 20-byte Ethereum address. + /// + /// Uses the same web3dart parser that every wallet credential class relies on + /// so the validity boundary here matches exactly the one that would otherwise + /// throw deep in the dashboard build. An empty string fails fast through the + /// [FormatException]/[ArgumentError] catch — no need to special-case it. + static bool _isValidEthAddress(String address) { + try { + EthereumAddress.fromHex(address); + return true; + } on FormatException { + return false; + } on ArgumentError { + return false; + } + } + + /// Non-throwing probe used at app load to decide whether the current wallet + /// is a BitBox row that was persisted with an empty/invalid address (the + /// pre-fix data corruption). Returns false for software/debug wallets, for a + /// missing current id, for an absent row, and for a BitBox row whose address + /// parses cleanly — i.e. it only flags the rows that would crash the + /// dashboard build, leaving every healthy wallet on the normal load path. + Future currentWalletNeedsAddressRecovery() async { + final id = _settingsRepository.currentWalletId; + if (id == null) return false; + final info = await _repository.getWalletInfo(id); + if (info == null) return false; + if (WalletType.values[info.type] != WalletType.bitbox) return false; + return !_isValidEthAddress(info.address); + } + + /// Re-derives the ETH address from a freshly re-paired BitBox and backfills it + /// onto the current wallet row, replacing the empty/invalid one. This is local + /// crypto/device recovery — the address is a deterministic function of the + /// device, not account state owned by the API — so it does not belong behind a + /// network round-trip. Throws [BitboxAddressUnavailableException] if the device + /// still returns an unusable address so the recovery UI can retry rather than + /// re-persist the corruption. + Future healCurrentBitboxAddress() async { + final id = _settingsRepository.currentWalletId!; + final info = (await _repository.getWalletInfo(id))!; + // Shares the retry + empty-guard boundary with createBitboxWallet; the + // format check stays as defence-in-depth (see that method). + final address = await _bitboxService.getEthAddress(); + if (!_isValidEthAddress(address)) { + throw const BitboxAddressUnavailableException(); + } + await _repository.updateAddress(id, address); + return BitboxWallet(id, info.name, address, _bitboxService); + } + /// Persists a user-supplied seed phrase immediately — the user typed an /// existing mnemonic, so there is no verify-seed quiz to gate the write /// behind. Deferring would not help: the seed is already known and the diff --git a/lib/screens/hardware_connect_bitbox/bitbox_address_recovery_page.dart b/lib/screens/hardware_connect_bitbox/bitbox_address_recovery_page.dart new file mode 100644 index 000000000..4035c0269 --- /dev/null +++ b/lib/screens/hardware_connect_bitbox/bitbox_address_recovery_page.dart @@ -0,0 +1,49 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_bloc/flutter_bloc.dart'; +import 'package:realunit_wallet/packages/hardware_wallet/bitbox.dart'; +import 'package:realunit_wallet/packages/service/dfx/dfx_kyc_service.dart'; +import 'package:realunit_wallet/packages/service/wallet_service.dart'; +import 'package:realunit_wallet/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart'; +import 'package:realunit_wallet/screens/hardware_connect_bitbox/connect_bitbox_view.dart'; +import 'package:realunit_wallet/screens/home/bloc/home_bloc.dart'; +import 'package:realunit_wallet/setup/di.dart'; + +/// Full-page host for the BitBox pairing ceremony when an already-persisted +/// BitBox wallet is missing a valid address (the pre-fix data corruption that +/// crashed the dashboard build). Reuses the exact [ConnectBitboxView] from the +/// initial-pairing flow, but wires the cubit's wallet-acquisition step to +/// [WalletService.healCurrentBitboxAddress] so re-pairing backfills the address +/// onto the existing row instead of creating a second wallet. +/// +/// On finish it feeds the healed wallet back through [LoadWalletEvent]; the +/// HomeBloc listener in `main` then re-runs `_navigate`, which lands on the +/// dashboard now that the recovery flag is cleared. +class BitboxAddressRecoveryPage extends StatelessWidget { + const BitboxAddressRecoveryPage({super.key}); + + @override + Widget build(BuildContext context) => Scaffold( + body: BlocProvider( + create: (_) => ConnectBitboxCubit( + getIt(), + getIt(), + // DfxKycService is the smallest registered DFXAuthService — used only as + // a transport for ensureSignatureFor(account); no KYC-specific calls + // here. Mirrors ConnectBitboxPage. + getIt(), + acquireWallet: () => getIt().healCurrentBitboxAddress(), + ), + child: ConnectBitboxView( + onFinish: (wallet) => getIt().add(LoadWalletEvent(wallet)), + // This page is the only navigation-stack entry (reached via `goNamed`), + // so `context.pop` would throw `GoError`. The BitBox wallet is view-only + // (keys live on the device, address is deterministically re-derivable by + // re-pairing), so deleting the corrupted local row is non-destructive: + // the handler resets HomeState (hasWallet:false, + // bitboxAddressRecoveryNeeded:false) and `main._navigate` then lands on + // the welcome/onboarding screen — no throw, no re-route loop. + onCancel: () => getIt().add(const DeleteCurrentWalletEvent()), + ), + ), + ); +} diff --git a/lib/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart b/lib/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart index 60170bb63..96da7838a 100644 --- a/lib/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart +++ b/lib/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart @@ -31,9 +31,11 @@ class ConnectBitboxCubit extends Cubit { Duration confirmPairingTimeout = _defaultConfirmPairingTimeout, Duration createWalletTimeout = _defaultCreateWalletTimeout, Duration pairingPinTimeout = _defaultPairingPinTimeout, + Future Function()? acquireWallet, }) : _confirmPairingTimeout = confirmPairingTimeout, _createWalletTimeout = createWalletTimeout, _pairingPinTimeout = pairingPinTimeout, + _acquireWallet = acquireWallet, super(BitboxNotConnected()) { _startScanning(); } @@ -42,6 +44,16 @@ class ConnectBitboxCubit extends Cubit { final Duration _createWalletTimeout; final Duration _pairingPinTimeout; + /// Injectable wallet-acquisition step. `null` selects the default behaviour: + /// the initial-pairing flow creates a brand-new view wallet. The + /// address-recovery flow passes [WalletService.healCurrentBitboxAddress] so + /// the same pairing ceremony backfills the empty address onto the existing + /// row instead of creating a second wallet. + final Future Function()? _acquireWallet; + + Future _acquireWalletOrDefault() => + (_acquireWallet ?? (() => _walletService.createBitboxWallet('Luke-Skywallet')))(); + Future _startScanning() async { if (DeviceInfo.instance.isIOS) await _service.startScan(); _checkForTimer = Timer.periodic(const Duration(milliseconds: 500), (_) => checkForBitbox()); @@ -143,15 +155,13 @@ class ConnectBitboxCubit extends Cubit { 'Disconnect the device, restart the app, and re-pair.', ), ); - final wallet = await _walletService - .createBitboxWallet('Luke-Skywallet') - .timeout( - _createWalletTimeout, - onTimeout: () => throw TimeoutException( - 'BitBox did not return an ETH address within ' - '${_createWalletTimeout.inSeconds}s. Try disconnecting and re-pairing.', - ), - ); + final wallet = await _acquireWalletOrDefault().timeout( + _createWalletTimeout, + onTimeout: () => throw TimeoutException( + 'BitBox did not return an ETH address within ' + '${_createWalletTimeout.inSeconds}s. Try disconnecting and re-pairing.', + ), + ); _service.startConnectionStatusObserver(); await _captureAuthSignature(wallet); } catch (e) { diff --git a/lib/screens/hardware_connect_bitbox/connect_bitbox_view.dart b/lib/screens/hardware_connect_bitbox/connect_bitbox_view.dart index 6f64b6aa7..ee72a39fb 100644 --- a/lib/screens/hardware_connect_bitbox/connect_bitbox_view.dart +++ b/lib/screens/hardware_connect_bitbox/connect_bitbox_view.dart @@ -11,10 +11,17 @@ import 'package:realunit_wallet/styles/colors.dart'; import 'package:realunit_wallet/widgets/handlebars.dart'; class ConnectBitboxView extends StatelessWidget { - const ConnectBitboxView({super.key, required this.onFinish}); + const ConnectBitboxView({super.key, required this.onFinish, this.onCancel}); final void Function(AWallet wallet) onFinish; + /// Overrides the default cancel handler. Bottom-sheet hosts leave this null so + /// cancelling pops the sheet (`context.pop`). Full-page hosts reached via + /// `goNamed` (e.g. [BitboxAddressRecoveryPage]) must pass a safe callback — + /// there `context.pop` throws `GoError` because the recovery route is the only + /// stack entry and `canPop()` is false. + final VoidCallback? onCancel; + @override Widget build(BuildContext context) => SafeArea( child: SizedBox( @@ -64,7 +71,7 @@ class ConnectBitboxView extends StatelessWidget { title: S.of(context).connectBitboxTitle, imagePath: 'assets/images/illustrations/bitbox_connected.svg', onConfirm: context.read().confirmPairing, - onCancel: context.pop, + onCancel: onCancel ?? context.pop, child: Column( spacing: 16, children: [ @@ -162,7 +169,7 @@ class ConnectBitboxView extends StatelessWidget { _ => ConnectContent( title: S.of(context).connectBitboxTitle, imagePath: 'assets/images/illustrations/bitbox_connect.svg', - onCancel: context.pop, + onCancel: onCancel ?? context.pop, child: Text( DeviceInfo.instance.isIOS ? S.of(context).connectBitboxContentIos diff --git a/lib/screens/home/bloc/home_bloc.dart b/lib/screens/home/bloc/home_bloc.dart index 2ddc90f6c..f9f6569de 100644 --- a/lib/screens/home/bloc/home_bloc.dart +++ b/lib/screens/home/bloc/home_bloc.dart @@ -56,6 +56,16 @@ class HomeBloc extends Bloc { if (state.openWallet != null || !_walletService.hasWallet()) return; emit(state.copyWith(isLoadingWallet: true)); + + // Self-heal gate: a BitBox row persisted with an empty/invalid address would + // crash the dashboard build via `EthereumAddress.fromHex("")`. Divert to the + // address-recovery pairing flow before we ever load such a wallet into the + // AppStore. Non-throwing — a healthy wallet returns false and falls through. + if (await _walletService.currentWalletNeedsAddressRecovery()) { + emit(state.copyWith(isLoadingWallet: false, bitboxAddressRecoveryNeeded: true)); + return; + } + try { final wallet = await _walletService.getCurrentWallet(); _appStore.wallet = wallet; @@ -102,7 +112,17 @@ class HomeBloc extends Bloc { Future _onLoadWallet(LoadWalletEvent event, Emitter emit) async { _updateWallet(event.wallet); - emit(state.copyWith(hasWallet: true, openWallet: _appStore.wallet, isLoadingWallet: false)); + // A wallet that loaded cleanly here (including a freshly healed BitBox + // address) means recovery is done — clear the flag so `_navigate` routes to + // the dashboard instead of back to the recovery page. + emit( + state.copyWith( + hasWallet: true, + openWallet: _appStore.wallet, + isLoadingWallet: false, + bitboxAddressRecoveryNeeded: false, + ), + ); } void _onSyncWalletServices( diff --git a/lib/screens/home/bloc/home_state.dart b/lib/screens/home/bloc/home_state.dart index 051bb4e7a..52b6c0b30 100644 --- a/lib/screens/home/bloc/home_state.dart +++ b/lib/screens/home/bloc/home_state.dart @@ -7,6 +7,7 @@ final class HomeState { this.isLoadingWallet = false, this.onboardingCompleted = false, this.softwareTermsAccepted = false, + this.bitboxAddressRecoveryNeeded = false, }); final bool hasWallet; @@ -15,17 +16,26 @@ final class HomeState { final bool onboardingCompleted; final bool softwareTermsAccepted; + /// True when the persisted current wallet is a BitBox row whose address is + /// empty/invalid — the pre-fix data corruption that crashed the dashboard + /// build. Routes the app to the address-recovery pairing flow instead of the + /// dashboard until the device re-supplies a valid address. + final bool bitboxAddressRecoveryNeeded; + HomeState copyWith({ bool? hasWallet, AWallet? openWallet, bool? isLoadingWallet, bool? onboardingCompleted, bool? softwareTermsAccepted, + bool? bitboxAddressRecoveryNeeded, }) => HomeState( hasWallet: hasWallet ?? this.hasWallet, openWallet: openWallet ?? this.openWallet, isLoadingWallet: isLoadingWallet ?? this.isLoadingWallet, onboardingCompleted: onboardingCompleted ?? this.onboardingCompleted, softwareTermsAccepted: softwareTermsAccepted ?? this.softwareTermsAccepted, + bitboxAddressRecoveryNeeded: + bitboxAddressRecoveryNeeded ?? this.bitboxAddressRecoveryNeeded, ); } diff --git a/lib/setup/routing/router_config.dart b/lib/setup/routing/router_config.dart index 3d0b9e263..f25059ce8 100644 --- a/lib/setup/routing/router_config.dart +++ b/lib/setup/routing/router_config.dart @@ -6,6 +6,7 @@ import 'package:realunit_wallet/screens/buy/buy_page.dart'; import 'package:realunit_wallet/screens/create_wallet/create_wallet_page.dart'; import 'package:realunit_wallet/screens/dashboard/dashboard_page.dart'; import 'package:realunit_wallet/screens/debug_auth/debug_auth_page.dart'; +import 'package:realunit_wallet/screens/hardware_connect_bitbox/bitbox_address_recovery_page.dart'; import 'package:realunit_wallet/screens/home/home_page.dart'; import 'package:realunit_wallet/screens/kyc/kyc_page_manager.dart'; import 'package:realunit_wallet/screens/legal/legal_disclaimer_page.dart'; @@ -190,6 +191,12 @@ final GoRouter routerConfig = GoRouter( builder: (_, _) => const ReceivePage(isBottomSheet: false), ), + GoRoute( + name: AppRoutes.bitboxAddressRecovery, + path: '/bitboxAddressRecovery', + builder: (_, _) => const BitboxAddressRecoveryPage(), + ), + GoRoute( name: SettingsRoutes.settings, path: '/settings', diff --git a/lib/setup/routing/routes/app_routes.dart b/lib/setup/routing/routes/app_routes.dart index 1ec2a07a9..ebac4c9a9 100644 --- a/lib/setup/routing/routes/app_routes.dart +++ b/lib/setup/routing/routes/app_routes.dart @@ -7,6 +7,7 @@ abstract final class AppRoutes { static const sellBitbox = 'sellBitbox'; static const kyc = 'kyc'; static const receive = 'receive'; + static const bitboxAddressRecovery = 'bitboxAddressRecovery'; static const webView = 'webView'; } diff --git a/test/packages/hardware_wallet/bitbox_service_test.dart b/test/packages/hardware_wallet/bitbox_service_test.dart index bfec6b09e..43ee21c83 100644 --- a/test/packages/hardware_wallet/bitbox_service_test.dart +++ b/test/packages/hardware_wallet/bitbox_service_test.dart @@ -4,6 +4,7 @@ import 'package:bitbox_flutter/usb/bitbox_usb_platform_interface.dart'; import 'package:fake_async/fake_async.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:realunit_wallet/packages/hardware_wallet/bitbox.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart'; // Service-lifecycle suite. Drives the official bitbox_flutter simulator // (installed at the BitboxUsbPlatform.instance seam) so the tests exercise @@ -453,5 +454,93 @@ void main() { }); }, ); + + // The empty-address self-heal boundary. The SDK coerces a native `null` + // into `""` at its transport layer (`return result ?? ''`), so a device + // that stalls right after channel-hash verify resolves getETHAddress with + // an empty string instead of throwing. getEthAddress retries that transient + // empty read across `attempts`; only a persistent empty throws. Both the + // create and heal flows route through this method, so an empty read can + // never be persisted. + group('getEthAddress', () { + const validAddress = '0x1111111111111111111111111111111111111111'; + const retryDelay = Duration(milliseconds: 200); + + test('returns the address when the first read is non-empty', () { + fakeAsync((async) { + final service = pairedServiceSync(async); + platform.when(SimulatedBitboxMethod.getETHAddress, (_) async => validAddress); + + String? result; + service.getEthAddress(retryDelay: retryDelay).then((value) => result = value); + async.flushMicrotasks(); + + expect(result, validAddress); + expect( + platform.count(SimulatedBitboxMethod.getETHAddress), + 1, + reason: 'a non-empty first read must short-circuit the retry loop', + ); + }); + }); + + test('retries an empty read and succeeds once the device returns a valid address', () { + fakeAsync((async) { + final service = pairedServiceSync(async); + // First read stalls (empty), second read recovers — the exact BLE + // stall the boundary exists to absorb. + var call = 0; + platform.when( + SimulatedBitboxMethod.getETHAddress, + (_) async => call++ == 0 ? '' : validAddress, + ); + + String? result; + service.getEthAddress(retryDelay: retryDelay).then((value) => result = value); + // The first (empty) read resolves on microtasks; the retry waits the + // backoff before the second read. + async.flushMicrotasks(); + expect(result, isNull, reason: 'must wait the retry delay before the second read'); + + async.elapse(retryDelay); + + expect(result, validAddress); + expect( + platform.count(SimulatedBitboxMethod.getETHAddress), + 2, + reason: 'exactly one retry was needed to recover', + ); + }); + }); + + test('throws BitboxAddressUnavailableException when every read is empty', () { + fakeAsync((async) { + final service = pairedServiceSync(async); + platform.when(SimulatedBitboxMethod.getETHAddress, (_) async => ''); + + Object? caught; + service + .getEthAddress(attempts: 3, retryDelay: retryDelay) + .catchError((Object e) { + caught = e; + return ''; + }); + // Drive past both inter-attempt delays so all three reads run. + async.flushMicrotasks(); + async.elapse(retryDelay * 3); + + expect( + caught, + isA(), + reason: 'a persistent empty read must throw, never return ""', + ); + expect( + platform.count(SimulatedBitboxMethod.getETHAddress), + 3, + reason: 'all configured attempts must be exhausted before throwing', + ); + }); + }); + }); }); } diff --git a/test/packages/service/dfx/exceptions/exception_surface_test.dart b/test/packages/service/dfx/exceptions/exception_surface_test.dart index d187e5a66..891ee5689 100644 --- a/test/packages/service/dfx/exceptions/exception_surface_test.dart +++ b/test/packages/service/dfx/exceptions/exception_surface_test.dart @@ -1,5 +1,6 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/api_exception.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_exception.dart'; import 'package:realunit_wallet/packages/service/dfx/exceptions/payment/buy_exceptions.dart'; import 'package:realunit_wallet/packages/wallet/exceptions/signing_cancelled_exception.dart'; @@ -15,6 +16,7 @@ void main() { group('exception toString surface', () { final exceptions = [ const BitboxNotConnectedException(), + const BitboxAddressUnavailableException(), const SigningCancelledException(), const ApiException(code: 'TEST', message: 'test'), const RegistrationRequiredException(code: 'TEST', message: 'test'), diff --git a/test/packages/service/wallet_service_test.dart b/test/packages/service/wallet_service_test.dart index 24f19cfa4..95ce405f3 100644 --- a/test/packages/service/wallet_service_test.dart +++ b/test/packages/service/wallet_service_test.dart @@ -1,6 +1,5 @@ import 'dart:async'; -import 'package:bitbox_flutter/bitbox_manager.dart'; import 'package:fake_async/fake_async.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mocktail/mocktail.dart'; @@ -9,6 +8,7 @@ import 'package:realunit_wallet/packages/hardware_wallet/bitbox_credentials.dart import 'package:realunit_wallet/packages/repository/settings_repository.dart'; import 'package:realunit_wallet/packages/repository/wallet_repository.dart'; import 'package:realunit_wallet/packages/service/app_store.dart'; +import 'package:realunit_wallet/packages/service/dfx/exceptions/bitbox_address_unavailable_exception.dart'; import 'package:realunit_wallet/packages/service/wallet_service.dart'; import 'package:realunit_wallet/packages/storage/database.dart'; import 'package:realunit_wallet/packages/wallet/wallet.dart'; @@ -19,8 +19,6 @@ class _MockSettingsRepository extends Mock implements SettingsRepository {} class _MockBitboxService extends Mock implements BitboxService {} -class _MockBitboxManager extends Mock implements BitboxManager {} - class _MockAppStore extends Mock implements AppStore {} const _testMnemonic = 'test test test test test test test test test test test junk'; @@ -221,22 +219,14 @@ void main() { }); group('createBitboxWallet', () { - // Drives the BitBox-pairing happy path end-to-end at this layer: derive - // the EIP-55 address from the device, persist a view-row in `walletInfos` - // (encrypted-seed column is `null` for hardware wallets), mark the row - // current, and return a typed BitboxWallet so the caller can immediately - // request a signature in the same flow. - late _MockBitboxManager manager; - - setUp(() { - manager = _MockBitboxManager(); - when(() => bitbox.bitboxManager).thenReturn(manager); - }); - - test('derives the BIP-44 ETH address from the device and persists a view row', () async { - when( - () => manager.getETHAddress(1, "m/44'/60'/0'/0/0"), - ).thenAnswer((_) async => _debugAddress); + // Drives the BitBox-pairing happy path end-to-end at this layer: fetch + // the EIP-55 address from the device via the retry boundary, persist a + // view-row in `walletInfos` (encrypted-seed column is `null` for hardware + // wallets), mark the row current, and return a typed BitboxWallet so the + // caller can immediately request a signature in the same flow. + + test('fetches the ETH address via the service boundary and persists a view row', () async { + when(() => bitbox.getEthAddress()).thenAnswer((_) async => _debugAddress); when(() => repo.createViewWallet(any(), any(), any())).thenAnswer((_) async => 11); // BitboxWallet ctor pulls credentials from the service — return a // fake handle so the test exercises the WalletService logic and not @@ -248,10 +238,9 @@ void main() { expect(wallet, isA()); expect(wallet.id, 11); expect(wallet.name, 'Hardware'); - // The BitBox keypath is non-negotiable: chainId 1 + ETH's canonical - // BIP-44 path. A drifting keypath would silently quote a different - // address than the rest of the app expects. - verify(() => manager.getETHAddress(1, "m/44'/60'/0'/0/0")).called(1); + // The address fetch goes through the centralised retry boundary, not a + // raw manager call — that's where the empty-read self-heal lives. + verify(() => bitbox.getEthAddress()).called(1); verify( () => repo.createViewWallet('Hardware', WalletType.bitbox, _debugAddress), ).called(1); @@ -261,9 +250,7 @@ void main() { }); test('propagates a BitBox derivation failure without writing to the repo', () async { - when( - () => manager.getETHAddress(any(), any()), - ).thenThrow(Exception('USB transport dropped')); + when(() => bitbox.getEthAddress()).thenThrow(Exception('USB transport dropped')); expect( () => service.createBitboxWallet('Hardware'), @@ -272,6 +259,124 @@ void main() { verifyNever(() => repo.createViewWallet(any(), any(), any())); verifyNever(() => settings.saveCurrentWalletId(any())); }); + + // The retry boundary throws on a persistent empty read; createBitboxWallet + // must propagate it so nothing lands on disk and the pairing flow's retry + // path takes over. + test('propagates BitboxAddressUnavailableException from the boundary — nothing persisted', + () async { + when(() => bitbox.getEthAddress()).thenThrow(const BitboxAddressUnavailableException()); + + await expectLater( + () => service.createBitboxWallet('Hardware'), + throwsA(isA()), + ); + verifyNever(() => repo.createViewWallet(any(), any(), any())); + verifyNever(() => settings.saveCurrentWalletId(any())); + }); + + // Defence-in-depth: the boundary can't be empty, but a non-empty yet + // malformed read still has to be rejected by the format guard before + // `EthereumAddress.fromHex` would crash the dashboard build. + test('throws BitboxAddressUnavailableException on a malformed address', () async { + when(() => bitbox.getEthAddress()).thenAnswer((_) async => 'not-a-hex-address'); + + await expectLater( + () => service.createBitboxWallet('Hardware'), + throwsA(isA()), + ); + verifyNever(() => repo.createViewWallet(any(), any(), any())); + verifyNever(() => settings.saveCurrentWalletId(any())); + }); + }); + + group('currentWalletNeedsAddressRecovery', () { + test('true for a BitBox row persisted with an empty address', () async { + when(() => settings.currentWalletId).thenReturn(5); + when(() => repo.getWalletInfo(5)).thenAnswer( + (_) async => _info(id: 5, name: 'Hardware', address: '', type: WalletType.bitbox), + ); + + expect(await service.currentWalletNeedsAddressRecovery(), isTrue); + }); + + test('true for a BitBox row persisted with a malformed address', () async { + when(() => settings.currentWalletId).thenReturn(5); + when(() => repo.getWalletInfo(5)).thenAnswer( + (_) async => _info(id: 5, name: 'Hardware', address: 'garbage', type: WalletType.bitbox), + ); + + expect(await service.currentWalletNeedsAddressRecovery(), isTrue); + }); + + test('false for a BitBox row with a valid address', () async { + when(() => settings.currentWalletId).thenReturn(5); + when(() => repo.getWalletInfo(5)).thenAnswer( + (_) async => + _info(id: 5, name: 'Hardware', address: _debugAddress, type: WalletType.bitbox), + ); + + expect(await service.currentWalletNeedsAddressRecovery(), isFalse); + }); + + test('false for a software wallet even with an empty address', () async { + when(() => settings.currentWalletId).thenReturn(5); + when(() => repo.getWalletInfo(5)).thenAnswer( + (_) async => _info(id: 5, name: 'Main', address: '', type: WalletType.software), + ); + + expect(await service.currentWalletNeedsAddressRecovery(), isFalse); + }); + + test('false when no current wallet id is set', () async { + when(() => settings.currentWalletId).thenReturn(null); + + expect(await service.currentWalletNeedsAddressRecovery(), isFalse); + verifyNever(() => repo.getWalletInfo(any())); + }); + + test('false when the row is missing', () async { + when(() => settings.currentWalletId).thenReturn(5); + when(() => repo.getWalletInfo(5)).thenAnswer((_) async => null); + + expect(await service.currentWalletNeedsAddressRecovery(), isFalse); + }); + }); + + group('healCurrentBitboxAddress', () { + setUp(() { + when(() => bitbox.getCredentials(any())).thenReturn(BitboxCredentials(_debugAddress)); + }); + + test('re-derives the address, backfills the row, and returns a BitboxWallet', () async { + when(() => settings.currentWalletId).thenReturn(5); + when(() => repo.getWalletInfo(5)).thenAnswer( + (_) async => _info(id: 5, name: 'Hardware', address: '', type: WalletType.bitbox), + ); + when(() => bitbox.getEthAddress()).thenAnswer((_) async => _debugAddress); + + final wallet = await service.healCurrentBitboxAddress(); + + expect(wallet, isA()); + expect(wallet.id, 5); + expect(wallet.name, 'Hardware'); + verify(() => bitbox.getEthAddress()).called(1); + verify(() => repo.updateAddress(5, _debugAddress)).called(1); + }); + + test('propagates BitboxAddressUnavailableException and does NOT persist', () async { + when(() => settings.currentWalletId).thenReturn(5); + when(() => repo.getWalletInfo(5)).thenAnswer( + (_) async => _info(id: 5, name: 'Hardware', address: '', type: WalletType.bitbox), + ); + when(() => bitbox.getEthAddress()).thenThrow(const BitboxAddressUnavailableException()); + + await expectLater( + () => service.healCurrentBitboxAddress(), + throwsA(isA()), + ); + verifyNever(() => repo.updateAddress(any(), any())); + }); }); group('createDebugWallet', () { diff --git a/test/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit_test.dart b/test/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit_test.dart index aa0420f27..11b764c55 100644 --- a/test/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit_test.dart +++ b/test/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit_test.dart @@ -61,6 +61,7 @@ void main() { Duration confirmPairingTimeout = const Duration(milliseconds: 500), Duration createWalletTimeout = const Duration(milliseconds: 500), Duration pairingPinTimeout = const Duration(milliseconds: 500), + Future Function()? acquireWallet, }) => ConnectBitboxCubit( service, walletService, @@ -68,6 +69,7 @@ void main() { confirmPairingTimeout: confirmPairingTimeout, createWalletTimeout: createWalletTimeout, pairingPinTimeout: pairingPinTimeout, + acquireWallet: acquireWallet, ); Future waitForState( @@ -398,5 +400,58 @@ void main() { .timeout(const Duration(seconds: 2)); expect(cubit.state, isA()); }); + + // Recovery flow injection: when an `acquireWallet` override is supplied + // (the address-recovery page passes `healCurrentBitboxAddress`), the cubit + // must call IT instead of the default `createBitboxWallet`, so re-pairing + // backfills the existing row rather than creating a second wallet. + test('confirmPairing uses the injected acquireWallet instead of createBitboxWallet', () async { + var pollCount = 0; + when(() => service.getAllUsbDevices()).thenAnswer((_) async => [device]); + when(() => service.init(any())).thenAnswer((_) async => true); + when(() => service.getChannelHash()).thenAnswer((_) async { + pollCount++; + return pollCount < 3 ? '' : 'HASH-ok'; + }); + when(() => service.confirmPairing()).thenAnswer((_) async {}); + + var acquireCalls = 0; + final cubit = makeCubit( + acquireWallet: () async { + acquireCalls++; + return wallet; + }, + ); + addTearDown(cubit.close); + + await waitForState(cubit); + await cubit.confirmPairing(); + + expect(cubit.state, isA()); + expect(acquireCalls, 1, reason: 'the injected acquireWallet must be the wallet source'); + // The default path must NOT run when an override is supplied. + verifyNever(() => walletService.createBitboxWallet(any())); + verify(() => authService.ensureSignatureFor(any())).called(1); + }); + + test('confirmPairing falls back to createBitboxWallet when no override is given', () async { + var pollCount = 0; + when(() => service.getAllUsbDevices()).thenAnswer((_) async => [device]); + when(() => service.init(any())).thenAnswer((_) async => true); + when(() => service.getChannelHash()).thenAnswer((_) async { + pollCount++; + return pollCount < 3 ? '' : 'HASH-ok'; + }); + when(() => service.confirmPairing()).thenAnswer((_) async {}); + + final cubit = makeCubit(); + addTearDown(cubit.close); + + await waitForState(cubit); + await cubit.confirmPairing(); + + expect(cubit.state, isA()); + verify(() => walletService.createBitboxWallet('Luke-Skywallet')).called(1); + }); }); } diff --git a/test/screens/hardware_connect_bitbox/connect_bitbox_view_test.dart b/test/screens/hardware_connect_bitbox/connect_bitbox_view_test.dart index 965021e08..478ad0d0c 100644 --- a/test/screens/hardware_connect_bitbox/connect_bitbox_view_test.dart +++ b/test/screens/hardware_connect_bitbox/connect_bitbox_view_test.dart @@ -1,8 +1,12 @@ import 'package:bloc_test/bloc_test.dart'; import 'package:flutter/cupertino.dart'; +import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; +import 'package:flutter_localizations/flutter_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; import 'package:mocktail/mocktail.dart'; +import 'package:realunit_wallet/generated/i18n.dart'; import 'package:realunit_wallet/packages/wallet/wallet.dart'; import 'package:realunit_wallet/screens/hardware_connect_bitbox/bloc/connect_bitbox_cubit.dart'; import 'package:realunit_wallet/screens/hardware_connect_bitbox/connect_bitbox_view.dart'; @@ -36,6 +40,37 @@ void main() { ); } + // Mounts the view as the only entry of a GoRouter stack — mirrors + // BitboxAddressRecoveryPage being reached via `goNamed`, where `canPop()` is + // false and a bare `context.pop` would throw GoError. + Future pumpViewOnSingleEntryStack( + WidgetTester tester, + BitboxConnectionState state, { + VoidCallback? onCancel, + }) async { + when(() => cubit.state).thenReturn(state); + whenListen(cubit, const Stream.empty(), initialState: state); + final router = GoRouter( + routes: [ + GoRoute( + path: '/', + builder: (_, _) => BlocProvider.value( + value: cubit, + child: ConnectBitboxView(onFinish: (_) {}, onCancel: onCancel), + ), + ), + ], + ); + addTearDown(router.dispose); + await tester.pumpWidget( + MaterialApp.router( + routerConfig: router, + localizationsDelegates: [S.delegate, GlobalMaterialLocalizations.delegate], + supportedLocales: S.delegate.supportedLocales, + ), + ); + } + group('$ConnectBitboxView', () { testWidgets('BitboxCapturingSignature shows a spinner and no buttons', (tester) async { await pumpView(tester, BitboxCapturingSignature(wallet)); @@ -87,5 +122,39 @@ void main() { expect(buttons[0].label, 'Retry now'); expect(buttons[1].label, 'Skip'); }); + + testWidgets( + 'cancel on a single-entry stack invokes the injected onCancel without throwing GoError', + (tester) async { + var cancelled = 0; + await pumpViewOnSingleEntryStack( + tester, + BitboxNotConnected(), + onCancel: () => cancelled++, + ); + + // BitboxNotConnected renders the default `_` case, which shows Cancel. + await tester.tap(find.byType(AppFilledButton)); + await tester.pump(); + + expect(cancelled, 1); + expect(tester.takeException(), isNull); + }, + ); + + testWidgets( + 'cancel on a single-entry stack with no onCancel falls back to context.pop and throws GoError', + (tester) async { + // Guards the regression: without an injected onCancel the default + // `context.pop` is wired, which throws on a stack with nothing to pop — + // exactly why BitboxAddressRecoveryPage must pass a safe onCancel. + await pumpViewOnSingleEntryStack(tester, BitboxNotConnected()); + + await tester.tap(find.byType(AppFilledButton)); + await tester.pump(); + + expect(tester.takeException(), isA()); + }, + ); }); } diff --git a/test/screens/home/home_bloc_test.dart b/test/screens/home/home_bloc_test.dart index 0ace26b29..70a3ace67 100644 --- a/test/screens/home/home_bloc_test.dart +++ b/test/screens/home/home_bloc_test.dart @@ -55,6 +55,9 @@ void main() { // and the AppStore-driven side effects (`primaryAddress`, `sessionCache`, // `wallet =`) all resolve without throwing. when(() => walletService.hasWallet()).thenReturn(false); + // Default: a healthy wallet does not need address recovery, so + // LoadCurrentWalletEvent falls through to the normal load path. + when(() => walletService.currentWalletNeedsAddressRecovery()).thenAnswer((_) async => false); when(() => settingsService.isSoftwareTermsAccepted).thenReturn(false); when(() => settingsService.isTermsAccepted).thenReturn(false); when(() => settingsService.setTermsAccepted(any())).thenReturn(null); @@ -186,6 +189,33 @@ void main() { verifyNever(() => transactionHistoryService.apiBasedSync()); }); + test( + 'BitBox address recovery needed → emits bitboxAddressRecoveryNeeded, does NOT load wallet', + () async { + when(() => walletService.hasWallet()).thenReturn(true); + when( + () => walletService.currentWalletNeedsAddressRecovery(), + ).thenAnswer((_) async => true); + + final bloc = build(); + await bloc.stream.firstWhere((s) => s.hasWallet); + + bloc.add(const LoadCurrentWalletEvent()); + await bloc.stream.firstWhere( + (s) => s.bitboxAddressRecoveryNeeded && !s.isLoadingWallet, + ); + + expect(bloc.state.bitboxAddressRecoveryNeeded, isTrue); + expect(bloc.state.isLoadingWallet, isFalse); + expect(bloc.state.openWallet, isNull); + // The corrupt wallet must never be loaded into the AppStore — that is + // the exact path that crashes the dashboard build. + verifyNever(() => walletService.getCurrentWallet()); + verifyNever(() => balanceService.updateBalance(any())); + verifyNever(() => transactionHistoryService.apiBasedSync()); + }, + ); + test( 'getCurrentWallet throws → isLoadingWallet flips back to false, no sync side effects', () async { @@ -231,6 +261,28 @@ void main() { verify(() => balanceService.startSync(_primary)).called(1); verify(() => transactionHistoryService.apiBasedSync()).called(1); }); + + test('clears bitboxAddressRecoveryNeeded once a wallet loads cleanly', () async { + // Drive the bloc into the recovery state first, then prove that loading + // the healed wallet via LoadWalletEvent flips the flag back off so + // `_navigate` routes to the dashboard instead of back to recovery. + final wallet = DebugWallet(1, 'Healed', _debugAddress); + when(() => appStore.wallet).thenReturn(wallet); + when(() => walletService.hasWallet()).thenReturn(true); + when( + () => walletService.currentWalletNeedsAddressRecovery(), + ).thenAnswer((_) async => true); + + final bloc = build(); + await bloc.stream.firstWhere((s) => s.hasWallet); + bloc.add(const LoadCurrentWalletEvent()); + await bloc.stream.firstWhere((s) => s.bitboxAddressRecoveryNeeded); + + bloc.add(LoadWalletEvent(wallet)); + await bloc.stream.firstWhere((s) => s.openWallet == wallet); + + expect(bloc.state.bitboxAddressRecoveryNeeded, isFalse); + }); }); group('SyncWalletServicesEvent', () {