Skip to content

fix(sell): close the post-broadcast double-payment window - #2535

Draft
BullishNode wants to merge 1 commit into
mainfrom
fix/sell-hardening
Draft

fix(sell): close the post-broadcast double-payment window#2535
BullishNode wants to merge 1 commit into
mainfrom
fix/sell-hardening

Conversation

@BullishNode

Copy link
Copy Markdown
Contributor

⚠️ Requires emulator/device review before merge — draft until verified. Test: sell success page shows "You sold X sats for Y [fiat]" and closes (button AND back gesture) to wallet home; Confirm shows an in-flight spinner; SINPE Móvil recipients without an owner name appear in the recipients list (name falls back to label/phone). The double-payment latch itself is not manually testable (requires a mid-broadcast network failure) — it is covered by 5 bloc regression tests, each proven to fail on the old code.

Problems

  1. Double-payment risk (Sell: post-broadcast error path re-enables Confirm — double-payment risk #2522): after broadcasting, the confirm handler fetched the order; any failure re-enabled Confirm with the transaction already on the wire — a second tap re-ran prepare/sign/broadcast. Concurrent handlers (fee recalc, utxo load, order poll, price-lock refresh) also emitted pre-await snapshots that could silently revert in-flight state.
  2. Success state carried the stale pre-broadcast order (Sell: success state receives stale pre-broadcast order #2530).
  3. Success screen lacked the amount line and diverged from buy (Sell success: add 'You sold X sats for Y [fiat]', align with buy via shared success scaffold #2523).
  4. SINPE recipients were dropped when ownerName was absent — the server schema has it nullable by design (Ridivi lookup can fail) (SINPE Móvil recipients silently dropped when ownerName is absent #2529).

Changes

  • Broadcast latch (payinBroadcastTxid) emitted immediately after broadcast on both bitcoin and liquid paths; a latched bloc never re-enters the send path, never surfaces a retryable error, keeps Confirm disabled, and lets the existing poll carry the order to success
  • Concurrent emits merge into live state (incl. an order poll spanning the broadcast — reviewer-caught blocker with a deterministic regression test)
  • Price-lock refresh preserves in-flight state; a pre-broadcast failure past the deadline re-arms the countdown
  • In-flight spinner + status next to Confirm; Advanced settings disabled mid-confirmation
  • Success screen on a new shared SuccessScreenScaffold (buy/pay can adopt later): "You sold {amount} for {fiatAmount}", credit message gated on balance payouts (Order.isBalancePayout), closes to wallet home
  • SINPE ownerName optional with display fallback chain

Validation

5 bloc regression tests (double-confirm, poll race, stale order, countdown race, deadline recovery) — each demonstrated failing without its fix; analyze clean; full suite green. Reviewed with a verification pass (1 blocker + 2 should-fixes applied).

Closes #2522
Closes #2523
Closes #2529
Closes #2530

After broadcasting, the confirm handler fetched the order; any failure
in that fetch re-enabled Confirm with the transaction already on the
wire, so a second tap re-ran prepare, sign and broadcast and could pay
the order twice. Concurrent handlers (fee recalculation, utxo load,
order poll, price-lock refresh) also emitted pre-await snapshots that
could silently revert in-flight state.

- Latch on broadcast: payinBroadcastTxid is emitted immediately after
  the transaction is on the wire (bitcoin and liquid). A latched bloc
  never re-enters the send path, never surfaces a retryable error, and
  keeps Confirm disabled while the existing poll carries the order to
  success.
- Merge concurrent emits into live state so the latch survives fee
  recalculation, utxo loads and an order poll spanning the broadcast.
- Price-lock refresh no longer clears the in-flight flag; a failure
  after the deadline re-arms the countdown so the price refreshes.
- Show an in-flight spinner and status next to Confirm; disable
  Advanced settings during confirmation.
- Success state now carries the post-broadcast order instead of the
  stale pre-broadcast snapshot.
- Sell success screen: adds 'You sold {amount} for {fiatAmount}', gates
  the balance-credit message on balance payouts, and closes (button and
  back gesture) to wallet home like buy, on a new shared success
  scaffold widget.
- SINPE recipients: ownerName is optional, matching the server schema;
  display falls back to label, then phone or IBAN. Recipients missing
  the Ridivi-derived name are no longer dropped.

Closes #2522
Closes #2523
Closes #2529
Closes #2530
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