diff --git a/src/integration/blockchain/shared/services/tx-validation.service.ts b/src/integration/blockchain/shared/services/tx-validation.service.ts index 33d862b450..7700582460 100644 --- a/src/integration/blockchain/shared/services/tx-validation.service.ts +++ b/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 { SolanaTransactionDto } from '../../solana/dto/solana.dto'; import { EvmUtil } from '../evm/evm.util'; export interface TxValidationResult { @@ -80,6 +81,40 @@ export class TxValidationService { return { recipient, amount }; } + // Solana transfer verification: match at least one destination in the tx that pays the expected + // owner + asset + amount. `SolanaTransactionDto.destinations[].to` carries the recipient's wallet + // OWNER address for both native SOL and SPL transfers (SolanaClient resolves each SPL transfer's + // destination ATA to its owner via postTokenBalances), so the same owner-equality check works + // for both. The mint disambiguates SPL from SOL and enforces the correct asset. Overpayment is + // accepted (mirrors validateParsedTransaction); underpayment / wrong owner / wrong mint fails. + // Fixes BUG-1260 (Solana anon payment completion accepted any finalized tx). + validateSolanaTransaction( + tx: SolanaTransactionDto, + expectedOwner: string, + expectedAmount: number, + expectedAsset: Asset, + ): TxValidationResult { + try { + const isCoin = expectedAsset.type === AssetType.COIN; + const expectedMint = isCoin ? undefined : expectedAsset.chainId; + + if (!isCoin && !expectedMint) throw new Error('Asset has no chainId (mint address)'); + + const match = tx.destinations.find((d) => { + if (isCoin) return !d.tokenInfo && d.to === expectedOwner; + return d.tokenInfo?.address === expectedMint && d.to === expectedOwner; + }); + + if (!match) throw new Error(`No transfer to ${expectedOwner} for ${expectedAsset.name} found`); + if (match.amount < expectedAmount) + throw new Error(`Insufficient amount: expected ${expectedAmount}, got ${match.amount}`); + + return { isValid: true, sender: tx.from?.[0] }; + } catch (e) { + return { isValid: false, error: e.message }; + } + } + private parseErc20Transfer( parsedTx: { to?: string; data: string }, asset: Asset, diff --git a/src/integration/blockchain/solana/solana-client.ts b/src/integration/blockchain/solana/solana-client.ts index ea5f07d457..2fc3967235 100644 --- a/src/integration/blockchain/solana/solana-client.ts +++ b/src/integration/blockchain/solana/solana-client.ts @@ -12,7 +12,6 @@ import { BlockchainClient, BlockchainToken } from '../shared/util/blockchain-cli import { SolanaNativeInstructionsDto, SolanaTokenDto, - SolanaTokenInstructionsDto, SolanaTransactionDestinationDto, SolanaTransactionDto, } from './dto/solana.dto'; @@ -427,8 +426,7 @@ export class SolanaClient extends BlockchainClient { if (isNativeTransaction) { transaction.destinations.push(...this.getNativeTransactionDestinations(parsedInstructions)); } else if (isTokenTransaction) { - const tokenInstruction = this.getTokenInstructions(parsedTransaction); - transaction.destinations.push(await this.getTokenTransactionDestination(tokenInstruction)); + transaction.destinations.push(...(await this.getTokenTransactionDestinations(parsedTransaction))); } return transaction; @@ -461,79 +459,47 @@ export class SolanaClient extends BlockchainClient { return transactionDestinations; } - private async getTokenTransactionDestination( - tokenInstruction: Partial, - ): Promise { - const token = await this.getTokenByAddress(tokenInstruction.mint); - - return { - to: tokenInstruction.destination, - amount: SolanaUtil.fromLamportAmount(tokenInstruction.amount ?? 0, token.decimals), - tokenInfo: { - address: tokenInstruction.mint, - decimals: token.decimals, - }, - }; - } - - private getTokenInstructions( + // Emit one destination per SPL transfer/transferChecked instruction in the tx, so a single tx + // that bundles unrelated instructions cannot glue DFX's ATA (from a `create`) to a foreign + // transfer's amount (BUG-1260 parser bypass). Resolves each destination ATA to its owner via + // postTokenBalances so callers can match on wallet-owner equality. Non-transfer instructions + // (`create`, `closeAccount`) are ignored — they don't move asset value; a real "create and fund" + // tx also has a transfer that carries the destination + amount. + private async getTokenTransactionDestinations( parsedTransaction: Solana.ParsedTransactionWithMeta, - ): Partial { + ): Promise { const parsedInstructions = parsedTransaction.transaction.message.instructions as Solana.ParsedInstruction[]; + const accountKeys = parsedTransaction.transaction.message.accountKeys; + const postTokenBalances = parsedTransaction.meta.postTokenBalances ?? []; - const tokenInstruction: Partial = {}; + const destinations: SolanaTransactionDestinationDto[] = []; for (const instruction of parsedInstructions) { const info = instruction.parsed?.info; if (!info) continue; - switch (instruction.parsed.type) { - case 'create': - tokenInstruction.mint = info.mint; - tokenInstruction.source = info.source; - tokenInstruction.destination = info.wallet; - break; - - case 'closeAccount': - tokenInstruction.destination = info.destination; - tokenInstruction.source = info.owner; - break; - - case 'transfer': - tokenInstruction.authority = info.authority; - tokenInstruction.amount = info.amount; - break; - - case 'transferChecked': - tokenInstruction.authority = info.authority ?? info.multisigAuthority; - tokenInstruction.amount = info.tokenAmount.amount; - break; - } - } - - if (!tokenInstruction.source && !tokenInstruction.destination && !tokenInstruction.mint) { - this.updateTokenInstruction(parsedTransaction, tokenInstruction); - } + const type = instruction.parsed.type; + if (type !== 'transfer' && type !== 'transferChecked') continue; - return tokenInstruction; - } + const destinationAta: string | undefined = info.destination; + const rawAmount: string | number | undefined = + type === 'transferChecked' ? info.tokenAmount?.amount : info.amount; + if (!destinationAta || rawAmount == null) continue; - private updateTokenInstruction( - parsedTransaction: Solana.ParsedTransactionWithMeta, - tokenInstruction: Partial, - ) { - const authority = tokenInstruction.authority; - if (!authority) return; + const destinationBalance = postTokenBalances.find( + (b) => accountKeys[b.accountIndex]?.pubkey.toBase58() === destinationAta, + ); + if (!destinationBalance?.mint || !destinationBalance?.owner) continue; - const tokenBalances = parsedTransaction.meta.postTokenBalances; + const token = await this.getTokenByAddress(destinationBalance.mint); - const sourceTokenBalance = tokenBalances.find((b) => b.owner === authority); - const destinationTokenBalance = tokenBalances.find( - (b) => b.owner !== authority && b.mint === sourceTokenBalance?.mint, - ); + destinations.push({ + to: destinationBalance.owner, + amount: SolanaUtil.fromLamportAmount(rawAmount, token.decimals), + tokenInfo: { address: destinationBalance.mint, decimals: token.decimals }, + }); + } - tokenInstruction.source = sourceTokenBalance.owner; - tokenInstruction.destination = destinationTokenBalance.owner; - tokenInstruction.mint = destinationTokenBalance.mint; + return destinations; } } diff --git a/src/subdomains/core/payment-link/services/__tests__/payment-link-sepolia.spec.ts b/src/subdomains/core/payment-link/services/__tests__/payment-link-sepolia.spec.ts index 95b9c64eb8..09e6c4a25b 100644 --- a/src/subdomains/core/payment-link/services/__tests__/payment-link-sepolia.spec.ts +++ b/src/subdomains/core/payment-link/services/__tests__/payment-link-sepolia.spec.ts @@ -3,6 +3,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ethers } from 'ethers'; import * as ConfigModule from 'src/config/config'; import { InternetComputerService } from 'src/integration/blockchain/icp/services/icp.service'; +import { SolanaService } from 'src/integration/blockchain/solana/services/solana.service'; import { Blockchain } from 'src/integration/blockchain/shared/enums/blockchain.enum'; import { EvmClient } from 'src/integration/blockchain/shared/evm/evm-client'; import { BlockchainRegistryService } from 'src/integration/blockchain/shared/services/blockchain-registry.service'; @@ -189,6 +190,7 @@ describe('Payment-link engine - Sepolia routing', () => { { provide: PaymentBalanceService, useValue: createMock() }, { provide: TxValidationService, useValue: createMock() }, { provide: InternetComputerService, useValue: createMock() }, + { provide: SolanaService, useValue: createMock() }, ], }).compile(); diff --git a/src/subdomains/core/payment-link/services/__tests__/payment-quote-firo.spec.ts b/src/subdomains/core/payment-link/services/__tests__/payment-quote-firo.spec.ts index 23156e87dd..67a61ef509 100644 --- a/src/subdomains/core/payment-link/services/__tests__/payment-quote-firo.spec.ts +++ b/src/subdomains/core/payment-link/services/__tests__/payment-quote-firo.spec.ts @@ -8,6 +8,7 @@ import { TestSharedModule } from 'src/shared/utils/test.shared.module'; import { Util } from 'src/shared/utils/util'; import { Blockchain } from 'src/integration/blockchain/shared/enums/blockchain.enum'; import { InternetComputerService } from 'src/integration/blockchain/icp/services/icp.service'; +import { SolanaService } from 'src/integration/blockchain/solana/services/solana.service'; import { PricingService } from 'src/subdomains/supporting/pricing/services/pricing.service'; import { PaymentQuoteRepository } from '../../repositories/payment-quote.repository'; import { PaymentQuoteService } from '../payment-quote.service'; @@ -98,6 +99,7 @@ describe('PaymentQuoteService - doFiroTxIdPayment', () => { { provide: PaymentBalanceService, useValue: paymentBalanceService }, { provide: TxValidationService, useValue: createMock() }, { provide: InternetComputerService, useValue: createMock() }, + { provide: SolanaService, useValue: createMock() }, ], }).compile(); diff --git a/src/subdomains/core/payment-link/services/payment-quote.service.ts b/src/subdomains/core/payment-link/services/payment-quote.service.ts index 41f8ef5f58..0849b0a07b 100644 --- a/src/subdomains/core/payment-link/services/payment-quote.service.ts +++ b/src/subdomains/core/payment-link/services/payment-quote.service.ts @@ -8,6 +8,7 @@ import { Blockchain } from 'src/integration/blockchain/shared/enums/blockchain.e import { EvmUtil } from 'src/integration/blockchain/shared/evm/evm.util'; import { BlockchainRegistryService } from 'src/integration/blockchain/shared/services/blockchain-registry.service'; import { TxValidationService } from 'src/integration/blockchain/shared/services/tx-validation.service'; +import { SolanaService } from 'src/integration/blockchain/solana/services/solana.service'; import { LightningHelper } from 'src/integration/lightning/lightning-helper'; import { Asset, AssetType } from 'src/shared/models/asset/asset.entity'; import { AssetService } from 'src/shared/models/asset/asset.service'; @@ -67,6 +68,7 @@ export class PaymentQuoteService { private readonly paymentBalanceService: PaymentBalanceService, private readonly txValidationService: TxValidationService, private readonly internetComputerService: InternetComputerService, + private readonly solanaService: SolanaService, ) {} // --- JOBS --- // @@ -146,6 +148,23 @@ export class PaymentQuoteService { }); } + // Replay guard for txId reuse across quotes (BUG-1260 secondary). Includes TX_FAILED because + // txFailed() does NOT clear txId (see PaymentQuote entity), so a failed quote still lays claim + // to its tx; excluding it would let a foreign quote replay the same tx as if it were fresh. + // ORDER BY id ASC so the earliest claimant is returned deterministically — required because the + // caller (executeHexPayment) saves the current quote with TX_RECEIVED + txId BEFORE reaching the + // per-chain handler, so both the earlier owner and the current quote satisfy the filter. + async getEarliestQuoteClaimingTx(txBlockchain: Blockchain, txId: string): Promise { + return this.paymentQuoteRepo.findOne({ + where: { + txBlockchain: Equal(txBlockchain), + txId: Equal(txId), + status: In([...PaymentQuoteTxStates, PaymentQuoteStatus.TX_FAILED]), + }, + order: { id: 'ASC' }, + }); + } + async getConfirmingQuotes(): Promise { return this.paymentQuoteRepo.find({ where: { status: PaymentQuoteStatus.TX_BLOCKCHAIN }, @@ -401,7 +420,7 @@ export class PaymentQuoteService { break; case Blockchain.SOLANA: - await this.doVerifiedTxIdPayment(Blockchain.SOLANA, transferInfo, quote); + await this.doSolanaTxIdPayment(transferInfo, quote); break; case Blockchain.INTERNET_COMPUTER: @@ -688,6 +707,60 @@ export class PaymentQuoteService { } } + // 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. + private async doSolanaTxIdPayment(transferInfo: TransferInfo, quote: PaymentQuote): Promise { + try { + if (!transferInfo.tx) { + quote.txFailed('Transaction Id not found'); + return; + } + + // Guard against replay of a legitimate DFX-bound tx satisfying an unrelated future quote — + // PaymentQuote.txId has no unique index, so this rejects re-use across quotes. + const earliestClaim = await this.getEarliestQuoteClaimingTx(Blockchain.SOLANA, transferInfo.tx); + if (earliestClaim && earliestClaim.uniqueId !== quote.uniqueId) { + quote.txFailed(`Transaction ${transferInfo.tx} already assigned to another quote`); + return; + } + + await this.waitForTxConfirmation(Blockchain.SOLANA, transferInfo.tx); + + const tx = await this.solanaService.getTransaction(transferInfo.tx); + const paymentAddress = this.paymentBalanceService.getDepositAddress(Blockchain.SOLANA); + const methodActivations = quote.activations?.filter((a) => a.method === Blockchain.SOLANA) ?? []; + + for (const activation of methodActivations) { + if (activation.asset.type !== AssetType.COIN && !activation.asset.chainId) continue; + + const result = this.txValidationService.validateSolanaTransaction( + tx, + paymentAddress, + activation.amount, + activation.asset, + ); + + if (result.isValid) { + quote.txInBlockchain(transferInfo.tx); + return; + } + } + + quote.txFailed(`Transaction ${transferInfo.tx} does not pay any matching activation`); + } catch (e) { + quote.txFailed( + e.message === 'not confirmed' + ? `Transaction ${transferInfo.tx} not confirmed in blockchain ${Blockchain.SOLANA}` + : e.message, + ); + } + } + private async doIcpPayment(transferInfo: TransferInfo, quote: PaymentQuote): Promise { if (!transferInfo.sender) { return this.doVerifiedTxIdPayment(Blockchain.INTERNET_COMPUTER, transferInfo, quote);