feat(recoverbull): isolate key-server Tor sessions

[ethicnology]

Stacked on the feat/bull-tor-package PR. Review that one first.

RecoverBull now opens its own isolated Tor session through bull_tor instead of borrowing a shared global proxy, so key-server traffic is not correlatable with the app's other Tor traffic.

Startup gains InitializeRequiredTorUsecase behind an AppStartupWalletPort, which is what lets the Tor bring-up move out of WalletBloc. Tor was being started from the wallet layer for a reason unrelated to wallets; it now belongs to app startup, and wallet_bloc loses that responsibility entirely.

ConnectToKeyServerUsecase owns the retry budget and backoff for reaching the key server. That logic used to sit in the bloc, which owns presentation only — as a use case it is testable without pumping a bloc, and the attempt number is still published so the screen can name the attempt in flight.

Two behaviours in the bloc are worth review attention:

• Readiness is latched. Arti's directory fraction is not monotonic once traffic is usable: a background directory refresh reports TorConnecting again while the established SOCKS route stays valid. RecoverBull only needs that route, so a benign refresh no longer undoes readiness. A refresh that carries a diagnostic is not treated as benign and still propagates, so a real loss of connectivity is not masked.
• A closed bloc no longer emits. Tor readiness can arrive after the screen is gone, which produced Bad state: Cannot add new events after calling close on device. Guarded.

[claude]

Claude finished @ethicnology's task in 10m 16s —— View job

---

Code review — feat(recoverbull): isolate key-server Tor sessions

• Read repo guidelines and gather PR context
• Review diff against feat/bull-tor-package for correctness bugs, security issues, regressions
• Post findings (inline comments not possible — see note at bottom)
• Post summary

Overall this is a solid refactor: the session-per-operation isolation with client.close(force: true) in finally, the droppable transformers, the latched readiness with the diagnostic escape hatch, cancelling the Tor subscription before super.close(), and the regression-pinning tests are all careful work. I verified the two behaviours called out in the PR body (latching, closed-bloc guard) against the bull_tor repository implementation and they hold. Findings below, most severe first.

---

Findings

1. session.close() in finally can throw out of a Result-returning use case, and the bloc has no catch for it

bullbitcoin-mobile/lib/core/recoverbull/domain/usecases/fetch_vault_key_from_server_usecase.dart

Lines 39 to 42 in 28bcafe

	);
	} finally {
	await session.close();
	}

TorSession.close() forwards to the onion session's stop (OnionTorBackend.openSession passes session.stop as the close callback), a native call that can throw — the backend's own stop() wraps service.stop() in try/catch precisely because it does, e.g. when the parent client is torn down by a concurrent retry(). Thrown from this finally, the exception escapes execute(), breaking the Result contract. The blast radius:

• _onFetchVaultKey (lib/features/recoverbull/presentation/bloc.dart:390-413) and _onVaultCreation (bloc.dart:322-388) wrap these calls in try/finally with no catch, so this becomes an unhandled bloc error: the spinner ends and the user sees no failure at all.
• If the fetch itself succeeded, the already-retrieved vault key is discarded by the throwing finally.
• Same pattern in store_vault_key_into_server_usecase.dart:44-45 — and there it fires after the vault file was already saved to Drive/disk, leaving the flow half-completed with no typed failure.
• Milder variant in check_server_connection_usecase.dart:28-30: the outer catch absorbs it, but a throwing close flips a successful check to false.

Suggest wrapping the close in its own try/catch that only logs. Fix this →

2. Stale-generation arm can leave the connecting screen stuck on "Waiting" with no error and no retry button

bullbitcoin-mobile/lib/features/recoverbull/presentation/bloc.dart

Lines 155 to 158 in 28bcafe

	case tor.TorUninitialized() || tor.TorStopped() || tor.TorConnecting():
	// A newer retry generation replaced this operation. Its stream owns the
	// current state; the stale handler must not publish a failure.
	return;

