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..21cb91cb6 100644 --- a/src/app/offers/storage/Redis.ts +++ b/src/app/offers/storage/Redis.ts @@ -1,7 +1,14 @@ 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 { UuidRegex } from "@domain/shared" +import { RedisCacheService, consumeCacheKey } from "@services/cache" +import { baseLogger } from "@services/logger" import ValidOffer from "../ValidOffer" @@ -10,30 +17,100 @@ 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. 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. +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) }, get: async (id: OfferId): Promise => { + if (!isOfferId(id)) return new OfferNotFound() 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). + * 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 + // 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/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..c9e7bdf67 100644 --- a/src/graphql/error-map.ts +++ b/src/graphql/error-map.ts @@ -636,6 +636,16 @@ export const mapError = (error: ApplicationError): CustomApolloError => { message: "Offer not available. Try again.", 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 cashout offer was already submitted and can't be reused. Check your transactions before starting a new cashout.", + }) case "BridgeInvalidAmountError": message = 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/integration/offers/execute-offer.spec.ts b/test/flash/integration/offers/execute-offer.spec.ts index 77f1b77ac..357f0dded 100644 --- a/test/flash/integration/offers/execute-offer.spec.ts +++ b/test/flash/integration/offers/execute-offer.spec.ts @@ -1,8 +1,12 @@ import CashoutManager from "@app/offers/CashoutManager" -import { JMDAmount, USDAmount } from "@domain/shared" +import { Cashout } from "@config" +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" @@ -109,4 +113,123 @@ 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. + // 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() + await bothArrived + return [bankAccount] + }) + + const results = await Promise.all([executeOffer(id), executeOffer(id)]).finally( + () => clearTimeout(safetyRelease), + ) + + 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) + }) + + // 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/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..cb364425b --- /dev/null +++ b/test/flash/unit/app/offers/storage/redis-consume.spec.ts @@ -0,0 +1,266 @@ +// 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" + +// 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 = `offer-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(), + ) + +// 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) + }) + + 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, + }) + }) + }) + + // 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 c3d1d4c8e..fa75740dc 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,20 @@ 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. 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 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 new file mode 100644 index 000000000..c15b039ba --- /dev/null +++ b/test/flash/unit/graphql/public/root/mutation/initiate-cash-out.spec.ts @@ -0,0 +1,102 @@ +// 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 cashout offer was already submitted and can't be reused. Check your transactions before starting a new cashout.", + }), + ], + 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 }), + ) + }) +})