perf(buy): resolve the user and the active vIBAN once per payment-info request

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<string, unknown>, tradeApprovalDate?: Date): User {
+    return { id: 1, userData: { kycLevel: KycLevel.LEVEL_50, tradeApprovalDate }, wallet } as unknown as User;
+  }
+
+  beforeEach(() => {
+    fiatService = createMock<FiatService>();
+    assetService = createMock<AssetService>();
+    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();
+  });
+});

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', () => {

src/subdomains/core/buy-crypto/routes/buy/buy.service.ts

@@ -35,14 +35,26 @@ 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';
 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<User> = {
+  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<BuyPaymentInfoDto> {
-    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<Buy> {
@@ -267,11 +279,21 @@ export class BuyService {
     return this.buyRepo;
   }
 
-  async toPaymentInfoDto(userId: number, buy: Buy, dto: GetBuyPaymentInfoDto): Promise<BuyPaymentInfoDto> {
-    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<BuyPaymentInfoDto> {
+    // 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 (