feat(payjoin): harden core engine (watchers, resume, guards)

[ethicnology]

Hardens the core payjoin engine: watchers, session-resume logic, and guards against invalid states. Models the fallback-to-normal-tx path explicitly via PayjoinStatus.aborted + a DB isAborted flag, instead of deriving it from isCompleted && txId == null. Extends the Payjoin entity with logRef, canManuallyBroadcastOriginal, and isCompleted/isAborted/isExpired getters.

Builds on #2443's typestate API. Addresses part of #2416 (payjoin improvements).

Merge order: 3rd, after PR1. Base of PR3, PR4, PR5 — must merge before all three.
PR1 → PR2 (this) → {PR3, PR4, PR5}

[ethicnology]

@claude audit this PR please

[claude]

Claude finished @ethicnology's task in 8m 37s —— View job

---

Audit of PR #2478 — payjoin core engine hardening

• Gather context (diff vs pj/00-settings-db, entity/model in base)
• Review core engine: repository impl (watchers, resume, fallback)
• Review datasources & entity/model changes
• Review usecases, locators, bloc changes
• Review tests
• Post audit findings

Overall: solid work. The engine design is coherent — explicit aborted modelling, dual passive+active watchers with bounded backoff, terminal-state guards on the expiry/proposal handlers, per-session resume error isolation, logRef privacy discipline, and a real dispose lifecycle. The ~3.2k lines of tests (mocktail + fake_async) cover the tricky paths (races, idempotency, watcher arming). The findings below are mostly race windows and asymmetries at the edges of that design, ranked by severity.

---

Findings

1. SendCubit bypasses the new settings-based session lifetime (Medium)

SendWithPayjoinUsecase now resolves settings.payjoinExpireAfterSec, with a comment saying callers only override it in tests — but the one production caller does exactly that: lib/features/send/presentation/bloc/send_cubit.dart:1884 passes expireAfterSec: PayjoinConstants.defaultExpireAfterSec explicitly, so the user-configured lifetime never applies to sends (the receive path is correct). If this is fixed in a later stacked PR, fine — but as merged, the send half of the settings feature is dead code. Relatedly, payjoin_repository_impl.dart:709 references SendCubit._watchPayjoin, which doesn't exist on this branch yet. Fix this →

2. Terminal-state rows can still be clobbered after the entry guards (Medium, race)

The new re-fetch guards check terminal flags at handler entry, but insertOnConflictUpdate replaces the whole row at write time, and the fallback watcher can mark the session aborted in between:

• _processPayjoinProposal failure path (payjoin_repository_impl.dart:716): persists payjoinModel.copyWith(isExpired: true, txId: null) from the stale event copy. The guard at line 642 ran before the multi-second sign/broadcast round-trip. The dangerous case is correlated, not random: the original tx landing on-chain (fallback watcher → isAborted) is exactly what makes the payjoin broadcast fail on already-spent inputs, and the subsequent original re-broadcast fail as already-known — landing in this branch, which then overwrites the watcher's correct aborted with expired. Re-fetch and bail on terminal (mirroring the entry guard) before persisting here.
• _proposePayjoin (payjoin_repository_impl.dart:978-993): fetches freshModel inside the lock but never checks its terminal flags. A manual "receive payment normally" tap (allowed by canManuallyBroadcastOriginal while proposalPsbt == null) during the PDK proposePayjoin round-trip gets its aborted row resurrected to proposed at line 993.

A shared updateIfNotTerminal-style write helper (or terminal re-checks at each write point) would close this class of bug once instead of per-call-site. Fix this →

3. Expired-at-resume receiver with a proposal sent loses its fallback watch (Low-Medium)

_processExpiredPayjoin's else-branch (payjoin_repository_impl.dart:822) re-arms only _watchForBroadcast for a resumed, expired receiver whose proposal went out — but _resumeOne's live equivalent (lines 905-916) arms both watchers. A sender that fell back post-proposal (its own _processPayjoinProposal catch broadcasts the original) resolves via the original tx; this receiver watches only the payjoin txid, never sees it, and stays "expired" forever despite having been paid. Arming _watchForFallback when originalTxId != null in that branch restores the symmetry.

4. Startup sweep retries forever once the original tx is already on-chain (Low)

resumePayjoinsOnStartup (payjoin_repository_impl.dart:850) re-broadcasts the original for stranded expired receivers on every app start. If the tx already landed via the other side, broadcast errors ("already in chain"), the session is never marked aborted, and the sweep retries — with a SEVERE log — on every start, indefinitely. Arming _watchForFallback for swept sessions (letting _onOriginalTransactionSeen resolve them once the tx is visible in the wallet) would make the loop converge. Also note the sweep calls fetchReceivers() unfiltered — a full-table scan per start that grows unboundedly with session history; a filtered query would be cheap here.

5. Unhandled rejection risk in AppLocator.setup (Low)

lib/locator.dart:78 — unawaited(resumePayjoinsOnStartup()) discards the future, but the method's top-level fetchReceivers()/fetchAll() awaits are outside its per-session try/catch. A DB failure there becomes an unhandled zone error during startup. Wrap the call (or the method body) in a catch-and-log.

6. Sender's post-completion watch uses the pre-finalization txid (Low)

_broadcastPsbt arms _watchForBroadcast with completedModel.txId (payjoin_repository_impl.dart:1062) — the proposal-derived txid, which the adjacent comment in _processPayjoinProposal itself explains can differ from the actually-broadcast txid for non-native-segwit inputs. When it differs, the watch never fires: the full 12-attempt forced-sync budget (~35 min) burns plus a passive watcher persists for the app session (harmless for state — the session is already completed — but pure overhead). Unlike the receiver side, the sender has the finalized bytes: computing BitcoinTx.fromPsbt(finalizedPsbt).txid inside _broadcastPsbt and persisting it as txId would fix both the watch and the displayed/stored txid.

7. Guard skipped when the fresh model is missing (Info)

tryBroadcastOriginalTransaction (payjoin_repository_impl.dart:325): when the fresh fetch returns null the guard is bypassed and the broadcast proceeds with the caller's stale copy — and _broadcastOriginalTransaction broadcasts to the network before discovering the model is gone. Pre-existing ordering, but now that the guard exists, bailing on a missing row would be more consistent.

---

Notes

• Freezed copyWith(txId: null) genuinely clears the field (sentinel-based), so the abort-path txId hygiene works as intended — verified against the model.
• The widened WatchPayjoinUsecase return type is safe at both existing call sites (ReceiveBloc filters PayjoinReceiver; TransactionDetailsCubit ignores the payload).
• Locator ordering claims check out: PayjoinLocator.registerRepositories (core_locator.dart:60) does run before WalletLocator.registerRepositories (:65) and LabelsLocator.registerFacade (:99), justifying the lazy closures and the deferred resumePayjoinsOnStartup.
• The 35s dio receiveTimeout vs the directory's ~30s long-poll hold is a well-documented real fix.
• I could not run flutter analyze or the test suite — no Dart/Flutter SDK is available in this environment, and no CI runs were found for this PR yet. The findings above are from manual review only.