fix(payment-link): validate ICP OCP tx recipient/amount/asset in the sender-less fallback (BUG-1260 sibling)

src/integration/blockchain/icp/icp-client.ts

@@ -349,6 +349,60 @@ export class InternetComputerClient extends BlockchainClient {
     return tx?.transaction.operations.every((op) => op.status === 'COMPLETED') ?? false;
   }
 
+  // Introspect a single tx by ID, handling all three ICP txId formats (Rosetta hash, ICRC-3
+  // `canisterId:blockIndex`, native block-index). `asset` is required because ICRC-3 needs its
+  // decimals to normalize the amount; the caller always has one (the activation being matched).
+  // Returns undefined if the tx cannot be located. Used by the OCP verification path to bind
+  // recipient/amount/asset instead of only checking finality (BUG-1260 sibling).
+  async getTransferByTxId(txId: string, asset: Asset): Promise<IcpTransfer | undefined> {
+    // Rosetta hash (64 hex): pull the tx and map its debit/credit ops the same way
+    // getNativeTransfersForAddress does — this is a native-ledger transfer either way.
+    if (/^[a-f0-9]{64}$/i.test(txId)) {
+      const response = await this.http.post<RosettaTransactionsResponse>(`${this.rosettaApiUrl}/search/transactions`, {
+        network_identifier: ROSETTA_NETWORK_ID,
+        transaction_identifier: { hash: txId },
+      });
+      const entry = response.transactions[0];
+      if (!entry) return undefined;
+
+      const ops = entry.transaction.operations.filter((op) => op.type === 'TRANSACTION' && op.status === 'COMPLETED');
+      const creditOp = ops.find((op) => op.amount && BigInt(op.amount.value) > 0n);
+      const debitOp = ops.find((op) => op.amount && BigInt(op.amount.value) < 0n);
+      if (!creditOp?.amount || !debitOp?.amount) return undefined;
+
+      return {
+        blockIndex: entry.block_identifier.index,
+        from: debitOp.account.address,
+        to: creditOp.account.address,
+        amount: InternetComputerUtil.fromSmallestUnit(BigInt(creditOp.amount.value)),
+        fee: 0,
+        memo: BigInt(entry.transaction.metadata.memo),
+        timestamp: Math.floor(entry.transaction.metadata.timestamp / 1_000_000_000),
+      };
+    }
+
+    // ICRC-3 `canisterId:blockIndex` — fetch that single block via the length=1 query
+    const parts = txId.split(':');
+    if (parts.length === 2) {
+      const [canisterId, indexStr] = parts;
+      const index = Number(indexStr);
+      const { transfers } = await this.getIcrcTransfers(canisterId, asset.decimals, index, 1);
+      // getIcrcTransfers/getTransfers can skip archived ranges and return a later block; fail-closed
+      // if the returned transfer is not the one the txId names (would otherwise let a foreign block
+      // satisfy an activation via recipient/amount collision).
+      return transfers[0]?.blockIndex === index ? transfers[0] : undefined;
+    }
+
+    // Native ICP: plain block index — fetch that single block via length=1
+    const index = Number(txId);
+    if (Number.isFinite(index)) {
+      const { transfers } = await this.getTransfers(index, 1);
+      return transfers[0]?.blockIndex === index ? transfers[0] : undefined;
+    }
+
+    return undefined;
+  }
+
   // --- Send native coin ---
 
   async sendNativeCoinFromDex(toAddress: string, amount: number): Promise<string> {

src/integration/blockchain/icp/icp.util.ts

@@ -16,6 +16,28 @@ export class InternetComputerUtil {
     return BigInt(Math.round(amount * Math.pow(10, decimals)));
   }
 
+  // Canonicalize an ICP txId to a single form per underlying block, so replay-guard string
+  // equality is not defeated by Number-coerced aliases (`"42"` vs `"042"` vs `"+42"` vs `"4.2e1"`
+  // all resolve to the same native block; `canA:42` vs `canA:042` to the same ICRC-3 block).
+  // Formats:
+  //   - Rosetta 64-hex hash: lowercased.
+  //   - ICRC-3 `canisterId:blockIndex`: block-index re-serialized via Number(...) if numeric.
+  //   - Native block-index: re-serialized via Number(...) if numeric.
+  // Non-numeric / unknown-shape txIds are returned untouched (the downstream lookup will fail).
+  static canonicalizeTxId(txId: string): string {
+    if (/^[a-f0-9]{64}$/i.test(txId)) return txId.toLowerCase();
+
+    const parts = txId.split(':');
+    if (parts.length === 2) {
+      const [canisterId, indexStr] = parts;
+      const index = Number(indexStr);
+      return Number.isFinite(index) ? `${canisterId}:${index}` : txId;
+    }
+
+    const index = Number(txId);
+    return Number.isFinite(index) && txId.trim().length > 0 ? String(index) : txId;
+  }
+
   static accountIdentifier(address: string, subaccount?: Uint8Array): string {
     const principal = Principal.fromText(address);
     const padding = Buffer.from('\x0Aaccount-id');

src/integration/blockchain/shared/services/tx-validation.service.ts

@@ -1,6 +1,7 @@
 import { Injectable } from '@nestjs/common';
 import { BigNumber, ethers } from 'ethers';
 import { Asset, AssetType } from 'src/shared/models/asset/asset.entity';
+import { IcpTransfer } from '../../icp/dto/icp.dto';
 import { SolanaTransactionDto } from '../../solana/dto/solana.dto';
 import { EvmUtil } from '../evm/evm.util';
 
@@ -115,6 +116,25 @@ export class TxValidationService {
     }
   }
 
+  // ICP transfer verification for the `?tx=<txId>` OCP path (used when the ICRC-2 approve/pull
+  // flow is unavailable — no `sender`). `IcpTransfer.to` is: (a) an account-identifier hex for
+  // native ICP (Rosetta + native ledger map from the byte array), (b) a Principal text for ICRC-3
+  // tokens. Caller passes the pre-normalized expected recipient in the format matching the
+  // asset. Overpayment accepted (mirrors validateParsedTransaction); wrong recipient / insufficient
+  // amount fails.
+  validateIcpTransfer(transfer: IcpTransfer, expectedRecipient: string, expectedAmount: number): TxValidationResult {
+    try {
+      if (transfer.to !== expectedRecipient)
+        throw new Error(`Invalid recipient: expected ${expectedRecipient}, got ${transfer.to}`);
+      if (transfer.amount < expectedAmount)
+        throw new Error(`Insufficient amount: expected ${expectedAmount}, got ${transfer.amount}`);
+
+      return { isValid: true, sender: transfer.from };
+    } catch (e) {
+      return { isValid: false, error: e.message };
+    }
+  }
+
   private parseErc20Transfer(
     parsedTx: { to?: string; data: string },
     asset: Asset,

src/subdomains/core/payment-link/services/payment-quote.service.ts

@@ -3,6 +3,7 @@ import { ethers } from 'ethers';
 import { Config } from 'src/config/config';
 import { BitcoinBasedClient } from 'src/integration/blockchain/bitcoin/node/bitcoin-based-client';
 import { BitcoinNodeType } from 'src/integration/blockchain/bitcoin/services/bitcoin.service';
+import { InternetComputerUtil } from 'src/integration/blockchain/icp/icp.util';
 import { InternetComputerService } from 'src/integration/blockchain/icp/services/icp.service';
 import { Blockchain } from 'src/integration/blockchain/shared/enums/blockchain.enum';
 import { EvmUtil } from 'src/integration/blockchain/shared/evm/evm.util';
@@ -684,36 +685,13 @@ export class PaymentQuoteService {
     );
   }
 
-  private async doVerifiedTxIdPayment(
-    method: Blockchain,
-    transferInfo: TransferInfo,
-    quote: PaymentQuote,
-  ): Promise<void> {
-    try {
-      if (!transferInfo.tx) {
-        quote.txFailed('Transaction Id not found');
-        return;
-      }
-
-      await this.waitForTxConfirmation(method, transferInfo.tx);
-
-      quote.txInBlockchain(transferInfo.tx);
-    } catch (e) {
-      quote.txFailed(
-        e.message === 'not confirmed'
-          ? `Transaction ${transferInfo.tx} not confirmed in blockchain ${method}`
-          : e.message,
-      );
-    }
-  }
-
-  // Fixes BUG-1260 (Solana anon payment completion): previously only checked finality via
-  // isTxComplete — recipient/amount/asset were never validated, so any finalized Solana tx
-  // accepted. Mirrors the EVM/Firo model: fetch the parsed tx, match a destination against the
-  // expected recipient + asset + amount. `SolanaTransactionDto.destinations[].to` is the wallet
-  // owner for both native and SPL (SolanaClient resolves each SPL transfer's destination ATA to
-  // its owner via postTokenBalances), so the owner address is compared for both paths; the mint
-  // disambiguates the asset.
+  // Fixes BUG-1260 (Solana anon payment completion): previously routed through a "verified txId"
+  // path that only checked finality via isTxComplete — recipient/amount/asset were never validated,
+  // so any finalized Solana tx accepted. Mirrors the EVM/Firo model: fetch the parsed tx, match a
+  // destination against the expected recipient + asset + amount. `SolanaTransactionDto.destinations[].to`
+  // is the wallet owner for both native and SPL (SolanaClient resolves each SPL transfer's
+  // destination ATA to its owner via postTokenBalances), so the owner address is compared for both
+  // paths; the mint disambiguates the asset.
   private async doSolanaTxIdPayment(transferInfo: TransferInfo, quote: PaymentQuote): Promise<void> {
     try {
       if (!transferInfo.tx) {
@@ -762,8 +740,13 @@ export class PaymentQuoteService {
   }
 
   private async doIcpPayment(transferInfo: TransferInfo, quote: PaymentQuote): Promise<void> {
+    // When the client cannot use the ICRC-2 approve/pull flow (no `sender` principal), it self-
+    // submits the payment and posts back a txId. That path used to only check finality —
+    // recipient/amount/asset were unverified and any finalized ICP tx would settle a foreign quote.
+    // Now applies the same shape as the Solana fix (BUG-1260): replay guard + fetch the transfer
+    // and match it against an activation.
     if (!transferInfo.sender) {
-      return this.doVerifiedTxIdPayment(Blockchain.INTERNET_COMPUTER, transferInfo, quote);
+      return this.doIcpTxIdPayment(transferInfo, quote);
     }
 
     try {
@@ -806,6 +789,76 @@ export class PaymentQuoteService {
     }
   }
 
+  // Client-broadcast ICP path (no `sender` — no ICRC-2 approve available). Same shape as the
+  // Solana fix (BUG-1260): replay guard, wait for finality, then bind the tx to an activation by
+  // fetching the transfer and validating recipient/amount. Recipient is normalized to the format
+  // the ICP client returns — native transfers surface an account-identifier hex, ICRC-3 transfers
+  // surface the owner principal.
+  private async doIcpTxIdPayment(transferInfo: TransferInfo, quote: PaymentQuote): Promise<void> {
+    try {
+      if (!transferInfo.tx) {
+        quote.txFailed('Transaction Id not found');
+        return;
+      }
+
+      // Canonicalize the txId before both the replay guard and the ledger lookup — otherwise
+      // Number-coerced aliases (`"42"` / `"042"` / `"+42"` / `"4.2e1"` all name block 42; same
+      // for `canA:42` vs `canA:042`) each pass the exact-string `Equal(...)` guard independently
+      // and settle the same on-chain block against multiple quotes. Persist the canonical form
+      // so downstream state (getConfirmingQuotes, webhooks, later replays) all agree.
+      const txId = InternetComputerUtil.canonicalizeTxId(transferInfo.tx);
+      if (txId !== quote.txId) await this.saveTransaction(quote, Blockchain.INTERNET_COMPUTER, txId);
+
+      const earliestClaim = await this.getEarliestQuoteClaimingTx(Blockchain.INTERNET_COMPUTER, txId);
+      if (earliestClaim && earliestClaim.uniqueId !== quote.uniqueId) {
+        quote.txFailed(`Transaction ${txId} already assigned to another quote`);
+        return;
+      }
+
+      await this.waitForTxConfirmation(Blockchain.INTERNET_COMPUTER, txId);
+
+      const icpClient = this.internetComputerService.getDefaultClient();
+      const paymentAddress = this.paymentBalanceService.getDepositAddress(Blockchain.INTERNET_COMPUTER);
+      const methodActivations = quote.activations?.filter((a) => a.method === Blockchain.INTERNET_COMPUTER) ?? [];
+
+      for (const activation of methodActivations) {
+        const isCoin = activation.asset.type === AssetType.COIN;
+        if (!isCoin && !activation.asset.chainId) continue;
+
+        // For ICRC-3 (`canisterId:blockIndex`), reject the tx up-front when its canister does not
+        // match the activation's asset canister. Without this, a legit transfer of 100 units of a
+        // cheap ICRC-3 token to DFX would satisfy a quote for 100 units of a different (expensive)
+        // ICRC-3 token — same principal, same amount, but a different asset entirely.
+        if (!isCoin) {
+          const canisterId = txId.split(':')[0];
+          if (canisterId !== activation.asset.chainId) continue;
+        }
+
+        const transfer = await icpClient.getTransferByTxId(txId, activation.asset);
+        if (!transfer) continue;
+
+        // Native ICP transfers surface an account-identifier hex; ICRC-3 transfers surface the
+        // owner principal — match the payment address to the same form the transfer carries.
+        const expectedRecipient = isCoin ? InternetComputerUtil.accountIdentifier(paymentAddress) : paymentAddress;
+
+        const result = this.txValidationService.validateIcpTransfer(transfer, expectedRecipient, activation.amount);
+
+        if (result.isValid) {
+          quote.txInBlockchain(txId);
+          return;
+        }
+      }
+
+      quote.txFailed(`Transaction ${txId} does not pay any matching activation`);
+    } catch (e) {
+      quote.txFailed(
+        e.message === 'not confirmed'
+          ? `Transaction ${transferInfo.tx} not confirmed in blockchain ${Blockchain.INTERNET_COMPUTER}`
+          : e.message,
+      );
+    }
+  }
+
   private async getAndCheckQuote(transferInfo: TransferInfo): Promise<PaymentQuote> {
     const quoteUniqueId = transferInfo.quoteUniqueId;