Skip to content

feat(payjoin): upgrade to official payjoin pub.dev bindings, drop isolate architecture - #2443

Merged
i5hi merged 29 commits into
developfrom
payjoin-upgrade
Jul 21, 2026
Merged

feat(payjoin): upgrade to official payjoin pub.dev bindings, drop isolate architecture#2443
i5hi merged 29 commits into
developfrom
payjoin-upgrade

Conversation

@ethicnology

Copy link
Copy Markdown
Member

Migrates the payjoin integration from our custom payjoin_flutter fork (SatoshiPortal/payjoin-dart, git dependency) to the official payjoin package on pub.dev (payjoin.org, uniffi-dart bindings, ^0.1.1), and rewrites PdkPayjoinDatasource around its new, synchronous typestate API.

This upgrade was worked on over several months (PR #2041, opened 2026-04-22). Thanks to everyone who helped get it across the line:

  • spacebear — authored the upgrade and the isolate-removal rewrite
  • Dan Gould — upstream payjoin/PDK guidance and review
  • Ben Allen — review

What changed

  • Dependency: payjoin_flutter (git ref) → payjoin: ^0.1.1 (hosted, verified publisher). Removes the platform plugin wiring (ios/Podfile.lock, linux/flutter/generated_plugins.cmake) that came with the old Flutter plugin fork, and the now-unneeded PConfig.initializeApp() call in main.dart.
  • Dropped the isolate-based background architecture. Receiver/sender polling used to run in two long-lived Isolates spawned lazily and communicating over SendPort/ReceivePort. The new PDK exposes a synchronous typestate machine with an in-memory JSON session persister, so polling now runs as plain Timer.periodic in the main isolate (PdkPayjoinDatasource). Net effect: much less code, no isolate bring-up latency, no serialization boundary between isolate and main.
  • BdkWalletDatasource: added createIsMineChecker / createPsbtSigner, which preload a bdk wallet once and return plain synchronous closures, matching the new PDK callback signatures (bool Function(Uint8List), String Function(String) instead of FutureOr<...> Function). Also hardens Electrum client creation against a rustls CryptoProvider install race that could otherwise fail one of two concurrently-syncing wallets.
  • Re-enabled integration_test/payjoin_test.dart, which had been fully commented out on develop. Rewrote it against the new API and added draining of leftover in-flight payjoin state in setUpAll to stop previous (possibly crashed) runs from starving the test wallets.

Known follow-ups (not addressed in this PR)

  • The payjoin typestate walk (processReceiveSession and its _check*/_commit*/_finalize* chain) has no unit test coverage — only the integration test, which isn't run in CI and needs funded testnet wallets. Worth a follow-up with fakes around the repository/datasource seam.
  • integration_test/payjoin_test.dart still has three empty stub tests (resume-after-restart, insufficient-funds paths) and a commented-out multi-payjoin test (TODO: Fix this test) — pre-existing gaps, now visible again now that the file is no longer fully disabled.

spacebear21 and others added 17 commits May 11, 2026 15:10
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.
@ethicnology ethicnology self-assigned this Jul 13, 2026
@claude

This comment was marked as resolved.

@ethicnology

Copy link
Copy Markdown
Member Author

Following the review feedback, this branch now includes a hardening pass on top of the original upgrade. All fixes were verified end-to-end with real payjoin roundtrips on testnet

Review fixes

  • 5b79418 fix(payjoin): guard polls against overlap, stranding and missed expiry — addresses the 🔴 race condition: Timer.periodic doesn't await its async callback, so a slow OHTTP relay could make ticks overlap, double-process the same session, and end up broadcasting the original transaction — silently cancelling the payjoin. Added per-session in-flight guards, moved timer.cancel() after all fallible work (a throw now leaves the timer armed so the next tick retries instead of stranding the session), re-check timer.isActive before emitting, and added a local expiry backstop (createdAt + expireAfterSec) so polling is bounded even if the PDK never surfaces an "expired" error. Also bounds each poll with Dio connect/receive timeouts.
  • 135dd68 fix(wallet): restrict payjoin psbt signer to standard sighashescreatePsbtSigner now uses the same sign options as signPsbt (allowAllSighashes: false) since it signs the receiver's contribution to an externally-supplied transaction, and logs when the signed PSBT isn't finalized instead of silently returning it.
  • 4d7aad4 fix(wallet): extend CryptoProvider race guard to dry scans — the rustls CryptoProvider install-race retry now covers _performDryScan too, via a shared _createElectrumClient helper.
  • fe8b712 refactor(payjoin): surface underlying errors in datasource logs — relay fetch failures, the PDK's input-selection rejection reason, and corrupt persisted event logs are no longer swallowed silently. This is what made the root cause below diagnosable.

Tests

  • db48dea test(payjoin): unit-test session persisters and relay fallback — first unit tests for the payjoin module (15 tests, fully offline): JSON round-trip/corruption handling for both session persisters, and the multi-relay fallback loop via an injected OhttpKeysFetcher seam.
  • 1215ed5 test(payjoin): keep integration wallets payjoin-capable across runs — the integration test used to degrade the shared testnet wallets with every run (accumulating dust and mismatched UTXOs) until payjoins permanently failed. Root cause is an upstream rust-payjoin bug: WantsInputs::try_preserving_privacy's documented fallback ("a simple consolidation is otherwise chosen if available") never runs, because avoid_uih drains the shared Peekable candidate iterator before returning its error, so select_first_candidate always sees an empty iterator and fails with "No candidates available for selection" (payjoin crate 0.23.0, src/receive/v1/mod.rs — reproduced in isolation). The test now sweeps each wallet to a single UTXO before the roundtrip, giving the selection heuristic one unambiguous candidate and making the test self-sustaining: verified with consecutive successful payjoin roundtrips on a physical device. Mnemonics can now also be passed via --dart-define for on-device runs (Platform.environment doesn't reach the app process on a physical device); CI's env-var path is unchanged.

