From ff11f24d5dc60095865db7e71d86f1e7cbfb3202 Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 24 Aug 2026 13:02:33 -0700 Subject: [PATCH 1/3] feat(payments): idempotency on the two send paths that bypassed it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ENG-533 discovery: the ENG-530 exactly-once wrapper covers the four send functions in @app/payments — but two mutations never call those functions. lnNoAmountUsdInvoicePaymentSend and lnurlPaymentSend are FLASH-FORK resolvers that execute IBEX directly, so they had no idempotencyKey input, no lock, no dedupe. A double-fire on the most common USD send path double-paid, exactly the 2026-07-23 incident class (intraLedgerPaymentSend fired twice ~1.5s apart, $280 debited for a $140 send). Both resolvers now accept the same optional idempotencyKey and route ONLY the money-moving IBEX call through withPaymentIdempotency: - scoped to the ROUTED wallet (the one actually debited), so the same key behaves identically across the cash-wallet compat redirect - fingerprinted on the REQUEST AS SENT (invoice/lnurl + input amount). Deliberately not amountMsat on the lnurl path: the msat figure moves with the dealer rate, and a legitimate same-key retry must not be rejected as a different payment because the price ticked - decode, metadata fetch, routing and amount conversion stay OUTSIDE execute(), so a cached replay touches neither IBEX nor the lnurl server - no key = passthrough, existing clients unaffected Onchain send mutations audited too: their resolvers are stubbed (onchain moved client-side to Breez), so there is nothing to cover there. Tests pin the WIRING in both resolver specs — key, wallet scope, fingerprint shape, cached-replay-skips-IBEX, wrapper-error mapping, absent-key passthrough — while the wrapper's own dedupe/lock behavior stays covered by app/payments/idempotency.spec.ts. The wrapper is mocked in resolver specs because it constructs Redis/Lock clients at import. SDL + supergraph regenerated. 205 suites / 2228 tests green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV --- dev/apollo-federation/supergraph.graphql | 10 +++ .../ln-noamount-usd-invoice-payment-send.ts | 42 ++++++++--- .../root/mutation/lnurl-payment-send.ts | 41 +++++++++-- src/graphql/public/schema.graphql | 10 +++ ...-noamount-usd-invoice-payment-send.spec.ts | 73 +++++++++++++++++++ .../root/mutation/lnurl-payment-send.spec.ts | 43 +++++++++++ 6 files changed, 201 insertions(+), 18 deletions(-) diff --git a/dev/apollo-federation/supergraph.graphql b/dev/apollo-federation/supergraph.graphql index 6e854094f..31a0e7dfa 100644 --- a/dev/apollo-federation/supergraph.graphql +++ b/dev/apollo-federation/supergraph.graphql @@ -1767,6 +1767,11 @@ input LnNoAmountUsdInvoicePaymentInput """Amount to pay in USD cents.""" amount: FractionalCentAmount! + """ + Optional client-supplied key; a repeated send with the same key returns the original result instead of paying again. + """ + idempotencyKey: String + """Optional memo to associate with the lightning invoice.""" memo: Memo @@ -1809,6 +1814,11 @@ input LnurlPaymentSendInput """Amount to spend from the USD/USDT wallet, in USD cents.""" amount: FractionalCentAmount! + """ + Optional client-supplied key; a repeated send with the same key returns the original result instead of paying again. + """ + idempotencyKey: String + """LNURL-pay value to decode and pay.""" lnurl: Lnurl! diff --git a/src/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.ts b/src/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.ts index 59a10993a..f7c525c92 100644 --- a/src/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.ts +++ b/src/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.ts @@ -16,6 +16,7 @@ import { resolveCashWalletMutationWalletIdForAccount } from "@app/cash-wallet-cu import Ibex from "@services/ibex/client" import { IbexError } from "@services/ibex/errors" +import { withPaymentIdempotency } from "@app/payments/idempotency" import { paymentSendStatusOrPending } from "@services/ibex/payment-status" const LnNoAmountUsdInvoicePaymentInput = GT.Input({ @@ -38,6 +39,11 @@ const LnNoAmountUsdInvoicePaymentInput = GT.Input({ type: Memo, description: "Optional memo to associate with the lightning invoice.", }, + idempotencyKey: { + type: GT.String, + description: + "Optional client-supplied key; a repeated send with the same key returns the original result instead of paying again.", + }, }), }) @@ -50,6 +56,7 @@ const LnNoAmountUsdInvoicePaymentSendMutation = GT.Field< paymentRequest: string | InputValidationError amount: FractionalCentAmount | InputValidationError memo?: string | InputValidationError + idempotencyKey?: string | null } } >({ @@ -64,7 +71,7 @@ const LnNoAmountUsdInvoicePaymentSendMutation = GT.Field< input: { type: GT.NonNull(LnNoAmountUsdInvoicePaymentInput) }, }, resolve: async (_, args, { domainAccount, cashWalletClientCapabilities }) => { - const { walletId, paymentRequest, amount, memo } = args.input + const { walletId, paymentRequest, amount, memo, idempotencyKey } = args.input if (walletId instanceof InputValidationError) { return { errors: [{ message: walletId.message }] } @@ -112,24 +119,39 @@ const LnNoAmountUsdInvoicePaymentSendMutation = GT.Field< errors: [mapAndParseErrorForGqlResponse(usCents)], } } - const PayLightningInvoice = await Ibex.payInvoice({ - invoice: paymentRequest as Bolt11, - accountId: routedWalletId, - send: usCents, + // ENG-533: this resolver executes IBEX directly (FLASH FORK above), so the + // exactly-once wrapper the covered send functions get in @app/payments + // never ran here — a double-fire on the most common USD send path + // double-paid, exactly the 2026-07-23 incident class. Scoped to the ROUTED + // wallet (the one actually debited) so the same key behaves identically + // across the cash-wallet compat redirect. Only the money-moving call sits + // inside execute(); routing and amount conversion stay outside so a cached + // replay does no IBEX work at all. + const outcome = await withPaymentIdempotency({ + idempotencyKey, + senderWalletId: routedWalletId, + requestFingerprint: `ln-noamount-usd|${paymentRequest}|${amount}`, + execute: async () => { + const PayLightningInvoice = await Ibex.payInvoice({ + invoice: paymentRequest as Bolt11, + accountId: routedWalletId, + send: usCents, + }) + if (PayLightningInvoice instanceof IbexError) return PayLightningInvoice + return paymentSendStatusOrPending(PayLightningInvoice) + }, }) - if (PayLightningInvoice instanceof IbexError) { + if (outcome instanceof Error) { return { status: "failed", - errors: [mapAndParseErrorForGqlResponse(PayLightningInvoice)], + errors: [mapAndParseErrorForGqlResponse(outcome)], } } - const status = paymentSendStatusOrPending(PayLightningInvoice) - return { errors: [], - status: status.value, + status: outcome.value, } }, }) diff --git a/src/graphql/public/root/mutation/lnurl-payment-send.ts b/src/graphql/public/root/mutation/lnurl-payment-send.ts index 13431a4bf..391470678 100644 --- a/src/graphql/public/root/mutation/lnurl-payment-send.ts +++ b/src/graphql/public/root/mutation/lnurl-payment-send.ts @@ -1,3 +1,4 @@ +import { withPaymentIdempotency } from "@app/payments/idempotency" import axios from "axios" import dedent from "dedent" @@ -48,6 +49,11 @@ const LnurlPaymentSendInput = GT.Input({ type: Memo, description: "Optional memo for the Lightning payment.", }, + idempotencyKey: { + type: GT.String, + description: + "Optional client-supplied key; a repeated send with the same key returns the original result instead of paying again.", + }, }), }) @@ -90,6 +96,7 @@ const LnurlPaymentSendMutation = GT.Field< lnurl: Lnurl | InputValidationError amount: FractionalCentAmount | InputValidationError memo?: Memo | InputValidationError + idempotencyKey?: string | null } } >({ @@ -103,7 +110,7 @@ const LnurlPaymentSendMutation = GT.Field< input: { type: GT.NonNull(LnurlPaymentSendInput) }, }, resolve: async (_, args, { domainAccount, cashWalletClientCapabilities }) => { - const { walletId, lnurl, amount, memo } = args.input + const { walletId, lnurl, amount, memo, idempotencyKey } = args.input if (walletId instanceof InputValidationError) { return { status: "failed", errors: [{ message: walletId.message }] } @@ -190,21 +197,39 @@ const LnurlPaymentSendMutation = GT.Field< } } - const payment = await Ibex.payToLnurl({ - accountId: routedWalletId, - amountMsat, - params: paramsFromMetadata(metadata), + // ENG-533: direct-IBEX execution, so the exactly-once wrapper never ran on + // this path. Scoped to the ROUTED wallet; only the money-moving call sits + // inside execute() — decode, metadata fetch and amount validation stay + // outside, so a cached replay touches neither IBEX nor the lnurl server. + // The fingerprint uses the client's lnurl + amount (the request as sent), + // not amountMsat: the msat figure moves with the dealer rate, and a + // legitimate same-key retry must not be rejected as a different payment + // because the price ticked. + const outcome = await withPaymentIdempotency({ + idempotencyKey, + senderWalletId: routedWalletId, + requestFingerprint: `lnurl|${lnurl}|${amount}`, + execute: async () => { + const payment = await Ibex.payToLnurl({ + accountId: routedWalletId, + amountMsat, + params: paramsFromMetadata(metadata), + }) + if (payment instanceof IbexError) return payment + return lnurlPaymentSendStatusOrPending(payment) + }, }) - if (payment instanceof IbexError) { + + if (outcome instanceof Error) { return { status: "failed", - errors: [mapAndParseErrorForGqlResponse(payment)], + errors: [mapAndParseErrorForGqlResponse(outcome)], } } return { errors: [], - status: lnurlPaymentSendStatusOrPending(payment).value, + status: outcome.value, } }, }) diff --git a/src/graphql/public/schema.graphql b/src/graphql/public/schema.graphql index 4666bb75c..e989126ee 100644 --- a/src/graphql/public/schema.graphql +++ b/src/graphql/public/schema.graphql @@ -1429,6 +1429,11 @@ input LnNoAmountUsdInvoicePaymentInput { """Amount to pay in USD cents.""" amount: FractionalCentAmount! + """ + Optional client-supplied key; a repeated send with the same key returns the original result instead of paying again. + """ + idempotencyKey: String + """Optional memo to associate with the lightning invoice.""" memo: Memo @@ -1499,6 +1504,11 @@ input LnurlPaymentSendInput { """Amount to spend from the USD/USDT wallet, in USD cents.""" amount: FractionalCentAmount! + """ + Optional client-supplied key; a repeated send with the same key returns the original result instead of paying again. + """ + idempotencyKey: String + """LNURL-pay value to decode and pay.""" lnurl: Lnurl! diff --git a/test/flash/unit/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.spec.ts b/test/flash/unit/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.spec.ts index 5340b6fec..e63c75450 100644 --- a/test/flash/unit/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.spec.ts +++ b/test/flash/unit/graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send.spec.ts @@ -22,12 +22,26 @@ jest.mock("@app/cash-wallet-cutover", () => ({ ) => mockResolveCashWalletMutationWalletIdForAccount(...args), })) +const mockWithPaymentIdempotency = jest.fn() + +// Passthrough by default: the wrapper's own dedupe/lock/fingerprint behavior is +// covered exhaustively by app/payments/idempotency.spec.ts. THIS spec pins the +// WIRING — that the resolver routes its IBEX execution through the wrapper with +// the right key, wallet scope and fingerprint — which is the half a resolver +// can get wrong (and did: this path bypassed the wrapper entirely until +// ENG-533). +jest.mock("@app/payments/idempotency", () => ({ + withPaymentIdempotency: (...args: Parameters) => + mockWithPaymentIdempotency(...args), +})) + jest.mock("@app/wallets", () => ({ usdWalletAmountFromWalletId: ( ...args: Parameters ) => mockUsdWalletAmountFromWalletId(...args), })) +import { IdempotencyKeyReuseError } from "@domain/errors" import { ErrorLevel, USDTAmount } from "@domain/shared" import LnNoAmountUsdInvoicePaymentSendMutation from "@graphql/public/root/mutation/ln-noamount-usd-invoice-payment-send" import { IbexError, UnconfirmedIbexPayment } from "@services/ibex/errors" @@ -75,6 +89,9 @@ describe("LnNoAmountUsdInvoicePaymentSendMutation", () => { status: 0, transaction: { payment: { status: { id: 2 } } }, }) + mockWithPaymentIdempotency.mockImplementation( + async ({ execute }: { execute: () => Promise }) => execute(), + ) }) it("pays the routed wallet with the resolved cent amount", async () => { @@ -182,4 +199,60 @@ describe("LnNoAmountUsdInvoicePaymentSendMutation", () => { expect(result.errors[0].message).toBeTruthy() expect(mockRecordExceptionInCurrentSpan).not.toHaveBeenCalled() }) + + describe("idempotency wiring (ENG-533)", () => { + it("routes the IBEX execution through withPaymentIdempotency with key, routed wallet and fingerprint", async () => { + const result = (await resolveMutation({ + idempotencyKey: "11111111-2222-4333-8444-555555555555", + })) as MutationResult + + expect(result.errors).toEqual([]) + expect(mockWithPaymentIdempotency).toHaveBeenCalledTimes(1) + const call = mockWithPaymentIdempotency.mock.calls[0][0] + expect(call.idempotencyKey).toBe("11111111-2222-4333-8444-555555555555") + // Scoped to the ROUTED wallet — the one actually debited — so the same + // key behaves identically across the cash-wallet compat redirect. + expect(call.senderWalletId).toBe(routedWalletId) + // Fingerprints the REQUEST as the client sent it (invoice + input + // amount), so a legitimate same-key retry is recognized as the same + // payment. + expect(call.requestFingerprint).toBe("ln-noamount-usd|lnbc1noamount|1234") + }) + + it("serves the wrapper's cached outcome without touching IBEX", async () => { + // The replay case the wrapper exists for: a double-fire's second request + // must produce the first result and no second payment. If this fails, + // the 2026-07-23 double-pay class is back on this path. + mockWithPaymentIdempotency.mockResolvedValue({ value: "success" }) + + const result = (await resolveMutation({ + idempotencyKey: "11111111-2222-4333-8444-555555555555", + })) as MutationResult + + expect(result.status).toBe("success") + expect(mockPayInvoice).not.toHaveBeenCalled() + }) + + it("maps a wrapper error (key reuse / lock busy) to a failed payload", async () => { + mockWithPaymentIdempotency.mockResolvedValue( + new IdempotencyKeyReuseError("same key, different payment"), + ) + + const result = (await resolveMutation({ + idempotencyKey: "11111111-2222-4333-8444-555555555555", + })) as MutationResult + + expect(result.status).toBe("failed") + expect(result.errors.length).toBeGreaterThan(0) + expect(mockPayInvoice).not.toHaveBeenCalled() + }) + + it("still executes without a key — absent key means passthrough, not rejection", async () => { + const result = (await resolveMutation()) as MutationResult + + expect(result.errors).toEqual([]) + expect(mockWithPaymentIdempotency.mock.calls[0][0].idempotencyKey).toBeUndefined() + expect(mockPayInvoice).toHaveBeenCalledTimes(1) + }) + }) }) diff --git a/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts b/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts index 4c1191b7c..dd5a0a131 100644 --- a/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts +++ b/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts @@ -5,6 +5,17 @@ const mockPayToLnurl = jest.fn() const mockGetSatsFromCentsForImmediateSell = jest.fn() const mockAxiosGet = jest.fn() +const mockWithPaymentIdempotency = jest.fn() + +// Passthrough by default — the wrapper's behavior is covered by +// app/payments/idempotency.spec.ts; this spec pins the resolver WIRING +// (key, wallet scope, fingerprint), the half this path got wrong by +// bypassing the wrapper entirely until ENG-533. +jest.mock("@app/payments/idempotency", () => ({ + withPaymentIdempotency: (...args: Parameters) => + mockWithPaymentIdempotency(...args), +})) + jest.mock("@app/cash-wallet-cutover", () => ({ resolveCashWalletMutationWalletIdForAccount: ( ...args: Parameters @@ -106,6 +117,9 @@ describe("LnurlPaymentSendMutation", () => { mockPayToLnurl.mockResolvedValue({ transaction: { payment: { status: { id: 2 } } }, }) + mockWithPaymentIdempotency.mockImplementation( + async ({ execute }: { execute: () => Promise }) => execute(), + ) }) it("decodes LNURL metadata, converts USDT wallet amount to msats, and pays IBEX", async () => { @@ -213,4 +227,33 @@ describe("LnurlPaymentSendMutation", () => { expect(result?.status).toBe("failed") expect(result?.errors[0].message).toBeTruthy() }) + + describe("idempotency wiring (ENG-533)", () => { + it("routes payToLnurl through withPaymentIdempotency, fingerprinting the request as sent", async () => { + const result = (await resolveMutation({ + idempotencyKey: "11111111-2222-4333-8444-555555555555", + })) as { errors: unknown[] } + + expect(result.errors).toEqual([]) + const call = mockWithPaymentIdempotency.mock.calls[0][0] + expect(call.idempotencyKey).toBe("11111111-2222-4333-8444-555555555555") + expect(call.senderWalletId).toBe(routedWalletId) + // The fingerprint uses the client's lnurl + input amount — NOT amountMsat, + // which moves with the dealer rate. A legitimate same-key retry must not + // be rejected as a different payment because the price ticked. + expect(call.requestFingerprint).toMatch(/^lnurl\|/) + expect(call.requestFingerprint).not.toContain("Msat") + }) + + it("serves the wrapper's cached outcome without touching IBEX or the lnurl server", async () => { + mockWithPaymentIdempotency.mockResolvedValue({ value: "success" }) + + const result = (await resolveMutation({ + idempotencyKey: "11111111-2222-4333-8444-555555555555", + })) as { status?: string } + + expect(result.status).toBe("success") + expect(mockPayToLnurl).not.toHaveBeenCalled() + }) + }) }) From fa0f62347406f05be58111041b2eaab0176be221 Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 24 Aug 2026 13:10:13 -0700 Subject: [PATCH 2/3] fix(payments): make the lnurl cached replay actually skip IBEX and the lnurl server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes on PR #494: - Move decodeLnurl, the metadata fetch, the msat conversion and validateLnurlPayAmountMsat inside withPaymentIdempotency's execute(). They previously ran on every replay, so a cache hit was unreachable unless the flaky lnurl server (or the dealer rate) cooperated on the retry — the retry could report failure while the cached success sat unreachable, prompting a fresh-key re-send: the double-pay class this PR exists to close. The fingerprint only needs lnurl + amount + routed wallet, all available before the wrapper. - Catch axios.get rejection in the metadata fetch and return InvalidLnurlError so it maps to the typed failed payload instead of escaping the redlock callback as a bare GraphQL error. - Strengthen the cached-replay spec to assert decodeLnurl and axios.get are also untouched (it previously only checked payToLnurl, certifying a property the code did not have). - Add the wrapper-error → failed payload and absent-key passthrough specs the PR body claimed, mirroring the noamount spec. - Add a spec for the metadata-fetch rejection path. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV --- .../root/mutation/lnurl-payment-send.ts | 97 ++++++++----------- .../root/mutation/lnurl-payment-send.spec.ts | 45 +++++++++ 2 files changed, 88 insertions(+), 54 deletions(-) diff --git a/src/graphql/public/root/mutation/lnurl-payment-send.ts b/src/graphql/public/root/mutation/lnurl-payment-send.ts index 391470678..0170174d3 100644 --- a/src/graphql/public/root/mutation/lnurl-payment-send.ts +++ b/src/graphql/public/root/mutation/lnurl-payment-send.ts @@ -150,66 +150,55 @@ const LnurlPaymentSendMutation = GT.Field< } } - const decoded = await Ibex.decodeLnurl({ lnurl }) - if (decoded instanceof IbexError) { - return { - status: "failed", - errors: [mapAndParseErrorForGqlResponse(decoded)], - } - } - if (!decoded.decodedLnurl) { - return { - status: "failed", - errors: [mapAndParseErrorForGqlResponse(new InvalidLnurlError())], - } - } - - const metadataResponse = await axios.get(decoded.decodedLnurl) - const metadata = metadataResponse.data - if (!isLnurlPayMetadata(metadata)) { - return { - status: "failed", - errors: [mapAndParseErrorForGqlResponse(new InvalidLnurlError())], - } - } - - const dealer = DealerPriceService() - const amountMsat = await amountMsatFromUsdWalletAmount({ - amount: walletAmount, - btcFromUsd: dealer.getSatsFromCentsForImmediateSell, - }) - if (amountMsat instanceof Error) { - return { - status: "failed", - errors: [mapAndParseErrorForGqlResponse(amountMsat)], - } - } - - const validAmount = validateLnurlPayAmountMsat({ - amountMsat, - minSendable: metadata.minSendable, - maxSendable: metadata.maxSendable, - }) - if (validAmount instanceof Error) { - return { - status: "failed", - errors: [mapAndParseErrorForGqlResponse(validAmount)], - } - } - // ENG-533: direct-IBEX execution, so the exactly-once wrapper never ran on - // this path. Scoped to the ROUTED wallet; only the money-moving call sits - // inside execute() — decode, metadata fetch and amount validation stay - // outside, so a cached replay touches neither IBEX nor the lnurl server. - // The fingerprint uses the client's lnurl + amount (the request as sent), - // not amountMsat: the msat figure moves with the dealer rate, and a - // legitimate same-key retry must not be rejected as a different payment - // because the price ticked. + // this path. Scoped to the ROUTED wallet. EVERYTHING after routing — + // decode, metadata fetch, msat conversion, amount validation and the + // money-moving call — sits inside execute(), so a cached replay + // short-circuits before touching IBEX or the lnurl server. That matters + // precisely on the retry path this wrapper exists for: the flaky lnurl + // server (or a moved dealer rate) must not be able to mask a cached + // success as a failure. The fingerprint needs only the client's + // lnurl + amount (the request as sent) — not amountMsat, which moves with + // the dealer rate; a legitimate same-key retry must not be rejected as a + // different payment because the price ticked. Failure branches return + // ApplicationErrors, which the wrapper never caches, so first-attempt + // failures stay retryable. const outcome = await withPaymentIdempotency({ idempotencyKey, senderWalletId: routedWalletId, requestFingerprint: `lnurl|${lnurl}|${amount}`, execute: async () => { + const decoded = await Ibex.decodeLnurl({ lnurl }) + if (decoded instanceof IbexError) return decoded + if (!decoded.decodedLnurl) return new InvalidLnurlError() + + // A metadata-fetch rejection (non-2xx or network error) must become a + // typed error like every sibling branch — a bare throw here would + // propagate through the redlock callback as an unhandled GraphQL error + // instead of the failed payload. + let metadata: unknown + try { + const metadataResponse = await axios.get(decoded.decodedLnurl) + metadata = metadataResponse.data + } catch { + return new InvalidLnurlError() + } + if (!isLnurlPayMetadata(metadata)) return new InvalidLnurlError() + + const dealer = DealerPriceService() + const amountMsat = await amountMsatFromUsdWalletAmount({ + amount: walletAmount, + btcFromUsd: dealer.getSatsFromCentsForImmediateSell, + }) + if (amountMsat instanceof Error) return amountMsat + + const validAmount = validateLnurlPayAmountMsat({ + amountMsat, + minSendable: metadata.minSendable, + maxSendable: metadata.maxSendable, + }) + if (validAmount instanceof Error) return validAmount + const payment = await Ibex.payToLnurl({ accountId: routedWalletId, amountMsat, diff --git a/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts b/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts index dd5a0a131..288efb27a 100644 --- a/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts +++ b/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts @@ -50,6 +50,7 @@ jest.mock("axios", () => ({ })) import LnurlPaymentSendMutation from "@graphql/public/root/mutation/lnurl-payment-send" +import { IdempotencyKeyReuseError } from "@domain/errors" import { paymentAmountFromNumber, USDTAmount, WalletCurrency } from "@domain/shared" import { IbexError } from "@services/ibex/errors" @@ -228,6 +229,20 @@ describe("LnurlPaymentSendMutation", () => { expect(result?.errors[0].message).toBeTruthy() }) + it("returns a failed payload — not a bare GraphQL error — when the lnurl metadata fetch rejects", async () => { + // axios.get rejects on non-2xx and on network errors. Inside execute() a + // bare throw would propagate through the redlock callback as an unhandled + // GraphQL error, so the rejection must be caught and mapped like every + // sibling branch. + mockAxiosGet.mockRejectedValueOnce(new Error("connect ECONNREFUSED")) + + const result = await resolveMutation() + + expect(result?.status).toBe("failed") + expect(result?.errors[0].message).toBeTruthy() + expect(mockPayToLnurl).not.toHaveBeenCalled() + }) + describe("idempotency wiring (ENG-533)", () => { it("routes payToLnurl through withPaymentIdempotency, fingerprinting the request as sent", async () => { const result = (await resolveMutation({ @@ -253,7 +268,37 @@ describe("LnurlPaymentSendMutation", () => { })) as { status?: string } expect(result.status).toBe("success") + // The replay path the wrapper exists for is exactly the one where the + // lnurl server (the flaky dependency) may be down or the dealer rate may + // have moved — so a cache hit must short-circuit EVERY external call, + // not just the money-moving one. If decode or the metadata fetch runs + // here, a cached success can be masked as a fresh failure and the user + // re-sends with a new key: the double-pay class this PR closes. + expect(mockDecodeLnurl).not.toHaveBeenCalled() + expect(mockAxiosGet).not.toHaveBeenCalled() expect(mockPayToLnurl).not.toHaveBeenCalled() }) + + it("maps a wrapper error (key reuse / lock busy) to a failed payload", async () => { + mockWithPaymentIdempotency.mockResolvedValue( + new IdempotencyKeyReuseError("same key, different payment"), + ) + + const result = (await resolveMutation({ + idempotencyKey: "11111111-2222-4333-8444-555555555555", + })) as MutationResult + + expect(result.status).toBe("failed") + expect(result.errors.length).toBeGreaterThan(0) + expect(mockPayToLnurl).not.toHaveBeenCalled() + }) + + it("still executes without a key — absent key means passthrough, not rejection", async () => { + const result = (await resolveMutation()) as MutationResult + + expect(result.errors).toEqual([]) + expect(mockWithPaymentIdempotency.mock.calls[0][0].idempotencyKey).toBeUndefined() + expect(mockPayToLnurl).toHaveBeenCalledTimes(1) + }) }) }) From aac9a8bd483ef5a4980d2897feeb5a77397336ea Mon Sep 17 00:00:00 2001 From: Dread Date: Mon, 24 Aug 2026 13:17:03 -0700 Subject: [PATCH 3/3] fix(review): move wallet-amount conversion inside the idempotency wrapper; pin exact fingerprint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - usdWalletAmountFromWalletId (a Mongo read + three failure branches) ran after routing but outside execute(), contradicting the comment claiming everything after routing sits inside the wrapper. Moved it into execute() before the msat conversion — its errors are ApplicationErrors the wrapper never caches, so behavior is preserved — and the cached-replay spec now asserts it is never called on a replay. - Replaced the weak fingerprint assertions (toMatch(/^lnurl|/) + not.toContain("Msat"), which could not fail under the regression they guard) with the exact string match, matching the sibling noamount spec. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV --- .../root/mutation/lnurl-payment-send.ts | 22 ++++++++----------- .../root/mutation/lnurl-payment-send.spec.ts | 4 ++-- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/graphql/public/root/mutation/lnurl-payment-send.ts b/src/graphql/public/root/mutation/lnurl-payment-send.ts index 0170174d3..dc8b481b1 100644 --- a/src/graphql/public/root/mutation/lnurl-payment-send.ts +++ b/src/graphql/public/root/mutation/lnurl-payment-send.ts @@ -139,21 +139,11 @@ const LnurlPaymentSendMutation = GT.Field< } } - const walletAmount = await usdWalletAmountFromWalletId({ - walletId: routedWalletId, - amount: amount.toString(), - }) - if (walletAmount instanceof Error) { - return { - status: "failed", - errors: [mapAndParseErrorForGqlResponse(walletAmount)], - } - } - // ENG-533: direct-IBEX execution, so the exactly-once wrapper never ran on // this path. Scoped to the ROUTED wallet. EVERYTHING after routing — - // decode, metadata fetch, msat conversion, amount validation and the - // money-moving call — sits inside execute(), so a cached replay + // decode, metadata fetch, wallet-amount conversion, msat conversion, + // amount validation and the money-moving call — sits inside execute(), so + // a cached replay // short-circuits before touching IBEX or the lnurl server. That matters // precisely on the retry path this wrapper exists for: the flaky lnurl // server (or a moved dealer rate) must not be able to mask a cached @@ -185,6 +175,12 @@ const LnurlPaymentSendMutation = GT.Field< } if (!isLnurlPayMetadata(metadata)) return new InvalidLnurlError() + const walletAmount = await usdWalletAmountFromWalletId({ + walletId: routedWalletId, + amount: amount.toString(), + }) + if (walletAmount instanceof Error) return walletAmount + const dealer = DealerPriceService() const amountMsat = await amountMsatFromUsdWalletAmount({ amount: walletAmount, diff --git a/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts b/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts index 288efb27a..fb12a3696 100644 --- a/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts +++ b/test/flash/unit/graphql/public/root/mutation/lnurl-payment-send.spec.ts @@ -256,8 +256,7 @@ describe("LnurlPaymentSendMutation", () => { // The fingerprint uses the client's lnurl + input amount — NOT amountMsat, // which moves with the dealer rate. A legitimate same-key retry must not // be rejected as a different payment because the price ticked. - expect(call.requestFingerprint).toMatch(/^lnurl\|/) - expect(call.requestFingerprint).not.toContain("Msat") + expect(call.requestFingerprint).toBe("lnurl|LNURL1DP68GURN8GHJ7MRWW4EXCTN|19446") }) it("serves the wrapper's cached outcome without touching IBEX or the lnurl server", async () => { @@ -276,6 +275,7 @@ describe("LnurlPaymentSendMutation", () => { // re-sends with a new key: the double-pay class this PR closes. expect(mockDecodeLnurl).not.toHaveBeenCalled() expect(mockAxiosGet).not.toHaveBeenCalled() + expect(mockUsdWalletAmountFromWalletId).not.toHaveBeenCalled() expect(mockPayToLnurl).not.toHaveBeenCalled() })