From 3758f001897470d649ab4afdc3094b38947a3ff5 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Fri, 31 Jul 2026 14:20:47 -0300 Subject: [PATCH] perf(buy): resolve the user and the active vIBAN once per payment-info request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PUT /v1/buy/paymentInfos issued the same two lookups twice per request, which matters because this route is the heaviest database consumer among the customer-facing endpoints and therefore the first to suffer whenever the database is under load. - The user was loaded in createBuyPaymentInfo and again in toPaymentInfoDto. Each load costs two queries, because TypeORM emits its DISTINCT-id pattern for findOne with relations. toPaymentInfoDto now accepts the already-loaded user; the standalone caller keeps its previous behaviour through the optional parameter, and a mismatched (userId, preloadedUser) pair is rejected rather than attributing the transaction request to another account. - getTxDetails already resolves the user's active vIBAN to pick the receiving bank for the fee. It now returns that result so the deposit-destination step reuses it instead of repeating the query. The lookup still happens inside getTxDetails, in the original order, so the ordering that keeps a failed quote from creating an external account is unchanged. Only a positive result is reused. A negative one is deliberately re-read: by the time resolveBankInfo sees it, it is a whole getTxDetails old and the branch that follows issues an IBAN, so a vIBAN issued concurrently in that window would be missed — createForUser would hit a duplicate, swallow the ConflictException, and the request would fail closed with PersonalIbanIssuanceFailed where a fresh read returns the customer's IBAN. To stop that being re-introduced as a later optimisation, getBankIn normalises "none found" to undefined at the single point that produces it. That is enforced by tests rather than by the compiler, since the repo sets neither strict nor strictNullChecks. Sharing one relation set between both loads also fixes a latent defect: createBuyPaymentInfo requested userData.wallet, while buyCheck reads user.wallet — which was therefore always undefined there. Both escape hatches guarded by it were inert: SKIP_AML_CHECK, and autoTradeApproval, which is meant to let a wallet without a tradeApprovalDate through rather than rejecting it with RecommendationRequired. The equivalent check in getTxDetails does have the wallet loaded and only emits a soft QuoteError, so the two disagreed on identical input. The resulting behaviour change is gated by DisabledProcess(TRADE_APPROVAL_DATE) and is strictly loosening; see the pull request for the rollout consideration. userData.users is dropped: nothing reachable from this endpoint reads it, and it fanned the user query out by the number of users on the account. userData.organization is kept, because UserData.address reads organization.country and TypeORM joins the eager relations of a requested relation one level only. --- .../__tests__/payment-info.service.spec.ts | 62 +++++++++ .../routes/buy/__tests__/buy.service.spec.ts | 127 ++++++++++++++++++ .../core/buy-crypto/routes/buy/buy.service.ts | 56 ++++++-- .../__tests__/transaction-helper.spec.ts | 70 ++++++++-- .../transaction-details.dto.ts | 12 ++ .../payment/services/transaction-helper.ts | 28 +++- 6 files changed, 326 insertions(+), 29 deletions(-) create mode 100644 src/shared/services/__tests__/payment-info.service.spec.ts diff --git a/src/shared/services/__tests__/payment-info.service.spec.ts b/src/shared/services/__tests__/payment-info.service.spec.ts new file mode 100644 index 0000000000..c63c873ef6 --- /dev/null +++ b/src/shared/services/__tests__/payment-info.service.spec.ts @@ -0,0 +1,62 @@ +import { createMock } from '@golevelup/ts-jest'; +import { BadRequestException } from '@nestjs/common'; +import { Asset } from 'src/shared/models/asset/asset.entity'; +import { AssetService } from 'src/shared/models/asset/asset.service'; +import { Fiat } from 'src/shared/models/fiat/fiat.entity'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; +import * as processServiceModule from 'src/shared/services/process.service'; +import { GetBuyPaymentInfoDto } from 'src/subdomains/core/buy-crypto/routes/buy/dto/get-buy-payment-info.dto'; +import { KycLevel } from 'src/subdomains/generic/user/models/user-data/user-data.enum'; +import { User } from 'src/subdomains/generic/user/models/user/user.entity'; +import { FiatPaymentMethod } from 'src/subdomains/supporting/payment/dto/payment-method.enum'; +import { PaymentInfoService } from '../payment-info.service'; + +// The trade-approval gate reads user.wallet. Callers that load the user without the wallet relation see +// an undefined wallet, which silently disables both escape hatches below - so these assert the gate +// against a user that actually carries one. +describe('PaymentInfoService buyCheck trade-approval gate', () => { + let service: PaymentInfoService; + let fiatService: FiatService; + let assetService: AssetService; + + const currency = { id: 2, name: 'EUR', sellable: true } as Fiat; + const asset = { id: 10, name: 'BTC', buyable: true } as Asset; + + function dto(): GetBuyPaymentInfoDto { + return { amount: 100, currency, asset, paymentMethod: FiatPaymentMethod.BANK } as GetBuyPaymentInfoDto; + } + + function user(wallet: Record, tradeApprovalDate?: Date): User { + return { id: 1, userData: { kycLevel: KycLevel.LEVEL_50, tradeApprovalDate }, wallet } as unknown as User; + } + + beforeEach(() => { + fiatService = createMock(); + assetService = createMock(); + jest.spyOn(fiatService, 'getFiat').mockResolvedValue(currency); + jest.spyOn(assetService, 'getAssetById').mockResolvedValue(asset); + jest.spyOn(processServiceModule, 'DisabledProcess').mockReturnValue(false); + + service = new PaymentInfoService(fiatService, assetService); + }); + + afterEach(() => jest.restoreAllMocks()); + + it('lets an autoTradeApproval wallet through without a tradeApprovalDate', async () => { + await expect( + service.buyCheck(dto(), undefined, user({ autoTradeApproval: true, amlRuleList: [] })), + ).resolves.toBeDefined(); + }); + + it('still rejects a wallet without autoTradeApproval and without a tradeApprovalDate', async () => { + await expect( + service.buyCheck(dto(), undefined, user({ autoTradeApproval: false, amlRuleList: [] })), + ).rejects.toThrow(BadRequestException); + }); + + it('accepts a wallet without autoTradeApproval once a tradeApprovalDate is set', async () => { + await expect( + service.buyCheck(dto(), undefined, user({ autoTradeApproval: false, amlRuleList: [] }, new Date())), + ).resolves.toBeDefined(); + }); +}); diff --git a/src/subdomains/core/buy-crypto/routes/buy/__tests__/buy.service.spec.ts b/src/subdomains/core/buy-crypto/routes/buy/__tests__/buy.service.spec.ts index 2d6cf3d2a9..1542e3da03 100644 --- a/src/subdomains/core/buy-crypto/routes/buy/__tests__/buy.service.spec.ts +++ b/src/subdomains/core/buy-crypto/routes/buy/__tests__/buy.service.spec.ts @@ -167,6 +167,54 @@ describe('BuyService', () => { expect(virtualIbanService.getOrCreateFrickForUser).not.toHaveBeenCalled(); }); + // The user is loaded once and handed to toPaymentInfoDto. Loading it a second time there costs two + // extra queries, and letting the two loads drift apart is what left user.wallet unloaded for buyCheck. + it('loads the user once and reuses it for the payment-info DTO', async () => { + const loadedUser = { id: 1, userData, wallet: {} } as any; + const getUser = jest.spyOn(userService, 'getUser').mockResolvedValue(loadedUser); + jest.spyOn(paymentInfoService, 'buyCheck').mockImplementation(async (d) => d as any); + jest.spyOn(service, 'createBuy').mockResolvedValue(buy); + jest.spyOn(virtualIbanService, 'getOrCreateFrickForUser').mockResolvedValue(virtualIban); + const fees = { min: 0, rate: 0.01, fixed: 0, dfx: 1, network: 0, platform: 0, bank: 0, total: 1 }; + jest.spyOn(transactionHelper, 'getTxDetails').mockResolvedValue({ + timestamp: new Date('2026-07-24T00:00:00Z'), + minVolume: 10, + minVolumeTarget: 0.001, + maxVolume: 10000, + maxVolumeTarget: 1, + exchangeRate: 100000, + rate: 101000, + estimatedAmount: 0.00099, + sourceAmount: 100, + isValid: false, + exactPrice: false, + feeSource: fees, + feeTarget: fees, + priceSteps: [], + } as any); + + await service.createBuyPaymentInfo({ user: 1, address: '0x123' } as any, dto()); + + expect(getUser).toHaveBeenCalledTimes(1); + // wallet is the relation buyCheck reads (as user.wallet, not userData.wallet) and organization is + // what UserData.address needs - asserted explicitly, because getUser is mocked and would otherwise + // hand back a fully-populated user no matter which relations were requested. + expect(getUser).toHaveBeenCalledWith(1, { userData: { organization: true }, wallet: true }); + // and the fee calculation sees the very same instance + expect((transactionHelper.getTxDetails as jest.Mock).mock.calls[0][7]).toBe(loadedUser); + }); + + // the transaction request is attributed to userId, so a preloaded user for someone else would book + // the request against the wrong account instead of failing + it('refuses a preloaded user that does not match the requested user', async () => { + await expect(service.toPaymentInfoDto(1, buy, dto(), { id: 2 } as any)).rejects.toThrow( + 'Preloaded user does not match userId', + ); + + expect(transactionHelper.getTxDetails).not.toHaveBeenCalled(); + expect(transactionRequestService.create).not.toHaveBeenCalled(); + }); + it('selects Frick once before fee calculation, persists exact IDs, and does not leak IDs publicly', async () => { const events: string[] = []; jest.spyOn(userService, 'getUser').mockResolvedValue({ id: 1, userData, wallet: {} } as any); @@ -1293,6 +1341,85 @@ describe('BuyService', () => { expect(virtualIbanService.isUserEligible).not.toHaveBeenCalled(); expect(bankService.getBank).not.toHaveBeenCalled(); }); + + // getTxDetails already resolves the user's active vIBAN to pick the receiving bank for the fee. It + // hands that result back so the deposit-destination step reuses it instead of repeating the query. + describe('active vIBAN reuse between fee calculation and deposit destination', () => { + const userLevelVirtualIban = { + id: 778, + iban: 'CH4431999123000889013', + bank: defaultRouteBank, + currency, + userData, + active: true, + status: VirtualIbanStatus.ACTIVE, + } as VirtualIban; + + beforeEach(() => { + jest.spyOn(userService, 'getUser').mockResolvedValue({ id: 1, userData, wallet } as any); + jest.spyOn(bankService, 'getBank').mockResolvedValue(defaultRouteBank); + }); + + it('does not repeat the lookup when getTxDetails resolved an active vIBAN', async () => { + jest + .spyOn(transactionHelper, 'getTxDetails') + .mockResolvedValue({ ...feeResult(), activeVirtualIban: userLevelVirtualIban } as any); + + const response = await service.toPaymentInfoDto(1, buy, { + amount: 100, + currency, + asset, + paymentMethod: FiatPaymentMethod.BANK, + exactPrice: false, + } as GetBuyPaymentInfoDto); + + expect(response.iban).toBe(userLevelVirtualIban.iban); + expect(virtualIbanService.getActiveReceivingForUserAndCurrency).not.toHaveBeenCalled(); + }); + + // A negative result is deliberately re-read: it is a whole getTxDetails old, and the issuance branch + // follows. The fresh read here returns a vIBAN, standing in for one issued concurrently in that + // window — reusing the stale negative would lose it and fail closed with PersonalIbanIssuanceFailed. + it('re-resolves when getTxDetails found none, so a concurrently issued vIBAN is not missed', async () => { + jest + .spyOn(transactionHelper, 'getTxDetails') + .mockResolvedValue({ ...feeResult(), activeVirtualIban: undefined } as any); + jest.spyOn(virtualIbanService, 'getActiveReceivingForUserAndCurrency').mockResolvedValue(userLevelVirtualIban); + + const response = await service.toPaymentInfoDto(1, buy, { + amount: 100, + currency, + asset, + paymentMethod: FiatPaymentMethod.BANK, + exactPrice: false, + } as GetBuyPaymentInfoDto); + + expect(virtualIbanService.getActiveReceivingForUserAndCurrency).toHaveBeenCalledTimes(1); + // the freshly-read IBAN must reach the response, not the stale "none" + expect(response.iban).toBe(userLevelVirtualIban.iban); + expect(virtualIbanService.createForUser).not.toHaveBeenCalled(); + }); + + // CARD on purpose: a BANK transfer that resolves no personal IBAN fails closed by design, so the + // "nothing found either way" outcome can only be observed on the payment method that still + // resolves a bank. + it('resolves it itself when getTxDetails ran no lookup', async () => { + jest + .spyOn(transactionHelper, 'getTxDetails') + .mockResolvedValue({ ...feeResult(), activeVirtualIban: undefined } as any); + jest.spyOn(virtualIbanService, 'getActiveReceivingForUserAndCurrency').mockResolvedValue(null); + + await service.toPaymentInfoDto(1, buy, { + amount: 100, + currency, + asset, + paymentMethod: FiatPaymentMethod.CARD, + exactPrice: false, + } as GetBuyPaymentInfoDto); + + expect(virtualIbanService.getActiveReceivingForUserAndCurrency).toHaveBeenCalledTimes(1); + }); + }); }); describe('createBuy route persistence', () => { diff --git a/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts b/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts index e2c92bfe9a..2bf6ac85e9 100644 --- a/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts +++ b/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts @@ -35,7 +35,7 @@ import { TransactionRequestType } from 'src/subdomains/supporting/payment/entiti import { SwissQRService } from 'src/subdomains/supporting/payment/services/swiss-qr.service'; import { TransactionHelper } from 'src/subdomains/supporting/payment/services/transaction-helper'; import { TransactionRequestService } from 'src/subdomains/supporting/payment/services/transaction-request.service'; -import { In, IsNull, Not, Repository } from 'typeorm'; +import { FindOptionsRelations, In, IsNull, Not, Repository } from 'typeorm'; import { Buy } from './buy.entity'; import { BuyRepository } from './buy.repository'; import { BankInfoDto, BuyPaymentInfoDto } from './dto/buy-payment-info.dto'; @@ -43,6 +43,18 @@ import { CreateBuyDto } from './dto/create-buy.dto'; import { GetBuyPaymentInfoDto, PersonalIbanProvider } from './dto/get-buy-payment-info.dto'; import { UpdateBuyDto } from './dto/update-buy.dto'; +// Single relation set for the whole payment-info request, loaded once and passed down. Both halves of +// the request used to load the user separately with different relations, which is how `user.wallet` +// went missing in createBuyPaymentInfo (buyCheck reads it) while toPaymentInfoDto had it. +// - userData.organization: `UserData.address` reads organization.street/country for ORGANIZATION and +// SOLE_PROPRIETORSHIP accounts. TypeORM joins the eager relations of a requested relation one level +// only, so organization.country is joined only when organization is requested explicitly. +// - wallet: read as user.wallet (not userData.wallet) by buyCheck and getTxDetails. +const PAYMENT_INFO_USER_RELATIONS: FindOptionsRelations = { + userData: { organization: true }, + wallet: true, +}; + @Injectable() export class BuyService { private cache: { id: number; bankUsage: string }[] = undefined; @@ -141,7 +153,7 @@ export class BuyService { } async createBuyPaymentInfo(jwt: JwtPayload, dto: GetBuyPaymentInfoDto): Promise { - const user = await this.userService.getUser(jwt.user, { userData: { wallet: true } }); + const user = await this.userService.getUser(jwt.user, PAYMENT_INFO_USER_RELATIONS); if (dto.personalIbanProvider === PersonalIbanProvider.FRICK && dto.paymentMethod !== FiatPaymentMethod.BANK) { throw new BadRequestException(QuoteError.PAYMENT_METHOD_NOT_ALLOWED); } @@ -154,7 +166,7 @@ export class BuyService { (e) => e.message?.includes('duplicate key'), ); - return this.toPaymentInfoDto(jwt.user, buy, dto); + return this.toPaymentInfoDto(jwt.user, buy, dto, user); } async createBuy(user: User, userAddress: string, dto: CreateBuyDto, ignoreExisting = false): Promise { @@ -267,11 +279,21 @@ export class BuyService { return this.buyRepo; } - async toPaymentInfoDto(userId: number, buy: Buy, dto: GetBuyPaymentInfoDto): Promise { - const user = await this.userService.getUser(userId, { - userData: { users: true, organization: true }, - wallet: true, - }); + /** + * @param preloadedUser the user for `userId`, saving a second load. Must be loaded with the relations + * `{ userData: { organization: true }, wallet: true }` — `getTxErrors` dereferences `user.wallet` + * without optional chaining, so a differently-loaded user fails at runtime rather than degrading. + */ + async toPaymentInfoDto( + userId: number, + buy: Buy, + dto: GetBuyPaymentInfoDto, + preloadedUser?: User, + ): Promise { + // the request is attributed to userId further down, so a mismatch would book it against another account + if (preloadedUser && preloadedUser.id !== userId) throw new Error('Preloaded user does not match userId'); + + const user = preloadedUser ?? (await this.userService.getUser(userId, PAYMENT_INFO_USER_RELATIONS)); // Explicit personal-IBAN selector dispatch is exhaustive and fail-closed. Frick resolves the // deposit destination before fee calculation so bankInOverride can pass the Frick bank name @@ -327,6 +349,7 @@ export class BuyService { feeSource, feeTarget, priceSteps, + activeVirtualIban, } = await this.transactionHelper.getTxDetails( dto.amount, dto.targetAmount, @@ -354,6 +377,8 @@ export class BuyService { buy, dto.asset, user.wallet, + undefined, + activeVirtualIban, ); } @@ -476,6 +501,7 @@ export class BuyService { asset?: Asset, wallet?: Wallet, personalIbanProvider?: PersonalIbanProvider, + activeVirtualIban?: VirtualIban, ): Promise<{ bankInfo: BankInfoDto & { isPersonalIban: boolean; reference?: string }; bankId: number; @@ -536,11 +562,15 @@ export class BuyService { } } - // user-level vIBAN - let virtualIban = await this.virtualIbanService.getActiveReceivingForUserAndCurrency( - selector.userData, - selector.currency, - ); + // user-level vIBAN — reuse the caller's lookup only when it actually found one. A negative result is + // deliberately NOT reused: it is up to a full getTxDetails (pricing, fees, limits) old by now, and the + // branch below issues an IBAN. A concurrent request that issued one in that window would otherwise be + // missed here, createForUser would hit a duplicate and swallow it, and the customer would get a + // fail-closed PersonalIbanIssuanceFailed where a fresh read returns the IBAN. Re-reading costs one + // SELECT on a path that is about to make an external issuance call anyway. + let virtualIban = + activeVirtualIban ?? + (await this.virtualIbanService.getActiveReceivingForUserAndCurrency(selector.userData, selector.currency)); // create a personal IBAN for an eligible KYC 50+ user if ( diff --git a/src/subdomains/core/history/__tests__/transaction-helper.spec.ts b/src/subdomains/core/history/__tests__/transaction-helper.spec.ts index 0ed3d5dc16..50ace5a7ff 100644 --- a/src/subdomains/core/history/__tests__/transaction-helper.spec.ts +++ b/src/subdomains/core/history/__tests__/transaction-helper.spec.ts @@ -259,23 +259,25 @@ describe('TransactionHelper', () => { describe('getBankIn', () => { const eur = createCustomFiat({ name: 'EUR' }); + // getBankIn returns the resolved bank together with the vIBAN it looked up, so getTxDetails can hand + // that vIBAN to the caller instead of having it repeat the lookup. Only a positive result is reusable, + // so "none found" and "no lookup ran" are both undefined. it('should return the deposit bank for bank transfers', async () => { jest.spyOn(virtualIbanService, 'getActiveReceivingForUserAndCurrency').mockResolvedValue(null); jest.spyOn(bankService, 'getBank').mockResolvedValue(olkyEUR); await expect( txHelper['getBankIn'](eur, FiatPaymentMethod.BANK, createCustomUserData({ kycLevel: KycLevel.LEVEL_30 })), - ).resolves.toBe(IbanBankName.OLKY); + ).resolves.toEqual({ bankName: IbanBankName.OLKY, activeVirtualIban: undefined }); }); it('should return the vIBAN bank for users with an active vIBAN', async () => { - jest - .spyOn(virtualIbanService, 'getActiveReceivingForUserAndCurrency') - .mockResolvedValue(createCustomVirtualIban({ bank: yapealEUR })); + const activeVirtualIban = createCustomVirtualIban({ bank: yapealEUR }); + jest.spyOn(virtualIbanService, 'getActiveReceivingForUserAndCurrency').mockResolvedValue(activeVirtualIban); await expect( txHelper['getBankIn'](eur, FiatPaymentMethod.BANK, createCustomUserData({ kycLevel: KycLevel.LEVEL_50 })), - ).resolves.toBe(IbanBankName.YAPEAL); + ).resolves.toEqual({ bankName: IbanBankName.YAPEAL, activeVirtualIban }); }); it('should return the deposit bank for users without an active vIBAN', async () => { @@ -284,23 +286,32 @@ describe('TransactionHelper', () => { await expect( txHelper['getBankIn'](eur, FiatPaymentMethod.BANK, createCustomUserData({ kycLevel: KycLevel.LEVEL_50 })), - ).resolves.toBe(IbanBankName.OLKY); + ).resolves.toEqual({ bankName: IbanBankName.OLKY, activeVirtualIban: undefined }); }); it('should return the instant bank for instant transfers', async () => { jest.spyOn(bankService, 'getBank').mockResolvedValue(olkyEUR); - await expect(txHelper['getBankIn'](eur, FiatPaymentMethod.INSTANT, undefined)).resolves.toBe(IbanBankName.OLKY); + await expect(txHelper['getBankIn'](eur, FiatPaymentMethod.INSTANT, undefined)).resolves.toEqual({ + bankName: IbanBankName.OLKY, + activeVirtualIban: undefined, + }); }); it('should return the default bank for card payments', async () => { - await expect(txHelper['getBankIn'](eur, FiatPaymentMethod.CARD, undefined)).resolves.toBe(CardBankName.CHECKOUT); + await expect(txHelper['getBankIn'](eur, FiatPaymentMethod.CARD, undefined)).resolves.toEqual({ + bankName: CardBankName.CHECKOUT, + activeVirtualIban: undefined, + }); }); it('should fall back to the default bank if no deposit bank is found', async () => { jest.spyOn(bankService, 'getBank').mockResolvedValue(undefined); - await expect(txHelper['getBankIn'](eur, FiatPaymentMethod.BANK, undefined)).resolves.toBe(IbanBankName.YAPEAL); + await expect(txHelper['getBankIn'](eur, FiatPaymentMethod.BANK, undefined)).resolves.toEqual({ + bankName: IbanBankName.YAPEAL, + activeVirtualIban: undefined, + }); }); }); @@ -322,7 +333,7 @@ describe('TransactionHelper', () => { jest.spyOn(txHelper as any, 'getTargetSpecs').mockResolvedValue({ volume: { min: 0, max: 1000 } }); jest.spyOn(txHelper as any, 'getTargetEstimation').mockResolvedValue({ sourceAmount: 100 }); - await txHelper.getTxDetails( + const details = await txHelper.getTxDetails( 100, undefined, from, @@ -340,6 +351,45 @@ describe('TransactionHelper', () => { expect(getBankIn).not.toHaveBeenCalled(); expect(getAllFees.mock.calls[0][4]).toBe(IbanBankName.FRICK); + // no lookup ran, so the caller must resolve the vIBAN itself rather than treat this as "none found" + expect(details.activeVirtualIban).toBeUndefined(); + }); + + it('hands the resolved active vIBAN to the caller so the deposit destination is not looked up again', async () => { + const from = createCustomFiat({ name: 'EUR' }); + const to = createCustomFiat({ name: 'CHF' }); + const activeVirtualIban = createCustomVirtualIban({ bank: yapealEUR }); + const user = { userData: createCustomUserData({ kycLevel: KycLevel.LEVEL_50 }) } as any; + jest.spyOn(pricingService, 'getPrice').mockResolvedValue({ convert: () => 100 } as any); + jest.spyOn(virtualIbanService, 'getActiveReceivingForUserAndCurrency').mockResolvedValue(activeVirtualIban); + jest + .spyOn(txHelper as any, 'getAllFees') + .mockResolvedValue([ + { network: 0, dfx: { rate: 0, fixed: 0 }, bank: { rate: 0, fixed: 0 }, partner: { rate: 0, fixed: 0 } }, + 0, + ]); + jest.spyOn(txHelper as any, 'getMinSpecs').mockReturnValue({ minFee: 0, minVolume: 0 }); + jest.spyOn(txHelper as any, 'getLimits').mockResolvedValue({ kycLimit: 1000, defaultLimit: 1000 }); + jest.spyOn(txHelper as any, 'getTxErrors').mockReturnValue([]); + jest.spyOn(txHelper as any, 'getSourceSpecs').mockResolvedValue({ volume: { min: 0, max: 1000 } }); + jest.spyOn(txHelper as any, 'getTargetSpecs').mockResolvedValue({ volume: { min: 0, max: 1000 } }); + jest.spyOn(txHelper as any, 'getTargetEstimation').mockResolvedValue({ sourceAmount: 100 }); + + const details = await txHelper.getTxDetails( + 100, + undefined, + from, + to, + FiatPaymentMethod.BANK, + FiatPaymentMethod.BANK, + false, + user, + undefined, + [], + ); + + expect(virtualIbanService.getActiveReceivingForUserAndCurrency).toHaveBeenCalledTimes(1); + expect(details.activeVirtualIban).toBe(activeVirtualIban); }); it('uses the persisted bank selection when regenerating a completed buy invoice', async () => { diff --git a/src/subdomains/supporting/payment/dto/transaction-helper/transaction-details.dto.ts b/src/subdomains/supporting/payment/dto/transaction-helper/transaction-details.dto.ts index 3a5313db87..f3a7811b4f 100644 --- a/src/subdomains/supporting/payment/dto/transaction-helper/transaction-details.dto.ts +++ b/src/subdomains/supporting/payment/dto/transaction-helper/transaction-details.dto.ts @@ -1,3 +1,4 @@ +import { VirtualIban } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.entity'; import { PriceStep } from 'src/subdomains/supporting/pricing/domain/entities/price'; import { FeeDto } from '../fee.dto'; import { QuoteError } from './quote-error.enum'; @@ -23,4 +24,15 @@ export interface TransactionDetails extends TargetEstimation { /** @deprecated Use `errors` instead */ error?: QuoteError; errors: QuoteError[]; + /** + * The user's active receiving vIBAN as resolved while picking the receiving bank, so a caller that also + * needs it for the deposit destination can reuse it instead of repeating the lookup. Undefined when none + * was found, or when no lookup ran at all (non-bank transfer, no userData, or an overridden bank). + * + * Only a positive result is reusable. A caller must NOT treat undefined as a settled "there is none" + * and skip its own read before issuing an IBAN: by then this value is a whole getTxDetails old, and a + * vIBAN issued concurrently in that window would be missed — which surfaces as a fail-closed + * PersonalIbanIssuanceFailed once the duplicate issuance is swallowed. + */ + activeVirtualIban?: VirtualIban; } diff --git a/src/subdomains/supporting/payment/services/transaction-helper.ts b/src/subdomains/supporting/payment/services/transaction-helper.ts index 711cb2a708..5e437c25fc 100644 --- a/src/subdomains/supporting/payment/services/transaction-helper.ts +++ b/src/subdomains/supporting/payment/services/transaction-helper.ts @@ -39,6 +39,7 @@ import { BankTxReturn } from '../../bank-tx/bank-tx-return/bank-tx-return.entity import { BankTx } from '../../bank-tx/bank-tx/entities/bank-tx.entity'; import { BankService } from '../../bank/bank/bank.service'; import { CardBankName, IbanBankName } from '../../bank/bank/dto/bank.dto'; +import { VirtualIban } from '../../bank/virtual-iban/virtual-iban.entity'; import { VirtualIbanService } from '../../bank/virtual-iban/virtual-iban.service'; import { CryptoInput, PayInConfirmationType } from '../../payin/entities/crypto-input.entity'; import { PriceCurrency, PriceValidity, PricingService } from '../../pricing/services/pricing.service'; @@ -274,7 +275,12 @@ export class TransactionHelper implements OnModuleInit { const chfPrice = await this.pricingService.getPrice(txAsset, PriceCurrency.CHF, PriceValidity.ANY); const txAmountChf = chfPrice.convert(txAmount); - const bankIn = bankInOverride ?? (await this.getBankIn(from, paymentMethodIn, user?.userData)); + // getBankIn resolves the user's active vIBAN to pick the receiving bank. It is surfaced on the result + // so the caller can reuse it for the deposit destination instead of repeating the same lookup. + const resolvedBankIn = bankInOverride + ? { bankName: bankInOverride, activeVirtualIban: undefined } + : await this.getBankIn(from, paymentMethodIn, user?.userData); + const bankIn = resolvedBankIn.bankName; const bankOut = TransactionHelper.getDefaultBankByPaymentMethod(paymentMethodOut); const wallet = walletName ? await this.walletService.getByIdOrName(undefined, walletName) : undefined; @@ -355,6 +361,7 @@ export class TransactionHelper implements OnModuleInit { isValid: !errors.length, error: errors[0], errors, + activeVirtualIban: resolvedBankIn.activeVirtualIban, }; } @@ -876,16 +883,22 @@ export class TransactionHelper implements OnModuleInit { from: Active, paymentMethodIn: PaymentMethod, userData?: UserData, - ): Promise { + ): Promise<{ bankName: CardBankName | IbanBankName | undefined; activeVirtualIban?: VirtualIban }> { const isBankTransfer = isFiat(from) && [FiatPaymentMethod.BANK, FiatPaymentMethod.INSTANT].includes(paymentMethodIn as FiatPaymentMethod); - if (!isBankTransfer) return TransactionHelper.getDefaultBankByPaymentMethod(paymentMethodIn); + if (!isBankTransfer) + return { + bankName: TransactionHelper.getDefaultBankByPaymentMethod(paymentMethodIn), + activeVirtualIban: undefined, + }; // vIBAN deposits are received at the vIBAN bank + let activeVirtualIban: VirtualIban | undefined; if (userData) { - const virtualIban = await this.virtualIbanService.getActiveReceivingForUserAndCurrency(userData, from.name); - if (virtualIban?.bank.receive) return virtualIban.bank.name; + activeVirtualIban = + (await this.virtualIbanService.getActiveReceivingForUserAndCurrency(userData, from.name)) ?? undefined; + if (activeVirtualIban?.bank.receive) return { bankName: activeVirtualIban.bank.name, activeVirtualIban }; } const bank = await this.bankService.getBank({ @@ -894,7 +907,10 @@ export class TransactionHelper implements OnModuleInit { userData, }); - return bank?.name ?? TransactionHelper.getDefaultBankByPaymentMethod(paymentMethodIn); + return { + bankName: bank?.name ?? TransactionHelper.getDefaultBankByPaymentMethod(paymentMethodIn), + activeVirtualIban, + }; } static getDefaultBankByPaymentMethod(paymentMethod: PaymentMethod): CardBankName | IbanBankName {