DRAFT: ENG-533 app half — needs redesign (client/server fingerprints disagree) by islandbitcoin · Pull Request #713 · lnflash/flash-mobile

islandbitcoin · 2026-08-27T22:36:46Z

Two commits, reviewable separately.

1. Schema hygiene — the snapshot had drifted 224 lines

app/graphql/public-schema.graphql is a hand-copied snapshot of the server SDL, and codegen types the entire app against it. Nothing ever compared it to the real schema.

Missing fields do not fail graphql-check — that validates our operations against the snapshot, so a new server field is simply invisible. The app can't ask for something that exists, and the symptom reads as "the backend never shipped it." Among the things missing: the idempotencyKey inputs from flash#494, and singlePaymentLimit / minimum on the allowance payload from flash#487.

Refreshed from lnflash/flash main — generated by that repo's write-sdl and gated by its own check:sdl, so authoritative without needing a running server — then re-ran codegen. All existing operations still validate.

Adds .github/workflows/schema-drift.yml, which diffs the snapshot against the server SDL on PRs touching app/graphql and weekly on a schedule — the drift is caused by the server moving, so a PR-only trigger would never fire. Verified it fails on a perturbed file, not merely that it passes on a matching one.

2. ENG-533 app half — a repeated USD send settling twice

Two protections that are easy to conflate:

The in-flight guard stops a double tap. hasAttemptedSend gates whether sendPayment is defined, and that's decided at render time — two taps in one frame both capture the closure from the render where it was still defined, and both run before React re-renders. A ref is written synchronously, so the second tap sees it.

The idempotency key stops a repeated request. This is the dangerous one: a send whose response was lost — dropped socket, gateway 502, app backgrounded mid-flight — has already moved the money, and the client cannot tell. The same key lets the backend recognise the repeat and return the original outcome.

The lifecycle is the part to argue with. The key is cleared only on a definitive, server-confirmed Failure: there we know nothing settled, and reusing it would make the backend replay the recorded failure so the customer could never succeed. Every other exit keeps the key — the conservative side, since a repeat the server already committed returns the original result instead of paying again.

lnurlPaymentSend gained the input server-side too, but the app never calls it (LNURL goes through Breez), so only the one call site is wired.

Tests cover both rules and both ways each can be wrong, plus a test pinning the un-guarded double-send so deleting the guard fails a test rather than silently restoring the bug.

92 suites / 903 tests green, tsc clean, changed-lines lint clean.

