feat: freeze-the-attempt send idempotency (ENG-533) - #717
Merged
Conversation
An attempt is an object — the send closure captured at the first tap plus one random uuid key — never a content fingerprint. Retained on throw (unknown outcome: retry re-runs the identical input under the same key so the backend replays), cleared on ANY server status (fresh key by construction: no deliberate-repeat collision, no 24h cached-FAILURE lockout). Deliberately not persisted; cross-session recovery belongs to a server-side lookup. Supersedes the fingerprint approach parked in #713. - payment-details: idempotencyKey threaded into all five send inputs behind a runtime capability gate (GraphQL input coercion rejects unknown fields pre-execution, so backends without the field get a keyless send and the gate remembers per endpoint+input type) - use-send-payment: inFlightRef double-tap guard (synchronous, closes the same-frame window the render-time gate can't); Breez Failure now re-arms the button like the GraphQL path - confirmation screen: early-return on ignored taps; retry enabled after Failure/throw (paymentError no longer pins the button; cleared on tap)
Contributor
|
this PR needs to be updated to follow conventional commits message |
- withIdempotencyKey: keyless coercion-refusal fallback now fires only on an attempt's FIRST dispatch. Retries of a dispatched attempt always go out keyed and surface UnresolvedAttemptKeyRefusedError instead of silently re-executing a payment whose earlier outcome is unknown (mixed-fleet rolling-deploy double-pay). attemptIsRetry threads from the hook's attemptRef through SendPaymentMutationParams into all five gated inputs. - use-send-payment: wire isIdempotencyKeyReuseError in as defense in depth — an IdempotencyKeyReuseError failure no longer re-arms the button with a fresh key; the user is told to check their transaction history. - confirmation screen: an ignored duplicate tap returns before touching the activity indicator, so it can no longer clobber the owning tap's spinner. - finish/remove editing-debris comment fragments in lightning.ts, intraledger.ts and stray blank lines in index.types.ts/lightning.ts. Tests: retry-path gate cases, attemptIsRetry lifecycle + key-reuse case in the hook suite, and a confirmation-screen duplicate-tap spinner contract test (verified to fail against the unfixed screen). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Contributor
|
this PR needs to be updated to follow conventional commits message |
…cs, and duplicated user copy Review fixes for ENG-533 (PR #717), round 2: - A retry of an attempt whose earlier dispatch went out KEYLESS is no longer auto-dispatched. The gate now reports every un-keyed send via onKeylessDispatch (both the fallback after a first-dispatch refusal and the latched-gate path); use-send-payment records it on the attempt and a retry tap of such an attempt surfaces the check-your-history failure without calling the mutation — the server never saw the key, so there is nothing to replay and a keyed re-send could pay twice. - An ignored duplicate tap no longer logs payment_attempt: the screen reads the in-flight guard synchronously (isInFlight) and returns before any side effect, so attempt counts stay honest against results. - The 'check your transaction history' sentence is now a single shared constant (CHECK_TRANSACTION_HISTORY_MESSAGE) used by both UnresolvedAttemptKeyRefusedError and the reuse-error branch, so the copy cannot drift between modules. Tests: hook spec pins the keyless-retry refusal (exactly one dispatch, history error, button stays disarmed); gate spec pins when onKeylessDispatch fires and when it must not; the duplicate-tap spec now asserts payment_attempt is logged exactly once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Contributor
|
this PR needs to be updated to follow conventional commits message |
islandbitcoin
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes ENG-533. Supersedes #713 (six review rounds established that content-fingerprinting a send attempt doesn't converge — every fingerprint either collided two deliberate payments or mismatched the server's own fingerprint, turning the one retry this feature exists for into `IdempotencyKeyReuseError` → double pay).
Design
An attempt is an object, not a fingerprint. At the first tap we freeze the current send closure together with one random uuid key. Nothing ever recognises a repeat — a repeat is simply the retained object being re-run.
Deliberately not persisted: force-quit mid-send leaves exactly today's pre-existing risk. Cross-session recovery wants a server-side lookup, not client heuristics.
Capability gate
GraphQL input coercion rejects unknown fields before execution, so a backend without the `idempotencyKey` inputs would fail every send. `idempotency-support.ts` gates the field per (endpoint, input type): on coercion refusal the send retries keyless (safe — refusal proves nothing settled) and the gate remembers.
UI contract
Tests
Full suite: 949 passed. tsc clean. No new lint findings (three `require-atomic-updates` suppressions are refs-as-synchronous-flags — the write-after-await is the design).