From 1978b1a6281f24f48ed217b2519be841406dc91b Mon Sep 17 00:00:00 2001 From: Dread Date: Thu, 15 Oct 2026 09:28:00 -0700 Subject: [PATCH 1/5] feat(offers): add OfferAlreadyExecuted and map it to OFFER_ALREADY_EXECUTED A cashout offer is about to become single use (ENG-695). Add the typed error a second execution will get, and map it to a stable code the mobile app can branch on, with copy that is safe to show as is: "This offer has already been used. Request a new cashout offer." It extends CacheServiceError beside OfferNotFound, so it joins ApplicationErrors through the existing CacheErrors spread, and the exhaustive switch in error-map.ts maps it through the generic forward-to-client helper the Fygaro and bank-account errors use. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01XkM7Bm1cnzugSFi6pGnoWj --- src/domain/cache/errors.ts | 1 + src/graphql/error-map.ts | 5 +++++ test/flash/unit/graphql/error-map.spec.ts | 15 +++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/src/domain/cache/errors.ts b/src/domain/cache/errors.ts index 07fc888eb..022c1533a 100644 --- a/src/domain/cache/errors.ts +++ b/src/domain/cache/errors.ts @@ -10,3 +10,4 @@ export class UnknownCacheServiceError extends CacheServiceError { } export class OfferNotFound extends CacheServiceError {} +export class OfferAlreadyExecuted extends CacheServiceError {} diff --git a/src/graphql/error-map.ts b/src/graphql/error-map.ts index 2a4339adf..c57c2dda3 100644 --- a/src/graphql/error-map.ts +++ b/src/graphql/error-map.ts @@ -636,6 +636,11 @@ export const mapError = (error: ApplicationError): CustomApolloError => { message: "Offer not available. Try again.", logger: baseLogger, }) + case "OfferAlreadyExecuted": + return bridgeGqlError({ + code: "OFFER_ALREADY_EXECUTED", + message: "This offer has already been used. Request a new cashout offer.", + }) case "BridgeInvalidAmountError": message = diff --git a/test/flash/unit/graphql/error-map.spec.ts b/test/flash/unit/graphql/error-map.spec.ts index c3d1d4c8e..d468020fc 100644 --- a/test/flash/unit/graphql/error-map.spec.ts +++ b/test/flash/unit/graphql/error-map.spec.ts @@ -23,6 +23,7 @@ import { SendLimitsUnavailableError, } from "@domain/payments/errors" import { PaymentSendRateLimiterExceededError } from "@domain/rate-limit/errors" +import { OfferAlreadyExecuted, OfferNotFound } from "@domain/cache" describe("error-map", () => { it("maps PhoneNotAllowedForRegistrationError to a user-readable validation error, not the catch-all", () => { @@ -250,4 +251,18 @@ describe("error-map", () => { ) }) }) + + // ENG-695: a cashout offer executes at most once. The client branches on the + // code to ask for a new offer, so it must be stable, forwarded, and distinct + // from the expired/unknown-offer case. + it("maps OfferAlreadyExecuted to OFFER_ALREADY_EXECUTED with client-safe copy", () => { + const result = mapError(new OfferAlreadyExecuted()) + + expect(result.extensions.code).toBe("OFFER_ALREADY_EXECUTED") + expect(result.message).toBe( + "This offer has already been used. Request a new cashout offer.", + ) + expect(result.forwardToClient).toBe(true) + expect(mapError(new OfferNotFound()).extensions.code).toBe("NOT_FOUND") + }) }) From a3528d8c5b288a272a51d7cf952abdbfe5d0289a Mon Sep 17 00:00:00 2001 From: Dread Date: Thu, 15 Oct 2026 09:28:36 -0700 Subject: [PATCH 2/5] fix(offers): consume a cashout offer atomically before executing it executeCashout loaded the offer with a plain GET and never removed it, so every call with the same offerId inside the offer's one-hour TTL reached ValidOffer.execute again. Run against unmodified main, a sequential retry and a concurrent duplicate each drafted and submitted a second ERPNext cashout and called Ibex.payInvoice a second time; the only remaining guard was IBEX refusing an already-paid invoice. Storage.consume now takes the offer out of Redis with consumeCacheKey, one DEL whose removed-count is true for exactly one caller. It runs after revalidation, so a transient validation failure (an ERPNext bank-account lookup, an expired invoice) does not burn the offer, and before execute(), the only step that drafts and pays. The loser of a concurrent race gets OfferAlreadyExecuted and a failed ops event with step "consumeOffer". A Redis error on the DEL refuses the payout (fail closed) and fires the same event with the error name. Before the DEL, consume writes a short-lived tombstone (offers:executed:, same TTL as the offer). Storage.get reads it only when the offer record is missing, so a sequential retry gets OfferAlreadyExecuted instead of OfferNotFound, without reaching validation, ops events or IBEX. The tombstone is best effort and only makes the error specific; the DEL alone guarantees one execution. A consumed offer is never executed again, even if execute() fails before or after paying: the user requests a new offer, and partial failures stay manual-reconcile as before. The offer key and value shape do not change, so offers created before the deploy are consumed the same way. The integration cases turn off cashout.skipPayment (set in the dev config the suite loads) so the IBEX leg is on the path and its call count is evidence. The concurrent case holds both executions inside validation until both have read the offer, so the DEL decides the race. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01XkM7Bm1cnzugSFi6pGnoWj --- src/app/offers/CashoutManager.ts | 22 ++ src/app/offers/storage/Redis.ts | 78 ++++++- .../integration/offers/execute-offer.spec.ts | 87 +++++++- .../app/offers/cashout-manager-authz.spec.ts | 119 +++++++++++ .../offers/ops-events-cashout-manager.spec.ts | 64 +++++- .../app/offers/storage/redis-consume.spec.ts | 193 ++++++++++++++++++ 6 files changed, 555 insertions(+), 8 deletions(-) create mode 100644 test/flash/unit/app/offers/storage/redis-consume.spec.ts diff --git a/src/app/offers/CashoutManager.ts b/src/app/offers/CashoutManager.ts index 8831ff79c..a60a11264 100644 --- a/src/app/offers/CashoutManager.ts +++ b/src/app/offers/CashoutManager.ts @@ -179,6 +179,28 @@ const CashoutManager = { return validOffer } + // Single-use gate (ENG-695): exactly one execution per offer gets past it. + // It sits after validation, so an offer that fails a transient check (an + // ERPNext bank-account lookup, say) is not burned and can be retried. It + // sits before execute(), the only step that drafts the ERPNext cashout and + // pays IBEX. A Redis error here refuses the payout (fail closed). Once + // consumed, an offer is never executed again, even if execute() fails: the + // user needs a new offer, and a partial failure is reconciled by ops. + const consumed = await Storage.consume(id) + if (consumed instanceof Error) { + notifyOpsEvent({ + flow: "cashout", + phase: "failed", + status: "failed", + accountId: providedWallet.accountId, + amount: displayAmount, + step: "consumeOffer", + error: consumed.constructor.name, + meta: { offerId: id }, + }) + return consumed + } + const executedOffer = await validOffer.execute() if (executedOffer instanceof Error) return executedOffer else { diff --git a/src/app/offers/storage/Redis.ts b/src/app/offers/storage/Redis.ts index cd401da3d..273f57c6a 100644 --- a/src/app/offers/storage/Redis.ts +++ b/src/app/offers/storage/Redis.ts @@ -1,7 +1,13 @@ import { randomUUID } from "crypto" -import { CacheServiceError, CacheUndefinedError, OfferNotFound } from "@domain/cache" -import { RedisCacheService } from "@services/cache" +import { + CacheServiceError, + CacheUndefinedError, + OfferAlreadyExecuted, + OfferNotFound, +} from "@domain/cache" +import { RedisCacheService, consumeCacheKey } from "@services/cache" +import { baseLogger } from "@services/logger" import ValidOffer from "../ValidOffer" @@ -10,13 +16,38 @@ import { parseRepositoryError } from "../../../services/mongoose/utils" import { OffersSerde } from "./OffersSerde" import PersistedOffer from "./PersistedOffer" +const OFFER_TTL_SECS = 3600 as Seconds + +const offerKey = (id: OfferId): string => `offers:${id}` +// Left behind when an offer is consumed, so a later attempt to execute it is +// told the offer was already used rather than that it never existed. It is for +// error specificity only: the DEL in `consume` alone decides who executes. +const executedKey = (id: OfferId): string => `offers:executed:${id}` + +// The offer record is gone: it expired, never existed, or an execution consumed +// it. Only the last leaves a tombstone. +const missingOfferError = async ( + id: OfferId, +): Promise => { + const tombstone = await RedisCacheService().get({ key: executedKey(id) }) + if (tombstone instanceof CacheUndefinedError) return new OfferNotFound() + if (tombstone instanceof CacheServiceError) { + baseLogger.warn( + { offerId: id, err: tombstone }, + "Could not read the executed-offer tombstone; reporting the offer as not found", + ) + return new OfferNotFound() + } + return new OfferAlreadyExecuted() +} + const Redis = { add: async (o: ValidOffer): Promise => { const id = randomUUID() as OfferId // could use hash of offer details with getOrSet const result = await RedisCacheService().set({ - key: `offers:${id}`, + key: offerKey(id), value: OffersSerde.serialize(o.details), - ttlSecs: 3600 as Seconds, + ttlSecs: OFFER_TTL_SECS, }) if (result instanceof CacheServiceError) return result return new PersistedOffer(id, o.details) @@ -25,15 +56,50 @@ const Redis = { get: async (id: OfferId): Promise => { try { const result: string | CacheServiceError = await RedisCacheService().get({ - key: `offers:${id}`, + key: offerKey(id), }) - if (result instanceof CacheUndefinedError) return new OfferNotFound() + if (result instanceof CacheUndefinedError) return await missingOfferError(id) if (result instanceof CacheServiceError) return result else return new PersistedOffer(id, OffersSerde.deserialize(result)) } catch (err) { return parseRepositoryError(err) } }, + + /** + * Single-use gate for executing an offer: takes the record out of Redis with + * one DEL, so exactly one caller per offer gets `true`. A concurrent + * duplicate gets OfferAlreadyExecuted. A Redis error is returned as is, and + * the caller must not execute an offer it could not consume (fail closed). + */ + consume: async ( + id: OfferId, + ): Promise => { + // The tombstone goes first, so no retry can land between a won DEL and the + // tombstone write. It is best effort. If it fails, a later retry sees + // OfferNotFound instead of OfferAlreadyExecuted, and nothing is paid twice + // either way. A tombstone left by a caller whose DEL then fails or loses is + // harmless, because `get` reads it only once the offer record is gone. + const tombstone = await RedisCacheService().set({ + key: executedKey(id), + value: "1", + ttlSecs: OFFER_TTL_SECS, + }) + if (tombstone instanceof CacheServiceError) { + baseLogger.warn( + { offerId: id, err: tombstone }, + "Could not write the executed-offer tombstone; consuming the offer anyway", + ) + } + + const consumed = await consumeCacheKey({ key: offerKey(id) }) + if (consumed instanceof CacheServiceError) return consumed + // The caller read this offer moments ago, so a key that is already gone was + // removed by a concurrent execution of the same offer (or, at the very edge + // of its TTL, expired). Either way, this caller must not execute it. + if (!consumed) return new OfferAlreadyExecuted() + return true + }, } export default Redis diff --git a/test/flash/integration/offers/execute-offer.spec.ts b/test/flash/integration/offers/execute-offer.spec.ts index 77f1b77ac..e6e33b985 100644 --- a/test/flash/integration/offers/execute-offer.spec.ts +++ b/test/flash/integration/offers/execute-offer.spec.ts @@ -1,7 +1,10 @@ import CashoutManager from "@app/offers/CashoutManager" -import { JMDAmount, USDAmount } from "@domain/shared" +import { Cashout } from "@config" +import { OfferAlreadyExecuted } from "@domain/cache" +import { JMDAmount, USDAmount, ValidationError } from "@domain/shared" import ErpNext, { CashoutId } from "@services/frappe/ErpNext" +import { BankAccountQueryError } from "@services/frappe/errors" import Ibex from "@services/ibex/client" import { alice } from "../jest.setup" @@ -109,4 +112,86 @@ describe("Offers", () => { // make assertions against ledger expect(status).toBeDefined() }) + + describe("single use per offerId", () => { + // dev/config/base-config.yaml sets cashout.skipPayment, which skips the + // IBEX leg of ValidOffer.execute entirely. Put it back on the path so the + // payInvoice call count below is evidence of what a duplicate would pay. + let skipPayment: jest.ReplaceProperty + beforeEach(() => { + skipPayment = jest.replaceProperty(Cashout, "SkipPayment", false) + }) + afterEach(() => { + skipPayment.restore() + }) + + const createOffer = async () => { + const offer = await CashoutManager.createOffer( + alice.usdWalletD.id, + send, + "", + alice.account.id, + ) + if (offer instanceof Error) throw offer + return offer + } + + const executeOffer = (id: OfferId) => + CashoutManager.executeCashout(id, alice.usdWalletD.id, alice.account.id) + + it("rejects a second execution of the same offer before ERPNext or IBEX", async () => { + const { id } = await createOffer() + + const first = await executeOffer(id) + const second = await executeOffer(id) + + // the money-moving side effects ran for the first call only + expect(mockedErpNext.draftCashout).toHaveBeenCalledTimes(1) + expect(mockedIbex.payInvoice).toHaveBeenCalledTimes(1) + expect(first).not.toBeInstanceOf(Error) + expect(second).toBeInstanceOf(OfferAlreadyExecuted) + }) + + it("lets exactly one of two concurrent executions through", async () => { + const { id } = await createOffer() + + // Hold both executions inside offer validation, after Storage.get and + // before the consume gate, until both have arrived. Both callers have + // then read the offer, so only the atomic consume can stop the second. + let arrived = 0 + let releaseBoth: () => void = () => undefined + const bothArrived = new Promise((resolve) => { + releaseBoth = resolve + }) + mockedErpNext.getBankAccountsByCustomer.mockImplementation(async () => { + arrived += 1 + if (arrived === 2) releaseBoth() + await bothArrived + return [bankAccount] + }) + + const results = await Promise.all([executeOffer(id), executeOffer(id)]) + + expect(arrived).toBe(2) + expect(mockedErpNext.draftCashout).toHaveBeenCalledTimes(1) + expect(mockedIbex.payInvoice).toHaveBeenCalledTimes(1) + expect(results.filter((r) => !(r instanceof Error))).toHaveLength(1) + expect(results.filter((r) => r instanceof OfferAlreadyExecuted)).toHaveLength(1) + }) + + it("keeps an offer whose execution fails validation, so a retry can run it", async () => { + const { id } = await createOffer() + mockedErpNext.getBankAccountsByCustomer.mockResolvedValueOnce( + new BankAccountQueryError("ERPNext unavailable"), + ) + + const failed = await executeOffer(id) + const retried = await executeOffer(id) + + expect(failed).toBeInstanceOf(ValidationError) + expect(retried).not.toBeInstanceOf(Error) + expect(mockedErpNext.draftCashout).toHaveBeenCalledTimes(1) + expect(mockedIbex.payInvoice).toHaveBeenCalledTimes(1) + }) + }) }) diff --git a/test/flash/unit/app/offers/cashout-manager-authz.spec.ts b/test/flash/unit/app/offers/cashout-manager-authz.spec.ts index 1d6e52439..5b247c64b 100644 --- a/test/flash/unit/app/offers/cashout-manager-authz.spec.ts +++ b/test/flash/unit/app/offers/cashout-manager-authz.spec.ts @@ -1,5 +1,6 @@ const mockStorageGet = jest.fn() const mockStorageAdd = jest.fn() +const mockStorageConsume = jest.fn() const mockFindWalletById = jest.fn() const mockFindAccountById = jest.fn() const mockValidOfferFrom = jest.fn() @@ -59,6 +60,7 @@ jest.mock("@app/offers/storage/Redis", () => ({ default: { get: (...args: unknown[]) => mockStorageGet(...args), add: (...args: unknown[]) => mockStorageAdd(...args), + consume: (...args: unknown[]) => mockStorageConsume(...args), }, })) @@ -91,6 +93,7 @@ jest.mock("@domain/bitcoin/lightning", () => { }) import CashoutManager from "@app/offers/CashoutManager" +import { OfferAlreadyExecuted, UnknownCacheServiceError } from "@domain/cache" import { USDAmount, ValidationError } from "@domain/shared" const offerId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" as OfferId @@ -126,6 +129,7 @@ describe("CashoutManager authorization", () => { }, }) mockStorageAdd.mockResolvedValue({ id: offerId, details: {} }) + mockStorageConsume.mockResolvedValue(true) }) describe("createOffer", () => { @@ -198,4 +202,119 @@ describe("CashoutManager authorization", () => { expect(mockValidOfferFrom).toHaveBeenCalled() }) }) + + describe("executeCashout single-use gate", () => { + const mockExecute = jest.fn() + + beforeEach(() => { + mockExecute.mockResolvedValue({ erpSubmitted: true, cashoutId: "c-1" }) + mockValidOfferFrom.mockResolvedValue({ details: {}, execute: mockExecute }) + }) + + it("consumes the offer, then executes it", async () => { + // Given the default fixtures (consume wins) + + // When the caller executes their offer + const result = await CashoutManager.executeCashout( + offerId, + walletId, + callerAccountId, + ) + + // Then the offer is consumed before it is executed + expect(result).not.toBeInstanceOf(Error) + expect(mockStorageConsume).toHaveBeenCalledTimes(1) + expect(mockStorageConsume).toHaveBeenCalledWith(offerId) + expect(mockExecute).toHaveBeenCalledTimes(1) + expect(mockStorageConsume.mock.invocationCallOrder[0]).toBeLessThan( + mockExecute.mock.invocationCallOrder[0], + ) + }) + + it("returns OfferAlreadyExecuted without executing when the consume is lost", async () => { + // Given a concurrent execution of the same offer consumed it first + const alreadyExecuted = new OfferAlreadyExecuted() + mockStorageConsume.mockResolvedValue(alreadyExecuted) + + // When the caller executes the offer + const result = await CashoutManager.executeCashout( + offerId, + walletId, + callerAccountId, + ) + + // Then nothing is drafted or paid and the typed error comes back + expect(result).toBe(alreadyExecuted) + expect(mockExecute).not.toHaveBeenCalled() + }) + + it("does not execute when the consume fails on a Redis error", async () => { + // Given Redis fails the DEL + const redisDown = new UnknownCacheServiceError("connection reset") + mockStorageConsume.mockResolvedValue(redisDown) + + // When the caller executes the offer + const result = await CashoutManager.executeCashout( + offerId, + walletId, + callerAccountId, + ) + + // Then the payout is refused (fail closed) + expect(result).toBe(redisDown) + expect(mockExecute).not.toHaveBeenCalled() + }) + + it("does not consume an offer that fails validation", async () => { + // Given the offer no longer validates (e.g. ERPNext bank lookup failed) + mockValidOfferFrom.mockResolvedValue(new ValidationError("bank lookup failed")) + + // When the caller executes the offer + const result = await CashoutManager.executeCashout( + offerId, + walletId, + callerAccountId, + ) + + // Then the offer stays available for a retry + expect(result).toBeInstanceOf(ValidationError) + expect(mockStorageConsume).not.toHaveBeenCalled() + }) + + it("does not consume when the presented wallet belongs to another account", async () => { + // Given a caller presenting someone else's wallet + mockFindWalletById.mockResolvedValue({ id: walletId, accountId: otherAccountId }) + + // When the caller executes the offer + const result = await CashoutManager.executeCashout( + offerId, + walletId, + callerAccountId, + ) + + // Then it is rejected and the owner's offer is left intact + expect(result).toBeInstanceOf(ValidationError) + expect(mockStorageConsume).not.toHaveBeenCalled() + }) + + it("does not consume another account's offer presented with the caller's own wallet", async () => { + // Given the caller's own wallet, but an offer that settles from a wallet + // on another account + mockFindWalletById + .mockResolvedValueOnce({ id: walletId, accountId: callerAccountId }) + .mockResolvedValueOnce({ id: walletId, accountId: otherAccountId }) + + // When the caller executes that offer + const result = await CashoutManager.executeCashout( + offerId, + walletId, + callerAccountId, + ) + + // Then it is rejected, so the caller cannot burn the owner's offer + expect(result).toBeInstanceOf(ValidationError) + expect(mockStorageConsume).not.toHaveBeenCalled() + expect(mockExecute).not.toHaveBeenCalled() + }) + }) }) diff --git a/test/flash/unit/app/offers/ops-events-cashout-manager.spec.ts b/test/flash/unit/app/offers/ops-events-cashout-manager.spec.ts index e2b1ab6f9..42140d362 100644 --- a/test/flash/unit/app/offers/ops-events-cashout-manager.spec.ts +++ b/test/flash/unit/app/offers/ops-events-cashout-manager.spec.ts @@ -1,4 +1,5 @@ const mockStorageGet = jest.fn() +const mockStorageConsume = jest.fn() const mockFindWalletById = jest.fn() const mockValidOfferFrom = jest.fn() @@ -43,7 +44,10 @@ jest.mock("@services/mongoose", () => ({ jest.mock("@app/offers/storage/Redis", () => ({ __esModule: true, - default: { get: (...args: unknown[]) => mockStorageGet(...args) }, + default: { + get: (...args: unknown[]) => mockStorageGet(...args), + consume: (...args: unknown[]) => mockStorageConsume(...args), + }, })) jest.mock("@app/offers/ValidOffer", () => ({ @@ -53,6 +57,7 @@ jest.mock("@app/offers/ValidOffer", () => ({ import CashoutManager from "@app/offers/CashoutManager" import { EmailService } from "@services/email" +import { OfferAlreadyExecuted, UnknownCacheServiceError } from "@domain/cache" import { USDAmount } from "@domain/shared" import { notifyOpsEvent } from "@services/alerts/ops-events" @@ -74,6 +79,7 @@ describe("ops events — CashoutManager.executeCashout", () => { beforeEach(() => { jest.clearAllMocks() mockStorageGet.mockResolvedValue(makeOffer()) + mockStorageConsume.mockResolvedValue(true) mockFindWalletById.mockResolvedValue({ id: walletId, accountId }) }) @@ -172,6 +178,62 @@ describe("ops events — CashoutManager.executeCashout", () => { ) }) + it("notifies a terminal failed event when a concurrent execution consumed the offer", async () => { + const execute = jest.fn() + mockValidOfferFrom.mockResolvedValue({ execute }) + mockStorageConsume.mockResolvedValue(new OfferAlreadyExecuted()) + + const result = await CashoutManager.executeCashout(offerId, walletId, accountId) + + expect(result).toBeInstanceOf(OfferAlreadyExecuted) + expect(execute).not.toHaveBeenCalled() + expect(EmailService.sendCashoutInitiatedEmail).not.toHaveBeenCalled() + expect(notifyOpsEvent).toHaveBeenCalledTimes(2) + expect(notifyOpsEvent).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ flow: "cashout", phase: "initiated" }), + ) + expect(notifyOpsEvent).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + flow: "cashout", + phase: "failed", + status: "failed", + accountId, + amount: { value: "500.00", currency: "USD" }, + step: "consumeOffer", + error: "OfferAlreadyExecuted", + meta: { offerId }, + }), + ) + expect(notifyOpsEvent).not.toHaveBeenCalledWith( + expect.objectContaining({ phase: "succeeded" }), + ) + }) + + it("notifies a terminal failed event when Redis refuses the payout", async () => { + const execute = jest.fn() + mockValidOfferFrom.mockResolvedValue({ execute }) + mockStorageConsume.mockResolvedValue(new UnknownCacheServiceError("connection reset")) + + const result = await CashoutManager.executeCashout(offerId, walletId, accountId) + + expect(result).toBeInstanceOf(UnknownCacheServiceError) + expect(execute).not.toHaveBeenCalled() + expect(notifyOpsEvent).toHaveBeenCalledTimes(2) + expect(notifyOpsEvent).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + flow: "cashout", + phase: "failed", + status: "failed", + step: "consumeOffer", + error: "UnknownCacheServiceError", + meta: { offerId }, + }), + ) + }) + it("does not notify when the caller wallet does not own the offer", async () => { mockFindWalletById .mockResolvedValueOnce({ id: walletId, accountId: "other-account" }) diff --git a/test/flash/unit/app/offers/storage/redis-consume.spec.ts b/test/flash/unit/app/offers/storage/redis-consume.spec.ts new file mode 100644 index 000000000..bc6c2611f --- /dev/null +++ b/test/flash/unit/app/offers/storage/redis-consume.spec.ts @@ -0,0 +1,193 @@ +// ENG-695: an offer is executed at most once. Storage.consume takes the offer +// record out of Redis with a single DEL (consumeCacheKey); a tombstone left +// beside it lets a later attempt be told the offer was already used. + +const mockCacheSet = jest.fn() +const mockCacheGet = jest.fn() +const mockConsumeCacheKey = jest.fn() +const mockLoggerWarn = jest.fn() + +jest.mock("@services/cache", () => ({ + RedisCacheService: () => ({ + set: (...args: unknown[]) => mockCacheSet(...args), + get: (...args: unknown[]) => mockCacheGet(...args), + }), + consumeCacheKey: (...args: unknown[]) => mockConsumeCacheKey(...args), +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { + info: jest.fn(), + warn: (...args: unknown[]) => mockLoggerWarn(...args), + error: jest.fn(), + }, +})) + +import PersistedOffer from "@app/offers/storage/PersistedOffer" +import { OffersSerde } from "@app/offers/storage/OffersSerde" +import Storage from "@app/offers/storage/Redis" +import { CashoutDetails } from "@app/offers/types" +import { + CacheUndefinedError, + OfferAlreadyExecuted, + OfferNotFound, + UnknownCacheServiceError, +} from "@domain/cache" +import { USDAmount } from "@domain/shared" + +const offerId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" as OfferId +const offerKey = `offers:${offerId}` +const executedKey = `offers:executed:${offerId}` + +const usd = (cents: string) => { + const amount = USDAmount.cents(cents) + if (amount instanceof Error) throw amount + return amount +} + +const details: CashoutDetails = { + payment: { + userAcct: "22222222-2222-4222-8222-222222222222" as WalletId, + flashAcct: "44444444-4444-4444-8444-444444444444" as WalletId, + invoice: { + paymentRequest: "lnbc1test" as Bolt11, + expiresAt: new Date(Date.now() + 60_000), + } as unknown as LnInvoice, + amount: usd("100"), + }, + payout: { + bankAccountId: "12345 - First Global", + amount: usd("98"), + serviceFee: usd("2"), + }, +} + +// Serve RedisCacheService().get from a key -> value map; a missing key reads as +// CacheUndefinedError, which is what the real service returns on a miss. +const stubRedis = (entries: Record) => + mockCacheGet.mockImplementation(async ({ key }: { key: string }) => + key in entries ? entries[key] : new CacheUndefinedError(), + ) + +describe("offer storage: single-use consume", () => { + beforeEach(() => { + jest.clearAllMocks() + mockCacheSet.mockImplementation(async ({ value }: { value: unknown }) => value) + mockConsumeCacheKey.mockResolvedValue(true) + }) + + describe("consume", () => { + it("returns true for the caller whose DEL removed the offer", async () => { + const result = await Storage.consume(offerId) + + expect(result).toBe(true) + expect(mockConsumeCacheKey).toHaveBeenCalledTimes(1) + expect(mockConsumeCacheKey).toHaveBeenCalledWith({ key: offerKey }) + }) + + it("returns OfferAlreadyExecuted when a concurrent execution removed the offer first", async () => { + mockConsumeCacheKey.mockResolvedValue(false) + + const result = await Storage.consume(offerId) + + expect(result).toBeInstanceOf(OfferAlreadyExecuted) + }) + + it("returns a Redis error on the DEL as is, so the caller fails closed", async () => { + const redisDown = new UnknownCacheServiceError("connection reset") + mockConsumeCacheKey.mockResolvedValue(redisDown) + + const result = await Storage.consume(offerId) + + expect(result).toBe(redisDown) + }) + + it("writes the tombstone, with the offer TTL, before the DEL", async () => { + await Storage.consume(offerId) + + expect(mockCacheSet).toHaveBeenCalledTimes(1) + expect(mockCacheSet).toHaveBeenCalledWith({ + key: executedKey, + value: "1", + ttlSecs: 3600, + }) + expect(mockCacheSet.mock.invocationCallOrder[0]).toBeLessThan( + mockConsumeCacheKey.mock.invocationCallOrder[0], + ) + }) + + it("still consumes the offer, and logs, when the tombstone write fails", async () => { + mockCacheSet.mockResolvedValue(new UnknownCacheServiceError("timeout")) + + const result = await Storage.consume(offerId) + + expect(result).toBe(true) + expect(mockConsumeCacheKey).toHaveBeenCalledWith({ key: offerKey }) + expect(mockLoggerWarn).toHaveBeenCalledWith( + expect.objectContaining({ offerId, err: expect.any(UnknownCacheServiceError) }), + expect.any(String), + ) + }) + }) + + describe("get", () => { + it("returns the stored offer without reading the tombstone", async () => { + stubRedis({ [offerKey]: OffersSerde.serialize(details) }) + + const result = await Storage.get(offerId) + + expect(result).toBeInstanceOf(PersistedOffer) + if (!(result instanceof PersistedOffer)) throw result + expect(result.id).toBe(offerId) + expect(result.details.payout.bankAccountId).toBe(details.payout.bankAccountId) + expect(mockCacheGet).toHaveBeenCalledTimes(1) + expect(mockCacheGet).toHaveBeenCalledWith({ key: offerKey }) + }) + + it("returns OfferAlreadyExecuted when the offer is gone and its tombstone is present", async () => { + stubRedis({ [executedKey]: "1" }) + + const result = await Storage.get(offerId) + + expect(result).toBeInstanceOf(OfferAlreadyExecuted) + expect(mockCacheGet).toHaveBeenCalledWith({ key: executedKey }) + }) + + it("returns OfferNotFound when the offer is gone and there is no tombstone", async () => { + stubRedis({}) + + const result = await Storage.get(offerId) + + expect(result).toBeInstanceOf(OfferNotFound) + expect(result).not.toBeInstanceOf(OfferAlreadyExecuted) + }) + + it("returns OfferNotFound, and logs, when the tombstone cannot be read", async () => { + stubRedis({ [executedKey]: new UnknownCacheServiceError("timeout") }) + + const result = await Storage.get(offerId) + + expect(result).toBeInstanceOf(OfferNotFound) + expect(mockLoggerWarn).toHaveBeenCalledWith( + expect.objectContaining({ offerId }), + expect.any(String), + ) + }) + }) + + describe("add", () => { + it("stores the offer under offers: with the one-hour TTL", async () => { + const validOffer = { details } as Parameters[0] + + const result = await Storage.add(validOffer) + + expect(result).toBeInstanceOf(PersistedOffer) + if (!(result instanceof PersistedOffer)) throw result + expect(mockCacheSet).toHaveBeenCalledWith({ + key: `offers:${result.id}`, + value: OffersSerde.serialize(details), + ttlSecs: 3600, + }) + }) + }) +}) From 43bb1884dc460f81b283e157e7b0c940d4fcc407 Mon Sep 17 00:00:00 2001 From: Dread Date: Thu, 15 Oct 2026 09:28:36 -0700 Subject: [PATCH 3/5] fix(graphql): return OFFER_ALREADY_EXECUTED from initiateCashout The resolver turned every executeCashout error into a bare InternalServerError and recorded it as a Critical exception, so mapping OfferAlreadyExecuted in error-map.ts alone would never reach a client. Return it as { errors: [{ code: "OFFER_ALREADY_EXECUTED", ... }], id: null } instead, and keep it out of the Critical exception stream: a repeated or concurrent execution of one offer is expected, not a fault. The client can branch on the code and request a new offer. Every other error still takes the existing catch-all, unchanged; the new resolver spec pins that so this change cannot widen it by accident. No schema change: InitiatedCashoutResponse already has errors and a nullable id, and Error already carries code. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01XkM7Bm1cnzugSFi6pGnoWj --- .../root/mutation/offers/initiate-cash-out.ts | 7 ++ .../root/mutation/initiate-cash-out.spec.ts | 101 ++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts diff --git a/src/graphql/public/root/mutation/offers/initiate-cash-out.ts b/src/graphql/public/root/mutation/offers/initiate-cash-out.ts index 450271fc8..3faf62c00 100644 --- a/src/graphql/public/root/mutation/offers/initiate-cash-out.ts +++ b/src/graphql/public/root/mutation/offers/initiate-cash-out.ts @@ -1,8 +1,10 @@ import CashoutManager from "@app/offers/CashoutManager" import { Cashout } from "@config" +import { OfferAlreadyExecuted } from "@domain/cache" import { NotImplementedError } from "@domain/errors" import { ErrorLevel } from "@domain/shared" import { InternalServerError } from "@graphql/error" +import { mapToGqlErrorList } from "@graphql/error-map" import { GT } from "@graphql/index" import IError from "@graphql/shared/types/abstract/error" import WalletId from "@graphql/shared/types/scalar/wallet-id" @@ -54,6 +56,11 @@ const InitiateCashoutMutation = GT.Field({ } const offer = await CashoutManager.executeCashout(offerId, walletId, domainAccount.id) + // Expected, not a fault: this offer was already executed, by an earlier + // call or a concurrent duplicate. Return the typed code so the client can + // ask for a new offer, and keep it out of the Critical exception stream. + if (offer instanceof OfferAlreadyExecuted) + return { errors: mapToGqlErrorList(offer), id: null } if (offer instanceof Error) { recordExceptionInCurrentSpan({ error: offer, diff --git a/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts b/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts new file mode 100644 index 000000000..1f2e262aa --- /dev/null +++ b/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts @@ -0,0 +1,101 @@ +// ENG-695: initiateCashout answers a second execution of the same offer with a +// typed OFFER_ALREADY_EXECUTED error the client can branch on, not the generic +// server error, and keeps it out of the Critical exception stream. + +const mockExecuteCashout = jest.fn() +const mockRecordException = jest.fn() + +jest.mock("@app/offers/CashoutManager", () => ({ + __esModule: true, + default: { + executeCashout: (...args: unknown[]) => mockExecuteCashout(...args), + }, +})) + +jest.mock("@config", () => ({ + Cashout: { Enabled: true }, + // read at module load by @graphql/error + getOnChainWalletConfig: () => ({ dustThreshold: 546 }), +})) + +jest.mock("@services/tracing", () => ({ + recordExceptionInCurrentSpan: (...args: unknown[]) => mockRecordException(...args), +})) + +jest.mock("@services/logger", () => ({ + baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})) + +import { OfferAlreadyExecuted, OfferNotFound } from "@domain/cache" +import { ErrorLevel } from "@domain/shared" +import { InternalServerError } from "@graphql/error" +import InitiateCashoutMutation from "@graphql/public/root/mutation/offers/initiate-cash-out" + +const offerId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" +const walletId = "11111111-1111-4111-8111-111111111111" +const accountId = "64df1a2b3c4d5e6f78901234" as AccountId +const context = { domainAccount: { id: accountId } } + +type Resolvable = { + resolve: ( + source: unknown, + args: { input: Record }, + context: unknown, + info: never, + ) => Promise +} + +const initiateCashout = () => + (InitiateCashoutMutation as unknown as Resolvable).resolve( + null, + { input: { offerId, walletId } }, + context, + {} as never, + ) + +describe("initiateCashout resolver", () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it("returns the cashout id when the offer executes", async () => { + mockExecuteCashout.mockResolvedValue({ cashoutId: "ACC-CSH-2026-00001" }) + + const result = await initiateCashout() + + expect(mockExecuteCashout).toHaveBeenCalledWith(offerId, walletId, accountId) + expect(result).toEqual({ errors: [], id: "ACC-CSH-2026-00001" }) + expect(mockRecordException).not.toHaveBeenCalled() + }) + + it("returns OFFER_ALREADY_EXECUTED and no id when the offer was already executed", async () => { + mockExecuteCashout.mockResolvedValue(new OfferAlreadyExecuted()) + + const result = await initiateCashout() + + expect(result).toEqual({ + errors: [ + expect.objectContaining({ + code: "OFFER_ALREADY_EXECUTED", + message: "This offer has already been used. Request a new cashout offer.", + }), + ], + id: null, + }) + expect(mockRecordException).not.toHaveBeenCalled() + }) + + // Pins the unchanged catch-all for every other failure. Turning those into + // typed { errors, id } payloads is a separate follow-up, not ENG-695. + it("keeps every other failure on the generic server-error path", async () => { + const notFound = new OfferNotFound() + mockExecuteCashout.mockResolvedValue(notFound) + + const result = await initiateCashout() + + expect(result).toBeInstanceOf(InternalServerError) + expect(mockRecordException).toHaveBeenCalledWith( + expect.objectContaining({ error: notFound, level: ErrorLevel.Critical }), + ) + }) +}) From a527e0d48d3af6023226eb3008cfeaf5b9ba31d8 Mon Sep 17 00:00:00 2001 From: Dread Date: Thu, 15 Oct 2026 09:35:42 -0700 Subject: [PATCH 4/5] test(offers): fail the concurrent-execution case fast when a caller never arrives The concurrent case holds both executions inside validation until both have read the offer. If a regression stopped one of them earlier (for example, consuming the offer before validation), the other waited on the barrier until the 30 s jest timeout. A 5 s safety release now lets it through, so the test fails on the arrived count (expected 2, received 1) with a clear message. The timer is cleared when both executions settle, and it cannot make the race pass falsely: a caller that arrives only after the release still had to read the offer first. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01XkM7Bm1cnzugSFi6pGnoWj --- test/flash/integration/offers/execute-offer.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/flash/integration/offers/execute-offer.spec.ts b/test/flash/integration/offers/execute-offer.spec.ts index e6e33b985..d22f0f02e 100644 --- a/test/flash/integration/offers/execute-offer.spec.ts +++ b/test/flash/integration/offers/execute-offer.spec.ts @@ -158,11 +158,15 @@ describe("Offers", () => { // Hold both executions inside offer validation, after Storage.get and // before the consume gate, until both have arrived. Both callers have // then read the offer, so only the atomic consume can stop the second. + // If a regression stops one execution before validation, the timer lets + // the other through, so the arrived count fails the test rather than the + // jest timeout. let arrived = 0 let releaseBoth: () => void = () => undefined const bothArrived = new Promise((resolve) => { releaseBoth = resolve }) + const safetyRelease = setTimeout(() => releaseBoth(), 5_000) mockedErpNext.getBankAccountsByCustomer.mockImplementation(async () => { arrived += 1 if (arrived === 2) releaseBoth() @@ -170,7 +174,9 @@ describe("Offers", () => { return [bankAccount] }) - const results = await Promise.all([executeOffer(id), executeOffer(id)]) + const results = await Promise.all([executeOffer(id), executeOffer(id)]).finally( + () => clearTimeout(safetyRelease), + ) expect(arrived).toBe(2) expect(mockedErpNext.draftCashout).toHaveBeenCalledTimes(1) From 87c887215a9624b186df1e493b5d61e3f9df9f25 Mon Sep 17 00:00:00 2001 From: Dread Date: Thu, 15 Oct 2026 10:23:32 -0700 Subject: [PATCH 5/5] fix(offers): keep the offer tombstone out of client reach; retry-safe copy Review fixes for ENG-695 (PR #528). - Storage: move the consumed-offer tombstone from offers:executed: to offer-executed:. initiateCashout passes the client's offerId straight to Storage.get, so the id "executed:" read the tombstone back as a PersistedOffer whose details were 1, and executeCashout threw a TypeError instead of returning a typed error. Storage.get and Storage.consume now also answer any id that is not a UUID with OfferNotFound, before a key is built. - OFFER_ALREADY_EXECUTED copy: the app shows the message as is, and the error mostly follows a retry after a lost response, when the first cashout went through. "Request a new cashout offer" invited a second withdrawal and a second fee. The new copy sends the user to their transactions first and stays true when the first execution failed before any money moved. - Integration: an offer whose execute() fails at IBEX (timeout) is not executed again on retry (one draftCashout, one payInvoice), and an offerId that names a tombstone returns OfferNotFound. Unit: over an in-memory Redis with ioredis-cache's JSON encoding, the forged id gets OfferNotFound and nothing but offer records stays under offers:; non-UUID ids never reach Redis. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01XkM7Bm1cnzugSFi6pGnoWj --- src/app/offers/storage/Redis.ts | 15 +++- src/graphql/error-map.ts | 7 +- .../integration/offers/execute-offer.spec.ts | 34 +++++++- .../app/offers/storage/redis-consume.spec.ts | 77 ++++++++++++++++++- test/flash/unit/graphql/error-map.spec.ts | 6 +- .../root/mutation/initiate-cash-out.spec.ts | 3 +- 6 files changed, 133 insertions(+), 9 deletions(-) diff --git a/src/app/offers/storage/Redis.ts b/src/app/offers/storage/Redis.ts index 273f57c6a..21cb91cb6 100644 --- a/src/app/offers/storage/Redis.ts +++ b/src/app/offers/storage/Redis.ts @@ -6,6 +6,7 @@ import { OfferAlreadyExecuted, OfferNotFound, } from "@domain/cache" +import { UuidRegex } from "@domain/shared" import { RedisCacheService, consumeCacheKey } from "@services/cache" import { baseLogger } from "@services/logger" @@ -21,8 +22,14 @@ const OFFER_TTL_SECS = 3600 as Seconds const offerKey = (id: OfferId): string => `offers:${id}` // Left behind when an offer is consumed, so a later attempt to execute it is // told the offer was already used rather than that it never existed. It is for -// error specificity only: the DEL in `consume` alone decides who executes. -const executedKey = (id: OfferId): string => `offers:executed:${id}` +// error specificity only: the DEL in `consume` alone decides who executes. It +// sits outside the offers: prefix, so no offer id can make offerKey name it. +const executedKey = (id: OfferId): string => `offer-executed:${id}` + +// `add` mints every offer id with randomUUID(). The id `get` and `consume` +// receive is client input (initiateCashout's offerId), so anything that is not +// a UUID is refused before it becomes part of a Redis key. +const isOfferId = (id: OfferId): boolean => UuidRegex.test(id) // The offer record is gone: it expired, never existed, or an execution consumed // it. Only the last leaves a tombstone. @@ -54,6 +61,7 @@ const Redis = { }, get: async (id: OfferId): Promise => { + if (!isOfferId(id)) return new OfferNotFound() try { const result: string | CacheServiceError = await RedisCacheService().get({ key: offerKey(id), @@ -71,10 +79,13 @@ const Redis = { * one DEL, so exactly one caller per offer gets `true`. A concurrent * duplicate gets OfferAlreadyExecuted. A Redis error is returned as is, and * the caller must not execute an offer it could not consume (fail closed). + * An id that is not a UUID names no offer: OfferNotFound, and no Redis call. */ consume: async ( id: OfferId, ): Promise => { + if (!isOfferId(id)) return new OfferNotFound() + // The tombstone goes first, so no retry can land between a won DEL and the // tombstone write. It is best effort. If it fails, a later retry sees // OfferNotFound instead of OfferAlreadyExecuted, and nothing is paid twice diff --git a/src/graphql/error-map.ts b/src/graphql/error-map.ts index c57c2dda3..c9e7bdf67 100644 --- a/src/graphql/error-map.ts +++ b/src/graphql/error-map.ts @@ -637,9 +637,14 @@ export const mapError = (error: ApplicationError): CustomApolloError => { logger: baseLogger, }) case "OfferAlreadyExecuted": + // Mostly a retry after a lost response, so the first execution may have + // paid, and the client has no cashout id to check (ENG-716). The copy + // sends the user to their transactions before a new cashout, and stays + // true when that first execution failed before any money moved. return bridgeGqlError({ code: "OFFER_ALREADY_EXECUTED", - message: "This offer has already been used. Request a new cashout offer.", + message: + "This cashout offer was already submitted and can't be reused. Check your transactions before starting a new cashout.", }) case "BridgeInvalidAmountError": diff --git a/test/flash/integration/offers/execute-offer.spec.ts b/test/flash/integration/offers/execute-offer.spec.ts index d22f0f02e..357f0dded 100644 --- a/test/flash/integration/offers/execute-offer.spec.ts +++ b/test/flash/integration/offers/execute-offer.spec.ts @@ -1,11 +1,12 @@ import CashoutManager from "@app/offers/CashoutManager" import { Cashout } from "@config" -import { OfferAlreadyExecuted } from "@domain/cache" +import { OfferAlreadyExecuted, OfferNotFound } from "@domain/cache" import { JMDAmount, USDAmount, ValidationError } from "@domain/shared" import ErpNext, { CashoutId } from "@services/frappe/ErpNext" import { BankAccountQueryError } from "@services/frappe/errors" import Ibex from "@services/ibex/client" +import { IbexError, UnexpectedIbexResponse } from "@services/ibex/errors" import { alice } from "../jest.setup" @@ -199,5 +200,36 @@ describe("Offers", () => { expect(mockedErpNext.draftCashout).toHaveBeenCalledTimes(1) expect(mockedIbex.payInvoice).toHaveBeenCalledTimes(1) }) + + // The offer stays consumed when execute() fails. After an IBEX timeout the + // payment may still have gone through, so handing the offer back for a + // retry could pay the same cashout twice. + it("never executes an offer again after its execution fails at IBEX", async () => { + const { id } = await createOffer() + mockedIbex.payInvoice.mockResolvedValueOnce(new UnexpectedIbexResponse("timeout")) + + const failed = await executeOffer(id) + const retried = await executeOffer(id) + + expect(failed).toBeInstanceOf(IbexError) + expect(retried).toBeInstanceOf(OfferAlreadyExecuted) + expect(mockedErpNext.draftCashout).toHaveBeenCalledTimes(1) + expect(mockedIbex.payInvoice).toHaveBeenCalledTimes(1) + }) + + // offerId is client input. A consumed offer's tombstone must not come back + // as an offer when the client names it, and the call must return a typed + // error rather than throw. + it("answers an offerId that names a tombstone with OfferNotFound", async () => { + const { id } = await createOffer() + const executed = await executeOffer(id) + + const forged = await executeOffer(`executed:${id}` as OfferId) + + expect(executed).not.toBeInstanceOf(Error) + expect(forged).toBeInstanceOf(OfferNotFound) + expect(mockedErpNext.draftCashout).toHaveBeenCalledTimes(1) + expect(mockedIbex.payInvoice).toHaveBeenCalledTimes(1) + }) }) }) diff --git a/test/flash/unit/app/offers/storage/redis-consume.spec.ts b/test/flash/unit/app/offers/storage/redis-consume.spec.ts index bc6c2611f..cb364425b 100644 --- a/test/flash/unit/app/offers/storage/redis-consume.spec.ts +++ b/test/flash/unit/app/offers/storage/redis-consume.spec.ts @@ -35,9 +35,10 @@ import { } from "@domain/cache" import { USDAmount } from "@domain/shared" -const offerId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" as OfferId +// A v4 UUID, the shape Storage.add mints with randomUUID(). +const offerId = "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee" as OfferId const offerKey = `offers:${offerId}` -const executedKey = `offers:executed:${offerId}` +const executedKey = `offer-executed:${offerId}` const usd = (cents: string) => { const amount = USDAmount.cents(cents) @@ -69,9 +70,31 @@ const stubRedis = (entries: Record) => key in entries ? entries[key] : new CacheUndefinedError(), ) +// An in-memory Redis behind all three cache mocks. Values are JSON encoded the +// way ioredis-cache encodes them, so a tombstone reads back exactly as it would +// in production, and a DEL removes a key for one caller only. +const useInMemoryRedis = (): Map => { + const store = new Map() + mockCacheSet.mockImplementation( + async ({ key, value }: { key: string; value: unknown }) => { + store.set(key, JSON.stringify(value)) + return value + }, + ) + mockCacheGet.mockImplementation(async ({ key }: { key: string }) => { + const data = store.get(key) + return data === undefined ? new CacheUndefinedError() : JSON.parse(data) + }) + mockConsumeCacheKey.mockImplementation(async ({ key }: { key: string }) => + store.delete(key), + ) + return store +} + describe("offer storage: single-use consume", () => { beforeEach(() => { jest.clearAllMocks() + stubRedis({}) mockCacheSet.mockImplementation(async ({ value }: { value: unknown }) => value) mockConsumeCacheKey.mockResolvedValue(true) }) @@ -190,4 +213,54 @@ describe("offer storage: single-use consume", () => { }) }) }) + + // initiateCashout hands the client's offerId straight to get. When the + // tombstone lived at offers:executed:, the id "executed:" named it, + // and get returned it as an offer whose details were the number 1. + describe("offer ids are client input", () => { + const addOffer = async (): Promise => { + const added = await Storage.add({ details } as Parameters[0]) + if (added instanceof Error) throw added + return added.id + } + + it("does not return a consumed offer's tombstone as an offer", async () => { + useInMemoryRedis() + const id = await addOffer() + expect(await Storage.consume(id)).toBe(true) + + const forged = await Storage.get(`executed:${id}` as OfferId) + + expect(forged).toBeInstanceOf(OfferNotFound) + expect(forged).not.toBeInstanceOf(PersistedOffer) + expect(await Storage.get(id)).toBeInstanceOf(OfferAlreadyExecuted) + }) + + it("keeps nothing but offer records under offers:", async () => { + const store = useInMemoryRedis() + const id = await addOffer() + + await Storage.consume(id) + + // the offer record is gone and only its tombstone is left, elsewhere + expect(store.size).toBe(1) + expect([...store.keys()].filter((key) => key.startsWith("offers:"))).toEqual([]) + }) + + it.each([ + ["a tombstone name", `executed:${offerId}`], + ["a UUID with a suffix", `${offerId}:x`], + ["a key pattern", "*"], + ["an empty id", ""], + ])("refuses %s before it reaches a Redis key", async (_label, id) => { + const fetched = await Storage.get(id as OfferId) + const consumed = await Storage.consume(id as OfferId) + + expect(fetched).toBeInstanceOf(OfferNotFound) + expect(consumed).toBeInstanceOf(OfferNotFound) + expect(mockCacheGet).not.toHaveBeenCalled() + expect(mockCacheSet).not.toHaveBeenCalled() + expect(mockConsumeCacheKey).not.toHaveBeenCalled() + }) + }) }) diff --git a/test/flash/unit/graphql/error-map.spec.ts b/test/flash/unit/graphql/error-map.spec.ts index d468020fc..fa75740dc 100644 --- a/test/flash/unit/graphql/error-map.spec.ts +++ b/test/flash/unit/graphql/error-map.spec.ts @@ -254,13 +254,15 @@ describe("error-map", () => { // ENG-695: a cashout offer executes at most once. The client branches on the // code to ask for a new offer, so it must be stable, forwarded, and distinct - // from the expired/unknown-offer case. + // from the expired/unknown-offer case. The app shows the message as is, and + // the first execution may have paid, so the copy sends the user to their + // transactions before a new cashout. it("maps OfferAlreadyExecuted to OFFER_ALREADY_EXECUTED with client-safe copy", () => { const result = mapError(new OfferAlreadyExecuted()) expect(result.extensions.code).toBe("OFFER_ALREADY_EXECUTED") expect(result.message).toBe( - "This offer has already been used. Request a new cashout offer.", + "This cashout offer was already submitted and can't be reused. Check your transactions before starting a new cashout.", ) expect(result.forwardToClient).toBe(true) expect(mapError(new OfferNotFound()).extensions.code).toBe("NOT_FOUND") diff --git a/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts b/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts index 1f2e262aa..c15b039ba 100644 --- a/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts +++ b/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts @@ -77,7 +77,8 @@ describe("initiateCashout resolver", () => { errors: [ expect.objectContaining({ code: "OFFER_ALREADY_EXECUTED", - message: "This offer has already been used. Request a new cashout offer.", + message: + "This cashout offer was already submitted and can't be reused. Check your transactions before starting a new cashout.", }), ], id: null,