@ethicnology

This comment was marked as outdated.

@claude

This comment was marked as resolved.

@spacebear21

Copy link
Copy Markdown
Contributor

https://pub.dev/packages/payjoin/versions/0.1.2 is now published! Importantly, it uses the latest version of uniffi-dart, which uses uniffi 0.31.2 ensuring reproducible builds.

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.
@ethicnology

Copy link
Copy Markdown
Member Author
  • dc2cca5 build(deps): bump payjoin to 0.1.2 — the 0.1.2 bindings use the latest uniffi-dart (uniffi 0.31.2), which restores reproducible native builds.
  • 3ea1abb fix(payjoin,wallet): address review findings_buildInputPair now throws on a missing input value instead of defaulting to zero (which would sign over the wrong segwit-committed amount and produce an invalid signature); _decodeEvents builds the list eagerly so a list with non-string entries is caught as a corrupt log instead of throwing later on every poll tick; _resumePayjoins emits the updated (expired) model rather than the stale one; dropped the misleading "not finalized" log on the receiver's partial signing, the full-PSBT info log, and the unused OhttpRelaysUnavailableException; surfaced terminal all-relays-failed errors to production logs; removed a stray required field from a freezed factory.
  • 49c7c0f test(payjoin): cover corrupt-log and relay-timeout edge cases — adds a list-of-non-strings decode case and covers the postBytes relay choke point (a Dio receive-timeout propagates unwrapped so the relay loops can fall back; success returns the response bytes).
  • d42d1c2 test(payjoin): correct multi-payjoin integration test timeout unit — a pre-existing Duration(minutes:)/seconds: typo (30 minutes instead of 30 seconds).

@ethicnology

This comment was marked as outdated.

@claude

This comment was marked as resolved.

_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.
@ethicnology
ethicnology requested a review from i5hi July 14, 2026 11:22

@ethicnology ethicnology left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@i5hi payjoin-upgrade is good to merge. Several non-blocking hardening items can be addressed in a follow-up branch, kept out of here to keep the scope tight:

  • Shorter session lifetime (anti-probing). Expiry drops 24h → 5min: a payjoin is a synchronous exchange, so a long window only lengthens the period a probing sender can hold an exposed UTXO and cheaply replace the original tx (BIP78).
  • Exposed-UTXO reuse (anti-probing). Once a proposal reveals a receiver UTXO, a later attempt should re-contribute that same already-burned coin rather than expose a fresh one. Follow-up labels contributed UTXOs (_labelExposedUtxos) and prefers them in selection (filterAvailableUtxos), closing a probing vector.
  • Don't race our own in-flight payjoin on expiry. _processExpiredPayjoin broadcasts the original only when proposalPsbt == null; once a proposal has gone out the sender owns broadcasting the payjoin (same inputs), so broadcasting the original would just race it.
  • Corrupt persisted log → replay loop. _decodeEvents resets silently to [], resurrecting the session empty and re-polling until expiry.
  • CorruptPayjoinSessionException that retires the session.
  • Datasource lifecycle leak. No dispose(), so a never-resolved session can leave Timer.periodic + StreamControllers alive. Follow-up adds an idempotent dispose(), delegated from the repository.
  • // Todo: listen for the broadcast of the transaction (_resumePayjoins). Follow-up implements watch-for-broadcast so the receiver completes/labels the session once the tx lands on-chain.
  • Transient relay blip. A momentary all-relays failure can irrevocably cancel a payjoin; follow-up wraps proposal posting in a bounded retry.
  • Signer robustness. createPsbtSigner throws FailedToSignPsbtException when the wallet matches no proposal input (fails at the boundary, not downstream); CryptoProvider install retry is bounded with backoff.
  • User disclosure. Receive screen now warns that payjoin reveals one of your UTXOs to the sender.

@DanGould

Copy link
Copy Markdown
Contributor

heads up: one database breaking change in payjoin/rust-payjoin#1747 coming before the change. This freezes the persisted data so it shouldn't be a problem when we introduce planned updates. This PR doesn't need to change with code, just gotta update the dep.

Your "Follow-up implements watch-for-broadcast so the receiver completes/labels the session once the tx lands on-chain." suggestion is something we're actively trying to accomodate in our own library with 1.1 here. I don't recommend building it on the current check_for_transaction API. The new API gives you:

classify(chain_view) function => Pending | Detected { outcome: Cooperative|Fallback|Other, confirmations }that maps directly onto the sending / confirming / complete / sent-as-regular-transfer / needs-review states, andcheck_for_transaction` gets deprecated at the same time. SatoshiPortal/payjoin can use the same exact approach for receiver classification when this lands

@i5hi
i5hi merged commit 53ed590 into develop Jul 21, 2026
3 checks passed
@github-project-automation github-project-automation Bot moved this from PR Review to User Testing in Wallet Releases Jul 21, 2026
This was linked to issues Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: User Testing

Development

Successfully merging this pull request may close these issues.

payjoin improments Upgrade to Payjoin 1.0.0

4 participants