Skip to content

fix(transactions): fall back to the payjoin txid when listing orders - #2696

Merged
ethicnology merged 1 commit into
developfrom
fix/buy-order-duplicate-transaction
Aug 20, 2026
Merged

fix(transactions): fall back to the payjoin txid when listing orders#2696
ethicnology merged 1 commit into
developfrom
fix/buy-order-duplicate-transaction

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

No description provided.

@wired-pasteque wired-pasteque self-assigned this Aug 19, 2026
@wired-pasteque
wired-pasteque force-pushed the fix/buy-order-duplicate-transaction branch from 863256b to c71d0c8 Compare August 19, 2026 22:24
@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude review this PR

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 2m 22s —— View job


Code review — PR #2696

  • Gather context (diff vs origin/develop, repo guidelines)
  • Read get_transactions_usecase.dart and the payjoin/order matching logic
  • Review the new test
  • Post review feedback

Verdict: LGTM. The fix is minimal, correctly placed, and well tested. No blocking issues found.

What the change does

When a buy settles through a payjoin, the exchange broadcasts the payjoin transaction before it reports it as the order's payout, so order.transactionId is still null while the wallet already sees the transaction. Previously the order never landed in orderBuckets, so the wallet transaction and the standalone order rendered as two separate rows ("Bitcoin" + "Buy Bitcoin"). The fix adds order.payjoin?.txid as a fallback bucket key (lib/features/transactions/application/usecases/get_transactions_usecase.dart:110), letting the wallet transaction absorb and consume the order.

Why I'm confident it's correct

  • The existing guards still apply on the payjoin-txid path. Bucketing only makes the order a candidate; the match at lines 178–194 still enforces the address check (o.toAddress != wt.toAddress), the direction check (buy → incoming), and _transactionCoversOrderAmount. So a hostile or wrong server-reported payjoin txid can't graft an order onto unrelated funds — same threat model as the transactionId path.
  • The set literal {order.transactionId, order.payjoin?.txid} dedupes the common case where both eventually name the same transaction, so the order is bucketed once — and it mirrors the established idiom used for swapBuckets (line 93) and payjoinBuckets (line 100).
  • It also covers sell and fiat-payment orders, which carry payjoinDetails too (order.dart:757–768). For a sell, the user's outgoing payjoin tx now binds before the settlement txid is reported; direction and amount guards hold there as well. That's a broader fix than the title suggests, and a desirable one.
  • OrderPayjoinDetails.txid stays null until the exchange has seen the payjoin (order.dart:304–308), so the fallback can't fire on a payjoin that fell back to a plain send — in the plainSend outcome the payjoin txid is null and transactionId carries the real settlement tx.
  • Double-attachment is impossible even when both keys are present and differ: consumedOrderIndices guards the order, and the conflicting (replaced) original tx isn't in the buckets anyway.

Minor observations (non-blocking)

  1. Test fixture inconsistency_buyOrder in the new test sets orderStatus: OrderStatus.inProgress but payinStatus/payoutStatus: completed. Harmless for what's asserted, but a real in-flight payjoin buy would presumably have payoutStatus not yet completed. Worth aligning if you touch this test again.
  2. The new test exercises the walletId == null path (where the duplicate standalone row appeared). The fix also improves the per-wallet path — where the order previously just failed to bind at all — but that's covered implicitly by the same bucket logic, so I don't think a second test is required.
  3. The remaining gap (inherent, not introduced here): between the wallet seeing the payjoin tx and the exchange reporting payjoin.txid, the duplicate can still appear briefly. That's unavoidable without local payjoin-session → order linkage, and out of scope for this PR.

Notes

  • I could not run the test suite in this environment (Flutter commands require approval not granted to this job), so this review is static analysis only. CI should confirm the new test passes.
  • No CLAUDE.md found in the repo, so no repo-specific guidelines applied.

@ethicnology
ethicnology merged commit cd1ad9f into develop Aug 20, 2026
2 checks passed
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