Skip to content

feat(payjoin): exchange buy/sell/pay integration + issue fixes - #2538

Merged
ethicnology merged 18 commits into
payjoin-reliability-hardeningfrom
payjoin-exchange-ux
Jul 30, 2026
Merged

feat(payjoin): exchange buy/sell/pay integration + issue fixes#2538
ethicnology merged 18 commits into
payjoin-reliability-hardeningfrom
payjoin-exchange-ux

Conversation

@ethicnology

Copy link
Copy Markdown
Member

Bundles the Payjoin Exchange integration (buy/sell/pay toggle, session negotiation, txid resolution, labels) with fixes for issues touching the same buy/sell/pay/transactions surfaces.

Closes #2515, #2516, #2517, #2518, #2521, #2522, #2523, #2524, #2525, #2526, #2527, #2530

@ethicnology ethicnology self-assigned this Jul 29, 2026
@ethicnology

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Claude finished @ethicnology's task in 6m 30s —— View job


Code review — PR #2538 (payjoin exchange buy/sell/pay integration + issue fixes)

  • Gather context (PR diff vs payjoin-reliability-hardening, 111 files, +6822/−778)
  • Review payjoin/exchange core changes (datasource, order model, repositories)
  • Review buy/sell/pay bloc + UI changes
  • Review transactions/labels changes
  • Review tests & localization
  • Post review feedback

Overview