_onTorInitialization first emits keyServerStatus: unknown and clears the failure, then awaits ensureReady()/retry(). If a newer generation supersedes the operation (transport-mode change, a concurrent retry from another surface), the repository returns a non-terminal state and this arm returns silently. The watch subscription will eventually update torConnection to TorReady, but nothing ever dispatches OnServerCheck again, so keyServerStatus stays unknown forever. On ConnectingPage that renders as "Waiting" with hasError == false — no error, no retry button, no way forward. Consider dispatching OnServerCheck from _onTorConnectionChanged when a TorReady arrives while keyServerStatus == KeyServerStatus.unknown.

3. Status screen can now cold-start a full Tor bootstrap and block every service row on it

bullbitcoin-mobile/lib/core/status/domain/usecases/check_all_service_status_usecase.dart

Lines 263 to 268 in 28bcafe

	if (!await _walletRepository.isTorRequired()) return status;
	return status.copyWith(
	status: switch (await _ensureTorReadyUsecase.execute()) {
	TorReady(:final route) when route.source == TorSource.embedded =>
	ServiceStatus.online,

Previously _checkTorConnection read the cached status and _checkRecoverbullConnection only contacted the server when Tor was already online. Now both call ensureReady() (the RecoverBull one via CheckServerConnectionUsecase → EnsureRecoverBullTorSessionUsecase), which starts a bootstrap when the client isn't running. The in-code justification (startup already warms Tor for exactly these wallets — and I confirmed isTorRequired() and hasMainnetBitcoinEncryptedBackup() are the same predicate) holds for the happy path. But when the warm-up failed (app launched offline, Tor blocked), execute()'s single Future.wait (lines 44-54) means every status row waits on a fresh full bootstrap — potentially including the snowflake fallback, i.e. minutes — and each subsequent status refresh re-bootstraps, since a failed attempt leaves no in-flight future to join. Consider a timeout around these two checks, or reporting EmbeddedTor.current when the state isn't already TorReady.

4. Behavior change to confirm: RecoverBull no longer honors the external Tor proxy setting

bullbitcoin-mobile/lib/core/recoverbull/data/repository/recoverbull_repository.dart

Lines 18 to 26 in 28bcafe

	class RecoverBullRepository {
	final RecoverBullRemoteDatasource remoteDatasource;
	final RecoverbullSettingsDatasource recoverbullSettingsDatasource;
	RecoverBullRepository({
	required this.remoteDatasource,
	required this.recoverbullSettingsDatasource,
	});

The repository no longer consults TorConfigPort, so a user who enabled useTorProxy in Tor settings (e.g. Orbot on 9050) now has key-server traffic forced through the embedded Arti client. This looks deliberate per the bull_tor design (Orbot is scoped to Electrum .onion only), and isolation is arguably better for privacy — but it is a user-visible regression for someone who routed RecoverBull through Orbot because Orbot has bridges configured and embedded Arti can't bootstrap on their network. If intentional, the Tor settings screen copy should probably say the proxy only applies to Electrum now.

Minor notes

• keyServerAttempt/keyServerAttempts are added to state (state.dart:32-33) and published by the bloc, but no UI reads them — ConnectingPage doesn't render the attempt counter the PR body describes. Dead state until the screen consumes it.
• bloc.dart:203 logs 'ready after ${state.keyServerAttempt} attempts' — if the bloc closed mid-run the isClosed guard skips the emit, so this can log a stale/0 count. Cosmetic.
• The legacy lib/core/tor stack (TorDatasource, InitTorUsecase, TorStatusUsecase, IsTorRequiredUsecase, TorConfigPort/TorConfigAdapter) now has no consumers left outside tor_settings' TorConnectivityService, yet TorLocator.registerDatasources still eagerly awaits TorDatasource.init() at startup. Worth a follow-up cleanup if the Electrum migration lands in a later stacked PR.
• FEATURES.md:126-129 adds a duplicate classDef packageStyle line in the mermaid diagram, silently overriding the one three lines above. Looks like a leftover.

---

Note: I couldn't post these as inline review comments — gh api calls aren't in this workflow's allowed tools. If you'd like inline comments next time, allow Bash(gh api:*) (or the inline-comment MCP tool) in the action's --allowedTools.