diff --git a/migration/1784117860216-AddBankTxTypeCreatedIndex.js b/migration/1784117860216-AddBankTxTypeCreatedIndex.js new file mode 100644 index 0000000000..a2d61642f8 --- /dev/null +++ b/migration/1784117860216-AddBankTxTypeCreatedIndex.js @@ -0,0 +1,33 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * Backs the monthly bank-cost aggregation (BankTxService.getBankTxFee), which runs every minute from + * the financial-log cron and filters bank_tx by (type = BANK_ACCOUNT_FEE, created >= from). Without a + * matching composite index the query degrades to a full scan over a table that grows continuously. + * + * bank_tx carries no primary key in production (MSSQL->Postgres cutover drift), so this migration adds + * ONLY a secondary index: no foreign key, no column alter, nothing that requires a unique constraint. + * + * @class + * @implements {MigrationInterface} + */ +module.exports = class AddBankTxTypeCreatedIndex1784117860216 { + name = 'AddBankTxTypeCreatedIndex1784117860216'; + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + await queryRunner.query(`CREATE INDEX "IDX_bank_tx_type_created" ON "bank_tx" ("type", "created")`); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + await queryRunner.query(`DROP INDEX "public"."IDX_bank_tx_type_created"`); + } +}; diff --git a/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts new file mode 100644 index 0000000000..a48b323daf --- /dev/null +++ b/src/subdomains/supporting/bank-tx/bank-tx/__tests__/bank-tx.service.spec.ts @@ -0,0 +1,185 @@ +import { createMock } from '@golevelup/ts-jest'; +import { ConfigService } from 'src/config/config'; +import { OlkypayService } from 'src/integration/bank/services/olkypay.service'; +import { YapealService } from 'src/integration/bank/services/yapeal.service'; +import { createCustomPrice } from 'src/integration/exchange/dto/__mocks__/price.dto.mock'; +import { createCustomFiat } from 'src/shared/models/fiat/__mocks__/fiat.entity.mock'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; +import { SettingService } from 'src/shared/models/setting/setting.service'; +import { BuyCryptoService } from 'src/subdomains/core/buy-crypto/process/services/buy-crypto.service'; +import { BuyService } from 'src/subdomains/core/buy-crypto/routes/buy/buy.service'; +import { BankDataService } from 'src/subdomains/generic/user/models/bank-data/bank-data.service'; +import { BankService } from 'src/subdomains/supporting/bank/bank/bank.service'; +import { VirtualIbanService } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.service'; +import { NotificationService } from 'src/subdomains/supporting/notification/services/notification.service'; +import { SpecialExternalAccountService } from 'src/subdomains/supporting/payment/services/special-external-account.service'; +import { TransactionNotificationService } from 'src/subdomains/supporting/payment/services/transaction-notification.service'; +import { TransactionService } from 'src/subdomains/supporting/payment/services/transaction.service'; +import { PricingService } from 'src/subdomains/supporting/pricing/services/pricing.service'; +import { BankTxRepeatService } from '../../bank-tx-repeat/bank-tx-repeat.service'; +import { BankTxReturnService } from '../../bank-tx-return/bank-tx-return.service'; +import { BankTxIndicator } from '../entities/bank-tx.entity'; +import { BankTxBatchRepository } from '../repositories/bank-tx-batch.repository'; +import { BankTxRepository } from '../repositories/bank-tx.repository'; +import { BankTxFrickService } from '../services/bank-tx-frick.service'; +import { BankTxService } from '../services/bank-tx.service'; +import { SepaParser } from '../services/sepa-parser.service'; + +// one raw aggregate row as returned by the GROUP BY currency, creditDebitIndicator query +interface FeeAggregate { + currency: string; + creditDebitIndicator: BankTxIndicator; + amount: string; // Postgres returns SUM() as a string +} + +describe('BankTxService', () => { + let service: BankTxService; + + let bankTxRepo: BankTxRepository; + let pricingService: PricingService; + let fiatService: FiatService; + + let qb: any; + + const from = new Date('2026-07-01'); + + // Distinct per-currency CHF prices. Price.convert divides the amount by the price + // (getPrice(fiat, CHF).convert(x) = x / price), so a wrong currency->price mapping or an + // inverted (multiply) direction produces a different total and turns these tests red. + const chfPriceByCurrency: Record = { EUR: 0.8, USD: 2 }; + + beforeAll(() => { + new ConfigService(); // sets module-level Config (defaultVolumeDecimal read by the CHF conversion) + }); + + beforeEach(() => { + jest.clearAllMocks(); + + bankTxRepo = createMock(); + pricingService = createMock(); + fiatService = createMock(); + + // one chainable query builder mock serves both queries: the legacy chargeAmountChf sum + // (getRawOne) and the per-currency/direction BankAccountFee aggregation (getRawMany). + qb = { + select: jest.fn(() => qb), + addSelect: jest.fn(() => qb), + where: jest.fn(() => qb), + andWhere: jest.fn(() => qb), + groupBy: jest.fn(() => qb), + addGroupBy: jest.fn(() => qb), + getRawOne: jest.fn(), + getRawMany: jest.fn(), + }; + (bankTxRepo.createQueryBuilder as jest.Mock).mockReturnValue(qb); + + (fiatService.getFiatByName as jest.Mock).mockImplementation((name: string) => createCustomFiat({ name })); + + service = new BankTxService( + bankTxRepo, + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + createMock(), + pricingService, + fiatService, + ); + }); + + function mockLegacyFee(fee: number | null): void { + qb.getRawOne.mockResolvedValue({ fee }); + } + + function mockFeeAggregates(rows: FeeAggregate[]): void { + qb.getRawMany.mockResolvedValue(rows); + } + + // per-currency prices from chfPriceByCurrency (distinct so mapping/direction errors are caught) + function mockChfPrices(): void { + (pricingService.getPrice as jest.Mock).mockImplementation(async (fiat: { name: string }) => + createCustomPrice({ source: fiat.name, target: 'CHF', price: chfPriceByCurrency[fiat.name] }), + ); + } + + it('converts each DBIT currency aggregate with its own price and sums the result', async () => { + mockLegacyFee(null); + mockFeeAggregates([ + { currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }, + { currency: 'USD', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '200' }, + ]); + mockChfPrices(); + + // EUR 100 / 0.8 = 125 CHF; USD 200 / 2 = 100 CHF; total 225 CHF + // (a multiply direction -> 80 + 400 = 480; a swapped currency->price -> 125 + 250 = 375) + await expect(service.getBankTxFee(from)).resolves.toBe(225); + + expect(fiatService.getFiatByName).toHaveBeenCalledWith('EUR'); + expect(fiatService.getFiatByName).toHaveBeenCalledWith('USD'); + expect(pricingService.getPrice).toHaveBeenCalledTimes(2); + }); + + it('nets a CRDT refund against a DBIT fee in the same currency before converting', async () => { + mockLegacyFee(null); + mockFeeAggregates([ + { currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }, + { currency: 'EUR', creditDebitIndicator: BankTxIndicator.CREDIT, amount: '30' }, + ]); + mockChfPrices(); + + // net EUR 70 / 0.8 = 87.5 CHF (converted once, after netting) + await expect(service.getBankTxFee(from)).resolves.toBe(87.5); + + expect(pricingService.getPrice).toHaveBeenCalledTimes(1); + }); + + it('adds the legacy chargeAmountChf sum on top of the converted BankAccountFee aggregates', async () => { + mockLegacyFee(5); + mockFeeAggregates([{ currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }]); + mockChfPrices(); + + // legacy 5 + EUR 100 / 0.8 (125) = 130 + await expect(service.getBankTxFee(from)).resolves.toBe(130); + }); + + it('rounds the assembled total to the default volume decimals', async () => { + mockLegacyFee(0.1); + mockFeeAggregates([{ currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '0.2' }]); + // price 1 so the converted amount is exactly 0.2 and only the final sum needs rounding + (pricingService.getPrice as jest.Mock).mockResolvedValue( + createCustomPrice({ source: 'EUR', target: 'CHF', price: 1 }), + ); + + // legacy 0.1 + EUR 0.2 = 0.30000000000000004 in float -> must be rounded to 0.3 + await expect(service.getBankTxFee(from)).resolves.toBe(0.3); + }); + + it('fails loud when the price is unavailable', async () => { + mockLegacyFee(null); + mockFeeAggregates([{ currency: 'EUR', creditDebitIndicator: BankTxIndicator.DEBIT, amount: '100' }]); + (pricingService.getPrice as jest.Mock).mockRejectedValue(new Error('No valid price')); + + await expect(service.getBankTxFee(from)).rejects.toThrow(); + }); + + it('returns only the legacy sum when there are no BankAccountFee aggregates', async () => { + mockLegacyFee(5); + mockFeeAggregates([]); + + await expect(service.getBankTxFee(from)).resolves.toBe(5); + + expect(pricingService.getPrice).not.toHaveBeenCalled(); + }); +}); 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 64c7c74838..1cb4cf61d4 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 @@ -9,7 +9,9 @@ import { } from '@nestjs/common'; import { CronExpression } from '@nestjs/schedule'; import { Observable, Subject } from 'rxjs'; +import { Config } from 'src/config/config'; import { YapealService } from 'src/integration/bank/services/yapeal.service'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; import { SettingService } from 'src/shared/models/setting/setting.service'; import { DfxLogger } from 'src/shared/services/dfx-logger'; import { Process } from 'src/shared/services/process.service'; @@ -26,6 +28,11 @@ import { MailContext, MailType } from 'src/subdomains/supporting/notification/en import { NotificationService } from 'src/subdomains/supporting/notification/services/notification.service'; import { SpecialExternalAccount } from 'src/subdomains/supporting/payment/entities/special-external-account.entity'; import { TransactionNotificationService } from 'src/subdomains/supporting/payment/services/transaction-notification.service'; +import { + PriceCurrency, + PriceValidity, + PricingService, +} from 'src/subdomains/supporting/pricing/services/pricing.service'; import { DeepPartial, FindOptionsRelations, @@ -114,6 +121,8 @@ export class BankTxService implements OnModuleInit { private readonly virtualIbanService: VirtualIbanService, @Inject(forwardRef(() => TransactionNotificationService)) private readonly transactionNotificationService: TransactionNotificationService, + private readonly pricingService: PricingService, + private readonly fiatService: FiatService, ) {} onModuleInit() { @@ -430,6 +439,10 @@ export class BankTxService implements OnModuleInit { ]); } + // Bank fees come from two sources: legacy per-tx charges (chargeAmountChf) plus dedicated BankAccountFee rows. + // Charge fields ceased with the bank migration in Dec 2025; fees arrive as dedicated BankAccountFee rows since, + // carrying the amount in the account currency (no amountChf column), so they are aggregated per currency and + // converted to CHF. async getBankTxFee(from: Date): Promise { const { fee } = await this.bankTxRepo .createQueryBuilder('bankTx') @@ -437,7 +450,51 @@ export class BankTxService implements OnModuleInit { .where('bankTx.created >= :from', { from }) .getRawOne<{ fee: number }>(); - return fee ?? 0; + let totalFeeChf = fee ?? 0; + + // Aggregate the dedicated BankAccountFee rows in the database (summed per currency and direction) + // rather than hydrating every row: this runs every minute from the financial-log cron and the table + // grows continuously. Column references use the alias.property form so TypeORM quotes the camelCase + // identifiers correctly for Postgres. + const feeAggregates = await this.bankTxRepo + .createQueryBuilder('bankTx') + .select('bankTx.currency', 'currency') + .addSelect('bankTx.creditDebitIndicator', 'creditDebitIndicator') + .addSelect('SUM(bankTx.amount)', 'amount') + .where('bankTx.type = :type', { type: BankTxType.BANK_ACCOUNT_FEE }) + .andWhere('bankTx.created >= :from', { from }) + .groupBy('bankTx.currency') + .addGroupBy('bankTx.creditDebitIndicator') + .getRawMany<{ currency: string; creditDebitIndicator: string; amount: string }>(); + + const amountByCurrency = new Map(); + for (const row of feeAggregates) { + const groupAmount = Number(row.amount); + + let signedAmount: number; + if (row.creditDebitIndicator === BankTxIndicator.DEBIT) { + signedAmount = groupAmount; + } else if (row.creditDebitIndicator === BankTxIndicator.CREDIT) { + signedAmount = -groupAmount; + } else { + this.logger.error( + `BankAccountFee aggregate (currency ${row.currency}) has unexpected creditDebitIndicator ${row.creditDebitIndicator}, skipping`, + ); + continue; + } + + amountByCurrency.set(row.currency, (amountByCurrency.get(row.currency) ?? 0) + signedAmount); + } + + for (const [currency, amount] of amountByCurrency) { + if (!amount) continue; + + const fiat = await this.fiatService.getFiatByName(currency); + const price = await this.pricingService.getPrice(fiat, PriceCurrency.CHF, PriceValidity.ANY); + totalFeeChf += price.convert(amount, Config.defaultVolumeDecimal); + } + + return Util.round(totalFeeChf, Config.defaultVolumeDecimal); } async getRecentBankToBankTx(fromIban: string, toIban: string): Promise { diff --git a/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts b/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts index e513fce5ce..093d1bdef3 100644 --- a/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts +++ b/src/subdomains/supporting/log/__tests__/log-job.service.spec.ts @@ -160,6 +160,59 @@ describe('LogJobService', () => { expect(result.minus.scrypt).toBeUndefined(); expect(result.minus.mexc).toBeUndefined(); }); + + it('flows the bank tx fee into minus.bank and the totals', async () => { + setupEmpty(); + jest.spyOn(exchangeTxService, 'getExchangeTx').mockResolvedValue([] as any); + jest.spyOn(bankTxService, 'getBankTxFee').mockResolvedValue(4196 as any); + + const result = await (service as any).getChangeLog(); + + expect(result.minus.bank).toBe(4196); + expect(result.minus.total).toBe(4196); + expect(result.total).toBe(-4196); + }); + }); + + describe('FinancialChangesLog isolation (reporting failure must not arm the equity safety mode)', () => { + // a healthy, finite book comfortably above the minimum -> the equity path leaves safety mode off + function setup() { + jest.spyOn(service as any, 'getTradingLog').mockResolvedValue({}); + jest.spyOn(service as any, 'getAssetLog').mockResolvedValue({}); + jest + .spyOn(service as any, 'getBalancesByFinancialType') + .mockReturnValue({ EUR: { plusBalance: 5000, plusBalanceChf: 5000, minusBalance: 0, minusBalanceChf: 0 } }); + jest.spyOn(assetService, 'getAssetsWith').mockResolvedValue([] as any); + jest.spyOn(settingService, 'getObj').mockResolvedValue(100 as any); + jest.spyOn(refRewardService, 'getOpenRefCreditLiability').mockResolvedValue({ amountEur: 0, amountChf: 0 }); + jest + .spyOn(logService, 'maxEntity') + .mockResolvedValue({ message: JSON.stringify({ balancesTotal: { totalBalanceChf: 5000 } }) } as any); + return jest.spyOn(logService, 'create').mockResolvedValue({} as any); + } + + it('writes the FinancialDataLog, keeps safety mode off, logs the error and omits the changes entry when getChangeLog throws', async () => { + const errorSpy = jest.spyOn(service['logger'], 'error'); + const createSpy = setup(); + // a transient reporting-price failure while building the changes log + jest.spyOn(service as any, 'getChangeLog').mockRejectedValue(new Error('No valid price')); + + await service.saveTradingLog(); + + // the equity path ran and still persisted the FinancialDataLog entry for this minute + const dataLog = createSpy.mock.calls.find(([dto]) => dto.subsystem === 'FinancialDataLog'); + expect(dataLog).toBeDefined(); + + // the reporting-price failure did NOT arm the equity safety mode: the healthy book set it to false + // (and the outer catch, which would set it true, never ran) + expect(processService.setSafetyModeActive).toHaveBeenCalledWith(false); + expect(processService.setSafetyModeActive).not.toHaveBeenCalledWith(true); + + // the failure is logged and this minute's changes entry is omitted (absence, not a wrong value) + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('financial changes log'), expect.any(Error)); + const changesLog = createSpy.mock.calls.find(([dto]) => dto.subsystem === 'FinancialChangesLog'); + expect(changesLog).toBeUndefined(); + }); }); describe('saveTradingLog (referral-credit liability)', () => { diff --git a/src/subdomains/supporting/log/log-job.service.ts b/src/subdomains/supporting/log/log-job.service.ts index de96da777a..3a293c117b 100644 --- a/src/subdomains/supporting/log/log-job.service.ts +++ b/src/subdomains/supporting/log/log-job.service.ts @@ -129,9 +129,6 @@ export class LogJobService { const refCreditLiability = await this.getRefCreditLiability(); if (refCreditLiability) balancesByFinancialType[REF_CREDIT_FINANCIAL_TYPE] = refCreditLiability; - // changes - const changeLog = await this.getChangeLog(); - // total balances — customer flow is balance-neutral, so totalBalanceChf moves only on // operating profit, FX, or an error/realised loss (see BalancesTotal). Hence the guardrails below. const plusBalanceChf = Util.sumObjValue(Object.values(balancesByFinancialType), 'plusBalanceChf'); @@ -190,14 +187,24 @@ export class LogJobService { category: null, }); - await this.logService.create({ - system: 'LogService', - subsystem: 'FinancialChangesLog', - severity: LogSeverity.INFO, - message: JSON.stringify({ changes: changeLog }), - valid: null, - category: null, - }); + // The changeLog feeds only the informative FinancialChangesLog and is independent of the equity + // path above, so it runs in its own try/catch: a reporting-price failure must not arm the equity + // safety mode; the equity path above has already run and set it correctly. On failure we log the + // error and omit this minute's changes entry (absence instead of a wrong value). + try { + const changeLog = await this.getChangeLog(); + + await this.logService.create({ + system: 'LogService', + subsystem: 'FinancialChangesLog', + severity: LogSeverity.INFO, + message: JSON.stringify({ changes: changeLog }), + valid: null, + category: null, + }); + } catch (e) { + this.logger.error("Failed to build the financial changes log; skipping this minute's changes entry", e); + } } catch (e) { await this.processService.setSafetyModeActive(true); throw e;