app/graphql/public-schema.graphql is a hand-copied snapshot of the server's SDL, and codegen types the entire app against it. Nothing checked it against the real schema, so it drifted 224 lines behind -- including the idempotencyKey inputs the send path needs (flash#494) and the allowance payload's singlePaymentLimit and minimum (flash#487). Stale-snapshot drift does not fail graphql-check: that validates our operations against the snapshot, so a missing server field is simply invisible. The app cannot ask for something that exists, and the symptom reads as "the backend never shipped it". Refreshed from lnflash/flash main (generated by its write-sdl and gated by its own check:sdl, so authoritative without a running server), then re-ran codegen. All existing operations still validate. Adds .github/workflows/schema-drift.yml: diffs the snapshot against the server SDL on PRs touching app/graphql and weekly on a schedule -- the drift is caused by the SERVER moving, so a PR-only trigger would not catch it. Verified the check fails on a perturbed file, not just passes on a matching one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

flash#494 added idempotencyKey to lnNoAmountUsdInvoicePaymentSend. This is the app half: generate the key, keep it stable across a repeat, and stop a double tap producing two requests in the first place. Two distinct protections, easily conflated: 1. In-flight guard. hasAttemptedSend gates whether sendPayment is DEFINED, and that is decided at render time -- two taps in one frame both capture the closure from the render where it was still defined and both run before React re-renders. A ref is written synchronously, so the second tap sees it. This is the double-tap case. 2. Idempotency key. The dangerous case is not the double tap but a send whose RESPONSE was lost: dropped socket, gateway 502, app backgrounded mid-flight. The server has already moved the money and the client cannot know. Sending the same key again lets the backend recognise the repeat and return the original outcome rather than paying twice. Key lifecycle is the part worth arguing with: the key is cleared ONLY on a definitive, server-confirmed Failure -- there we know nothing settled, and reusing the key would make the backend replay the recorded failure so the customer could never succeed. Every other exit keeps the key, which is the conservative side: a repeat the server already committed returns the original result instead of moving money again. lnurlPaymentSend also gained the input server-side but the app never calls it -- LNURL sends go through Breez -- so only the one call site is wired. Tests cover both rules and both ways they can be wrong (reuse on repeat, fresh key after definitive failure, no recycling across attempts), plus a test pinning the un-guarded double-send so deleting the guard fails a test instead of silently restoring the bug. 92 suites / 903 tests green, tsc clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

linear · 2026-08-27T22:36:49Z

ENG-533

… send path Review fixes for #713. The key was structurally unreusable. It lived in a ref, and the only retry this flow offers is a back-navigation — which unmounts useSendPayment and mints a fresh uuid for exactly the repeat the key is supposed to make recognisable. It is now derived from the ATTEMPT (wallet + destination + amount + memo) via uuidv5, in send-attempt-key.ts, so a rebuilt payment detail on a remounted screen reproduces it. A definitive FAILURE retires the key; every other exit keeps it. The thrown-mutation path — the dropped socket / gateway 502 / backgrounded app the whole design exists for — left the screen wedged in-flight and errored, so the retained key could never be resent by anyone. The reset policy now lives in one try/finally: an exception and a FAILURE payload land in the same known state, and the confirm screen distinguishes a retryable send error from a blocking one (fee error, dead invoice) instead of disabling the button for both. The suppressed second tap is now marked `ignored`, and the screen returns on it. It was indistinguishable from a real result with no status, so a double tap stopped the spinner mid-payment, logged a payment_result with an undefined status into the ENG-533 analytics, and showed a failure toast plus an error haptic over a payment about to succeed. idempotencyKey was sent unconditionally against LnNoAmountUsdInvoicePaymentInput, which only gained the field in flash#494. GraphQL rejects unknown input fields during coercion, so against an older API the whole mutation errors and every no-amount USD lightning send stops working — and graphql-check cannot see it. That one call site now goes through a runtime gate (idempotency-support.ts): a coercion refusal proves nothing executed, so it retries bare and remembers. The other four inputs have carried the field since ENG-530 and are long deployed, so intraLedgerPaymentSend, intraLedgerUsdPaymentSend, lnNoAmountInvoicePaymentSend and lnInvoicePaymentSend now pass it unconditionally — four live paths, including USD/USDT Flash-to-Flash, were running with the backend's exactly-once machinery switched off. Tests: the old idempotency spec asserted a local class and a local closure defined three lines above it, and passed with the guard deleted. Replaced with cases against the real code — the real screen, the real hook, the real builders. Verified they fail when the guard, the derived key or the `ignored` short-circuit is removed. schema-drift.yml: a byte-exact diff against another repo's moving main on pull_request goes red for drift no app PR caused. Additions now warn and pass on PRs and fail on the schedule; removals fail either way. A fetch failure now says so instead of reading as a drift alarm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

Six review findings on the ENG-533 app half, all of which left the key either unable to survive its own retry or unable to fire at all. - Fingerprint the attempt the USER authored, not the one the price derived. `settlementAmount` is a price-derived estimate for a USD/USDT sending wallet (`settlementAmountIsEstimated`), and the details screen re-derives it on every realtime-price tick while sitting mounted underneath the confirm screen — so the back-navigation that IS the retry handed back a detail whose settlement amount had moved by a cent, minting a different uuid and letting the backend book a second payment. It is worst on `lnInvoicePaymentSend`, whose input carries no amount at all: two byte-identical requests got different keys purely because BTC moved. Keys on `unitOfAccountAmount`, carried verbatim through `setConvertMoneyAmount`. - Drop the perishable bolt11 from an LNURL fingerprint. An LNURL detail's `paymentRequest` is re-minted on every pass forward through the details screen (IBEX caps those invoices at 60s), and that pass is the retry, so a USD LNURL send could never carry the same key twice — zero protection on a path that does reach GraphQL. Uses the lightning address plus the amount and memo, which survive the re-mint. - Anchor the runtime capability gate on the unknown-field sentence. graphql-js writes EVERY input-coercion error as `Variable "$input" got invalid value <the whole input object>; <reason>`, and that object contains `idempotencyKey` whenever we sent it — so "names the field" AND "says got invalid value" matched any coercion error at all. One added required field server-side would have disarmed idempotency for the process lifetime and sent every later no-amount USD lightning payment out bare. The spec's fabricated message is replaced with the shape graphql-js really emits. - Stop the suppressed tap recording a `payment_attempt`. The event fired before `sendPayment()`, so a double tap logged two attempts against one result and skewed the ratio ENG-533 is measured on by precisely the double-taps ENG-533 counts. The hook now exposes the in-flight reading synchronously, so the second tap returns before anything is recorded — rather than moving the log below the awaited send, which would lose the attempt entirely whenever the mutation throws. - Never hand back a spent key. The eviction cap deleted the insertion-oldest entry, sending that attempt back to generation 0 — the exact uuid the server had already answered with FAILURE, which the backend then replays forever. Absence has to keep meaning "never seen" for a key to be re-derivable after a remount, so the map does not forget; it only ever gains an entry on a definitive, server-confirmed failure. - Delete the unreachable fallback key. `sendPayment` is only defined when the payment detail it fingerprints is, so the `uuidv4` branch, its ref, its `as string` cast, its second retirement path and its `require-atomic-updates` disable were all dead. Narrowed once at the top of the memo instead. Tests: each new case was verified to fail against the code it pins — a price tick under the back-navigation, an LNURL re-mint, a coercion error about a different field, the double-tap attempt count, and a session of 200 retirements. 930 tests green, tsc clean, prettier clean, lint unchanged from before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

Review residual, and the worst bug in the PR so far: a deliberate repeat of an identical send silently never happened. The fingerprint is purely content-derived, so a Flashcard reload of J$2,000 to the same LNURL produces the same string every time. Retiring the key only on Failure meant that after a SUCCESS the key stayed retired-less: reloading the same card for the same amount later that day re-derived the first payment's key, the backend honoured its documented contract and returned the ORIGINAL success, the screen navigated to sendBitcoinSuccess -- and the second J$2,000 never left the wallet. The customer is shown success twice and paid once. Round 2's move to key on unitOfAccountAmount is what made this fully deterministic: it removed the price-tick jitter that had been accidentally masking the collision on USD/USDT wallets. Now retired on ANY server-supplied status. A status coming back at all means the client knows the outcome, so the next authoring of the same content is a new payment and deserves a new key. The case this design exists for is untouched and is the exact complement: a LOST response yields no status, throws, and leaves the key intact so the repeat carries it and settles once. Spec covers both halves of the rule -- same key while unresolved, fresh key after the outcome is known -- against identical content, which the existing "two different sends" case never exercised because it separated them by amount. 93 suites / 931 tests green, tsc clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

…nd input Review fixes for the ENG-533 app half. Freeze the send, don't just key it. The server binds its cached result to a requestFingerprint built from the WIRE input (`ln|${paymentRequest}`, `ln-noamount-usd|${paymentRequest}|${amount}`, `intraledger|${recipientWalletId}|${amount}`), while our attempt fingerprint is built from what SURVIVES the retry — deliberately excluding a re-minted LNURL bolt11 and a price-derived settlement amount. Resending the same key with the rebuilt payment detail therefore landed on IdempotencyKeyReuseError rather than a replay, and the screen read that as a definitive failure, retired the key and let the next tap pay a second time. `freezeAttempt` now pairs the key with the send closure captured on the first Confirm, so the two sides move in lockstep, and `isIdempotencyKeyReuseError` stops a reuse rejection ever being mistaken for a failure: the key is kept, the button stays locked, and the user is sent to their transaction history. Persist the retired generations. The server caches definitive outcomes — FAILURE included — for 24h (IDEMPOTENCY_TTL_SECS), so an in-memory-only map let a force-quit re-derive the generation-0 key the backend had already answered with FAILURE, locking the customer out of that exact payment for the rest of the day. Generations are now written through to AsyncStorage under a hash of the fingerprint, with a matching 24h expiry that bounds the store for free. Gate all five send inputs, not one. Nothing in this repo measures which environments carry `idempotencyKey`, and an unknown input field is rejected during coercion — taking the whole send path down rather than degrading. The gate is also scoped per (graphqlUri, input type) and re-armed on foreground, so one stale pod or one send against staging costs a single send's protection rather than the session's. Give the weekly schema-drift run an owner: on strict failure it opens (and thereafter updates) a labelled tracking issue carrying the diff, and closes it again once the snapshot matches. An unowned red X in the Actions tab is the same failure mode that let the snapshot sit 224 lines behind. Tests: frozen-input replay for both the re-minted LNURL bolt11 and the repriced settlement amount, with the four server requestFingerprint strings pinned; a cold-module reload proving a retired key survives a force-quit and expires with the server's window; per-input and per-endpoint gate scoping plus the foreground re-arm; all five inputs driven through their real builders; and a screen-level case proving a reuse rejection cannot become a second payment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

…goes out Three review findings on the ENG-533/ENG-555 work, all on the same seam: what the screen VALIDATES and what the app TRANSMITS had drifted apart. 1. The freeze held a closure, so half an attempt survived a force-quit. `generationByAttempt` was written through to storage while the frozen send was an in-memory closure — so a relaunch re-derived the same key for a REBUILT input (a re-minted LNURL bolt11, a repriced settlement amount), which is the one combination the backend answers with IdempotencyKeyReuseError: nothing settles, and that exact payment is impossible for the next 24h. The wire input is now DATA (`payment-details/send-wire-input.ts`), surfaced by each detail builder as `sendPaymentWireInput`, frozen and persisted under the same digest and 24h TTL as the generations, and fed back to the rebuilt detail's mutation through `SendPaymentMutationParams.frozenInput` — so the repeat is byte-identical whatever process it happens in. A frozen entry is honoured only while its key still matches the generation in force, so a torn write cannot resend a spent key. 2. The ENG-555 expiry guard judged the invoice on screen, not the one that would be sent. On the LNURL path those routinely differ: the details screen re-mints on every pass forward while the freeze holds the original. The guard now reads the transmitted bolt11 (`frozenSendInvoice`), refuses only a FIRST send of a dead invoice — a frozen replay is how a lost outcome is recovered, and refusing it would strand the attempt for 24h — and, when a frozen dead invoice IS rejected by the server, names the expiry instead of "Something went wrong". 3. `IdempotencyKeyReuseError` was logged as `paymentStatus: "FAILURE"`. The one event that proves the idempotency work fired was landing in the failure column of the ratio ENG-533 is measured on; it now logs `KEY_REUSED`. Tests: cold-module replay of both a re-minted bolt11 and a repriced settlement amount, the torn-write and corrupt-store cases, the stranded LNURL retry end to end at the screen (including that the remedy it names is reachable), and the KEY_REUSED analytics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

islandbitcoin · 2026-08-28T02:04:50Z

Converting to draft. The schema-hygiene half is split out to #714 and can merge independently.

Why this half is not mergeable

Six review rounds across two runs, and each found a real defect in the previous round's fix. That is a design that is wrong, not one fix away.

The last round verified it against the backend source, and it is fundamental: the client and server fingerprint different things.

Server: ln|${paymentRequest}, ln-noamount-usd|${paymentRequest}|${amount}, intraledger|${recipientWalletId}|${amount} — the bolt11 and the settlement amount.
This branch: deliberately drops the bolt11 for LNURL (so a re-minted invoice still matches) and uses unitOfAccount amount (so a price tick doesn't split an attempt).

Those choices are individually defensible and collectively fatal. On the retry this design exists for, the app resends the same key with a different server-side fingerprint → IdempotencyKeyReuseError → mapped to {status: "failed"} → the code retires the key → the user retries with a fresh one → the money leaves twice. The exact bug, reintroduced with extra steps. Two of the tests here pin the behaviour that causes it.

Second blocker: generationByAttempt is in-memory, but the server caches definitive outcomes — including FAILURE — for 24h. Force-quitting after a failed payment (the normal human reaction) resets generation to 0, the server replays its cached FAILURE, and the user cannot make that exact payment for a day with nothing explaining why.

What a correct version looks like

Freeze the attempt, don't just key it. On first Confirm, capture the exact mutation input (paymentRequest, settlementAmount.amount) alongside the derived key; on a repeat, resend that frozen input rather than a rebuilt payment detail. Client key and server fingerprint then move in lockstep by construction, and the LNURL/price-tick special-casing disappears.

Plus: persist generation state with a 24h expiry matching IDEMPOTENCY_TTL_SECS, and route all five inputs through the existing capability gate rather than asserting deployment state.

Also worth pinning the four server fingerprint strings in a test, so a change on either side breaks something.

Not blocking v0.7.0 — the double-send window it addresses is pre-existing, and shipping this version would make it worse.

) * chore(graphql): refresh the schema snapshot, and pin it against drift app/graphql/public-schema.graphql is a hand-copied snapshot of the server's SDL, and codegen types the entire app against it. Nothing checked it against the real schema, so it drifted 224 lines behind -- including the idempotencyKey inputs the send path needs (flash#494) and the allowance payload's singlePaymentLimit and minimum (flash#487). Stale-snapshot drift does not fail graphql-check: that validates our operations against the snapshot, so a missing server field is simply invisible. The app cannot ask for something that exists, and the symptom reads as "the backend never shipped it". Refreshed from lnflash/flash main (generated by its write-sdl and gated by its own check:sdl, so authoritative without a running server), then re-ran codegen. All existing operations still validate. Adds .github/workflows/schema-drift.yml. The two triggers deliberately do not judge alike: * schedule/manual -- strict. Any difference is drift somebody must go look at, and nobody's PR is blocked while they do. * pull_request -- only REMOVALS fail. A removal can invalidate an operation this app sends. Additions are the server shipping something new, which no app PR caused and none can fix; failing on those turns every app/graphql PR red the first time flash merges anything, and a red X everyone knows to ignore is worse than no check. A fetch failure and real drift are also reported differently, so a repo rename or a raw.githubusercontent 5xx does not land in someone's inbox looking like a schema alarm. Verified the check fails on a perturbed snapshot, not merely that it passes on a matching one. Split out of #713, which carried this plus ENG-533's app half. That half needs a design change (client and server fingerprint different things) and should not hold this back. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV * fix(ci): judge schema drift semantically, and give the weekly alarm an owner Three review findings on the drift workflow: 1. The removal check was a textual grep over the unified diff, so any CHANGED line read as a removal -- a docstring rewording server-side would fail every app PR touching app/graphql as if an operation-breaking removal had happened, which is the cry-wolf failure the workflow's own comments argue against. Now judged by graphql-inspector (already a repo dependency, it backs graphql-check): exit codes verified empirically -- identical 0, additions 0, docstring rewording 0, field removal 1. The textual diff is kept for display only. 2. The strict weekly run's only output was a red run in the Actions tab, which GitHub shows to the workflow file's last committer and nobody else. The 224-line drift this workflow exists to catch sat unnoticed for exactly that reason. A strict failure now files (or updates) a tracking issue carrying the inspector output and the refresh instructions. 3. The remediation text never said to commit the regenerated generated.ts, so the next person's first attempt would bounce off check:codegen. It says so now. All five decision paths simulated locally: identical passes, addition warns on a PR and fails the schedule, removal fails both, docstring edit passes a PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV --------- Co-authored-by: Dread <dread@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

islandbitcoin · 2026-09-01T00:01:24Z

Superseded by #717 — freeze-the-attempt (object identity + random key) replaces the content-fingerprint approach; six review rounds here established the fingerprint design doesn't converge.

Dread and others added 2 commits August 27, 2026 15:36

Dread and others added 5 commits August 27, 2026 16:02

islandbitcoin mentioned this pull request Aug 28, 2026

chore(graphql): refresh the schema snapshot + pin it against drift #714

Merged

islandbitcoin changed the title ~~chore(graphql): refresh the schema snapshot + pin it against drift; wire ENG-533's idempotency key~~ DRAFT: ENG-533 app half — needs redesign (client/server fingerprints disagree) Aug 28, 2026

islandbitcoin marked this pull request as draft August 28, 2026 02:04

islandbitcoin mentioned this pull request Sep 1, 2026

feat: freeze-the-attempt send idempotency (ENG-533) #717

Open

islandbitcoin closed this Sep 1, 2026

Navigation Menu

Uh oh!

DRAFT: ENG-533 app half — needs redesign (client/server fingerprints disagree) - #713

DRAFT: ENG-533 app half — needs redesign (client/server fingerprints disagree)#713
islandbitcoin wants to merge 7 commits into
mainfrom
chore/schema-hygiene

islandbitcoin commented Aug 27, 2026

Uh oh!

linear Bot commented Aug 27, 2026

Uh oh!

islandbitcoin commented Aug 28, 2026

Uh oh!

islandbitcoin commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

islandbitcoin commented Aug 27, 2026

1. Schema hygiene — the snapshot had drifted 224 lines

2. ENG-533 app half — a repeated USD send settling twice

Uh oh!

linear Bot commented Aug 27, 2026

Uh oh!

islandbitcoin commented Aug 28, 2026

Why this half is not mergeable

What a correct version looks like

Uh oh!

islandbitcoin commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant