feat(payjoin): upgrade, settings, anti-probing hardening, and status/reliability fixes - #2461
feat(payjoin): upgrade, settings, anti-probing hardening, and status/reliability fixes#2461ethicnology wants to merge 86 commits into
Conversation
Switch from payjoin_flutter to dart payjoin bindings, which are actively maintained and support the latest rust-payjoin versions.
These pre-load the wallet and return a synchronous callback compatible with the synchronous payjoin interface, for isMine and signPsbtSync.
These session persisters hold payjoin events in memory as a transitive step, so that DB migrations and complete event persistence may be implemented in a follow-up step.
Implements a chaining pattern with processReceiveSession to process and advance a session from any state to its terminal state. BBM needs the proposal PSBT to save to its model, so it needs to be extracted before transitioning to the Monitor typestate to be returned alongside the session.
This should be droppable once isolates architecture is replaced
Move receiver/sender polling onto the main isolate, keyed by session idin two Timer.periodic maps. The old isolate indirection existed because frb async FFI could block the UI isolate. With sync uniffi, `Timer.periodic` on the main isolate works and removes ~150 lines of accidental complexity. Co-Authored-By: Dan Gould <d@ngould.dev> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves conflicts against the current payjoin-upgrade (= develop) base: - pubspec.yaml/.lock: drop payjoin_flutter (SatoshiPortal fork), adopt the published payjoin 0.1.1 package, the whole point of this upgrade. - lib/main.dart: drop PConfig.initializeApp() (payjoin_flutter's FRB init, no longer needed); keep BullSdk.init()/BitBoxApi.initialize() from develop's bull_sdk consolidation, dropping the stale LibLwk/BoltzCore/LibBbqr/LibArk/BitBoxFlutterApi calls the PR branch still had from before that consolidation. - lib/core/wallet/data/datasources/bdk_wallet_datasource.dart: combine both independent fixes — the PR's rustls CryptoProvider install-race retry when building the ElectrumClient, and develop's try/catch logging around fullScan. - ios/Podfile.lock, linux/flutter/generated_plugins.cmake: drop the payjoin_flutter plugin entries; the new payjoin package needs none (native code via Rust native assets, not a CocoaPods/plugin registration). - integration_test/payjoin_test.dart: take the PR's active test body (develop's was fully commented out in d721ce9) and update it to the current API surface: PrepareBitcoinSendUsecase moved to core/wallet/domain/usecases and lost ignoreUnspendableInputs (unspendable filtering is now automatic); NetworkFee.relative no longer exists, use NetworkFee.relativeFromSatPerVbyte; mnemonics are read from Platform.environment at runtime (matching the CI step), not String.fromEnvironment/--dart-define. fvm flutter analyze, dart fix --dry-run, dart format --set-exit-if-changed and fvm flutter test test/ (523 tests) are all green after this merge.
The 0.1.2 bindings use the latest uniffi-dart (uniffi 0.31.2), which restores reproducible native builds.
- _buildInputPair: throw on a missing input value instead of silently defaulting to zero, which would sign over the wrong segwit-committed amount and produce an invalid signature surfacing far away. - _decodeEvents: eagerly build the event list inside the try/catch so a persisted list with non-string entries is caught as a corrupt log instead of slipping through .cast()'s lazy view and throwing later on every poll tick. - _resumePayjoins: emit the updated (expired) model on the stream, not the stale one, so listeners see the expired status. - createPsbtSigner: drop the 'not finalized' log — the receiver only signs its own contribution to a multi-party proposal, so a non-finalized PSBT is expected here, not an error. - Surface terminal all-relays-failed errors via logger.log.warning so they reach production logs, not just dart:developer. - Remove the unused OhttpRelaysUnavailableException and drop the stray 'required' field from PayjoinInputPairModel's freezed factory. - Stop logging the full proposal PSBT at info level.
Adds a list-of-non-strings case to the session persister decode tests, and covers the postBytes relay choke point: a Dio receive-timeout propagates unwrapped (so the relay loops can catch it and fall back to the next relay) and a success returns the response bytes.
The multi-payjoin group's Timeout used Duration(minutes: ...) where the interval math is expressed in seconds — 30 minutes instead of the intended 30 seconds. Pre-existing typo, fixed while here.
_resumePayjoins handled a session that expired while the app was closed by only updating the DB and emitting — it never broadcast the receiver's stored original transaction, unlike the live-expiry path (_processExpiredPayjoin). A receiver that had the sender's original tx but was killed before a proposal completed would, on next launch, silently drop it: neither the payjoin nor the fallback ever hit the chain, stranding the sender's payment. Delegate to _processExpiredPayjoin so restart-time expiry runs the same original-transaction fallback.
The Dio for OHTTP relay polling set connect and receive timeouts but left the request-body upload phase unbounded. The per-session in-flight guard turns any unbounded await into a permanent stall: if a relay stalls mid-send, postBytes never completes, the poll's finally never runs, the session id is never cleared from the in-flight set, and every later tick is skipped — polling for that session silently stops until app restart. Add sendTimeout (10s; OHTTP bodies are small) so all three phases are bounded, restoring the locator's 'a slow relay can't hold a session in flight' guarantee.
Tag a payjoin transaction with the payjoin system label once it completes, so it is recognisable as a payjoin in the transaction list. Labelling is wired at the two completion points in the repository: - _broadcastPsbt (sender success): labels the finalized payjoin tx (txId). - tryBroadcastOriginalTransaction (fallback): the negotiation didn't complete and the original tx landed on-chain, so it labels originalTxId (txId is the payjoin proposal tx and is typically null on this path). Best-effort and idempotent: a labelling failure is logged and swallowed so it never fails the already-broadcast payjoin, and the labels store dedupes on (label, reference). The labels facade is injected as a lazy closure because the payjoin repository is an eager singleton built before the facade is registered.
…button Matches the enable/disable toggle's immediate-persistence pattern: each field now saves on blur or keyboard submit, validated and persisted independently (they are two unrelated settings values, so an invalid edit in one must never block the other). The Save button and the Form wrapper it needed are gone.
Superseded by the global payjoin setting, which already hides this switch entirely when disabled — the per-address opt-out added no value on top of it. The UTXO-reveal disclosure it showed conditionally is already covered unconditionally by the settings screen's explanation. Removes the now-unreachable plumbing this left behind: ReceiveEvent.receiveAddressOnlyToggled, its handler, and ReceiveState.isAddressOnly, folding canPayjoin/isPayjoinLoading down to drop the dead condition.
…made it, and distinguish it from a real payjoin in status Both sides hold their own copy of the original transaction and can each independently decide to broadcast it (a receiver declining below the anti-probing minimum, either session's own expiry with no proposal exchanged, or a sender's negotiation failing). Whichever side actually broadcasts it persists completion itself, but the OTHER side previously had no way to find out — it just kept waiting on its own session with no signal that the payment had already landed via the counterparty's fallback. Observed live: a receiver declining below-minimum broadcasts immediately, while the sender's session sat on its prior status for up to a full expiry window with nothing to show for it — and if that second, now-redundant broadcast attempt then errored (already known to the network), the session never completed at all, since the failure was silently swallowed with nothing left watching for the transaction to land through any other path. Adds _watchForFallback, a passive+active watch for the ORIGINAL transaction (mirroring the existing _watchForBroadcast for the real payjoin one), armed as soon as originalTxId is known — session creation for a sender, request-received for a receiver — and resumed across app restarts. Whichever of the two watches fires first resolves the session. Also stops calling _stopWatching before attempting a fallback broadcast in _processExpiredPayjoin: it must survive a failed attempt so the watch stays alive to catch the transaction landing through any other path. Introduces PayjoinStatus.fallback as its own status, distinct from completed: both mean the session is done (isCompleted covers both), but only completed means a real payjoin happened. Threads through every consumer that read the old completed+no-txid combination or the isRealPayjoinCompletion heuristic: transaction details (status label and table row), the CSV export, and the receive in-progress screen.
The status names the outcome, not the mechanism: the payjoin was aborted and the payment fell back to a plain broadcast of the original transaction. User-facing wording follows (Payjoin aborted / Aborted, fr: Payjoin abandonné / Abandonné), the l10n keys are renamed to match, and the CSV export status value becomes 'aborted'.
The session row's persisted status can lag reality: completion detection runs on background polls, so right after a payment lands the row may still say requested/proposed while the broadcast transaction is already visible in the wallet. Transaction.displayPayjoinStatus makes the on-chain txid authoritative — the payjoin txid means completed, the original txid means aborted — and both the details heading and the payjoin status row now render it, localized for every case instead of leaking the raw enum name.
fromReceiverTable/fromSenderTable never mapped isCompleted/isExpired back from the row, so every by-id fetch silently fell back to their @default(false) and returned a never-completed session no matter what was persisted. Every terminal status derives from these flags, so this single gap surfaced everywhere: stale requested/proposed statuses on transaction details after a completed or aborted payjoin, expiry handlers re-broadcasting the original for already-resolved sessions, and completion handlers re-resolving sessions that had already resolved (all observed live).
A session resolving through the fallback left its directory poll running until expiry, which then re-broadcast the original transaction off a stale in-memory copy (observed live: a redundant second broadcast a minute after resolution). _stopWatching now also cancels the PDK poll, and _processExpiredPayjoin re-fetches the persisted row and bails when it already resolved, deciding and persisting on the fresh row instead of the event's copy. In the other direction, our own broadcasts now stay watched until visible: the single post-broadcast sync can be throttled by the sync coordinator or race the broadcast (observed live: a receiver's below-minimum fallback stayed invisible until manual resyncs), so tryBroadcastOriginalTransaction re-arms the original-tx watch and _broadcastPsbt watches the payjoin txid, both polls forcing direct electrum-backed lookups and tearing themselves down once the transaction lands. Arming the broadcast watch is now best-effort like its fallback sibling, so a failure to arm can never misreport an already-successful broadcast.
Opening details by payjoin id showed payjoin-session-only data until the next organic sync armed watchers fired — a stale placeholder even though the payment was already on-chain. The cubit now resolves the broadcast transaction immediately (payjoin txid, else the original on a fallback), and when a resolved session's broadcast isn't visible locally yet it awaits one forced lookup through the new GetWalletTransactionUsecase — the repository's per-transaction sync path bypasses the sync coordinator, so it can't be throttled away — landing the user straight on the transaction view. A still-invisible transaction falls back to the session view plus a targeted sync, and the armed watchers upgrade the screen when it arrives.
The receive side of a real payjoin gets less than the negotiated payment: per BIP78 the mining fee for the input the receiver contributed is deducted from its output (observed live: 1002 sats sent, 948 displayed), which looked like a wrong amount. Transaction.payjoinFeeContributionSat derives that gap and the details table shows it with an expandable explanation, only when it applies — completed real payjoin, receive side, positive gap.
… inputs Offering the full candidate list to tryPreservingPrivacy in one shot let its UIH2-avoidance heuristic pick a fresh coin over one already exposed by a prior proposal, so the reuse preference only acted through its narrow sort fallback. Try the exposed subset first and fall back to the full list, under a dedicated payjoin_exposed system label (distinct from the completed-payjoin tag) that is applied on proposal send, kept on failed negotiations, and removed once a real payjoin completes.
|
Claude finished @ethicnology's task in 11m 5s —— View job Reviewing this PR for correctness bugs, security issues, and regressions… Todo
Confirmed findings so far include a significant one: |
|
Hey @claude I expect your review/audit for this huge PR |
|
Claude finished @ethicnology's task in 3m 41s —— View job Review: payjoin upgrade, settings, anti-probing hardening & reliability fixesFull audit done — 6 parallel deep-review passes over the diff vs Overall: this is a genuinely strong PR. The stale-status root cause ( Major — security / protocol1. The min-amount anti-probing gate is bypassable at zero cost. The receiver skips BIP78's can-broadcast check ( 2. Receiver's max fee rate is hardcoded to 10,000 sat/vb ( 3. User-frozen coins can be contributed to a payjoin — and the new exposed-first preference can make them the first choice. Receiver input candidates exclude only payjoin-locked coins, never user-frozen ones ( 4. Major — resume after restart (
|
2 participants
Closes:
Non-exhaustive summary:
payjoin_flutterfork onto the officialpayjoinpackage (rust-payjoin via uniffi), currently pinned to 1.0.0-rc.4 bindings.PayjoinStatus.abortednames the outcome, not the mechanism: the payjoin was aborted and the payment landed via a plain broadcast of the original transaction instead.isCompleted/isExpiredback from the database row, so every fetch silently returned a "never completed" session no matter what had actually been persisted — surfacing as stuck "requested"/"proposed" statuses long after a payjoin had actually completed or aborted.