diff --git a/src/app/payment-splits/execute-pending-splits.ts b/src/app/payment-splits/execute-pending-splits.ts index 80cf99d2f..4f8d91f16 100644 --- a/src/app/payment-splits/execute-pending-splits.ts +++ b/src/app/payment-splits/execute-pending-splits.ts @@ -5,8 +5,9 @@ import { authorizeSend } from "@app/payments/authorize-send" import { isDefinitivePaymentRejection } from "@app/payments/mark-outgoing-invoice" import { intraledgerPaymentSendWalletIdForUsdWallet } from "@app/payments/send-intraledger" +import { AccountValidator } from "@domain/accounts" import { PaymentSendStatus } from "@domain/bitcoin/lightning" -import { CouldNotFindError } from "@domain/errors" +import { CouldNotFindError, InactiveAccountError } from "@domain/errors" import { LockServiceError, ResourceAttemptsLockServiceError } from "@domain/lock" import { InternalTransferMarkerError, splitRetryBackoffMs } from "@domain/payment-splits" import { SendLimitsUnavailableError } from "@domain/payments/errors" @@ -21,6 +22,7 @@ import { PaymentSplitExecutionsRepository, type PaymentSplitExecutionRecord, } from "@services/mongoose/payment-split-executions" +import { PaymentSplitRulesRepository } from "@services/mongoose/payment-split-rules" const logger = baseLogger.child({ module: "payment-splits" }) @@ -111,7 +113,7 @@ const formatCents = (cents: number): string => `US$${(cents / 100).toFixed(2)}` const notifyOwner = async ( accountId: string, - outcome: "sent" | "failed", + outcome: "sent" | "failed" | "disabled", execution: PaymentSplitExecutionRecord, ): Promise => sendOutcomeNotificationBestEffort({ @@ -128,6 +130,21 @@ const notifyOwner = async ( logContext: { executionId: execution.id, walletId: execution.walletId }, }) +/** + * ENG-660: whether the split's DESTINATION account is inactive right now. + * InactiveAccountError from the send does not say which side was inactive + * (the sender is checked first), so the destination is looked up directly. + * A lookup failure answers false: the row then fails as before and the rule + * stays enabled. + */ +const isDestinationInactive = async (destinationWalletId: string): Promise => { + const wallet = await WalletsRepository().findById(destinationWalletId as WalletId) + if (wallet instanceof Error) return false + const account = await AccountsRepository().findById(wallet.accountId) + if (account instanceof Error) return false + return AccountValidator(account).isActive() instanceof Error +} + type ExecuteOutcome = | "sent" | "ibex_pending" @@ -179,6 +196,62 @@ const executeOne = async ( return "failed" as const } + // ENG-660: the destination account is no longer active, so every later + // payout to it would fail the same way. Turn off the wallet's rules to that + // destination and tell the owner once (only the call that flipped `enabled` + // notifies); further rows for the same destination fail without another + // push. The rules are matched by destination, not by execution.ruleId: a + // save replaces every rule with a new id, and a delete removes it, so the + // recorded ruleId may no longer exist. When nothing was disabled AND no rule + // to the destination exists any more, the owner still gets the ordinary + // "failed" push, so a failed payout is never silent. + const failAndDisableRule = async (error: Error, accountId: string) => { + await finish({ status: "failed", attempts, lastError: error.name }) + const rules = PaymentSplitRulesRepository() + const logContext = { + executionId: execution.id, + ruleId: execution.ruleId, + walletId: execution.walletId, + destinationWalletId: execution.destinationWalletId, + } + const disabled = await rules.disableForDestination( + execution.walletId, + execution.destinationWalletId, + { now: stamp() }, + ) + if (disabled instanceof Error) { + logger.error( + { ...logContext, error: disabled }, + "could not disable payment split rule with an inactive destination", + ) + await notifyOwner(accountId, "failed", execution) + return "failed" as const + } + if (disabled) { + logger.warn( + logContext, + "payment split rule disabled: destination account is not active", + ) + await notifyOwner(accountId, "disabled", execution) + return "failed" as const + } + // Nothing flipped: either an earlier row already disabled the rule (it + // still exists, its owner was told) or the rule is gone (edited away or + // deleted), in which case nobody has told the owner about this payout. + const exists = await rules.existsForDestination( + execution.walletId, + execution.destinationWalletId, + ) + if (exists instanceof Error) { + logger.error( + { ...logContext, error: exists }, + "could not check payment split rule for an inactive destination", + ) + } + if (exists !== true) await notifyOwner(accountId, "failed", execution) + return "failed" as const + } + const retry = async (error: Error) => { await finish({ status: "pending", @@ -288,6 +361,12 @@ const executeOne = async ( { executionId: execution.id, walletId: execution.walletId, error: result }, "payment split failed", ) + if ( + result instanceof InactiveAccountError && + (await isDestinationInactive(execution.destinationWalletId)) + ) { + return failAndDisableRule(result, account.id) + } return fail(result, account.id) } diff --git a/src/app/payment-splits/set-splits.ts b/src/app/payment-splits/set-splits.ts index 58b418021..39d9efa85 100644 --- a/src/app/payment-splits/set-splits.ts +++ b/src/app/payment-splits/set-splits.ts @@ -1,9 +1,10 @@ import { PaymentSplitsConfig } from "@config" -import { checkedToUsername } from "@domain/accounts" +import { AccountValidator, checkedToUsername } from "@domain/accounts" import { InvalidWalletForAccountError } from "@domain/accounts/errors" import { PaymentSplitsDisabledError, + SplitDestinationInactiveError, SplitDestinationNotFoundError, validateSplitSet, type SplitSetEntry, @@ -40,6 +41,24 @@ export const resolveOwnedWallet = async ({ return wallet } +/** + * ENG-660: a destination whose account is not active (closed, locked, pending) + * would fail every payout with InactiveAccountError, so it is refused at save + * time. Uses the same AccountValidator check the intraledger send applies to + * the recipient. + */ +const checkDestinationActive = ( + account: Account, + destination: string, +): true | SplitDestinationInactiveError => { + if (AccountValidator(account).isActive() instanceof Error) { + return new SplitDestinationInactiveError( + `Split destination account is not active: ${destination}`, + ) + } + return true +} + /** * A destination is either a walletId or a Flash username. A username resolves * to that account's wallet in the SOURCE wallet's currency — not to its @@ -60,6 +79,12 @@ const resolveDestination = async ({ if (wallet instanceof Error) { return new SplitDestinationNotFoundError(`No wallet found for ${trimmed}`) } + const owner = await AccountsRepository().findById(wallet.accountId) + if (owner instanceof Error) { + return new SplitDestinationNotFoundError(`No wallet found for ${trimmed}`) + } + const active = checkDestinationActive(owner, trimmed) + if (active instanceof Error) return active return wallet } @@ -72,6 +97,8 @@ const resolveDestination = async ({ if (account instanceof Error) { return new SplitDestinationNotFoundError(`No Flash user found for ${trimmed}`) } + const active = checkDestinationActive(account, trimmed) + if (active instanceof Error) return active const wallets = await WalletsRepository().listByAccountId(account.id) if (wallets instanceof Error) return wallets diff --git a/src/config/locales/en.json b/src/config/locales/en.json index e0e5ab783..95f9ce844 100644 --- a/src/config/locales/en.json +++ b/src/config/locales/en.json @@ -143,6 +143,10 @@ "failed": { "title": "Payment split failed", "body": "We could not send a {{amount}} payment split. The money is still in your wallet." + }, + "disabled": { + "title": "Payment split turned off", + "body": "We could not send a {{amount}} payment split because the recipient's account is no longer active. We turned this split off. The money is still in your wallet." } } } diff --git a/src/config/locales/es.json b/src/config/locales/es.json index 6a7177e91..2dc655c60 100644 --- a/src/config/locales/es.json +++ b/src/config/locales/es.json @@ -139,6 +139,10 @@ "failed": { "title": "La división de pago falló", "body": "No pudimos enviar una división de pago de {{amount}}. El dinero sigue en tu billetera." + }, + "disabled": { + "title": "División de pago desactivada", + "body": "No pudimos enviar una división de pago de {{amount}} porque la cuenta del destinatario ya no está activa. Desactivamos esta división. El dinero sigue en tu billetera." } } } diff --git a/src/domain/payment-splits/errors.ts b/src/domain/payment-splits/errors.ts index ba1362a80..98b90e487 100644 --- a/src/domain/payment-splits/errors.ts +++ b/src/domain/payment-splits/errors.ts @@ -13,6 +13,9 @@ export class SplitRuleLimitExceededError extends ValidationError {} // The destination username or walletId does not resolve to a wallet in the // source wallet's currency. export class SplitDestinationNotFoundError extends ValidationError {} +// ENG-660: the destination account's current status is not active (closed, +// locked, pending). Every payout to it would fail with InactiveAccountError. +export class SplitDestinationInactiveError extends ValidationError {} export class SelfSplitNotAllowedError extends ValidationError {} // The destination wallet's currency differs from the source wallet's, or the // source wallet is not a USD/USDT wallet. send-intraledger rejects diff --git a/src/graphql/error-map.ts b/src/graphql/error-map.ts index 9d0b6cef4..2a4339adf 100644 --- a/src/graphql/error-map.ts +++ b/src/graphql/error-map.ts @@ -99,6 +99,10 @@ export const mapError = (error: ApplicationError): CustomApolloError => { message = error.message return new ValidationInternalError({ message, logger: baseLogger }) + case "SplitDestinationInactiveError": + message = "Split destination account is not active" + return new ValidationInternalError({ message, logger: baseLogger }) + case "SplitRuleNotFoundError": message = "Payment split not found" return new NotFoundError({ message, logger: baseLogger }) diff --git a/src/services/mongoose/payment-split-rules.ts b/src/services/mongoose/payment-split-rules.ts index 50950d220..27b2300e4 100644 --- a/src/services/mongoose/payment-split-rules.ts +++ b/src/services/mongoose/payment-split-rules.ts @@ -117,6 +117,50 @@ export const PaymentSplitRulesRepository = () => { } } + // ENG-660: turns off every enabled rule from `walletId` to a destination + // whose account went inactive. Matched by (wallet, destination), not by rule + // id: replaceForWallet re-inserts every rule with a new _id on each save, so + // an execution's ruleId may name a rule that no longer exists while a + // replacement to the same dead destination is still enabled. Only enabled + // rules are updated, so of several executions failing on the same + // destination exactly one sees `true` and notifies the owner. + const disableForDestination = async ( + walletId: string, + destinationWalletId: string, + { now = new Date() }: { now?: Date } = {}, + ): Promise => { + try { + const result = await PaymentSplitRuleModel.updateMany( + { + walletId: { $eq: walletId }, + destinationWalletId: { $eq: destinationWalletId }, + enabled: true, + }, + { $set: { enabled: false, updatedAt: now } }, + ) + return result.modifiedCount > 0 + } catch (error) { + return toError(error) + } + } + + // Whether any rule (enabled or not) from `walletId` to `destinationWalletId` + // exists. + const existsForDestination = async ( + walletId: string, + destinationWalletId: string, + ): Promise => { + try { + const doc = await PaymentSplitRuleModel.exists({ + walletId: { $eq: walletId }, + destinationWalletId: { $eq: destinationWalletId }, + }) + return doc !== null + } catch (error) { + return toError(error) + } + } + const listWalletIdsWithEnabledRules = async (): Promise => { try { const ids = await PaymentSplitRuleModel.distinct("walletId", { enabled: true }) @@ -131,6 +175,8 @@ export const PaymentSplitRulesRepository = () => { findById, replaceForWallet, deleteById, + disableForDestination, + existsForDestination, listWalletIdsWithEnabledRules, } } diff --git a/test/flash/integration/payment-splits/payment-splits-repositories.spec.ts b/test/flash/integration/payment-splits/payment-splits-repositories.spec.ts index 440e910e8..49344b6c2 100644 --- a/test/flash/integration/payment-splits/payment-splits-repositories.spec.ts +++ b/test/flash/integration/payment-splits/payment-splits-repositories.spec.ts @@ -201,6 +201,91 @@ describe("payment split repositories (ENG-653)", () => { } }) + it("disables a rule once, and a disabled rule records nothing (ENG-660)", async () => { + const enabled = PaymentSplitsConfig.enabled + PaymentSplitsConfig.enabled = true + try { + const rules = PaymentSplitRulesRepository() + const saved = await rules.replaceForWallet("wallet-d", [ + { destinationWalletId: "closed-partner", percent: 10 }, + { destinationWalletId: "open-partner", percent: 20 }, + ]) + if (saved instanceof Error) throw saved + const closed = saved.find((r) => r.destinationWalletId === "closed-partner") + if (!closed) throw new Error("rule not saved") + + const disabledAt = new Date("2026-10-12T12:00:00Z") + // Only the first call flips the flag, so only it notifies the owner. + expect( + await rules.disableForDestination("wallet-d", "closed-partner", { + now: disabledAt, + }), + ).toBe(true) + expect(await rules.disableForDestination("wallet-d", "closed-partner")).toBe(false) + expect(await rules.disableForDestination("wallet-d", "no-such-partner")).toBe(false) + expect(await rules.findById(closed.id)).toMatchObject({ + enabled: false, + updatedAt: disabledAt, + }) + // The disabled rule still exists; a destination never configured does not. + expect(await rules.existsForDestination("wallet-d", "closed-partner")).toBe(true) + expect(await rules.existsForDestination("wallet-d", "no-such-partner")).toBe(false) + + // Webhook and reconcile both record through recordSplitExecutions. + const triggerKey = "d".repeat(64) + const result = await recordSplitExecutions({ + walletId: "wallet-d", + triggerKey, + paymentHash: triggerKey, + amount: 100, + currency: "USDT", + }) + if (result instanceof Error) throw result + + expect(result).toMatchObject({ recorded: 1 }) + const rows = await PaymentSplitExecution.find({ triggerKey }).lean() + expect(rows.map((row) => row.destinationWalletId)).toEqual(["open-partner"]) + + // A wallet whose only rule is disabled is not reconciled at all. + const only = await rules.replaceForWallet("wallet-e", [ + { destinationWalletId: "closed-partner", percent: 10 }, + ]) + if (only instanceof Error) throw only + expect(await rules.disableForDestination("wallet-e", "closed-partner")).toBe(true) + expect(await rules.listWalletIdsWithEnabledRules()).toEqual(["wallet-d"]) + } finally { + PaymentSplitsConfig.enabled = enabled + } + }) + + it("disables a destination's rule after a re-save gave it a new id (ENG-660)", async () => { + const rules = PaymentSplitRulesRepository() + const first = await rules.replaceForWallet("wallet-f", [ + { destinationWalletId: "closed-partner", percent: 10 }, + ]) + if (first instanceof Error) throw first + // The owner edits the percentage: the rule is re-inserted with a new id, + // so an execution recorded under the first id no longer names a rule. + const second = await rules.replaceForWallet("wallet-f", [ + { destinationWalletId: "closed-partner", percent: 15 }, + ]) + if (second instanceof Error) throw second + expect(second[0].id).not.toEqual(first[0].id) + expect(await rules.findById(first[0].id)).toBeNull() + + // Another owner has an enabled rule to the same destination. + const other = await rules.replaceForWallet("wallet-g", [ + { destinationWalletId: "closed-partner", percent: 5 }, + ]) + if (other instanceof Error) throw other + + expect(await rules.disableForDestination("wallet-f", "closed-partner")).toBe(true) + expect(await rules.findById(second[0].id)).toMatchObject({ enabled: false }) + // Disabling wallet-f's rule must not touch wallet-g's rule. + expect(await rules.findById(other[0].id)).toMatchObject({ enabled: true }) + expect(await rules.disableForDestination("wallet-g", "closed-partner")).toBe(true) + }) + it("lets exactly one of several concurrent claimers claim a pending row", async () => { const repo = PaymentSplitExecutionsRepository() await repo.insert(executionFields("e".repeat(64), "rule-1")) diff --git a/test/flash/unit/app/payment-splits/execute-pending-splits.spec.ts b/test/flash/unit/app/payment-splits/execute-pending-splits.spec.ts index ccfe6196f..6147d8aa3 100644 --- a/test/flash/unit/app/payment-splits/execute-pending-splits.spec.ts +++ b/test/flash/unit/app/payment-splits/execute-pending-splits.spec.ts @@ -5,6 +5,8 @@ const mockFindWalletById = jest.fn() const mockFindAccountById = jest.fn() const mockIsInternal = jest.fn() const mockNotifyOps = jest.fn() +const mockDisableRule = jest.fn() +const mockRuleExists = jest.fn() jest.mock("@services/logger", () => { const logger = { trace: jest.fn(), @@ -46,6 +48,13 @@ jest.mock("@services/alerts/ops-events", () => ({ notifyOpsEvent: (...args: unknown[]) => mockNotifyOps(...args), })) +jest.mock("@services/mongoose/payment-split-rules", () => ({ + PaymentSplitRulesRepository: jest.fn(() => ({ + disableForDestination: (...args: unknown[]) => mockDisableRule(...args), + existsForDestination: (...args: unknown[]) => mockRuleExists(...args), + })), +})) + jest.mock("@services/mongoose/payment-split-executions", () => ({ PaymentSplitExecutionsRepository: jest.fn(() => mockStore.repo), })) @@ -91,13 +100,17 @@ const mockLogger = ( const SOURCE = "source-wallet" const DEST = "dest-wallet" -const OWNER = { id: "owner-account", level: 1 } +const OWNER = { id: "owner-account", level: 1, status: "active" } +const DEST_ACCOUNT = "dest-account" const T0 = new Date("2026-10-11T12:00:00Z") const minutes = (m: number) => new Date(T0.getTime() + m * 60_000) -const seed = async (shareCents = 250): Promise<{ id: string }> => { +const seed = async ( + shareCents = 250, + triggerKey = "a".repeat(64), +): Promise<{ id: string }> => { const inserted = await mockStore.repo.insert({ - triggerKey: "a".repeat(64), + triggerKey, ruleId: "rule-1", walletId: SOURCE, destinationWalletId: DEST, @@ -132,6 +145,8 @@ beforeEach(() => { mockFindAccountById.mockResolvedValue(OWNER) mockAuthorizeSend.mockResolvedValue(true) mockNotify.mockResolvedValue(undefined) + mockDisableRule.mockResolvedValue(true) + mockRuleExists.mockResolvedValue(true) }) afterAll(() => { @@ -221,6 +236,177 @@ describe("executePendingSplits", () => { ) }) + describe("inactive destination account (ENG-660)", () => { + const destinationStatus = (status: string) => { + mockFindWalletById.mockImplementation(async (id: string) => + id === DEST + ? { id: DEST, accountId: DEST_ACCOUNT, currency: WalletCurrency.Usdt } + : { id: SOURCE, accountId: OWNER.id, currency: WalletCurrency.Usdt }, + ) + mockFindAccountById.mockImplementation(async (id: string) => + id === DEST_ACCOUNT ? { id: DEST_ACCOUNT, level: 1, status } : OWNER, + ) + } + + it.each(["closed", "locked", "pending"])( + "disables the rule, fails the row and pushes the owner once when the destination is %s", + async (status) => { + destinationStatus(status) + await seed() + mockSend.mockResolvedValue(new InactiveAccountError(DEST_ACCOUNT)) + + const summary = await executePendingSplits({ now: T0 }) + + expect(summary).toMatchObject({ claimed: 1, failed: 1, retried: 0 }) + expect(onlyRow()).toMatchObject({ + status: "failed", + attempts: 1, + lastError: "InactiveAccountError", + }) + expect(mockDisableRule).toHaveBeenCalledTimes(1) + expect(mockDisableRule).toHaveBeenCalledWith(SOURCE, DEST, expect.anything()) + expect(mockNotify).toHaveBeenCalledTimes(1) + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ + accountId: OWNER.id, + phraseBase: "notification.paymentSplit.disabled", + dataType: "payment_split_disabled", + amountArg: "US$2.50", + }), + ) + }, + ) + + it("pushes once across several rows of the same rule", async () => { + destinationStatus("closed") + await seed(250, "a".repeat(64)) + await seed(300, "b".repeat(64)) + mockSend.mockResolvedValue(new InactiveAccountError(DEST_ACCOUNT)) + // Only the first disable flips `enabled`; the rule is already off after. + mockDisableRule.mockResolvedValueOnce(true).mockResolvedValue(false) + + const summary = await executePendingSplits({ now: T0 }) + + expect(summary).toMatchObject({ claimed: 2, failed: 2 }) + expect([...mockStore.rows.values()].map((row) => row.status)).toEqual([ + "failed", + "failed", + ]) + expect(mockDisableRule).toHaveBeenCalledTimes(2) + expect(mockNotify).toHaveBeenCalledTimes(1) + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ phraseBase: "notification.paymentSplit.disabled" }), + ) + }) + + it("still pushes a failure when the recorded rule no longer exists", async () => { + // The row was recorded under a rule the owner since edited away (a save + // gives every rule a new id) or deleted, so nothing is left to disable. + destinationStatus("closed") + await seed() + mockSend.mockResolvedValue(new InactiveAccountError(DEST_ACCOUNT)) + mockDisableRule.mockResolvedValue(false) + mockRuleExists.mockResolvedValue(false) + + const summary = await executePendingSplits({ now: T0 }) + + expect(summary).toMatchObject({ claimed: 1, failed: 1 }) + expect(onlyRow()).toMatchObject({ + status: "failed", + lastError: "InactiveAccountError", + }) + expect(mockRuleExists).toHaveBeenCalledWith(SOURCE, DEST) + expect(mockNotify).toHaveBeenCalledTimes(1) + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ + accountId: OWNER.id, + phraseBase: "notification.paymentSplit.failed", + }), + ) + }) + + it("pushes a failure when nothing was disabled and the rule lookup fails", async () => { + destinationStatus("closed") + await seed() + mockSend.mockResolvedValue(new InactiveAccountError(DEST_ACCOUNT)) + mockDisableRule.mockResolvedValue(false) + mockRuleExists.mockResolvedValue(new UnknownRepositoryError("mongo down")) + + await executePendingSplits({ now: T0 }) + + expect(onlyRow()).toMatchObject({ status: "failed" }) + expect(mockNotify).toHaveBeenCalledTimes(1) + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ phraseBase: "notification.paymentSplit.failed" }), + ) + expect(mockLogger.error).toHaveBeenCalled() + }) + + it("keeps the rule enabled when the SOURCE account is the inactive one", async () => { + destinationStatus("active") + await seed() + mockSend.mockResolvedValue(new InactiveAccountError(OWNER.id)) + + await executePendingSplits({ now: T0 }) + + expect(onlyRow()).toMatchObject({ + status: "failed", + lastError: "InactiveAccountError", + }) + expect(mockDisableRule).not.toHaveBeenCalled() + expect(mockNotify).toHaveBeenCalledTimes(1) + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ phraseBase: "notification.paymentSplit.failed" }), + ) + }) + + it("keeps the rule enabled when the destination account cannot be read", async () => { + destinationStatus("closed") + mockFindAccountById.mockImplementation(async (id: string) => + id === DEST_ACCOUNT ? new UnknownRepositoryError("mongo down") : OWNER, + ) + await seed() + mockSend.mockResolvedValue(new InactiveAccountError(DEST_ACCOUNT)) + + await executePendingSplits({ now: T0 }) + + expect(onlyRow()).toMatchObject({ status: "failed" }) + expect(mockDisableRule).not.toHaveBeenCalled() + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ phraseBase: "notification.paymentSplit.failed" }), + ) + }) + + it("never disables a rule for a destination that is inactive but failed another way", async () => { + destinationStatus("closed") + await seed() + mockSend.mockResolvedValue(new IntraledgerLimitsExceededError("over limit")) + + await executePendingSplits({ now: T0 }) + + expect(mockDisableRule).not.toHaveBeenCalled() + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ phraseBase: "notification.paymentSplit.failed" }), + ) + }) + + it("still fails the row and pushes a failure when the rule cannot be disabled", async () => { + destinationStatus("closed") + await seed() + mockSend.mockResolvedValue(new InactiveAccountError(DEST_ACCOUNT)) + mockDisableRule.mockResolvedValue(new UnknownRepositoryError("mongo down")) + + await executePendingSplits({ now: T0 }) + + expect(onlyRow()).toMatchObject({ status: "failed" }) + expect(mockNotify).toHaveBeenCalledTimes(1) + expect(mockNotify).toHaveBeenCalledWith( + expect.objectContaining({ phraseBase: "notification.paymentSplit.failed" }), + ) + expect(mockLogger.error).toHaveBeenCalled() + }) + }) + it("fails an IBEX Failure status and pushes the owner", async () => { await seed() mockSend.mockResolvedValue(PaymentSendStatus.Failure) diff --git a/test/flash/unit/app/payment-splits/record-split-executions.spec.ts b/test/flash/unit/app/payment-splits/record-split-executions.spec.ts index 14846ae1c..db8e1193c 100644 --- a/test/flash/unit/app/payment-splits/record-split-executions.spec.ts +++ b/test/flash/unit/app/payment-splits/record-split-executions.spec.ts @@ -73,6 +73,36 @@ afterAll(() => { }) describe("recordSplitExecutions", () => { + it("records nothing for a rule disabled after its destination went inactive (ENG-660)", async () => { + // The repository honours enabledOnly the way the Mongo filter does. + const withDisabled = [{ ...rules[0], enabled: false }, rules[1]] + mockListRules.mockImplementation( + async (_walletId: string, { enabledOnly = false } = {}) => + enabledOnly ? withDisabled.filter((rule) => rule.enabled) : withDisabled, + ) + + const result = await record({ amount: 100 }) + + expect(result).toMatchObject({ recorded: 1 }) + expect([...mockStore.rows.values()].map((row) => row.destinationWalletId)).toEqual([ + "dest-2", + ]) + }) + + it("records nothing at all when the wallet's only rule is disabled (ENG-660)", async () => { + mockListRules.mockImplementation( + async (_walletId: string, { enabledOnly = false } = {}) => + enabledOnly ? [] : [{ ...rules[0], enabled: false }], + ) + + expect(await record({ amount: 100 })).toEqual({ + recorded: 0, + duplicates: 0, + skippedMin: 0, + }) + expect(mockStore.rows.size).toBe(0) + }) + it("records one pending execution per enabled rule with the floored share", async () => { const result = await record() diff --git a/test/flash/unit/app/payment-splits/set-splits.spec.ts b/test/flash/unit/app/payment-splits/set-splits.spec.ts index 09a62ce76..02c7daf56 100644 --- a/test/flash/unit/app/payment-splits/set-splits.spec.ts +++ b/test/flash/unit/app/payment-splits/set-splits.spec.ts @@ -1,6 +1,7 @@ const mockFindWalletById = jest.fn() const mockListByAccountId = jest.fn() const mockFindByUsername = jest.fn() +const mockFindAccountById = jest.fn() const mockReplaceForWallet = jest.fn() const mockFindRuleById = jest.fn() const mockDeleteRuleById = jest.fn() @@ -9,6 +10,7 @@ const mockLockWalletId = jest.fn() jest.mock("@services/mongoose", () => ({ AccountsRepository: jest.fn(() => ({ findByUsername: (...args: unknown[]) => mockFindByUsername(...args), + findById: (...args: unknown[]) => mockFindAccountById(...args), })), WalletsRepository: jest.fn(() => ({ findById: (...args: unknown[]) => mockFindWalletById(...args), @@ -33,6 +35,7 @@ jest.mock("@services/lock", () => ({ import { PaymentSplitsConfig } from "@config" import { deletePaymentSplit } from "@app/payment-splits/delete-split" import { setPaymentSplits } from "@app/payment-splits/set-splits" +import { AccountStatus } from "@domain/accounts" import { InvalidWalletForAccountError } from "@domain/accounts/errors" import { CouldNotFindAccountFromUsernameError, @@ -41,6 +44,7 @@ import { import { PaymentSplitsDisabledError, SplitCurrencyMismatchError, + SplitDestinationInactiveError, SplitDestinationNotFoundError, SplitPercentExceededError, SplitRuleNotFoundError, @@ -68,6 +72,10 @@ const aliceWallets = [ { id: ALICE_USDT, accountId: "alice", currency: WalletCurrency.Usdt }, ] +// Latest account status per account id; anything not listed is active. +const accountStatuses: Record = {} +const statusOf = (id: string) => accountStatuses[id] ?? AccountStatus.Active + const originalEnabled = PaymentSplitsConfig.enabled beforeEach(() => { @@ -76,9 +84,14 @@ beforeEach(() => { mockFindWalletById.mockImplementation( async (id: string) => wallets[id] ?? new CouldNotFindWalletFromIdError(), ) + for (const id of Object.keys(accountStatuses)) delete accountStatuses[id] + mockFindAccountById.mockImplementation(async (id: string) => ({ + id, + status: statusOf(id), + })) mockFindByUsername.mockImplementation(async (username: string) => username.toLowerCase() === "alice" - ? { id: "alice", defaultWalletId: ALICE_BTC } + ? { id: "alice", defaultWalletId: ALICE_BTC, status: statusOf("alice") } : new CouldNotFindAccountFromUsernameError(username), ) mockListByAccountId.mockResolvedValue(aliceWallets) @@ -243,6 +256,88 @@ describe("setPaymentSplits", () => { expect(mockReplaceForWallet).not.toHaveBeenCalled() }) + describe("destination account status (ENG-660)", () => { + it.each([AccountStatus.Closed, AccountStatus.Locked, AccountStatus.Pending])( + "rejects a walletId destination whose account is %s and saves nothing", + async (status) => { + accountStatuses["dest-account"] = status + + const result = await setPaymentSplits({ + account: OWNER, + walletId: SOURCE_ID, + splits: [{ destination: DEST_ID, percent: 25 }], + }) + + expect(result).toBeInstanceOf(SplitDestinationInactiveError) + expect(mockFindAccountById).toHaveBeenCalledWith("dest-account") + expect(mockLockWalletId).not.toHaveBeenCalled() + expect(mockReplaceForWallet).not.toHaveBeenCalled() + }, + ) + + it.each([AccountStatus.Closed, AccountStatus.Locked, AccountStatus.Pending])( + "rejects a username destination whose account is %s and saves nothing", + async (status) => { + accountStatuses["alice"] = status + + const result = await setPaymentSplits({ + account: OWNER, + walletId: SOURCE_ID, + splits: [{ destination: "@alice", percent: 10 }], + }) + + expect(result).toBeInstanceOf(SplitDestinationInactiveError) + expect(mockReplaceForWallet).not.toHaveBeenCalled() + }, + ) + + it("still saves an active destination", async () => { + accountStatuses["dest-account"] = AccountStatus.Active + + const result = await setPaymentSplits({ + account: OWNER, + walletId: SOURCE_ID, + splits: [{ destination: DEST_ID, percent: 25 }], + }) + + expect(result).toEqual([ + expect.objectContaining({ destinationWalletId: DEST_ID, percent: 25 }), + ]) + expect(mockReplaceForWallet).toHaveBeenCalledTimes(1) + }) + + it("saves nothing when one destination of a mixed set is inactive", async () => { + accountStatuses["other-account"] = AccountStatus.Closed + + const result = await setPaymentSplits({ + account: OWNER, + walletId: SOURCE_ID, + splits: [ + { destination: DEST_ID, percent: 20 }, + { destination: "alice", percent: 10 }, + { destination: OTHER_ID, percent: 30 }, + ], + }) + + expect(result).toBeInstanceOf(SplitDestinationInactiveError) + expect(mockLockWalletId).not.toHaveBeenCalled() + expect(mockReplaceForWallet).not.toHaveBeenCalled() + }) + + it("treats a destination wallet whose account cannot be found as not found", async () => { + mockFindAccountById.mockResolvedValue(new CouldNotFindWalletFromIdError()) + + const result = await setPaymentSplits({ + account: OWNER, + walletId: SOURCE_ID, + splits: [{ destination: DEST_ID, percent: 25 }], + }) + + expect(result).toBeInstanceOf(SplitDestinationNotFoundError) + expect(mockReplaceForWallet).not.toHaveBeenCalled() + }) + }) + it("rejects everything while the feature flag is off", async () => { PaymentSplitsConfig.enabled = false const result = await setPaymentSplits({ diff --git a/test/flash/unit/graphql/public/root/mutation/wallet-splits-set.spec.ts b/test/flash/unit/graphql/public/root/mutation/wallet-splits-set.spec.ts index f00807da7..0b0112b07 100644 --- a/test/flash/unit/graphql/public/root/mutation/wallet-splits-set.spec.ts +++ b/test/flash/unit/graphql/public/root/mutation/wallet-splits-set.spec.ts @@ -2,9 +2,13 @@ const mockFindWalletById = jest.fn() const mockReplaceForWallet = jest.fn() const mockFindRuleById = jest.fn() const mockDeleteRuleById = jest.fn() +const mockFindAccountById = jest.fn() jest.mock("@services/mongoose", () => ({ - AccountsRepository: jest.fn(() => ({ findByUsername: jest.fn() })), + AccountsRepository: jest.fn(() => ({ + findByUsername: jest.fn(), + findById: (...args: unknown[]) => mockFindAccountById(...args), + })), WalletsRepository: jest.fn(() => ({ findById: (...args: unknown[]) => mockFindWalletById(...args), listByAccountId: jest.fn(), @@ -85,6 +89,10 @@ beforeEach(() => { mockFindWalletById.mockImplementation( async (id: string) => walletsById[id] ?? new CouldNotFindWalletFromIdError(), ) + mockFindAccountById.mockImplementation(async (id: string) => ({ + id, + status: "active", + })) mockReplaceForWallet.mockImplementation( async (walletId: string, rules: { destinationWalletId: string; percent: number }[]) => rules.map((r, i) => ({ @@ -164,6 +172,24 @@ describe("walletSplitsSet", () => { expect(result.errors[0].message).toMatch(/^No wallet found for/) }) + it("maps an inactive destination account (ENG-660)", async () => { + mockFindAccountById.mockImplementation(async (id: string) => ({ + id, + status: id === "dest-account" ? "closed" : "active", + })) + + const result = await resolveSet({ + walletId: SOURCE_ID, + splits: [{ destination: DEST_ID, percent: 10 }], + }) + + expect(result.splits).toBeUndefined() + expect(result.errors).toEqual([ + expect.objectContaining({ message: "Split destination account is not active" }), + ]) + expect(mockReplaceForWallet).not.toHaveBeenCalled() + }) + it("maps the disabled flag", async () => { PaymentSplitsConfig.enabled = false