From af4f45bd82b4c5e5bd58ff1c058fa9e43ee44fe0 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Wed, 22 Jul 2026 23:35:25 +0200 Subject: [PATCH 1/2] fix(fiat-output): reject spreadsheet date serials on admin date fields and repair affected valutaDate rows (#4315) * fix(fiat-output): reject spreadsheet date serials on admin date fields and repair affected valutaDate rows * fix(fiat-output): widen the serial repair window to all post-2000 serials and restore from the audit log on revert * fix(fiat-output): make the serial repair audit atomic and guard the rollback by the audited after value * fix(fiat-output): lock repair candidates and gate both mutations on their audit rows --- ...00000011-FixFiatOutputValutaDateSerials.js | 113 ++++++++++++++++++ .../__tests__/create-fiat-output.dto.spec.ts | 68 +++++++++++ .../fiat-output/dto/create-fiat-output.dto.ts | 7 +- .../fiat-output/dto/update-fiat-output.dto.ts | 24 +++- 4 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 migration/1784600000011-FixFiatOutputValutaDateSerials.js create mode 100644 src/subdomains/supporting/fiat-output/dto/__tests__/create-fiat-output.dto.spec.ts diff --git a/migration/1784600000011-FixFiatOutputValutaDateSerials.js b/migration/1784600000011-FixFiatOutputValutaDateSerials.js new file mode 100644 index 0000000000..6fcb23e1cb --- /dev/null +++ b/migration/1784600000011-FixFiatOutputValutaDateSerials.js @@ -0,0 +1,113 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * @class + * @implements {MigrationInterface} + */ +module.exports = class FixFiatOutputValutaDateSerials1784600000011 { + name = 'FixFiatOutputValutaDateSerials1784600000011'; + + /** + * Spreadsheet date serials were sent as numbers and class-transformer interpreted them as + * milliseconds since 1970-01-01, yielding broken 1970 timestamps. The millisecond portion equals + * the original serial (days since 1899-12-30); conversion is deterministic within the serial + * window for dates from 2000-01-01 (serial 36526) to the observed maximum (serial ~46300 / + * 1970-01-01 00:00:46.300): + * "valutaDate" >= TIMESTAMP '1970-01-01 00:00:36.526' AND "valutaDate" < TIMESTAMP '1970-01-01 00:00:46.300' + * Known 1970-01-01T00:00:00.001Z legacy values exist only in "isReadyDate", never in "valutaDate", + * and lie outside this window (no overlap). + * + * Audit and repair run in a single atomic statement with a shared row snapshot (data-modifying + * CTEs): there is no window between audit insert and mutation in which concurrent changes could + * be repaired without being logged. Affected rows are locked with FOR UPDATE. The final UPDATE + * is gated by EXISTS on the audit CTE so audit-before-mutation is structurally enforced, not only + * implied by statement atomicity. + * + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + // Millisecond epoch offset is the spreadsheet serial; add that many days to 1899-12-30. + // Audit + repair share one snapshot via data-modifying CTEs (fail closed if insert fails). + await queryRunner.query(` +WITH "affected" AS ( + SELECT "id", "valutaDate", + TIMESTAMP '1899-12-30 00:00:00' + ROUND(EXTRACT(EPOCH FROM "valutaDate") * 1000) * INTERVAL '1 day' AS "repairedDate" + FROM "fiat_output" + WHERE "valutaDate" >= TIMESTAMP '1970-01-01 00:00:36.526' AND "valutaDate" < TIMESTAMP '1970-01-01 00:00:46.300' + FOR UPDATE +), +"audit" AS ( + INSERT INTO "log" ("created", "updated", "system", "subsystem", "severity", "message") + SELECT now(), now(), 'FiatOutput', 'ValutaDateSerialRepair', 'Info', + json_agg(json_build_object( + 'id', "id", + 'before', to_char("valutaDate", 'YYYY-MM-DD HH24:MI:SS.MS'), + 'after', to_char("repairedDate", 'YYYY-MM-DD HH24:MI:SS.MS') + ))::text + FROM "affected" + HAVING count(*) > 0 + RETURNING 1 +) +UPDATE "fiat_output" f +SET "valutaDate" = a."repairedDate" +FROM "affected" a +WHERE f."id" = a."id" AND EXISTS (SELECT 1 FROM "audit"); +`); + } + + /** + * Restore previous "valutaDate" values exclusively from the newest audit log entry written by + * up() (system='FiatOutput', subsystem='ValutaDateSerialRepair'). Restores only rows still holding + * the audited 'after' value (guard against silently destroying legitimate post-repair corrections). + * Writes its own rollback audit log entry (subsystem='ValutaDateSerialRepairRollback') before the + * mutation in the same atomic statement. No calendar-date or midnight heuristic: legitimate + * production valutaDates frequently are exactly midnight UTC (00:00:00.000), so a + * date_trunc/midnight check would falsely reverse thousands of valid rows. Missing audit log + * yields a no-op (0 rows). Restorable rows are locked with FOR UPDATE OF f; the after-guard is + * re-checked at update time. The final UPDATE is gated by EXISTS on the rollback audit CTE so + * audit-before-mutation is structurally enforced, not only implied by statement atomicity. + * + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(` +WITH "auditSource" AS ( + SELECT "id" AS "logId", "message" FROM "log" + WHERE "system" = 'FiatOutput' AND "subsystem" = 'ValutaDateSerialRepair' + ORDER BY "id" DESC LIMIT 1 +), +"entries" AS ( + SELECT s."logId", jsonb_array_elements(s."message"::jsonb) AS "elem" FROM "auditSource" s +), +"restorable" AS ( + SELECT e."logId", (e."elem"->>'id')::int AS "rowId", + (e."elem"->>'before')::timestamp AS "beforeValue", + (e."elem"->>'after')::timestamp AS "afterValue" + FROM "entries" e + JOIN "fiat_output" f ON f."id" = (e."elem"->>'id')::int + WHERE f."valutaDate" = (e."elem"->>'after')::timestamp + FOR UPDATE OF f +), +"rollbackAudit" AS ( + INSERT INTO "log" ("created", "updated", "system", "subsystem", "severity", "message") + SELECT now(), now(), 'FiatOutput', 'ValutaDateSerialRepairRollback', 'Info', + json_agg(json_build_object( + 'sourceLogId', "logId", + 'id', "rowId", + 'before', to_char("afterValue", 'YYYY-MM-DD HH24:MI:SS.MS'), + 'after', to_char("beforeValue", 'YYYY-MM-DD HH24:MI:SS.MS') + ))::text + FROM "restorable" + HAVING count(*) > 0 + RETURNING 1 +) +UPDATE "fiat_output" f +SET "valutaDate" = r."beforeValue" +FROM "restorable" r +WHERE f."id" = r."rowId" AND f."valutaDate" = r."afterValue" AND EXISTS (SELECT 1 FROM "rollbackAudit"); +`); + } +}; diff --git a/src/subdomains/supporting/fiat-output/dto/__tests__/create-fiat-output.dto.spec.ts b/src/subdomains/supporting/fiat-output/dto/__tests__/create-fiat-output.dto.spec.ts new file mode 100644 index 0000000000..6d1714a4a3 --- /dev/null +++ b/src/subdomains/supporting/fiat-output/dto/__tests__/create-fiat-output.dto.spec.ts @@ -0,0 +1,68 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { CreateFiatOutputDto } from '../create-fiat-output.dto'; +import { UpdateFiatOutputDto } from '../update-fiat-output.dto'; +import { FiatOutputType } from '../../fiat-output.entity'; + +describe('CreateFiatOutputDto.valutaDate', () => { + const baseDto = { + type: FiatOutputType.BUY_FIAT, + amount: 100, + currency: 'EUR', + name: 'Test', + address: 'Street', + city: 'Zurich', + iban: 'CH9300762011623852957', + zip: '8000', + country: 'CH', + }; + + const validateDto = async (raw: Record) => { + const instance = plainToInstance(CreateFiatOutputDto, raw); + return validate(instance); + }; + + it('accepts a valid ISO date', async () => { + const errors = await validateDto({ ...baseDto, valutaDate: '2026-07-22' }); + expect(errors).toEqual([]); + }); + + it('accepts the inclusive MinDate boundary 2000-01-01T00:00:00Z', async () => { + const errors = await validateDto({ ...baseDto, valutaDate: '2000-01-01T00:00:00Z' }); + expect(errors).toEqual([]); + }); + + it('rejects a numeric spreadsheet date serial', async () => { + const errors = await validateDto({ ...baseDto, valutaDate: 46225 }); + expect(errors).toHaveLength(1); + expect(errors[0].constraints).toHaveProperty('minDate'); + }); +}); + +describe('UpdateFiatOutputDto date fields', () => { + const dateFields = [ + 'valutaDate', + 'isReadyDate', + 'isTransmittedDate', + 'isConfirmedDate', + 'isApprovedDate', + 'outputDate', + ] as const; + + const validateDto = async (raw: Record) => { + const instance = plainToInstance(UpdateFiatOutputDto, raw); + return validate(instance); + }; + + it.each(dateFields)('%s accepts a valid ISO date', async (field) => { + const errors = await validateDto({ [field]: '2026-07-22' }); + expect(errors.filter((e) => e.property === field && e.constraints?.minDate)).toEqual([]); + }); + + it.each(dateFields)('%s rejects a numeric spreadsheet date serial', async (field) => { + const errors = await validateDto({ [field]: 46225 }); + const fieldError = errors.find((e) => e.property === field); + expect(fieldError).toBeDefined(); + expect(fieldError!.constraints).toHaveProperty('minDate'); + }); +}); diff --git a/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts b/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts index 08354fd79c..527e01addc 100644 --- a/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts +++ b/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts @@ -1,7 +1,9 @@ import { Type } from 'class-transformer'; -import { IsDate, IsEnum, IsNotEmpty, IsNumber, IsOptional, IsString } from 'class-validator'; +import { IsDate, IsEnum, IsNotEmpty, IsNumber, IsOptional, IsString, MinDate } from 'class-validator'; import { FiatOutputType } from '../fiat-output.entity'; +export const MIN_FIAT_OUTPUT_DATE = new Date('2000-01-01T00:00:00Z'); + export class CreateFiatOutputDto { @IsOptional() @IsNumber() @@ -65,6 +67,9 @@ export class CreateFiatOutputDto { @IsOptional() @IsDate() + @MinDate(MIN_FIAT_OUTPUT_DATE, { + message: 'valutaDate must be an ISO date on or after 2000-01-01 (numeric spreadsheet date serials are rejected)', + }) @Type(() => Date) valutaDate?: Date; diff --git a/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts b/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts index 78044fa22e..103eaf83a6 100644 --- a/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts +++ b/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts @@ -1,6 +1,7 @@ import { Type } from 'class-transformer'; -import { IsBoolean, IsDate, IsNumber, IsOptional, IsString } from 'class-validator'; +import { IsBoolean, IsDate, IsNumber, IsOptional, IsString, MinDate } from 'class-validator'; import { TransactionCharge } from '../fiat-output.entity'; +import { MIN_FIAT_OUTPUT_DATE } from './create-fiat-output.dto'; export class UpdateFiatOutputDto { @IsOptional() @@ -29,6 +30,9 @@ export class UpdateFiatOutputDto { @IsOptional() @IsDate() + @MinDate(MIN_FIAT_OUTPUT_DATE, { + message: 'valutaDate must be an ISO date on or after 2000-01-01 (numeric spreadsheet date serials are rejected)', + }) @Type(() => Date) valutaDate?: Date; @@ -106,21 +110,36 @@ export class UpdateFiatOutputDto { @IsOptional() @IsDate() + @MinDate(MIN_FIAT_OUTPUT_DATE, { + message: 'isReadyDate must be an ISO date on or after 2000-01-01 (numeric spreadsheet date serials are rejected)', + }) @Type(() => Date) isReadyDate?: Date; @IsOptional() @IsDate() + @MinDate(MIN_FIAT_OUTPUT_DATE, { + message: + 'isTransmittedDate must be an ISO date on or after 2000-01-01 (numeric spreadsheet date serials are rejected)', + }) @Type(() => Date) isTransmittedDate?: Date; @IsOptional() @IsDate() + @MinDate(MIN_FIAT_OUTPUT_DATE, { + message: + 'isConfirmedDate must be an ISO date on or after 2000-01-01 (numeric spreadsheet date serials are rejected)', + }) @Type(() => Date) isConfirmedDate?: Date; @IsOptional() @IsDate() + @MinDate(MIN_FIAT_OUTPUT_DATE, { + message: + 'isApprovedDate must be an ISO date on or after 2000-01-01 (numeric spreadsheet date serials are rejected)', + }) @Type(() => Date) isApprovedDate?: Date; @@ -130,6 +149,9 @@ export class UpdateFiatOutputDto { @IsOptional() @IsDate() + @MinDate(MIN_FIAT_OUTPUT_DATE, { + message: 'outputDate must be an ISO date on or after 2000-01-01 (numeric spreadsheet date serials are rejected)', + }) @Type(() => Date) outputDate?: Date; From d0cedd309f27a9fcb190c80fa73eb6afb72a2455 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 23 Jul 2026 07:57:42 +0200 Subject: [PATCH 2/2] fix(liquidity): match Bank Frick balances by account IBAN and restrict automatic payout bank selection (#4314) * fix(liquidity): match Bank Frick balances by account IBAN and restrict automatic payout bank selection * fix(liquidity): scope the Frick availability guard to matched accounts and protect explicit bank assignments * fix(fiat-output): resolve the sender bank whenever an account IBAN is set and repair missing bank links * fix(fiat-output): reach rows with a set account IBAN but missing bank link in the assignment job * fix(fiat-output): reject an empty account IBAN and pin the assignment query contract in tests * fix(fiat-output): reject explicit null account IBANs and assert the repair query branch in tests * fix(fiat-output): guard assignment writes with compare-and-set criteria against concurrent admin updates * fix(fiat-output): match legacy empty-string account IBANs in the assignment compare-and-set --- .../balances/__tests__/bank.adapter.spec.ts | 105 ++++++++++- .../adapters/balances/bank.adapter.ts | 46 ++++- .../liquidity-management-balance.service.ts | 13 +- .../buy-fiat-preparation.service.spec.ts | 10 +- .../services/buy-fiat-preparation.service.ts | 3 +- src/subdomains/generic/gs/dto/gs.dto.ts | 2 + .../bank-tx/services/bank-tx.service.ts | 2 +- .../supporting/bank/bank/bank.service.ts | 4 + .../__tests__/fiat-output-job.service.spec.ts | 153 ++++++++++++++-- .../__tests__/fiat-output.service.spec.ts | 166 ++++++++++++++++++ .../fiat-output/dto/create-fiat-output.dto.ts | 9 +- .../fiat-output/dto/update-fiat-output.dto.ts | 6 +- .../fiat-output/fiat-output-job.service.ts | 51 +++++- .../fiat-output/fiat-output.service.ts | 50 +++--- 14 files changed, 541 insertions(+), 79 deletions(-) create mode 100644 src/subdomains/supporting/fiat-output/__tests__/fiat-output.service.spec.ts diff --git a/src/subdomains/core/liquidity-management/adapters/balances/__tests__/bank.adapter.spec.ts b/src/subdomains/core/liquidity-management/adapters/balances/__tests__/bank.adapter.spec.ts index a79914bee9..c2e0092d96 100644 --- a/src/subdomains/core/liquidity-management/adapters/balances/__tests__/bank.adapter.spec.ts +++ b/src/subdomains/core/liquidity-management/adapters/balances/__tests__/bank.adapter.spec.ts @@ -5,6 +5,7 @@ import { YapealService } from 'src/integration/bank/services/yapeal.service'; import { CheckoutService } from 'src/integration/checkout/services/checkout.service'; import { createCustomAsset } from 'src/shared/models/asset/__mocks__/asset.entity.mock'; import { BankTxBatchService } from 'src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx-batch.service'; +import { Bank } from 'src/subdomains/supporting/bank/bank/bank.entity'; import { BankService } from 'src/subdomains/supporting/bank/bank/bank.service'; import { IbanBankName } from 'src/subdomains/supporting/bank/bank/dto/bank.dto'; import { LiquidityManagementAsset } from '../../../interfaces'; @@ -37,32 +38,120 @@ describe('BankAdapter', () => { ); }); - function frickAsset(dexName: string): LiquidityManagementAsset { - return Object.assign(createCustomAsset({ dexName }), { context: IbanBankName.FRICK }); + function frickAsset(dexName: string, id?: number): LiquidityManagementAsset { + return Object.assign(createCustomAsset({ dexName, id }), { context: IbanBankName.FRICK }); } - it('routes FRICK to frickService.getBalances() and creates a LiquidityBalance per matching asset currency, mirroring the Yapeal case', async () => { + function frickBank(asset: LiquidityManagementAsset, iban: string): Bank { + return createMock({ asset, iban }); + } + + it('matches same-currency Bank Frick accounts to assets by their linked IBAN', async () => { + const firstEurAsset = frickAsset('EUR', 1); + const secondEurAsset = frickAsset('EUR', 2); + jest + .spyOn(bankService, 'getBanksWithAsset') + .mockResolvedValue([ + frickBank(firstEurAsset, 'LI01 0000 0000 0000 0001'), + frickBank(secondEurAsset, 'LI02 0000 0000 0000 0002'), + ]); + jest.spyOn(frickService, 'getBalances').mockResolvedValue([ + { + iban: 'LI010000000000000001', + currency: 'EUR', + balance: 1000, + availableBalance: 900, + }, + { + iban: 'li020000000000000002', + currency: 'EUR', + balance: 2000, + availableBalance: 1800, + }, + ]); + + const result = await adapter.getForBank(IbanBankName.FRICK, [firstEurAsset, secondEurAsset]); + + expect(result).toHaveLength(2); + expect(result[0].asset).toBe(firstEurAsset); + expect(result[0].amount).toBe(900); + expect(result[1].asset).toBe(secondEurAsset); + expect(result[1].amount).toBe(1800); + }); + + it('fails closed when a Bank Frick asset has no linked bank account', async () => { + jest.spyOn(bankService, 'getBanksWithAsset').mockResolvedValue([]); jest .spyOn(frickService, 'getBalances') .mockResolvedValue([{ iban: 'LI-EUR', currency: 'EUR', balance: 1000, availableBalance: 900 }]); - const eurAsset = frickAsset('EUR'); - const chfAsset = frickAsset('CHF'); - const result = await adapter.getForBank(IbanBankName.FRICK, [eurAsset, chfAsset]); + await expect(adapter.getForBank(IbanBankName.FRICK, [frickAsset('EUR', 1)])).rejects.toThrow( + 'Bank Frick account is not linked to asset', + ); + }); + + it('fails closed when the linked Bank Frick IBAN is missing from the API response', async () => { + const eurAsset = frickAsset('EUR', 1); + jest.spyOn(bankService, 'getBanksWithAsset').mockResolvedValue([frickBank(eurAsset, 'LI-LINKED')]); + jest + .spyOn(frickService, 'getBalances') + .mockResolvedValue([{ iban: 'LI-OTHER', currency: 'EUR', balance: 1000, availableBalance: 900 }]); + + await expect(adapter.getForBank(IbanBankName.FRICK, [eurAsset])).rejects.toThrow( + 'No Bank Frick account found for IBAN LI-LINKED', + ); + }); + + it('ignores Bank Frick API accounts that are not linked to a liquidity asset', async () => { + const eurAsset = frickAsset('EUR', 1); + jest.spyOn(bankService, 'getBanksWithAsset').mockResolvedValue([frickBank(eurAsset, 'LI-LINKED')]); + jest.spyOn(frickService, 'getBalances').mockResolvedValue([ + { iban: 'LI-LINKED', currency: 'EUR', balance: 1000, availableBalance: 900 }, + { iban: 'LI-OPERATING', currency: 'EUR', balance: 5000, availableBalance: 4500 }, + ]); + + const result = await adapter.getForBank(IbanBankName.FRICK, [eurAsset]); expect(result).toHaveLength(1); expect(result[0].asset).toBe(eurAsset); expect(result[0].amount).toBe(900); }); - it('fails closed when Bank Frick reports no available balance for an account, instead of silently using the booked balance', async () => { + it('fails closed when Bank Frick reports no available balance for a linked account, instead of silently using the booked balance', async () => { + const eurAsset = frickAsset('EUR', 1); + jest.spyOn(bankService, 'getBanksWithAsset').mockResolvedValue([frickBank(eurAsset, 'LI-EUR')]); jest.spyOn(frickService, 'getBalances').mockResolvedValue([{ iban: 'LI-EUR', currency: 'EUR', balance: 1000 }]); - await expect(adapter.getForBank(IbanBankName.FRICK, [frickAsset('EUR')])).rejects.toThrow( + await expect(adapter.getForBank(IbanBankName.FRICK, [eurAsset])).rejects.toThrow( 'Missing available balance for Bank Frick account LI-EUR', ); }); + it('ignores an unmatched Bank Frick account with no available balance and still processes the linked account', async () => { + const eurAsset = frickAsset('EUR', 1); + jest.spyOn(bankService, 'getBanksWithAsset').mockResolvedValue([frickBank(eurAsset, 'LI-LINKED')]); + jest.spyOn(frickService, 'getBalances').mockResolvedValue([ + { iban: 'LI-LINKED', currency: 'EUR', balance: 1000, availableBalance: 900 }, + { iban: 'LI-OPERATING', currency: 'EUR', balance: 5000 }, + ]); + + const result = await adapter.getForBank(IbanBankName.FRICK, [eurAsset]); + + expect(result).toHaveLength(1); + expect(result[0].asset).toBe(eurAsset); + expect(result[0].amount).toBe(900); + }); + + it('fails closed when the linked Bank Frick account currency does not match the asset', async () => { + const eurAsset = frickAsset('EUR', 1); + jest.spyOn(bankService, 'getBanksWithAsset').mockResolvedValue([frickBank(eurAsset, 'LI-LINKED')]); + jest + .spyOn(frickService, 'getBalances') + .mockResolvedValue([{ iban: 'LI-LINKED', currency: 'CHF', balance: 1000, availableBalance: 900 }]); + + await expect(adapter.getForBank(IbanBankName.FRICK, [eurAsset])).rejects.toThrow(/currency mismatch/i); + }); + it('propagates and logs a getBalances() failure the same way as the other cases', async () => { jest.spyOn(frickService, 'getBalances').mockRejectedValue(new Error('Bank Frick unavailable')); diff --git a/src/subdomains/core/liquidity-management/adapters/balances/bank.adapter.ts b/src/subdomains/core/liquidity-management/adapters/balances/bank.adapter.ts index db95b3ad52..ec588b1153 100644 --- a/src/subdomains/core/liquidity-management/adapters/balances/bank.adapter.ts +++ b/src/subdomains/core/liquidity-management/adapters/balances/bank.adapter.ts @@ -77,20 +77,52 @@ export class BankAdapter implements LiquidityBalanceIntegration { } case IbanBankName.FRICK: { + const normalizeIban = (iban: string): string => iban.replace(/\s/g, '').toUpperCase(); + const frickBalances = await this.frickService.getBalances(); - for (const balance of frickBalances) { - // Bank Frick's `available` field is optional in its own account-listing contract. Falling - // back to the booked `balance` would overstate spendable liquidity (it ignores pending - // debits) - exactly the overdraft risk this case exists to close - so a missing available - // balance fails loud instead of silently substituting a different, unsafe number. + const banks = await this.bankService.getBanksWithAsset(); + const matchedIbans = new Set(); + + for (const asset of assets) { + const bank = banks.find((bank) => bank.asset?.id === asset.id); + if (!bank) throw new Error(`Bank Frick account is not linked to asset ${asset.uniqueName}`); + + const bankIban = normalizeIban(bank.iban); + const balance = frickBalances.find((balance) => { + const balanceIban = normalizeIban(balance.iban); + return balanceIban === bankIban; + }); + if (!balance) + throw new Error(`No Bank Frick account found for IBAN ${bank.iban} (asset ${asset.uniqueName})`); + + // A wrongly linked IBAN (e.g. a CHF account linked to a Frick EUR asset) must fail loud instead of + // silently recording the wrong currency's balance as this asset's liquidity. + if (balance.currency !== asset.dexName) + throw new Error( + `Currency mismatch for Bank Frick account ${bank.iban}: ` + + `expected ${asset.dexName}, found ${balance.currency} (asset ${asset.uniqueName})`, + ); + + // Bank Frick's `available` field is optional in its own account-listing contract. Falling back to + // the booked `balance` would overstate spendable liquidity (it ignores pending debits) - exactly + // the overdraft risk this case exists to close - so a missing available balance fails loud instead + // of silently substituting a different, unsafe number. This only applies to the account actually + // matched to an asset here - an unmatched account without an available balance (e.g. the operating + // account) plays no role in any asset's liquidity figure and must not crash the refresh. if (!Number.isFinite(balance.availableBalance)) throw new Error(`Missing available balance for Bank Frick account ${balance.iban}`); - const matchingAssets = assets.filter((asset) => asset.dexName === balance.currency); - matchingAssets.forEach((asset) => balances.push(LiquidityBalance.create(asset, balance.availableBalance))); + matchedIbans.add(normalizeIban(balance.iban)); + balances.push(LiquidityBalance.create(asset, balance.availableBalance)); } + const unmatchedIbans = frickBalances + .filter((balance) => !matchedIbans.has(normalizeIban(balance.iban))) + .map((balance) => balance.iban); + if (unmatchedIbans.length) + this.logger.verbose(`Ignored unmatched Bank Frick accounts: ${unmatchedIbans.join(', ')}`); + break; } diff --git a/src/subdomains/core/liquidity-management/services/liquidity-management-balance.service.ts b/src/subdomains/core/liquidity-management/services/liquidity-management-balance.service.ts index bfe80b76f2..9d420241df 100644 --- a/src/subdomains/core/liquidity-management/services/liquidity-management-balance.service.ts +++ b/src/subdomains/core/liquidity-management/services/liquidity-management-balance.service.ts @@ -10,7 +10,8 @@ import { LiquidityBalanceIntegrationFactory } from '../factories/liquidity-balan import { LiquidityBalanceRepository } from '../repositories/liquidity-balance.repository'; export interface BankBalanceUpdate { - bank: Bank; + bank: Bank | null; + iban: string; balance: number; } @@ -59,7 +60,17 @@ export class LiquidityManagementBalanceService implements OnModuleInit { } async refreshBankBalance(dto: BankBalanceUpdate): Promise { + if (!dto.bank) { + this.logger.verbose(`Skipping bank balance refresh: no bank found for IBAN ${dto.iban}`); + return; + } + const entity = await this.balanceRepo.findOne({ where: { asset: { bank: { id: dto.bank.id } } } }); + if (!entity) { + this.logger.verbose(`Skipping bank balance refresh for bank ${dto.bank.id}: liquidity balance not found`); + return; + } + await this.balanceRepo.update(entity.id, { amount: dto.balance }); } diff --git a/src/subdomains/core/sell-crypto/process/services/__tests__/buy-fiat-preparation.service.spec.ts b/src/subdomains/core/sell-crypto/process/services/__tests__/buy-fiat-preparation.service.spec.ts index fc6ae59cec..fb73832f4b 100644 --- a/src/subdomains/core/sell-crypto/process/services/__tests__/buy-fiat-preparation.service.spec.ts +++ b/src/subdomains/core/sell-crypto/process/services/__tests__/buy-fiat-preparation.service.spec.ts @@ -160,7 +160,12 @@ describe('BuyFiatPreparationService', () => { it.each<[string, string, string, IbanBankName]>([ ['CHF through Yapeal', 'CHF', 'CH1234567890', IbanBankName.YAPEAL], ['EUR through Olkypay', 'EUR', 'DE1234567890', IbanBankName.OLKY], - ['Frick-eligible EUR through Bank Frick', 'EUR', 'LI1234567890', IbanBankName.FRICK], + [ + 'EUR through Olkypay for a Liechtenstein IBAN (Bank Frick is never auto-selected)', + 'EUR', + 'LI1234567890', + IbanBankName.OLKY, + ], ])('predicts %s and passes the selected bank to fee matching', async (_, currency, iban, bankName) => { const bank = createCustomBank({ name: bankName, currency }); const { entity, country } = arrangeRefreshFee(currency, iban, bank); @@ -172,7 +177,6 @@ describe('BuyFiatPreparationService', () => { entity.outputAsset.name, FiatOutputType.BUY_FIAT, entity.userData, - false, country, ); expect(transactionHelper.getTxFeeInfos).toHaveBeenCalledWith( @@ -188,7 +192,7 @@ describe('BuyFiatPreparationService', () => { entity.user, ); - if ([IbanBankName.OLKY, IbanBankName.FRICK].includes(bankName)) { + if (bankName === IbanBankName.OLKY) { const bankOut = jest.mocked(transactionHelper.getTxFeeInfos).mock.calls[0][8]; expect(bankOut).not.toBe(IbanBankName.YAPEAL); } diff --git a/src/subdomains/core/sell-crypto/process/services/buy-fiat-preparation.service.ts b/src/subdomains/core/sell-crypto/process/services/buy-fiat-preparation.service.ts index 4aa4988a9b..9aa73a679f 100644 --- a/src/subdomains/core/sell-crypto/process/services/buy-fiat-preparation.service.ts +++ b/src/subdomains/core/sell-crypto/process/services/buy-fiat-preparation.service.ts @@ -279,7 +279,6 @@ export class BuyFiatPreparationService { entity.outputAsset.name, FiatOutputType.BUY_FIAT, entity.userData, - false, country, ); @@ -294,7 +293,7 @@ export class BuyFiatPreparationService { undefined, // This prediction and the later FiatOutput bank assignment share the single source of truth, // FiatOutputService.selectPayoutBank, and agree while the underlying state is unchanged. Active - // virtual IBANs, sender-bank send/sendPriority, and Bank Frick availability are live and mutable; + // virtual IBANs and sender-bank send/sendPriority are live and mutable; // because this prediction is neither persisted nor reconciled, bankOut is best-effort and can // differ if, for example, a virtual IBAN activates or a bank's send/priority changes between calls. // Fee.verifyForTx in fee.entity.ts matches bank-scoped Fees against its banks array, so an unknown diff --git a/src/subdomains/generic/gs/dto/gs.dto.ts b/src/subdomains/generic/gs/dto/gs.dto.ts index 269b24cdbe..0e4b21d3c0 100644 --- a/src/subdomains/generic/gs/dto/gs.dto.ts +++ b/src/subdomains/generic/gs/dto/gs.dto.ts @@ -137,6 +137,7 @@ export const DebugAllowedColumns: Record = { 'created', 'updated', 'amlEnabled', + 'assetId', 'bic', 'currency', 'iban', @@ -144,6 +145,7 @@ export const DebugAllowedColumns: Record = { 'receive', 'sctInst', 'send', + 'sendPriority', ], }, bank_account: { diff --git a/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts b/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts index 21826fa71d..2853cdb53f 100644 --- a/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts +++ b/src/subdomains/supporting/bank-tx/bank-tx/services/bank-tx.service.ts @@ -583,7 +583,7 @@ export class BankTxService implements OnModuleInit { // update bank liq balance const bank = await this.bankService.getBankByIban(batch.iban); - this.bankBalanceSubject.next({ bank, balance: batch.bankBalanceAfter }); + this.bankBalanceSubject.next({ bank, iban: batch.iban, balance: batch.bankBalanceAfter }); // avoid infinite loop in JSON batch.transactions = newTxs.map((tx) => { diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index a994329a88..0bdedb57e1 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -28,6 +28,10 @@ export class BankService implements OnModuleInit { return this.bankRepo.findCached(`all`); } + async getBanksWithAsset(): Promise { + return this.bankRepo.find({ relations: { asset: true } }); + } + async getBanksByName(bankName: IbanBankName): Promise { return this.bankRepo.findCachedBy(bankName, { name: bankName }); } diff --git a/src/subdomains/supporting/fiat-output/__tests__/fiat-output-job.service.spec.ts b/src/subdomains/supporting/fiat-output/__tests__/fiat-output-job.service.spec.ts index 1ca2bfcb88..6c8bfb1f13 100644 --- a/src/subdomains/supporting/fiat-output/__tests__/fiat-output-job.service.spec.ts +++ b/src/subdomains/supporting/fiat-output/__tests__/fiat-output-job.service.spec.ts @@ -1,5 +1,6 @@ import { createMock } from '@golevelup/ts-jest'; import { Test, TestingModule } from '@nestjs/testing'; +import { In, IsNull, Not } from 'typeorm'; import { FrickPaymentState } from 'src/integration/bank/dto/frick.dto'; import { BankFrickService } from 'src/integration/bank/services/frick.service'; import { IbanService } from 'src/integration/bank/services/iban.service'; @@ -159,13 +160,36 @@ describe('FiatOutputJobService', () => { await service['assignBankAccount'](); const updateCalls = (fiatOutputRepo.update as jest.Mock).mock.calls; - expect(updateCalls[0][0]).toBe(1); + expect(updateCalls[0][0]).toEqual({ id: 1, accountIban: IsNull() }); expect(updateCalls[0][1]).toMatchObject({ originEntityId: 100, accountIban: yapealEUR.iban }); - expect(updateCalls[1][0]).toBe(3); + expect(updateCalls[1][0]).toEqual({ id: 3, accountIban: IsNull() }); expect(updateCalls[1][1]).toMatchObject({ originEntityId: 102, accountIban: yapealEUR.iban }); }); + it('should assign bank account when accountIban is an empty string (legacy rows)', async () => { + jest.spyOn(fiatOutputRepo, 'find').mockResolvedValue([ + createCustomFiatOutput({ + id: 1, + accountIban: '', + bank: undefined, + type: FiatOutputType.BUY_FIAT, + isComplete: false, + buyFiats: [createCustomBuyFiat({ id: 100, sell: createCustomSell({ iban: 'DE123456789' }) })], + }), + ]); + + jest.spyOn(countryService, 'getCountryWithSymbol').mockResolvedValue(createCustomCountry({ yapealEnable: true })); + + jest.spyOn(bankService, 'getSenderBanks').mockResolvedValue([yapealEUR]); + + await service['assignBankAccount'](); + + const updateCalls = (fiatOutputRepo.update as jest.Mock).mock.calls; + expect(updateCalls[0][0]).toEqual({ id: 1, accountIban: '' }); + expect(updateCalls[0][1]).toMatchObject({ originEntityId: 100, accountIban: yapealEUR.iban }); + }); + it('should use virtual IBAN when user has one for BuyFiat', async () => { const virtualIban = 'CH1234567890VIBAN'; @@ -188,7 +212,7 @@ describe('FiatOutputJobService', () => { await service['assignBankAccount'](); const updateCalls = (fiatOutputRepo.update as jest.Mock).mock.calls; - expect(updateCalls[0][0]).toBe(1); + expect(updateCalls[0][0]).toEqual({ id: 1, accountIban: IsNull() }); expect(updateCalls[0][1]).toMatchObject({ originEntityId: 100, accountIban: virtualIban }); }); @@ -214,11 +238,11 @@ describe('FiatOutputJobService', () => { await service['assignBankAccount'](); const updateCalls = (fiatOutputRepo.update as jest.Mock).mock.calls; - expect(updateCalls[0][0]).toBe(1); + expect(updateCalls[0][0]).toEqual({ id: 1, accountIban: IsNull() }); expect(updateCalls[0][1]).toMatchObject({ originEntityId: 100, accountIban: virtualIban }); }); - it('skips an unavailable Bank Frick sender and selects the next eligible sender', async () => { + it('excludes Bank Frick from automatic sender selection regardless of payout-creation availability', async () => { const frick = createCustomBank({ name: IbanBankName.FRICK, currency: 'EUR', @@ -236,7 +260,7 @@ describe('FiatOutputJobService', () => { expect(result).toEqual({ accountIban: yapealEUR.iban, bank: yapealEUR }); }); - it('does not select Bank Frick without the configured instant-payment capability', async () => { + it('excludes Bank Frick from automatic sender selection regardless of its instant-payment capability', async () => { const frick = createCustomBank({ name: IbanBankName.FRICK, currency: 'EUR', @@ -254,7 +278,7 @@ describe('FiatOutputJobService', () => { expect(result).toEqual({ accountIban: olkyEUR.iban, bank: olkyEUR }); }); - it('still routes an EUR payout to Olkypay while Frick EUR is send=true with the seeded (worse) default priority', async () => { + it('excludes Bank Frick from automatic sender selection even when its sender priority is worse', async () => { const frick = createCustomBank({ name: IbanBankName.FRICK, currency: 'EUR', @@ -272,7 +296,7 @@ describe('FiatOutputJobService', () => { expect(result).toEqual({ accountIban: olkyEUR.iban, bank: olkyEUR }); }); - it('routes to Frick once its priority is lowered below the incumbent sender', async () => { + it('never auto-selects Bank Frick even when it has better sender priority than the incumbent', async () => { const frick = createCustomBank({ name: IbanBankName.FRICK, currency: 'EUR', @@ -287,10 +311,10 @@ describe('FiatOutputJobService', () => { createCustomCountry({ yapealEnable: true }), ); - expect(result).toEqual({ accountIban: frick.iban, bank: frick }); + expect(result).toEqual({ accountIban: yapealEUR.iban, bank: yapealEUR }); }); - it('throws only when two eligible banks share the exact same priority, not merely because Frick coexists', async () => { + it('never throws on a priority tie with Bank Frick - Frick is filtered out before the tie can even occur', async () => { const frick = createCustomBank({ name: IbanBankName.FRICK, currency: 'EUR', @@ -300,12 +324,12 @@ describe('FiatOutputJobService', () => { }); jest.spyOn(bankService, 'getSenderBanks').mockResolvedValue([frick, yapealEUR]); - await expect( - service['getPayoutAccount']( - createCustomFiatOutput({ currency: 'EUR', buyFiats: [] }), - createCustomCountry({ yapealEnable: true }), - ), - ).rejects.toThrow('Ambiguous sender bank priority for EUR'); + const result = await service['getPayoutAccount']( + createCustomFiatOutput({ currency: 'EUR', buyFiats: [] }), + createCustomCountry({ yapealEnable: true }), + ); + + expect(result).toEqual({ accountIban: yapealEUR.iban, bank: yapealEUR }); }); it('routes to the highest-priority (lowest number) sender when multiple non-Frick banks are eligible', async () => { @@ -334,7 +358,7 @@ describe('FiatOutputJobService', () => { expect(result).toEqual({ accountIban: olkyEUR.iban, bank: olkyEUR }); }); - it('does not use an unavailable Bank Frick virtual IBAN', async () => { + it('excludes an unavailable Bank Frick virtual IBAN from automatic selection unconditionally', async () => { const frick = createCustomBank({ name: IbanBankName.FRICK, send: true }); jest.spyOn(frickPayoutService, 'canCreatePayments').mockReturnValue(false); jest @@ -352,6 +376,101 @@ describe('FiatOutputJobService', () => { expect(result).toEqual({ accountIban: undefined, bank: undefined }); }); + + it('keeps an already-assigned account IBAN and bank relation without re-resolving or auto-selecting', async () => { + const accountIban = 'LI75088110103524'; + jest.spyOn(fiatOutputRepo, 'find').mockResolvedValue([ + createCustomFiatOutput({ + id: 1, + accountIban, + bank: olkyEUR, + type: FiatOutputType.BUY_FIAT, + isComplete: false, + buyFiats: [createCustomBuyFiat({ id: 100, sell: createCustomSell({ iban: 'DE123456789' }) })], + }), + ]); + jest.spyOn(bankService, 'getBankByIban').mockResolvedValue(olkyEUR); + + await service['assignBankAccount'](); + + const updateCalls = (fiatOutputRepo.update as jest.Mock).mock.calls; + expect(updateCalls).toHaveLength(1); + expect(updateCalls[0][0]).toEqual({ id: 1, accountIban }); + expect(updateCalls[0][1]).toMatchObject({ originEntityId: 100, bank: olkyEUR }); + expect(bankService.getBankByIban).not.toHaveBeenCalled(); + expect(bankService.getSenderBanks).not.toHaveBeenCalled(); + }); + + it('repairs a missing bank relation from an already-assigned account IBAN without auto-selecting', async () => { + const accountIban = 'LI75088110103524'; + jest.spyOn(fiatOutputRepo, 'find').mockResolvedValue([ + createCustomFiatOutput({ + id: 1, + accountIban, + bank: undefined, + type: FiatOutputType.BUY_FIAT, + isComplete: false, + buyFiats: [createCustomBuyFiat({ id: 100, sell: createCustomSell({ iban: 'DE123456789' }) })], + }), + ]); + jest.spyOn(bankService, 'getBankByIban').mockResolvedValue(olkyEUR); + + await service['assignBankAccount'](); + + const updateCalls = (fiatOutputRepo.update as jest.Mock).mock.calls; + expect(updateCalls).toHaveLength(1); + expect(updateCalls[0][0]).toEqual({ id: 1, accountIban }); + expect(updateCalls[0][1]).toMatchObject({ originEntityId: 100, bank: olkyEUR }); + expect(bankService.getBankByIban).toHaveBeenCalledWith(accountIban); + expect(bankService.getSenderBanks).not.toHaveBeenCalled(); + }); + + it('logs a bank lookup miss and continues to repair the next entity in the same run', async () => { + const accountIban = 'LI75088110103524'; + jest.spyOn(fiatOutputRepo, 'find').mockResolvedValue([ + createCustomFiatOutput({ + id: 1, + originEntityId: 123, + accountIban, + bank: undefined, + type: FiatOutputType.BUY_FIAT, + isComplete: false, + buyFiats: [createCustomBuyFiat({ id: 100, sell: createCustomSell({ iban: 'DE123456789' }) })], + }), + createCustomFiatOutput({ + id: 2, + originEntityId: 123, + accountIban, + bank: undefined, + type: FiatOutputType.BUY_FIAT, + isComplete: false, + buyFiats: [createCustomBuyFiat({ id: 200, sell: createCustomSell({ iban: 'DE123456789' }) })], + }), + ]); + jest.spyOn(bankService, 'getBankByIban').mockResolvedValueOnce(undefined).mockResolvedValueOnce(olkyEUR); + const loggerErrorSpy = jest.spyOn(service['logger'], 'error'); + + await service['assignBankAccount'](); + + const findArgs = (fiatOutputRepo.find as jest.Mock).mock.calls[0][0]; + expect(findArgs.where).toHaveLength(3); + expect(findArgs.where[2]).toEqual({ + valutaDate: IsNull(), + isComplete: false, + type: In([FiatOutputType.BUY_CRYPTO_FAIL, FiatOutputType.BUY_FIAT, FiatOutputType.BANK_TX_RETURN]), + accountIban: Not(IsNull()), + bank: IsNull(), + }); + + expect(loggerErrorSpy).toHaveBeenCalledWith( + 'Error in fillPreValutaDate fiatOutput: 1:', + expect.objectContaining({ message: `No bank found for account IBAN ${accountIban} (fiat output 1)` }), + ); + const updateCalls = (fiatOutputRepo.update as jest.Mock).mock.calls; + expect(updateCalls).toHaveLength(1); + expect(updateCalls[0][0]).toEqual({ id: 2, accountIban }); + expect(updateCalls[0][1]).toMatchObject({ bank: olkyEUR }); + }); }); describe('setReadyDate', () => { diff --git a/src/subdomains/supporting/fiat-output/__tests__/fiat-output.service.spec.ts b/src/subdomains/supporting/fiat-output/__tests__/fiat-output.service.spec.ts new file mode 100644 index 0000000000..06b98d9d86 --- /dev/null +++ b/src/subdomains/supporting/fiat-output/__tests__/fiat-output.service.spec.ts @@ -0,0 +1,166 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { BadRequestException } from '@nestjs/common'; + +import { createCustomCountry } from 'src/shared/models/country/__mocks__/country.entity.mock'; +import { createCustomBank, frickEUR, olkyEUR } from 'src/subdomains/supporting/bank/bank/__mocks__/bank.entity.mock'; +import { IbanBankName } from 'src/subdomains/supporting/bank/bank/dto/bank.dto'; +import { createCustomVirtualIban } from '../../bank/virtual-iban/__mocks__/virtual-iban.entity.mock'; + +import { createCustomFiatOutput } from '../__mocks__/fiat-output.entity.mock'; +import { CreateFiatOutputDto } from '../dto/create-fiat-output.dto'; +import { UpdateFiatOutputDto } from '../dto/update-fiat-output.dto'; +import { FiatOutputType } from '../fiat-output.entity'; +import { FiatOutputService } from '../fiat-output.service'; + +type FiatOutputServiceConstructor = ConstructorParameters; +type SelectPayoutBankUserData = NonNullable[2]>; + +describe('FiatOutputService', () => { + let service: FiatOutputService; + let fiatOutputRepo: DeepMocked; + let bankService: DeepMocked; + let virtualIbanService: DeepMocked; + + beforeEach(() => { + fiatOutputRepo = createMock(); + bankService = createMock(); + virtualIbanService = createMock(); + service = new FiatOutputService( + fiatOutputRepo, + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + bankService, + createMock(), + virtualIbanService, + ); + }); + + describe('selectPayoutBank', () => { + const country = createCustomCountry({ yapealEnable: true }); + + it('skips a Bank Frick virtual IBAN and falls back to an incumbent sender bank', async () => { + const userData = createMock(); + virtualIbanService.getActiveForUserAndCurrency.mockResolvedValue( + createCustomVirtualIban({ bank: frickEUR, iban: 'SYNTHETIC-FRICK-VIBAN' }), + ); + bankService.getSenderBanks.mockResolvedValue([olkyEUR]); + + const result = await service.selectPayoutBank('EUR', FiatOutputType.BUY_FIAT, userData, country); + + expect(result).toEqual({ accountIban: olkyEUR.iban, bank: olkyEUR }); + }); + + it('returns an eligible incumbent virtual IBAN without loading sender banks', async () => { + const userData = createMock(); + const virtualIban = createCustomVirtualIban({ bank: olkyEUR, iban: 'SYNTHETIC-OLKY-VIBAN' }); + virtualIbanService.getActiveForUserAndCurrency.mockResolvedValue(virtualIban); + + const result = await service.selectPayoutBank('EUR', FiatOutputType.BUY_FIAT, userData, country); + + expect(result).toEqual({ accountIban: virtualIban.iban, bank: olkyEUR }); + expect(bankService.getSenderBanks).not.toHaveBeenCalled(); + }); + + it('selects the incumbent sender when Bank Frick has lower priority', async () => { + const userData = createMock(); + const frick = createCustomBank({ + name: IbanBankName.FRICK, + currency: 'EUR', + iban: 'SYNTHETIC-FRICK-ACCOUNT', + send: true, + sendPriority: 2000, + }); + virtualIbanService.getActiveForUserAndCurrency.mockResolvedValue(null); + bankService.getSenderBanks.mockResolvedValue([olkyEUR, frick]); + + const result = await service.selectPayoutBank('EUR', FiatOutputType.BUY_FIAT, userData, country); + + expect(result).toEqual({ accountIban: olkyEUR.iban, bank: olkyEUR }); + }); + + it('does not auto-select Bank Frick as the last available sender', async () => { + const userData = createMock(); + const frick = createCustomBank({ + name: IbanBankName.FRICK, + currency: 'EUR', + iban: 'SYNTHETIC-FRICK-ACCOUNT', + send: true, + sendPriority: 1000, + }); + virtualIbanService.getActiveForUserAndCurrency.mockResolvedValue(null); + bankService.getSenderBanks.mockResolvedValue([frick]); + + const result = await service.selectPayoutBank('EUR', FiatOutputType.BUY_FIAT, userData, country); + + expect(result).toEqual({ accountIban: undefined, bank: undefined }); + }); + + it('selects the incumbent without throwing when Bank Frick has the same priority', async () => { + const userData = createMock(); + const frick = createCustomBank({ + name: IbanBankName.FRICK, + currency: 'EUR', + iban: 'SYNTHETIC-FRICK-ACCOUNT', + send: true, + sendPriority: olkyEUR.sendPriority, + }); + virtualIbanService.getActiveForUserAndCurrency.mockResolvedValue(null); + bankService.getSenderBanks.mockResolvedValue([frick, olkyEUR]); + + const result = await service.selectPayoutBank('EUR', FiatOutputType.BUY_FIAT, userData, country); + + expect(result).toEqual({ accountIban: olkyEUR.iban, bank: olkyEUR }); + }); + }); + + describe('create', () => { + const baseDto: CreateFiatOutputDto = { + type: FiatOutputType.BUY_FIAT, + amount: 100, + currency: 'EUR', + name: 'John Doe', + address: 'Main Street', + zip: '8000', + city: 'Zurich', + country: 'CH', + iban: 'CH9300762011623852957', + accountIban: 'LI75088110103524', + }; + + it('fails loud when accountIban has no matching bank', async () => { + fiatOutputRepo.create.mockReturnValue(createCustomFiatOutput({ ...baseDto })); + bankService.getBankByIban.mockResolvedValue(undefined); + + await expect(service.create(baseDto)).rejects.toThrow(BadRequestException); + expect(bankService.getBankByIban).toHaveBeenCalledWith(baseDto.accountIban); + }); + }); + + describe('update', () => { + it('resolves and persists bank when accountIban is set', async () => { + const id = 1; + const dto: UpdateFiatOutputDto = { accountIban: 'LI75088110103524' }; + fiatOutputRepo.findOneBy.mockResolvedValue(createCustomFiatOutput({ id })); + bankService.getBankByIban.mockResolvedValue(olkyEUR); + fiatOutputRepo.save.mockImplementation(async (entity) => entity as never); + + await service.update(id, dto); + + expect(bankService.getBankByIban).toHaveBeenCalledWith(dto.accountIban); + expect(fiatOutputRepo.save).toHaveBeenCalledWith(expect.objectContaining({ bank: olkyEUR })); + }); + + it('fails loud when accountIban has no matching bank', async () => { + const id = 1; + const dto: UpdateFiatOutputDto = { accountIban: 'LI75088110103524' }; + fiatOutputRepo.findOneBy.mockResolvedValue(createCustomFiatOutput({ id })); + bankService.getBankByIban.mockResolvedValue(undefined); + + await expect(service.update(id, dto)).rejects.toThrow(BadRequestException); + expect(bankService.getBankByIban).toHaveBeenCalledWith(dto.accountIban); + }); + }); +}); diff --git a/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts b/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts index 527e01addc..6d7eb5e71d 100644 --- a/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts +++ b/src/subdomains/supporting/fiat-output/dto/create-fiat-output.dto.ts @@ -1,5 +1,5 @@ import { Type } from 'class-transformer'; -import { IsDate, IsEnum, IsNotEmpty, IsNumber, IsOptional, IsString, MinDate } from 'class-validator'; +import { IsDate, IsEnum, IsNotEmpty, IsNumber, IsOptional, IsString, MinDate, ValidateIf } from 'class-validator'; import { FiatOutputType } from '../fiat-output.entity'; export const MIN_FIAT_OUTPUT_DATE = new Date('2000-01-01T00:00:00Z'); @@ -61,7 +61,12 @@ export class CreateFiatOutputDto { @IsString() iban: string; - @IsOptional() + // Deliberate deviation from the documented create-DTO convention (where @IsOptional permits both + // undefined and null): an explicit null account IBAN has no meaning here (absent = automatic + // assignment) and would bypass the bank-resolution invariant this module enforces, so it is + // rejected instead of tolerated. + @ValidateIf((_o, v) => v !== undefined) + @IsNotEmpty() @IsString() accountIban?: string; diff --git a/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts b/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts index 103eaf83a6..4cd10abb0a 100644 --- a/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts +++ b/src/subdomains/supporting/fiat-output/dto/update-fiat-output.dto.ts @@ -1,5 +1,6 @@ import { Type } from 'class-transformer'; -import { IsBoolean, IsDate, IsNumber, IsOptional, IsString, MinDate } from 'class-validator'; +import { IsBoolean, IsDate, IsNotEmpty, IsNumber, IsOptional, IsString, MinDate } from 'class-validator'; +import { IsOptionalButNotNull } from 'src/shared/validators/is-not-null.validator'; import { TransactionCharge } from '../fiat-output.entity'; import { MIN_FIAT_OUTPUT_DATE } from './create-fiat-output.dto'; @@ -8,7 +9,8 @@ export class UpdateFiatOutputDto { @IsNumber() originEntityId?: number; - @IsOptional() + @IsOptionalButNotNull() + @IsNotEmpty() @IsString() accountIban?: string; diff --git a/src/subdomains/supporting/fiat-output/fiat-output-job.service.ts b/src/subdomains/supporting/fiat-output/fiat-output-job.service.ts index 0174588779..4aeea5f55f 100644 --- a/src/subdomains/supporting/fiat-output/fiat-output-job.service.ts +++ b/src/subdomains/supporting/fiat-output/fiat-output-job.service.ts @@ -28,6 +28,7 @@ import { CheckStatus } from 'src/subdomains/core/aml/enums/check-status.enum'; import { BuyFiatRepository } from 'src/subdomains/core/sell-crypto/process/buy-fiat.repository'; import { UserStatus } from 'src/subdomains/generic/user/models/user/user.enum'; import { Bank } from 'src/subdomains/supporting/bank/bank/bank.entity'; +import { BankService } from 'src/subdomains/supporting/bank/bank/bank.service'; import { LogService } from '../log/log.service'; import { Ep2ReportService } from './ep2-report.service'; import { FiatOutputFrickService } from './fiat-output-frick.service'; @@ -56,6 +57,7 @@ export class FiatOutputJobService { private readonly frickPayoutService: FiatOutputFrickService, private readonly fiatOutputService: FiatOutputService, private readonly scryptService: ScryptService, + private readonly bankService: BankService, ) {} @DfxCron(CronExpression.EVERY_MINUTE, { process: Process.FIAT_OUTPUT, timeout: 1800 }) @@ -144,7 +146,7 @@ export class FiatOutputJobService { country: Country, ): Promise<{ accountIban: string | undefined; bank: Bank | undefined }> { const currency = entity.currency ?? entity.bankAccountCurrency; - return this.fiatOutputService.selectPayoutBank(currency, entity.type, entity.userData, entity.isInstant, country); + return this.fiatOutputService.selectPayoutBank(currency, entity.type, entity.userData, country); } private async assignBankAccount(): Promise { @@ -156,10 +158,13 @@ export class FiatOutputJobService { type: In([FiatOutputType.BUY_CRYPTO_FAIL, FiatOutputType.BUY_FIAT, FiatOutputType.BANK_TX_RETURN]), }; + // OR branches: (1) missing originEntityId, (2) missing accountIban (full assignment), + // (3) accountIban set but bank relation missing — so the repair path in the loop can run. const entities = await this.fiatOutputRepo.find({ where: [ { ...request, originEntityId: IsNull() }, { ...request, accountIban: IsNull() }, + { ...request, accountIban: Not(IsNull()), bank: IsNull() }, ], relations: { buyCrypto: { bankTx: true, transaction: { userData: true } }, @@ -172,15 +177,35 @@ export class FiatOutputJobService { try { if (!entity.buyFiats?.length && !entity.buyCrypto && !entity.bankTxReturn) continue; + if (entity.accountIban) { + // An already-assigned account IBAN (set at creation, or a manual database assignment) must never + // be overwritten by the automatic bank selection below - only originEntityId (and a still-missing + // bank relation) can be repaired here. + let bank = entity.bank; + if (!bank) { + bank = await this.bankService.getBankByIban(entity.accountIban); + if (!bank) + throw new Error(`No bank found for account IBAN ${entity.accountIban} (fiat output ${entity.id})`); + } + + // CAS: match on the current accountIban so a concurrent write from the admin update endpoint, + // landing between this method's snapshot read and this write, is not silently overwritten. + await this.fiatOutputRepo.update( + { id: entity.id, accountIban: entity.accountIban }, + { originEntityId: entity.originEntity?.id, bank }, + ); + continue; + } + const country = await this.countryService.getCountryWithSymbol(entity.ibanCountry); const { accountIban, bank } = await this.getPayoutAccount(entity, country); - await this.fiatOutputRepo.update(entity.id, { - originEntityId: entity.originEntity?.id, - accountIban, - bank, - }); + // Legacy rows may hold an empty string instead of null; match whatever value was read. + await this.fiatOutputRepo.update( + { id: entity.id, accountIban: entity.accountIban == null ? IsNull() : entity.accountIban }, + { originEntityId: entity.originEntity?.id, accountIban, bank }, + ); } catch (e) { this.logger.error(`Error in fillPreValutaDate fiatOutput: ${entity.id}:`, e); } @@ -208,6 +233,8 @@ export class FiatOutputJobService { .getAssetsWith({ bank: true, balance: true }) .then((assets) => assets.filter((a) => a.type === AssetType.CUSTODY && a.bank)); + let skippedFrickFiatOutputs = 0; + for (const accountIbanGroup of groupedEntities.values()) { let updatedFiatOutputAmount = 0; @@ -236,7 +263,10 @@ export class FiatOutputJobService { for (const entity of sortedEntities.filter((e) => !e.isReadyDate)) { try { - if (entity.bank?.name === IbanBankName.FRICK && !this.frickPayoutService.canCreatePayments()) continue; + if (entity.bank?.name === IbanBankName.FRICK && !this.frickPayoutService.canCreatePayments()) { + skippedFrickFiatOutputs++; + continue; + } if ( (entity.user?.isBlockedOrDeleted || entity.userData?.isBlocked) && entity.type === FiatOutputType.BUY_FIAT @@ -293,6 +323,10 @@ export class FiatOutputJobService { ); } } else { + this.logger.verbose( + `FiatOutput ${entity.id} blocked: required ${entity.bankAmount}, ` + + `available ${availableBalance} ${asset.name}`, + ); break; } } catch (e) { @@ -300,6 +334,9 @@ export class FiatOutputJobService { } } } + + if (skippedFrickFiatOutputs) + this.logger.verbose(`Skipped ${skippedFrickFiatOutputs} Frick fiat outputs: payout creation disabled`); } private async createBatches(): Promise { diff --git a/src/subdomains/supporting/fiat-output/fiat-output.service.ts b/src/subdomains/supporting/fiat-output/fiat-output.service.ts index 5b221368d1..260147d99e 100644 --- a/src/subdomains/supporting/fiat-output/fiat-output.service.ts +++ b/src/subdomains/supporting/fiat-output/fiat-output.service.ts @@ -10,7 +10,6 @@ import { UserData } from 'src/subdomains/generic/user/models/user-data/user-data import { Bank } from 'src/subdomains/supporting/bank/bank/bank.entity'; import { IbanBankName } from 'src/subdomains/supporting/bank/bank/dto/bank.dto'; import { VirtualIbanService } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.service'; -import { FiatOutputFrickService } from 'src/subdomains/supporting/fiat-output/fiat-output-frick.service'; import { BankTxRepeatService } from '../bank-tx/bank-tx-repeat/bank-tx-repeat.service'; import { BankTxReturn } from '../bank-tx/bank-tx-return/bank-tx-return.entity'; import { BankTxReturnService } from '../bank-tx/bank-tx-return/bank-tx-return.service'; @@ -36,55 +35,42 @@ export class FiatOutputService { private readonly bankService: BankService, private readonly sellRepo: SellRepository, private readonly virtualIbanService: VirtualIbanService, - private readonly frickPayoutService: FiatOutputFrickService, ) {} + /** + * Automatic selection is restricted to incumbent banks. Bank Frick is payout-eligible only through + * an explicit per-output assignment at creation or in the database, never through automatic selection. + */ async selectPayoutBank( currency: string, type: FiatOutputType, userData: UserData | undefined, - isInstant: boolean, country: Country, ): Promise<{ accountIban: string | undefined; bank: Bank | undefined }> { - // A Frick instant payout is only ever supported for EUR (Bank Frick rejects instant CHF/FOREIGN - // orders outright) - gate on both the capability flag and the currency so an instant CHF output can - // never be assigned to Frick in the first place, rather than failing on every transmit retry. - const isEligibleFrickCandidate = (bank: Bank): boolean => - bank.name !== IbanBankName.FRICK || !isInstant || (bank.sctInst && currency === 'EUR'); - // use virtual IBAN if existing if (userData && [FiatOutputType.BUY_FIAT, FiatOutputType.BUY_CRYPTO_FAIL].includes(type)) { const virtualIban = await this.virtualIbanService.getActiveForUserAndCurrency(userData, currency); if ( virtualIban?.bank?.send && - isEligibleFrickCandidate(virtualIban.bank) && - virtualIban.bank.isCountryEnabled(country) && - (virtualIban.bank.name !== IbanBankName.FRICK || this.frickPayoutService.canCreatePayments()) + virtualIban.bank.name !== IbanBankName.FRICK && + virtualIban.bank.isCountryEnabled(country) ) return { accountIban: virtualIban.iban, bank: virtualIban.bank }; } - // fallback to standard bank account selection - const banks = await this.bankService.getSenderBanks(currency); - const eligibleBanks = banks.filter( - (candidate) => - isEligibleFrickCandidate(candidate) && - candidate.isCountryEnabled(country) && - (candidate.name !== IbanBankName.FRICK || this.frickPayoutService.canCreatePayments()), + // Automatic sender-bank selection is incumbent-banks-only. Bank Frick is payout-eligible exclusively + // through explicit per-output assignment (accountIban at creation or manual database assignment), + // mirroring the deliberate exclusion in BankService.getBank() for the customer-facing deposit selector. + const banks = (await this.bankService.getSenderBanks(currency)).filter( + (candidate) => candidate.name !== IbanBankName.FRICK, ); + const eligibleBanks = banks.filter((candidate) => candidate.isCountryEnabled(country)); // Sender priority (lower wins) is the deterministic tie-breaker between multiple eligible senders for // the same currency - an operational input (Bank.sendPriority), not a hardcoded bank-name preference. - // A throw is reserved for a genuine priority tie that involves Frick itself, never for a tie between - // two non-Frick incumbents (e.g. Olkypay EUR and Yapeal EUR both send=true at the shared default - // priority): Array.prototype.sort is stable, so when every candidate shares the same priority, the - // pre-existing first-match order is used instead of throwing away an otherwise-workable route. + // Array.prototype.sort is stable, so candidates with the same priority keep their pre-existing order. const sortedBanks = [...eligibleBanks].sort((a, b) => a.sendPriority - b.sendPriority); - const tiedForTop = sortedBanks.filter((candidate) => candidate.sendPriority === sortedBanks[0]?.sendPriority); - if (tiedForTop.length > 1 && tiedForTop.some((candidate) => candidate.name === IbanBankName.FRICK)) - throw new Error(`Ambiguous sender bank priority for ${currency}`); - const bank = sortedBanks[0]; return bank ? { accountIban: bank.iban, bank } : { accountIban: undefined, bank: undefined }; } @@ -135,7 +121,8 @@ export class FiatOutputService { if (entity.accountIban && !entity.bank) { const bank = await this.bankService.getBankByIban(entity.accountIban); - if (bank) entity.bank = bank; + if (!bank) throw new BadRequestException('No bank found for account IBAN'); + entity.bank = bank; } return this.fiatOutputRepo.save(entity); @@ -221,9 +208,14 @@ export class FiatOutputService { if (!entity.bankTx) throw new NotFoundException('BankTx not found'); } + if (dto.accountIban) { + entity.bank = await this.bankService.getBankByIban(dto.accountIban); + if (!entity.bank) throw new BadRequestException('No bank found for account IBAN'); + } + if (dto.amount != null) dto.amount = Util.roundReadable(dto.amount, AmountType.FIAT); - return this.fiatOutputRepo.save({ ...entity, ...dto }); + return this.fiatOutputRepo.save({ ...entity, ...dto, bank: entity.bank }); } async delete(id: number): Promise {