This PR wires payjoin into all three exchange flows (buy = receiver session whose BIP21 URI is handed to the exchange at order creation; sell/pay = sender session against the order's bip21URI), adds txid-based payjoin resolution and labeling, a shared fee modal for sell/pay (#2521), a broadcast latch against double payment (#2522), defensive order parsing (GenericOrder, unknown enum fallbacks, per-order try/catch), and richer success/details screens. The design commentary in doc comments is excellent — the "why" behind the session-window bound, the URI-can't-be-revised constraint, and the txid-gated payjoin claims are all clearly reasoned. Test coverage for the new mapping/outcome/bloc logic is solid.

High — the pay flow is missing the #2522 protections the sell flow received

The sell bloc got a careful broadcast latch (payinBroadcastTxid, _emitSendPaymentError, live-state re-reads). The pay flow now also creates payjoin sender sessions and broadcasts payins, but has none of it:

  1. Confirm re-arms after the payment is already committedlib/features/pay/presentation/pay_bloc.dart:592-601. In _onSendPaymentConfirmed, once _sendWithPayjoinUsecase.execute returns (original proposal posted — the session owns the payment) or the plain broadcast succeeds, any failure in the post-broadcast steps (the 5s-delayed _getOrderUsecase fetch, or the latestOrder is! FiatPaymentOrder throw at line 551) lands in the generic catch, which emits a retryable error with isConfirmingPayment: false onto the pre-await snapshot payPaymentState. The user taps Continue again and a second transaction/session is built, possibly from different UTXOs — the exact double-payment scenario Sell: post-broadcast error path re-enables Confirm — double-payment risk #2522 describes for sell. Pay needs the same latch: set it immediately after _sendWithPayjoinUsecase/broadcast, and swallow post-broadcast errors into a "payment sent, refreshing order" state. Fix this →

  2. Price-lock timeout mid-confirmation re-arms Confirmlib/features/pay/presentation/pay_bloc.dart:394-426. _onOrderRefreshTimePassed builds from state.cleanPaymentState, which resets isConfirmingPayment: false (pay_state.dart:170-177), then emits that snapshot after the refresh await. The countdown on pay_send_payment_screen.dart stays live during confirmation, and confirmation takes ≥5 s by construction, so a deadline expiring mid-confirm is a realistic race that visually re-arms Confirm over an in-flight payment. SellBloc._onOrderRefreshTimePassed guards isConfirmingPayment || isPayinBroadcast and re-reads the live state on both success and failure paths — pay should mirror it.

  3. Payjoin toggle handlers clear the confirming flagPayBloc._onPayjoinToggled (pay_bloc.dart:604) and SellBloc._onPayjoinToggled both go through cleanPaymentState/toCleanPaymentState, which resets isConfirmingPayment and error. The UI disables the switch while confirming, but an event queued in the race window would still un-latch the button state. Emitting copyWith(isPayjoinEnabled: ...) on the live state, with an early return when isConfirmingPayment/isPayinBroadcast, is both simpler and safe.

Medium

  1. Sender's minimum feerate degenerates to 1 sat/vB for absolute custom feespay_bloc.dart:529-531 and the identical code in sell_bloc.dart: networkFeesSatPerVb: networkFee.isRelative ? networkFee.value as double : 1. This value becomes minFeeRateSatPerKwu in SenderBuilder.buildRecommended (pdk_payjoin_datasource.dart:225), i.e. the lowest proposal feerate the sender will accept. With an absolute custom fee the receiver may return a proposal priced far below the tier the user committed. Since absoluteFeesUpdated and preparedSend.txSize are both in hand, deriving the actual rate (absoluteFees * 1.0 / txSize) would preserve the user's intent.

  2. Abandoned buy orders leak a 24-hour receiver session. CreateBuyOrderUsecase cancels the receiver only when placeBuyOrder throws (create_buy_order_usecase.dart). If the order is created but the user backs out of the confirm screen, or the confirmation deadline lapses, the session polls the directory for up to 24 h for an order that will never pay out. Keeping it alive after a confirmed order is clearly deliberate (payout can be hours away) — but cancelling on order expiry/user abandonment of an unconfirmed order seems worth a follow-up.

  3. GetPayjoinByTxIdUsecase double-wraps its own exceptionlib/core/payjoin/domain/usecases/get_payjoin_by_tx_id_usecase.dart:14-19. The GetPayjoinByTxIdException('Payjoin not found') thrown inside the try is caught by the same function's catch (e) and re-wrapped, yielding "GetPayjoinByTxIdException: Payjoin not found" nested in another exception. Add on GetPayjoinByTxIdException { rethrow; } or move the empty-check outside the try.

  4. Two near-duplicate pay bloc test suites: test/features/pay/pay_bloc_test.dart (252 lines) and test/features/pay/presentation/pay_bloc_test.dart (530 lines) share almost identical mock scaffolding. If the split isn't intentional, consolidating under presentation/ would avoid them drifting apart.

Low / nits

  1. createBuyOrder's address/bip21URI mutual-exclusion check (bullbitcoin_api_datasource.dart) is an assert, so release builds would silently send 'address': null if a caller ever violated it. An ArgumentError would fail loudly in production too.
  2. OrderModel.fromJson now defaults currencies/methods to '' for admin-initiated orders — statuses correctly map to unknown, but an empty currency string will render as a bare number in amountAndCurrencyToDisplay(). Probably acceptable for reward/funding orders; just noting the UI consequence.
  3. The CancelPayjoinReceiverUsecase doc comment says the buy payjoin toggle lives on "the confirmation screen", but this PR puts it on the input screen (buy_input_screen.dart) — worth syncing the comment.

What's done well

  • The sell Sell: post-broadcast error path re-enables Confirm — double-payment risk #2522 fix is thorough: latch set on the very next line after broadcast, txid derived pre-broadcast, live-state re-reads after every await, inert UI (canEditFees, disabled advanced settings), and the deliberate re-fire of a missed price-lock refresh in _emitSendPaymentError.
  • _throwOrderApiError + _OrderLimit.tryParse replaces four copies of fragile cast-heavy parsing with one defensive implementation that degrades to the generic error — and it's tested.
  • Per-element try/catch in order list parsing and entity mapping means one malformed order no longer wipes the whole history.
  • GenericOrder + unknown enum members make the app forward-compatible with new server-side order types instead of throwing.
  • PayjoinSessionWindow bounding sessions by the order deadline (never the global setting) closes a real funds-against-dead-order hazard, and the reasoning is documented where it matters.
  • LabelExchangeOrdersUsecase dedup by (label, reference) fixes the "labels only written once ever" reconciliation gap.

Verification notes

  • I could not run flutter analyze or the test suite in this environment (no Flutter toolchain installed); findings are from reading the diff and surrounding code.
  • CI: the Android build was still in progress at review time.

@ethicnology
ethicnology force-pushed the payjoin-exchange-ux branch from 97f097c to f31c632 Compare July 30, 2026 15:33
@ethicnology
ethicnology force-pushed the payjoin-reliability-hardening branch from 84dac62 to f0e92e7 Compare July 30, 2026 16:07
FrancisPouliot and others added 18 commits July 30, 2026 12:15
The API legitimately sends order values the app refused to parse:
status 'Expired' (distinct from 'Payment deadline expired'), payout
status 'Failed', order type 'Sell USDT', empty payin/payout statuses,
and null exchange rate and confirmation deadline fields on
admin-initiated orders. Both batch parse layers turned the first such
order into an empty list, hiding every exchange transaction in the app.

- Parse per element with skip-and-log at both layers (datasource
  listOrderSummaries and repository getOrders), and in getOrderByTxId.
- Make OrderStatus, OrderPayinStatus, OrderPayoutStatus and OrderType
  tolerant of unknown values; add the known missing members.
- Render unknown order types generically via a new Order.generic
  variant carrying the server-sent type name.
- Relax exchangeRateAmount, exchangeRateCurrency and
  confirmationDeadline to nullable, matching the server contract, with
  null guards at their consumers.
- Read reward amounts from the payout side; the payin side is empty
  for admin-initiated orders and rendered every reward as 0 sats.
- Derive fiat-vs-sats display from the order instead of a hardcoded
  variant list, fixing fiat refunds shown as sats and BTC balance
  adjustments formatted as fiat.

Closes #2526
Closes #2527
…method

- Show the network on default wallets in the buy dropdown: Instant
  payments (L-BTC on Liquid) / Secure Bitcoin (BTC on Bitcoin chain).
  External and custom wallets keep their own names.
- Split the confirm page's Payout method row into Payout wallet and
  Payout method, sharing the network phrases with the dropdown.
- Success message now includes what was paid: 'You bought {amount}
  with {fiatAmount}' from the order's payin side.
- Below-minimum (and any other) order-creation failures are no longer
  silent: the amount screen renders limit errors with the server's
  amount and currency, and everything else through a neutral fallback
  message. Previously only below-min/above-max variants rendered, and
  most server errors never mapped to them, leaving Continue dead.
- Make the createOrder error parsing total via a shared Never-typed
  helper across buy, sell, pay and withdraw: an error response can no
  longer fall through to the result cast. Parses the server's singular
  reason and new plural reasons shapes (API-Orders#859), tolerating
  empty and limit-less entries.
- Stop converting fiat limit amounts with btcToSats: a 20 CAD minimum
  rendered as 2,000,000,000 sats. Error entities now carry amount and
  currency verbatim; the render site formats fiat as fiat.
- Repair broken translations the changed keys touched (zh/fa/th/tr
  buyYouBought, zh payout-method and external-wallet strings).

Closes #2515
Closes #2516
Closes #2517
Closes #2518
…update

- 'Payment in Progress!' now says what is happening: 'The payment of
  {fiatAmount} to {recipient} will be sent after your transaction
  receives 1 confirmation onchain', with the recipient resolved from
  the order (name, then label, then account identifier). Same
  treatment on the completed screen, and the details table gains a
  Recipient row.
- Once the payin confirms, the copy switches to 'Your transaction is
  confirmed. The payment ... is being processed', driven by the
  existing 5s poll.
- The success state now carries the post-broadcast order instead of
  the stale pre-broadcast snapshot the bloc fetched and discarded.
- Transaction details: pull-to-refresh, and a proper error state with
  Retry instead of skeletons that spin forever when the load fails.
- New localization keys instead of editing the old ones, so stale
  translations fall back to English rather than silently dropping the
  amount and recipient.

Closes #2524
Closes #2525
The Network fee priority row on the sell and pay confirmation screens
was a debugPrint stub and every transaction paid the fastest rate.

- SellBloc and PayBloc implement the shared fee modal's view-state and
  actions ports, following SendCubit's semantics: preset selection,
  custom sat/vB or absolute fees with arm/disarm/finalize, epoch-guarded
  previews, and a relay-floor gate on finalize.
- The row opens the shared FeeOptionsModal and shows the committed
  selection; the summary fee reprices from a real PSBT built at the
  selected rate on every change.
- Confirm prepares, signs and broadcasts at the committed selection.
  Unlike send and swap, the preview PSBT is display-only: sell and pay
  rebuild at broadcast time because a price-lock refresh can move the
  order's payin amount. The built fee is re-asserted against the relay
  floor before signing.
- Fee editing locks while a confirmation is in flight or the payin is
  broadcast; handlers re-read live state so queued events are dropped,
  and every post-await emit merges into live state - a recalculation
  or preview landing after the flow moved on emits nothing instead of
  resurrecting a pre-broadcast state.
- Previews price against the order's real payin address.

Closes #2521
@ethicnology
ethicnology force-pushed the payjoin-exchange-ux branch from f31c632 to 40a9f6b Compare July 30, 2026 16:17
@ethicnology
ethicnology merged commit 7853102 into payjoin-reliability-hardening Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants