From 67a8d2d2700fe3237d61f8a09e0b4588dfbf2b56 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Fri, 26 Jun 2026 16:53:28 +0800 Subject: [PATCH 1/2] feat: enhance transaction alert scanning --- packages/snap/.env.example | 2 +- packages/snap/locales/en.json | 8 +- packages/snap/locales/es.json | 8 +- packages/snap/messages.json | 8 +- packages/snap/src/context.ts | 1 - .../clientRequest/confirmSend.test.ts | 2 +- .../src/handlers/clientRequest/confirmSend.ts | 2 +- .../scanRefresher.test.ts | 10 +- .../handlers/keyring/signTransaction.test.ts | 46 +- .../src/handlers/keyring/signTransaction.ts | 18 +- .../SecurityAlertsApiClient.ts | 6 +- .../TransactionScanService.test.ts | 661 +++++++++++++----- .../TransactionScanService.ts | 307 ++++---- .../security-alerts-api-response.fixture.ts | 325 +++++++++ .../snap/src/services/transaction-scan/api.ts | 177 +++-- .../EstimatedChanges.test.tsx | 4 +- .../EstimatedChanges/EstimatedChanges.tsx | 13 +- .../components/TransactionAlert.test.tsx | 22 +- .../components/TransactionAlert.tsx | 3 + 19 files changed, 1159 insertions(+), 464 deletions(-) create mode 100644 packages/snap/src/services/transaction-scan/__mocks__/security-alerts-api-response.fixture.ts diff --git a/packages/snap/.env.example b/packages/snap/.env.example index e4dd7855..234e353e 100644 --- a/packages/snap/.env.example +++ b/packages/snap/.env.example @@ -35,7 +35,7 @@ EXPLORER_TESTNET_BASE_URL=https://stellar.expert/explorer/testnet TOKEN_API_BASE_URL=http://tokens.api.cx.metamask.io # Static API Base URL -STATIC_API_BASE_URL=https://static.api.cx.metamask.io +STATIC_API_BASE_URL=https://static.cx.metamask.io # Price API Base URL PRICE_API_BASE_URL=https://price.api.cx.metamask.io diff --git a/packages/snap/locales/en.json b/packages/snap/locales/en.json index 9502ad74..ca211156 100644 --- a/packages/snap/locales/en.json +++ b/packages/snap/locales/en.json @@ -140,7 +140,7 @@ "message": "{reason}. Only continue if you trust every address involved." }, "confirmation.simulationErrorTitle": { - "message": "This transaction was reverted during simulation." + "message": "This transaction is expected to fail." }, "confirmation.simulationErrorSubtitle": { "message": "{reason}" @@ -454,6 +454,12 @@ "transactionScan.errors.insufficientFunds": { "message": "Insufficient funds" }, + "transactionScan.errors.noTrustline": { + "message": "Trustline not found" + }, + "transactionScan.errors.transactionExpired": { + "message": "Transaction expired" + }, "transactionScan.errors.invalidAddress": { "message": "Invalid address" }, diff --git a/packages/snap/locales/es.json b/packages/snap/locales/es.json index fb210737..f07d7aa8 100644 --- a/packages/snap/locales/es.json +++ b/packages/snap/locales/es.json @@ -140,7 +140,7 @@ "message": "{reason}. Only continue if you trust every address involved." }, "confirmation.simulationErrorTitle": { - "message": "This transaction was reverted during simulation." + "message": "This transaction is expected to fail." }, "confirmation.simulationErrorSubtitle": { "message": "{reason}" @@ -454,6 +454,12 @@ "transactionScan.errors.insufficientFunds": { "message": "Insufficient funds" }, + "transactionScan.errors.noTrustline": { + "message": "Trustline not found" + }, + "transactionScan.errors.transactionExpired": { + "message": "Transaction expired" + }, "transactionScan.errors.invalidAddress": { "message": "Invalid address" }, diff --git a/packages/snap/messages.json b/packages/snap/messages.json index 5e3af967..f6b69ca9 100644 --- a/packages/snap/messages.json +++ b/packages/snap/messages.json @@ -138,7 +138,7 @@ "message": "{reason}. Only continue if you trust every address involved." }, "confirmation.simulationErrorTitle": { - "message": "This transaction was reverted during simulation." + "message": "This transaction is expected to fail." }, "confirmation.simulationErrorSubtitle": { "message": "{reason}" @@ -452,6 +452,12 @@ "transactionScan.errors.insufficientFunds": { "message": "Insufficient funds" }, + "transactionScan.errors.noTrustline": { + "message": "Trustline not found" + }, + "transactionScan.errors.transactionExpired": { + "message": "Transaction expired" + }, "transactionScan.errors.invalidAddress": { "message": "Invalid address" }, diff --git a/packages/snap/src/context.ts b/packages/snap/src/context.ts index fd29ea8c..f44606aa 100644 --- a/packages/snap/src/context.ts +++ b/packages/snap/src/context.ts @@ -151,7 +151,6 @@ const accountResolver = new AccountResolver({ const signTransactionHandler = new SignTransactionHandler({ logger, accountResolver, - transactionService, confirmationUIController, }); diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts index 05f392bb..cf233a21 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts @@ -308,7 +308,7 @@ describe('ConfirmSendHandler', () => { assets: [ { type: AssetChangeDirection.Out, - value: 1, + value: '1', price: null, symbol: assetMetadata.symbol, name: assetMetadata.name, diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.ts b/packages/snap/src/handlers/clientRequest/confirmSend.ts index c4bcb22e..69219f1c 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.ts @@ -365,7 +365,7 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< assets: [ { type: AssetChangeDirection.Out, - value: Number(amount), + value: amount, price: null, symbol, name: name ?? symbol, diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts index 6f239960..0fd32a1b 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts @@ -28,7 +28,7 @@ describe('ConfirmationScanRefresher', () => { assets: [ { type: AssetChangeDirection.Out, - value: 2, + value: '2', price: null, symbol: 'USDC', name: 'USD Coin', @@ -136,7 +136,7 @@ describe('ConfirmationScanRefresher', () => { assets: [ { type: AssetChangeDirection.Out, - value: 1, + value: '1', price: null, symbol: 'XLM', name: 'Stellar Lumens', @@ -171,7 +171,7 @@ describe('ConfirmationScanRefresher', () => { assets: [ { type: AssetChangeDirection.Out, - value: 12.5, + value: '12.5', price: null, symbol: 'XLM', name: 'Stellar Lumens', @@ -216,7 +216,7 @@ describe('ConfirmationScanRefresher', () => { assets: [ { type: AssetChangeDirection.Out, - value: 7, + value: '7', price: null, symbol: 'XLM', name: 'Stellar Lumens', @@ -255,7 +255,7 @@ describe('ConfirmationScanRefresher', () => { assets: [ { type: AssetChangeDirection.Out, - value: 1, + value: '1', price: null, symbol: 'XLM', name: 'Stellar Lumens', diff --git a/packages/snap/src/handlers/keyring/signTransaction.test.ts b/packages/snap/src/handlers/keyring/signTransaction.test.ts index 0444a58f..a06d70e8 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.test.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.test.ts @@ -6,17 +6,10 @@ import { SignTransactionHandler } from './signTransaction'; import { KnownCaip2ChainId } from '../../api'; import { AccountService } from '../../services/account'; import { generateStellarKeyringAccount } from '../../services/account/__mocks__/account.fixtures'; -import { SimulationException } from '../../services/network/exceptions'; import { mockOnChainAccountService } from '../../services/on-chain-account/__mocks__/onChainAccount.fixtures'; import type { Transaction } from '../../services/transaction'; -import { - TransactionService, - Transaction as WrappedTransaction, -} from '../../services/transaction'; -import { - buildMockClassicTransaction, - createMockTransactionService, -} from '../../services/transaction/__mocks__/transaction.fixtures'; +import { Transaction as WrappedTransaction } from '../../services/transaction'; +import { buildMockClassicTransaction } from '../../services/transaction/__mocks__/transaction.fixtures'; import { WalletService } from '../../services/wallet'; import { getTestWallet } from '../../services/wallet/__mocks__/wallet.fixtures'; import type { ConfirmationUXController } from '../../ui/confirmation/controller'; @@ -41,7 +34,6 @@ describe('SignTransactionHandler', () => { 0, ); - const { transactionService } = createMockTransactionService(); const { accountService, onChainAccountService, walletService } = mockOnChainAccountService(); const accountResolver = new AccountResolver({ @@ -58,11 +50,6 @@ describe('SignTransactionHandler', () => { .spyOn(WalletService.prototype, 'resolveWallet') .mockResolvedValue(wallet); - // Default: pass-through fee (no Soroban simulation needed for classic tx). - jest - .spyOn(TransactionService.prototype, 'computingFee') - .mockImplementation(async (tx) => tx); - const renderConfirmationDialog = jest.fn(); const confirmationUIController = { renderConfirmationDialog, @@ -74,7 +61,6 @@ describe('SignTransactionHandler', () => { const handler = new SignTransactionHandler({ logger, accountResolver, - transactionService, confirmationUIController, }); @@ -82,7 +68,6 @@ describe('SignTransactionHandler', () => { handler, mockAccount, wallet, - transactionService, renderConfirmationDialog, }; } @@ -271,31 +256,4 @@ describe('SignTransactionHandler', () => { }); expect(renderConfirmationDialog).not.toHaveBeenCalled(); }); - - it('returns error -2 when fee simulation fails', async () => { - const { - handler, - mockAccount, - wallet, - transactionService, - renderConfirmationDialog, - } = setupHandler(); - - const transaction = buildMainnetPaymentFromWallet(wallet.address); - jest - .spyOn(transactionService, 'computingFee') - .mockRejectedValueOnce(new SimulationException('contract not found')); - - const result = await handler.handle( - buildRequest(mockAccount.id, transaction.getRaw().toXDR()), - ); - - expect(result).toMatchObject({ - error: { - code: Sep43ErrorCode.ExternalService, - ext: [expect.stringContaining('Failed to simulate transaction')], - }, - }); - expect(renderConfirmationDialog).not.toHaveBeenCalled(); - }); }); diff --git a/packages/snap/src/handlers/keyring/signTransaction.ts b/packages/snap/src/handlers/keyring/signTransaction.ts index b3d9e9f3..44dded59 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.ts @@ -9,7 +9,6 @@ import type { AccountResolver } from '../accountResolver'; import { BaseSep43KeyringHandler } from './base'; import type { Sep43Error } from './exceptions'; import type { StellarKeyringAccount } from '../../services/account'; -import type { TransactionService } from '../../services/transaction'; import { OperationMapper, Transaction } from '../../services/transaction'; import { assertTransactionScope, @@ -35,19 +34,15 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< SignTransactionRequest, SignTransactionResponse > { - readonly #transactionService: TransactionService; - readonly #confirmationUIController: ConfirmationUXController; constructor({ logger, accountResolver, - transactionService, confirmationUIController, }: { logger: ILogger; accountResolver: AccountResolver; - transactionService: TransactionService; confirmationUIController: ConfirmationUXController; }) { super({ @@ -57,7 +52,6 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< requestStruct: SignTransactionRequestStruct, responseStruct: SignTransactionResponseStruct, }); - this.#transactionService = transactionService; this.#confirmationUIController = confirmationUIController; } @@ -77,16 +71,14 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< // verify the transaction scope matches the requested scope assertTransactionScope(transaction, scope); - // Computing fee will inject the fee into the transaction - const transactionWithFee = - await this.#transactionService.computingFee(transaction); - - if (!(await this.#confirmation(request, transactionWithFee, account))) { + // We do not process RPC simulation here, we trust the fee that provided by the dapp. + // If the transaction is invalid, the security scan will output the error. + if (!(await this.#confirmation(request, transaction, account))) { throw new UserRejectedRequestError() as unknown as Error; } - wallet.signTransaction(transactionWithFee); - const signedTxXdr = transactionWithFee.getRaw().toXDR(); + wallet.signTransaction(transaction); + const signedTxXdr = transaction.getRaw().toXDR(); return { signedTxXdr, diff --git a/packages/snap/src/services/transaction-scan/SecurityAlertsApiClient.ts b/packages/snap/src/services/transaction-scan/SecurityAlertsApiClient.ts index ad597915..4e91aa50 100644 --- a/packages/snap/src/services/transaction-scan/SecurityAlertsApiClient.ts +++ b/packages/snap/src/services/transaction-scan/SecurityAlertsApiClient.ts @@ -8,7 +8,7 @@ import { import type { ScanTransactionRequest, SecurityAlertsMetadata, - StellarTransactionScanRequest, + SecurityAlertsApiRequest, StellarTransactionScanResponse, } from './api'; import { TransactionScanException } from './exceptions'; @@ -27,7 +27,7 @@ import { const SCOPE_TO_SECURITY_ALERTS_CHAIN: Record< KnownCaip2ChainId, - StellarTransactionScanRequest['chain'] + SecurityAlertsApiRequest['chain'] > = { [KnownCaip2ChainId.Mainnet]: 'pubnet', [KnownCaip2ChainId.Testnet]: 'testnet', @@ -72,7 +72,7 @@ export class SecurityAlertsApiClient { path: '/stellar/transaction/scan', }); - const requestBody: StellarTransactionScanRequest = { + const requestBody: SecurityAlertsApiRequest = { account_address: accountAddress, chain: SCOPE_TO_SECURITY_ALERTS_CHAIN[scope], metadata: this.#getMetadata(origin), diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts index 49497b08..e147a993 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts @@ -1,25 +1,48 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { Networks } from '@stellar/stellar-sdk'; + +import { + insufficientBalanceResponse, + noTrustlineResponse, + successPaymentSEP41Response, + successPaymentUSDCResponse, + successPaymentXLMResponse, + successSwapXLMToUSDCResponse, +} from './__mocks__/security-alerts-api-response.fixture'; import { AssetChangeDirection, + TransactionScanErrorId, TransactionScanOption, TransactionScanValidationType, } from './api'; +import type { StellarTransactionScanResponse } from './api'; import type { SecurityAlertsApiClient } from './SecurityAlertsApiClient'; import { TransactionScanService } from './TransactionScanService'; -/* eslint-disable @typescript-eslint/naming-convention */ import { KnownCaip2ChainId } from '../../api'; +import { xlmIcon } from '../../ui/images'; +import { toCaip19ClassicAssetId, toCaip19Sep41AssetId } from '../../utils'; import { logger } from '../../utils/logger'; +import { getIconUrl } from '../asset-metadata/utils'; +import { buildMockClassicTransaction } from '../transaction/__mocks__/transaction.fixtures'; jest.mock('../../utils/logger'); +const FIXTURE_ACCOUNT_ADDRESS = + 'GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO'; +const USDC_ISSUER = 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN'; +const SEP41_ADDRESS = + 'CBIJBDNZNF4X35BJ4FFZWCDBSCKOP5NB4PLG4SNENRMLAPYG4P5FM6VN'; + describe('TransactionScanService', () => { - const accountAddress = - 'GDPMFLKUGASUTWBN2XGYYKD27QGHCYH4BUFUTER4L23INYQ4JHDWFOIE'; const scanParams = { - accountAddress, + accountAddress: FIXTURE_ACCOUNT_ADDRESS, origin: 'https://example.com', scope: KnownCaip2ChainId.Mainnet, transaction: 'AAAAAgAAAAA=', - options: [TransactionScanOption.Validation], + options: [ + TransactionScanOption.Simulation, + TransactionScanOption.Validation, + ], }; function setup() { @@ -40,171 +63,220 @@ describe('TransactionScanService', () => { }; } - it('maps successful validation and simulation responses', async () => { - const { service, securityAlertsApiClient } = setup(); - securityAlertsApiClient.scanTransaction.mockResolvedValue({ - validation: { - status: 'Success', - result_type: TransactionScanValidationType.Warning, - reason: 'known_attacker', - description: 'Known attacker involved', - }, - simulation: { - status: 'Success', - account_summary: { - account_assets_diffs: [ + describe('security alerts API fixtures', () => { + it('maps insufficient balance simulation revert', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + insufficientBalanceResponse as StellarTransactionScanResponse, + ); + + const result = await service.scanTransactionSafe(scanParams); + + expect(result).toMatchObject({ + status: 'ERROR', + estimatedChanges: { assets: [] }, + validation: null, + error: { + type: 'simulation', + code: TransactionScanErrorId.InsufficientBalance, + message: insufficientBalanceResponse.simulation.error, + }, + }); + }); + + it('maps no trustline simulation revert', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + noTrustlineResponse as StellarTransactionScanResponse, + ); + + const result = await service.scanTransactionSafe(scanParams); + + expect(result).toMatchObject({ + status: 'ERROR', + estimatedChanges: { assets: [] }, + validation: null, + error: { + type: 'simulation', + code: TransactionScanErrorId.NoTrustline, + message: noTrustlineResponse.simulation.error, + }, + }); + }); + + it('maps XLM to USDC swap simulation and benign validation', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + successSwapXLMToUSDCResponse as StellarTransactionScanResponse, + ); + + const result = await service.scanTransactionSafe(scanParams); + + expect(result).toStrictEqual({ + status: 'SUCCESS', + estimatedChanges: { + assets: [ { - asset: { - code: 'XLM', - }, - asset_type: 'NATIVE', - out: { - raw_value: 10000000, - value: 1, - usd_price: 0.1, - }, + type: AssetChangeDirection.Out, + symbol: 'XLM', + name: 'XLM', + logo: xlmIcon, + value: '2', + price: 0.36, + }, + { + type: AssetChangeDirection.In, + symbol: 'USDC', + name: 'USDC', + logo: getIconUrl( + toCaip19ClassicAssetId( + KnownCaip2ChainId.Mainnet, + 'USDC', + USDC_ISSUER, + ), + ), + value: '1', + price: 1, }, ], }, - }, + validation: { + type: TransactionScanValidationType.Benign, + reason: '', + description: '', + }, + error: null, + }); }); - const result = await service.scanTransactionSafe(scanParams); + it('maps native XLM payment simulation', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + successPaymentXLMResponse as StellarTransactionScanResponse, + ); - expect(result).toStrictEqual({ - status: 'SUCCESS', - estimatedChanges: { - assets: [ - { - type: AssetChangeDirection.Out, - symbol: 'XLM', - name: 'XLM', - logo: null, - value: 1, - price: 0.1, - }, - ], - }, - validation: { - type: TransactionScanValidationType.Warning, - reason: 'known_attacker', - description: 'Known attacker involved', - }, - error: null, - }); - }); + const result = await service.scanTransactionSafe(scanParams); - it('reads the signer asset diffs from assets_diffs when account_summary is empty', async () => { - const { service, securityAlertsApiClient } = setup(); - securityAlertsApiClient.scanTransaction.mockResolvedValue({ - simulation: { - status: 'Success', - // Blockaid leaves the aggregate empty but populates per-account diffs. - account_summary: { account_assets_diffs: [] }, - assets_diffs: { - [accountAddress]: [ + expect(result).toStrictEqual({ + status: 'SUCCESS', + estimatedChanges: { + assets: [ { - asset: { type: 'NATIVE', code: 'XLM' }, - asset_type: 'NATIVE', - in: null, - out: { raw_value: 5000000, value: 0, summary: 'Sent 0.5 XLM' }, + type: AssetChangeDirection.Out, + symbol: 'XLM', + name: 'XLM', + logo: xlmIcon, + value: '1', + price: 0.18, }, ], }, - }, - validation: null, + validation: { + type: TransactionScanValidationType.Benign, + reason: '', + description: '', + }, + error: null, + }); }); - const result = await service.scanTransactionSafe({ - ...scanParams, - options: [TransactionScanOption.Simulation], - }); + it('maps classic USDC payment with sub-unit amount', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + successPaymentUSDCResponse as StellarTransactionScanResponse, + ); - const change = result?.estimatedChanges.assets[0]; - expect(change?.symbol).toBe('XLM'); - expect(change?.type).toBe(AssetChangeDirection.Out); - // raw_value wins over the rounded `value: 0`. - expect(change?.value).toBe(0.5); - }); + const result = await service.scanTransactionSafe(scanParams); - it('resolves an icon for classic issued assets from their code and issuer', async () => { - const { service, securityAlertsApiClient } = setup(); - const issuer = 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN'; - securityAlertsApiClient.scanTransaction.mockResolvedValue({ - simulation: { - status: 'Success', - account_summary: { - account_assets_diffs: [ + expect(result).toStrictEqual({ + status: 'SUCCESS', + estimatedChanges: { + assets: [ { - asset: { code: 'USDC', issuer, type: 'ASSET' }, - asset_type: 'ASSET', - out: { raw_value: 1000000, value: 0.1, usd_price: 0.1 }, + type: AssetChangeDirection.Out, + symbol: 'USDC', + name: 'USDC', + logo: getIconUrl( + toCaip19ClassicAssetId( + KnownCaip2ChainId.Mainnet, + 'USDC', + USDC_ISSUER, + ), + ), + value: '0.000001', + price: 0, }, ], }, - }, - validation: null, + validation: { + type: TransactionScanValidationType.Benign, + reason: '', + description: '', + }, + error: null, + }); }); - const result = await service.scanTransactionSafe({ - ...scanParams, - options: [TransactionScanOption.Simulation], - }); + it('maps SEP-41 token payment with token decimals', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + successPaymentSEP41Response as StellarTransactionScanResponse, + ); - const change = result?.estimatedChanges.assets[0]; - expect(change?.symbol).toBe('USDC'); - // Icon is derived from the classic asset id (code-issuer), not returned by Blockaid. - expect(change?.logo).toContain(`USDC-${issuer}`); - // Decimals resolve from the classic classification, so raw_value wins. - expect(change?.value).toBe(0.1); + const result = await service.scanTransactionSafe(scanParams); + + expect(result).toStrictEqual({ + status: 'SUCCESS', + estimatedChanges: { + assets: [ + { + type: AssetChangeDirection.Out, + symbol: 'SolvBTC', + name: 'Solv BTC', + logo: getIconUrl( + toCaip19Sep41AssetId(KnownCaip2ChainId.Mainnet, SEP41_ADDRESS), + ), + value: '0.00000001', + price: 0, + }, + ], + }, + validation: { + type: TransactionScanValidationType.Benign, + reason: '', + description: '', + }, + error: null, + }); + }); }); - it('maps API simulation errors', async () => { + it('prioritizes the simulation revert over a validation error', async () => { const { service, securityAlertsApiClient } = setup(); - securityAlertsApiClient.scanTransaction.mockResolvedValue({ - simulation: { - status: 'Error', - error: 'insufficient_balance', - }, - validation: null, - }); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + insufficientBalanceResponse as StellarTransactionScanResponse, + ); const result = await service.scanTransactionSafe(scanParams); - expect(result).toMatchObject({ - status: 'ERROR', - error: { - type: 'simulation', - code: 'insufficient_balance', - message: 'insufficient_balance', - }, + expect(result?.error).toMatchObject({ + type: 'simulation', + code: TransactionScanErrorId.InsufficientBalance, }); }); - it('prioritizes the simulation revert over a validation error', async () => { + it('maps API simulation errors with machine-readable codes', async () => { const { service, securityAlertsApiClient } = setup(); securityAlertsApiClient.scanTransaction.mockResolvedValue({ simulation: { status: 'Error', error: 'insufficient_balance', }, - validation: { - status: 'Error', - error: 'known_attacker', - }, + validation: null, }); - const result = await service.scanTransactionSafe({ - ...scanParams, - options: [ - TransactionScanOption.Simulation, - TransactionScanOption.Validation, - ], - }); + const result = await service.scanTransactionSafe(scanParams); - // A validation `Error` only means no verdict was produced (malicious comes - // from a validation `Success`), so the actionable simulation revert reason - // is surfaced instead of masking it behind a security failure. expect(result).toMatchObject({ status: 'ERROR', error: { @@ -244,13 +316,7 @@ describe('TransactionScanService', () => { }, }); - const result = await service.scanTransactionSafe({ - ...scanParams, - options: [ - TransactionScanOption.Simulation, - TransactionScanOption.Validation, - ], - }); + const result = await service.scanTransactionSafe(scanParams); expect(result).toStrictEqual({ status: 'SUCCESS', @@ -282,7 +348,7 @@ describe('TransactionScanService', () => { status: 'ERROR', error: { type: 'simulation', - code: null, + code: TransactionScanErrorId.InvalidTransaction, message: 'Could not simulate transaction: account not found', }, }); @@ -298,68 +364,296 @@ describe('TransactionScanService', () => { expect(result).toBeNull(); }); - describe('estimated changes decimal precision', () => { - it('computes display value from raw_value for fractional native XLM', async () => { + describe('preflight validation', () => { + it('returns a transaction expired error when the time bound has passed', async () => { + const { service, securityAlertsApiClient } = setup(); + const mockNow = 1_700_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(mockNow); + + try { + const expiredTransaction = buildMockClassicTransaction( + [ + { + type: 'payment', + params: { + destination: FIXTURE_ACCOUNT_ADDRESS, + asset: 'native', + amount: '1', + }, + }, + ], + { networkPassphrase: Networks.PUBLIC, timeout: 1 }, + ); + jest.advanceTimersByTime(2000); + + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [], + }, + assets_diffs: {}, + }, + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: expiredTransaction.getRaw().toXDR(), + }); + + expect(result).toMatchObject({ + status: 'ERROR', + error: { + type: 'simulation', + code: TransactionScanErrorId.TransactionExpired, + message: 'Transaction expired', + }, + }); + } finally { + jest.useRealTimers(); + } + }); + + it('does not block scan when XDR cannot be parsed locally', async () => { const { service, securityAlertsApiClient } = setup(); securityAlertsApiClient.scanTransaction.mockResolvedValue({ simulation: { status: 'Success', account_summary: { - account_assets_diffs: [ + account_assets_diffs: [], + }, + assets_diffs: {}, + }, + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: 'AAAAAgAAAAA=', + }); + + expect(result).toMatchObject({ + status: 'SUCCESS', + error: null, + }); + }); + + it('prioritizes simulation revert over preflight expiration error', async () => { + const { service, securityAlertsApiClient } = setup(); + const mockNow = 1_700_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(mockNow); + + try { + const expiredTransaction = buildMockClassicTransaction( + [ + { + type: 'payment', + params: { + destination: FIXTURE_ACCOUNT_ADDRESS, + asset: 'native', + amount: '1', + }, + }, + ], + { networkPassphrase: Networks.PUBLIC, timeout: 1 }, + ); + jest.advanceTimersByTime(2000); + + securityAlertsApiClient.scanTransaction.mockResolvedValue( + insufficientBalanceResponse as StellarTransactionScanResponse, + ); + + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: expiredTransaction.getRaw().toXDR(), + }); + + expect(result?.error).toMatchObject({ + type: 'simulation', + code: TransactionScanErrorId.InsufficientBalance, + }); + } finally { + jest.useRealTimers(); + } + }); + + it('returns estimated changes when preflight reports expiration but simulation succeeds', async () => { + const { service, securityAlertsApiClient } = setup(); + const mockNow = 1_700_000_000_000; + jest.useFakeTimers(); + jest.setSystemTime(mockNow); + + try { + const expiredTransaction = buildMockClassicTransaction( + [ + { + type: 'payment', + params: { + destination: FIXTURE_ACCOUNT_ADDRESS, + asset: 'native', + amount: '1', + }, + }, + ], + { networkPassphrase: Networks.PUBLIC, timeout: 1 }, + ); + jest.advanceTimersByTime(2000); + + securityAlertsApiClient.scanTransaction.mockResolvedValue( + successPaymentXLMResponse as StellarTransactionScanResponse, + ); + + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: expiredTransaction.getRaw().toXDR(), + }); + + expect(result).toMatchObject({ + status: 'ERROR', + error: { + code: TransactionScanErrorId.TransactionExpired, + }, + estimatedChanges: { + assets: [ + { + type: AssetChangeDirection.Out, + symbol: 'XLM', + value: '1', + }, + ], + }, + }); + } finally { + jest.useRealTimers(); + } + }); + }); + + describe('estimated change mapping', () => { + it('omits unsupported asset types from estimated changes', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [], + }, + assets_diffs: { + [FIXTURE_ACCOUNT_ADDRESS]: [ { - asset: { type: 'NATIVE', code: 'XLM' }, + asset: { + type: 'POOL_SHARE', + }, + asset_type: 'POOL_SHARE', + in: null, + out: { + usd_price: 1, + summary: 'Sent pool share', + value: 1, + raw_value: 10000000, + }, + }, + { + asset: { + type: 'NATIVE', + code: 'XLM', + }, asset_type: 'NATIVE', + in: null, out: { - raw_value: 5000000, - value: 0, - usd_price: 0.11, + usd_price: 0.18, + summary: 'Sent 1 XLM', + value: 1, + raw_value: 10000000, }, }, ], }, }, - validation: null, - }); + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + } as StellarTransactionScanResponse); - const result = await service.scanTransactionSafe({ - ...scanParams, - options: [TransactionScanOption.Simulation], - }); + const result = await service.scanTransactionSafe(scanParams); - expect(result?.estimatedChanges.assets[0]?.value).toBe(0.5); + expect(result).toMatchObject({ + status: 'SUCCESS', + estimatedChanges: { + assets: [ + { + type: AssetChangeDirection.Out, + symbol: 'XLM', + name: 'XLM', + logo: xlmIcon, + value: '1', + price: 0.18, + }, + ], + }, + }); }); - it('computes display value from raw_value when value is rounded', async () => { + it('maps null value when raw_value is missing from the transfer', async () => { const { service, securityAlertsApiClient } = setup(); securityAlertsApiClient.scanTransaction.mockResolvedValue({ simulation: { status: 'Success', account_summary: { - account_assets_diffs: [ + account_assets_diffs: [], + }, + assets_diffs: { + [FIXTURE_ACCOUNT_ADDRESS]: [ { - asset: { type: 'NATIVE', code: 'XLM' }, + asset: { + type: 'NATIVE', + code: 'XLM', + }, asset_type: 'NATIVE', + in: null, out: { - raw_value: 15000000, - value: 2, - usd_price: 0.33, + usd_price: 0.18, + summary: 'Sent 1 XLM', + value: 1, }, }, ], }, }, - validation: null, - }); + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + } as unknown as StellarTransactionScanResponse); - const result = await service.scanTransactionSafe({ - ...scanParams, - options: [TransactionScanOption.Simulation], - }); + const result = await service.scanTransactionSafe(scanParams); - expect(result?.estimatedChanges.assets[0]?.value).toBe(1.5); + expect(result).toMatchObject({ + status: 'SUCCESS', + estimatedChanges: { + assets: [ + { + type: AssetChangeDirection.Out, + symbol: 'XLM', + value: null, + price: 0.18, + }, + ], + }, + }); }); - it('falls back to value when asset decimals are unknown', async () => { + it('reads signer diffs from assets_diffs instead of account_summary', async () => { const { service, securityAlertsApiClient } = setup(); securityAlertsApiClient.scanTransaction.mockResolvedValue({ simulation: { @@ -368,31 +662,36 @@ describe('TransactionScanService', () => { account_assets_diffs: [ { asset: { - type: 'CONTRACT', - address: - 'CASUP2OPFVEHCWGP2XLBXOV7DQIQIT42AQISG4MXAZGNLVFFN63X7WRT', - symbol: 'USDC', - name: 'USD Coin', + type: 'NATIVE', + code: 'XLM', }, - asset_type: 'CONTRACT', + asset_type: 'NATIVE', + in: null, out: { - raw_value: 1500000, - value: 1.5, - usd_price: 1.5, + usd_price: 0.18, + summary: 'Sent 1 XLM', + value: 1, + raw_value: 10000000, }, }, ], }, + assets_diffs: {}, }, - validation: null, - }); + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + } as StellarTransactionScanResponse); - const result = await service.scanTransactionSafe({ - ...scanParams, - options: [TransactionScanOption.Simulation], - }); + const result = await service.scanTransactionSafe(scanParams); - expect(result?.estimatedChanges.assets[0]?.value).toBe(1.5); + expect(result).toMatchObject({ + status: 'SUCCESS', + estimatedChanges: { + assets: [], + }, + }); }); }); }); diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.ts index 44e3b31b..41172ec5 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -1,6 +1,13 @@ import { BigNumber } from 'bignumber.js'; -import { AssetChangeDirection, TransactionScanOption } from './api'; +import { + AssetChangeDirection, + StellarClassicAssetDetailsStruct, + StellarNativeAssetDetailsStruct, + StellarSep41AssetDetailsStruct, + TransactionScanErrorId, + TransactionScanOption, +} from './api'; import type { StellarAssetDiff, StellarTransactionScanResponse, @@ -12,7 +19,7 @@ import type { } from './api'; import type { SecurityAlertsApiClient } from './SecurityAlertsApiClient'; import type { KnownCaip2ChainId } from '../../api'; -import { STELLAR_DECIMAL_PLACES } from '../../constants'; +import { xlmIcon } from '../../ui/images'; import type { ILogger } from '../../utils'; import { createPrefixedLogger, @@ -20,8 +27,11 @@ import { toCaip19Sep41AssetId, trackError, } from '../../utils'; -import { normalizeAmount } from '../../utils/currency'; +import { toDisplayBalance } from '../../utils/currency'; import { getIconUrl } from '../asset-metadata/utils'; +import { TransactionExpireException } from '../transaction/exceptions'; +import { Transaction } from '../transaction/Transaction'; +import { assertTransactionTimeBound } from '../transaction/utils'; export class TransactionScanService { readonly #securityAlertsApiClient: SecurityAlertsApiClient; @@ -53,6 +63,11 @@ export class TransactionScanService { options: TransactionScanOption[]; }): Promise { try { + const preflightValidationErrorResult = this.#preflightValidation( + transaction, + scope, + ); + const result = await this.#securityAlertsApiClient.scanTransaction({ accountAddress, origin, @@ -61,7 +76,13 @@ export class TransactionScanService { options, }); - return this.#mapScan(result, options, scope, accountAddress); + return this.#mapScan( + result, + preflightValidationErrorResult, + options, + scope, + accountAddress, + ); } catch (error: unknown) { this.#logger.warn('Error scanning Stellar transaction', { reason: error, @@ -73,8 +94,56 @@ export class TransactionScanService { } } + /** + * Local checks Blockaid does not perform before calling the Security Alerts API. + * + * Currently verifies the transaction time bound has not passed. When the XDR + * cannot be parsed locally, returns `null` so Blockaid remains the source of + * truth for malformed envelopes. + * + * @param xdr - The transaction XDR. + * @param scope - The CAIP-2 chain of the transaction. + * @returns A preflight validation error, or `null` when no local issue applies. + */ + #preflightValidation( + xdr: string, + scope: KnownCaip2ChainId, + ): TransactionScanError | null { + try { + const transaction = Transaction.fromXdr({ + xdr, + scope, + }); + + assertTransactionTimeBound(transaction); + + return null; + } catch (error) { + if (error instanceof TransactionExpireException) { + return { + type: TransactionScanOption.Simulation, + code: TransactionScanErrorId.TransactionExpired, + message: 'Transaction expired', + }; + } + + return null; + } + } + + /** + * Maps a raw Security Alerts API response to the snap's scan result shape. + * + * @param result - Raw scan response from the Security Alerts API. + * @param preflightValidationErrorResult - Local preflight error, if any. + * @param options - Scan options that were requested. + * @param scope - CAIP-2 chain of the transaction. + * @param accountAddress - Signer address used to read per-account asset diffs. + * @returns Normalized scan result for confirmation UI and handlers. + */ #mapScan( result: StellarTransactionScanResponse, + preflightValidationErrorResult: TransactionScanError | null, options: TransactionScanOption[], scope: KnownCaip2ChainId, accountAddress: string, @@ -83,11 +152,14 @@ export class TransactionScanService { const validation = result.validation ?? null; const simulationError = simulation?.status === 'Error' - ? this.#mapError('simulation', simulation.error) + ? this.#mapError(TransactionScanOption.Simulation, simulation.error) : null; + + const preflightValidationError = preflightValidationErrorResult ?? null; + const validationError = validation?.status === 'Error' - ? this.#mapError('validation', validation.error) + ? this.#mapError(TransactionScanOption.Validation, validation.error) : null; const missingResultError = this.#getMissingResultError({ simulation, @@ -99,7 +171,11 @@ export class TransactionScanService { // not produce a verdict — never that the transaction is malicious (that // comes from a validation `Success` with a malicious/warning result_type) — // so it should not mask why the transaction would actually fail. - const error = simulationError ?? validationError ?? missingResultError; + const error = + simulationError ?? + validationError ?? + preflightValidationError ?? + missingResultError; return { status: error ? 'ERROR' : 'SUCCESS', @@ -125,8 +201,7 @@ export class TransactionScanService { * * Blockaid reports per-account diffs under `assets_diffs`, keyed by address. * The aggregated `account_summary.account_assets_diffs` is frequently empty - * (e.g. for tiny/zero-USD amounts), so we read the signer's own entry first - * and only fall back to the summary. + * (e.g. for tiny/zero-USD amounts), so we read the signer's own entry. * * @param simulation - The successful Blockaid simulation result. * @param accountAddress - The signer (scanned) account address. @@ -139,137 +214,98 @@ export class TransactionScanService { >, accountAddress: string, ): StellarAssetDiff[] { - return ( - simulation.assets_diffs?.[accountAddress] ?? - simulation.account_summary.account_assets_diffs ?? - [] - ); + return simulation.assets_diffs?.[accountAddress] ?? []; } #mapAssetChanges( assetDiffs: StellarAssetDiff[], scope: KnownCaip2ChainId, ): TransactionScanAssetChange[] { - return assetDiffs.flatMap((assetDiff) => { - const changes: TransactionScanAssetChange[] = []; - if (assetDiff.out) { - changes.push( - this.#mapAssetChange(assetDiff, AssetChangeDirection.Out, scope), - ); - } - if (assetDiff.in) { - changes.push( - this.#mapAssetChange(assetDiff, AssetChangeDirection.In, scope), - ); - } - return changes; - }); - } - - #mapAssetChange( - assetDiff: StellarAssetDiff, - type: AssetChangeDirection, - scope: KnownCaip2ChainId, - ): TransactionScanAssetChange { - const transfer = assetDiff[type]; - const symbol = - assetDiff.asset.symbol ?? assetDiff.asset.code ?? assetDiff.asset_type; - - return { - type, - symbol, - name: assetDiff.asset.name ?? symbol, - logo: this.#resolveLogo(assetDiff, scope), - value: this.#computeDisplayValue(transfer, assetDiff), - price: transfer?.usd_price ?? null, - }; + return assetDiffs + .filter((assetDiff) => assetDiff.out ?? assetDiff.in) + .map((assetDiff) => + this.#mapAssetChange( + assetDiff, + assetDiff.out ? AssetChangeDirection.Out : AssetChangeDirection.In, + scope, + ), + ) + .filter( + (change): change is TransactionScanAssetChange => change !== null, + ); } /** - * Resolves the asset icon URL for a Blockaid diff. Blockaid does not return an - * icon, so we derive it from the asset identity (the same static icon source - * the rest of the app uses). Native XLM is left to the UI's bundled icon. + * Maps a single asset diff to our internal asset change format. + * + * Returns `null` for asset types the confirmation UI cannot render yet (for + * example pool shares or other Blockaid classifications outside native, + * classic, and SEP-41). * - * @param assetDiff - The asset diff from Blockaid. + * @param assetDiff - The asset diff to map. + * @param type - The direction of the asset change. * @param scope - The CAIP-2 chain of the transaction. - * @returns The icon URL, or null when it cannot be derived. + * @returns The mapped asset change, or `null` when unsupported. */ - #resolveLogo( + #mapAssetChange( assetDiff: StellarAssetDiff, + type: AssetChangeDirection, scope: KnownCaip2ChainId, - ): string | null { - const { asset, asset_type: assetType } = assetDiff; - - if (assetType === 'NATIVE' || asset.type === 'NATIVE') { - return null; - } - if (asset.code !== undefined && asset.issuer !== undefined) { - return getIconUrl( - toCaip19ClassicAssetId(scope, asset.code, asset.issuer), - ); - } - if (asset.address !== undefined) { - return getIconUrl(toCaip19Sep41AssetId(scope, asset.address)); - } - return null; - } + ): TransactionScanAssetChange | null { + const { asset } = assetDiff; + const transfer = assetDiff[type]; + const hasTransfer = transfer !== undefined && transfer !== null; + const hasRawValue = + hasTransfer && + transfer.raw_value !== undefined && + transfer.raw_value !== null; + const usdPrice = + hasTransfer && transfer.usd_price !== undefined + ? transfer.usd_price + : null; - /** - * Computes the human-readable amount for an asset transfer. - * - * For assets with known decimals (native + classic = 7) we normalize - * `raw_value` ourselves, because Blockaid's `value` can be imprecise for - * fractional amounts. For contract (SEP-41) tokens we don't know the decimals, - * so we fall back to Blockaid's already-normalized `value`. - * - * @param transfer - The in/out transfer details from Blockaid. - * @param assetDiff - The parent asset diff (used to resolve decimals). - * @returns The display amount, or null when unavailable. - */ - #computeDisplayValue( - transfer: StellarAssetDiff['in'] | StellarAssetDiff['out'], - assetDiff: StellarAssetDiff, - ): number | null { - if (transfer === undefined || transfer === null) { - return null; + if (StellarNativeAssetDetailsStruct.is(asset)) { + return { + type, + symbol: asset.code, + name: asset.code, + logo: xlmIcon, + value: hasRawValue + ? toDisplayBalance(new BigNumber(transfer.raw_value)) + : null, + price: usdPrice, + }; } - - const decimals = this.#resolveAssetDecimals(assetDiff); - if (decimals !== undefined && transfer.raw_value !== undefined) { - return normalizeAmount( - new BigNumber(transfer.raw_value), - decimals, - ).toNumber(); + if (StellarClassicAssetDetailsStruct.is(asset)) { + return { + type, + symbol: asset.code, + name: asset.code, + logo: getIconUrl( + toCaip19ClassicAssetId(scope, asset.code, asset.issuer), + ), + value: hasRawValue + ? toDisplayBalance(new BigNumber(transfer.raw_value)) + : null, + price: usdPrice, + }; } - - return transfer.value ?? null; - } - - /** - * Resolves asset decimals for Blockaid simulation diffs. - * - * Native and classic Stellar assets use 7 decimal places; contract tokens do - * not expose decimals in the Blockaid payload today. Blockaid is inconsistent - * about where it puts the classification, so we check both the top-level - * `asset_type` and the nested `asset.type` (e.g. classic issued assets such as - * USDC surface `ASSET` on one but not always the other). - * - * @param assetDiff - The asset diff from Blockaid. - * @returns The decimals when known. - */ - #resolveAssetDecimals(assetDiff: StellarAssetDiff): number | undefined { - const { asset_type: assetType, asset } = assetDiff; - - if ( - assetType === 'NATIVE' || - asset.type === 'NATIVE' || - assetType === 'ASSET' || - asset.type === 'ASSET' - ) { - return STELLAR_DECIMAL_PLACES; + if (StellarSep41AssetDetailsStruct.is(asset)) { + return { + type, + symbol: asset.symbol, + name: asset.name, + logo: getIconUrl(toCaip19Sep41AssetId(scope, asset.address)), + value: hasRawValue + ? toDisplayBalance(new BigNumber(transfer.raw_value), asset.decimals) + : null, + price: usdPrice, + }; } - return undefined; + // If the asset is unknown, return null. + // We dont support unknown assets yet. + return null; } #mapValidation( @@ -288,11 +324,38 @@ export class TransactionScanService { #mapError(type: string, message: string): TransactionScanError { return { type, - code: this.#getErrorCode(message), + code: this.#mapErrorCode(message), message, }; } + /** + * Maps a Blockaid error message to a stable error code for localization. + * + * Blockaid may return either free-text revert messages (e.g. "insufficient + * balance") or machine-readable codes (e.g. `insufficient_balance`). Known + * free-text patterns are matched first; otherwise {@link #getErrorCode} is + * used to pass through compact API codes unchanged. + * + * @param message - Raw error string from the simulation or validation payload. + * @returns A normalized code aligned with {@link TransactionScanErrorId}. + */ + #mapErrorCode(message: string): string { + // Blockaid error message contains some keywords about the error. + // Map them to a known error id. + // - "insufficient balance" to "insufficientbalance". + // - "no trustline" to "notrustline". + if (message.toLowerCase().includes('insufficient balance')) { + return TransactionScanErrorId.InsufficientBalance; + } + if (message.toLowerCase().includes('no trustline')) { + return TransactionScanErrorId.NoTrustline; + } + return ( + this.#getErrorCode(message) ?? TransactionScanErrorId.InvalidTransaction + ); + } + #getMissingResultError({ simulation, validation, diff --git a/packages/snap/src/services/transaction-scan/__mocks__/security-alerts-api-response.fixture.ts b/packages/snap/src/services/transaction-scan/__mocks__/security-alerts-api-response.fixture.ts new file mode 100644 index 00000000..95fa1b3e --- /dev/null +++ b/packages/snap/src/services/transaction-scan/__mocks__/security-alerts-api-response.fixture.ts @@ -0,0 +1,325 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +export const insufficientBalanceResponse = { + simulation: { + status: 'Error', + error: + 'Reverted: Operation: Payment (operation index: 4) reverted: Account GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO has insufficient balance for asset $USDC after operation. Current unliable balance: 2.4694894, diff: 20', + error_details: null, + }, + validation: { + status: 'Error', + error: 'Simulation failed', + }, +}; + +export const invalidContractAddressResponse = { + simulation: { + status: 'Error', + error: + 'Reverted: HostError: Error(Contract, #100)\n\nEvent log (newest first):\n 0: [Diagnostic Event] contract:CBIJBDNZNF4X35BJ4FFZWCDBSCKOP5NB4PLG4SNENRMLAPYG4P5FM6VN, topics:[error, Error(Contract, #100)], data:"escalating error to VM trap from failed host function call: fail_with_error"\n 1: [Diagnostic Event] contract:CBIJBDNZNF4X35BJ4FFZWCDBSCKOP5NB4PLG4SNENRMLAPYG4P5FM6VN, topics:[error, Error(Contract, #100)], data:["failing with contract error", 100]\n 2: [Diagnostic Event] topics:[fn_call, CBIJBDNZNF4X35BJ4FFZWCDBSCKOP5NB4PLG4SNENRMLAPYG4P5FM6VN, transfer], data:[GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO, GB327AMKGJDXEMQREZRRVW7Y6KEKWPOWTJKCCYUQK7KKXVMCTNZEOYXU, 111111111]\n', + error_details: null, + }, + validation: { + status: 'Error', + error: 'Simulation failed', + }, +}; + +export const noTrustlineResponse = { + simulation: { + status: 'Error', + error: + 'Reverted: Operation: PathPaymentStrictSend (operation index: 1) reverted: Account GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO has no trustline for AAAd:GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN', + error_details: null, + }, + validation: { + status: 'Error', + error: 'Simulation failed', + }, +}; + +export const successSwapXLMToUSDCResponse = { + simulation: { + status: 'Success', + assets_diffs: { + GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO: [ + { + asset: { + type: 'NATIVE', + code: 'XLM', + }, + in: null, + out: { + usd_price: 0.36, + summary: 'Sent 2 XLM', + value: 2, + raw_value: 20000000, + }, + asset_type: 'NATIVE', + }, + { + asset: { + type: 'ASSET', + code: 'USDC', + issuer: 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN', + org_name: '', + org_url: '', + }, + in: { + usd_price: 1, + summary: 'Received 1 USDC', + value: 1, + raw_value: 10000000, + }, + out: null, + asset_type: 'ASSET', + }, + ], + GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN: [ + { + asset: { + type: 'NATIVE', + code: 'XLM', + }, + in: { + usd_price: 0.18, + summary: 'Received 1 XLM', + value: 1, + raw_value: 10000000, + }, + out: null, + asset_type: 'NATIVE', + }, + ], + }, + exposures: {}, + assets_ownership_diff: {}, + address_details: [], + account_summary: { + account_assets_diffs: [], + account_exposures: [], + account_ownerships_diff: [], + total_usd_diff: { + in: 0, + out: 0, + total: 0, + }, + total_usd_exposure: {}, + }, + transaction_actions: null, + }, + validation: { + status: 'Success', + result_type: 'Benign', + description: '', + reason: '', + classification: '', + features: [], + }, +}; + +export const successPaymentXLMResponse = { + simulation: { + status: 'Success', + assets_diffs: { + GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO: [ + { + asset: { + type: 'NATIVE', + code: 'XLM', + }, + in: null, + out: { + usd_price: 0.18, + summary: 'Sent 1 XLM', + value: 1, + raw_value: 10000000, + }, + asset_type: 'NATIVE', + }, + ], + GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN: [ + { + asset: { + type: 'NATIVE', + code: 'XLM', + }, + in: { + usd_price: 0.18, + summary: 'Received 1 XLM', + value: 1, + raw_value: 10000000, + }, + out: null, + asset_type: 'NATIVE', + }, + ], + }, + exposures: {}, + assets_ownership_diff: {}, + address_details: [], + account_summary: { + account_assets_diffs: [], + account_exposures: [], + account_ownerships_diff: [], + total_usd_diff: { + in: 0, + out: 0, + total: 0, + }, + total_usd_exposure: {}, + }, + transaction_actions: null, + }, + validation: { + status: 'Success', + result_type: 'Benign', + description: '', + reason: '', + classification: '', + features: [], + }, +}; + +export const successPaymentUSDCResponse = { + simulation: { + status: 'Success', + assets_diffs: { + GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO: [ + { + asset: { + type: 'ASSET', + code: 'USDC', + issuer: 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN', + org_name: '', + org_url: '', + }, + in: null, + out: { + usd_price: 0, + summary: 'Sent 0.000001 USDC', + value: 0, + raw_value: 10, + }, + asset_type: 'ASSET', + }, + ], + GB327AMKGJDXEMQREZRRVW7Y6KEKWPOWTJKCCYUQK7KKXVMCTNZEOYXU: [ + { + asset: { + type: 'ASSET', + code: 'USDC', + issuer: 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN', + org_name: '', + org_url: '', + }, + in: { + usd_price: 0, + summary: 'Received 0.000001 USDC', + value: 0, + raw_value: 10, + }, + out: null, + asset_type: 'ASSET', + }, + ], + }, + exposures: {}, + assets_ownership_diff: {}, + address_details: [], + account_summary: { + account_assets_diffs: [], + account_exposures: [], + account_ownerships_diff: [], + total_usd_diff: { + in: 0, + out: 0, + total: 0, + }, + total_usd_exposure: {}, + }, + transaction_actions: null, + }, + validation: { + status: 'Success', + result_type: 'Benign', + description: '', + reason: '', + classification: '', + features: [], + }, +}; + +export const successPaymentSEP41Response = { + simulation: { + status: 'Success', + assets_diffs: { + GA7UCNSASSOPQYTRGJ2NC7TDBSXHMWK6JHS7AO6X2ZQAIQSTB5ELNFSO: [ + { + asset: { + type: 'SEP41', + address: 'CBIJBDNZNF4X35BJ4FFZWCDBSCKOP5NB4PLG4SNENRMLAPYG4P5FM6VN', + name: 'Solv BTC', + symbol: 'SolvBTC', + decimals: 8, + }, + in: null, + out: { + usd_price: 0, + summary: 'Sent 1E-8 SolvBTC', + value: 0, + raw_value: 1, + }, + asset_type: 'SEP41', + }, + ], + GB327AMKGJDXEMQREZRRVW7Y6KEKWPOWTJKCCYUQK7KKXVMCTNZEOYXU: [ + { + asset: { + type: 'SEP41', + address: 'CBIJBDNZNF4X35BJ4FFZWCDBSCKOP5NB4PLG4SNENRMLAPYG4P5FM6VN', + name: 'Solv BTC', + symbol: 'SolvBTC', + decimals: 8, + }, + in: { + usd_price: 0, + summary: 'Received 1E-8 SolvBTC', + value: 0, + raw_value: 1, + }, + out: null, + asset_type: 'SEP41', + }, + ], + }, + exposures: {}, + assets_ownership_diff: {}, + address_details: [], + account_summary: { + account_assets_diffs: [], + account_exposures: [], + account_ownerships_diff: [], + total_usd_diff: { + in: 0, + out: 0, + total: 0, + }, + total_usd_exposure: {}, + }, + transaction_actions: null, + }, + validation: { + status: 'Success', + result_type: 'Benign', + description: '', + reason: '', + classification: '', + features: [], + }, +}; + +export const invalidRequestResponse = { + statusCode: 400, + message: 'account_address must be a valid Stellar public key.', +}; diff --git a/packages/snap/src/services/transaction-scan/api.ts b/packages/snap/src/services/transaction-scan/api.ts index 2455978f..032e41a5 100644 --- a/packages/snap/src/services/transaction-scan/api.ts +++ b/packages/snap/src/services/transaction-scan/api.ts @@ -17,18 +17,47 @@ import { import { KnownCaip2ChainId, XdrStruct } from '../../api'; +/** + * TransactionScanErrorId + * + * Error IDs for the transaction scan. + * These are used to map error messages to localized messages for the transaction alert. + * + * Values are lowercase and punctuation-free so they match the normalized API + * error codes looked up in {@link TransactionAlert} (`ERROR_MESSAGE_IDS`). + * + * @see packages/snap/src/ui/confirmation/components/TransactionAlert.tsx + */ +export enum TransactionScanErrorId { + InsufficientBalance = 'insufficientbalance', + InsufficientFunds = 'insufficientfunds', + InvalidTransaction = 'invalidtransaction', + InvalidAddress = 'invalidaddress', + NoTrustline = 'notrustline', + TransactionExpired = 'transactionexpired', +} + +/** TransactionScanOption - Options for the transaction scan. */ export enum TransactionScanOption { Simulation = 'simulation', Validation = 'validation', } +/** TransactionScanValidationType */ export enum TransactionScanValidationType { Benign = 'Benign', Warning = 'Warning', Malicious = 'Malicious', } -export type StellarSecurityAlertsChain = 'pubnet' | 'testnet' | 'futurenet'; +/** AssetChangeDirection - Direction of an estimated balance change relative to the signer. */ +export enum AssetChangeDirection { + In = 'in', + Out = 'out', +} + +/** Security Alerts API chain identifier. */ +export type SecurityAlertsChain = 'pubnet' | 'testnet' | 'futurenet'; export type SecurityAlertsMetadata = | { @@ -39,14 +68,38 @@ export type SecurityAlertsMetadata = type: 'in_app'; }; -export type StellarTransactionScanRequest = { +/** + * Request body sent to `POST /stellar/transaction/scan`. + */ +export type SecurityAlertsApiRequest = { account_address: string; - chain: StellarSecurityAlertsChain; + chain: SecurityAlertsChain; metadata: SecurityAlertsMetadata; transaction: string; options?: TransactionScanOption[]; }; +/** ScanTransactionRequest */ +export const SecurityScanRequestStruct = type({ + accountAddress: string(), + origin: string(), + scope: enums(Object.values(KnownCaip2ChainId)), + transaction: string(), +}); + +export const ScanTransactionRequestStruct = type({ + accountAddress: nonempty(string()), + origin: string(), + scope: enums(Object.values(KnownCaip2ChainId)), + transaction: XdrStruct, + options: optional(array(enums(Object.values(TransactionScanOption)))), +}); + +export type SecurityScanRequest = Infer; + +export type ScanTransactionRequest = Infer; + +/** StellarTransactionScanResponse */ const StellarAssetTransferDetailsStruct = type({ raw_value: number(), value: number(), @@ -54,17 +107,39 @@ const StellarAssetTransferDetailsStruct = type({ usd_price: optional(nullable(number())), }); -const StellarAssetDetailsStruct = type({ - code: optional(string()), - issuer: optional(string()), - org_name: optional(string()), - org_url: optional(string()), - address: optional(string()), - name: optional(string()), - symbol: optional(string()), - type: optional(string()), +export const StellarClassicAssetDetailsStruct = type({ + type: literal('ASSET'), + code: string(), + issuer: string(), + org_name: string(), + org_url: string(), +}); + +export const StellarNativeAssetDetailsStruct = type({ + type: literal('NATIVE'), + code: literal('XLM'), }); +export const StellarSep41AssetDetailsStruct = type({ + type: literal('SEP41'), + address: string(), + name: string(), + symbol: string(), + decimals: number(), +}); + +/** Catch-all asset shape so Blockaid responses with unsupported types still parse. */ +export const StellarUnknownAssetDetailsStruct = type({ + type: string(), +}); + +const StellarAssetDetailsStruct = union([ + StellarNativeAssetDetailsStruct, + StellarClassicAssetDetailsStruct, + StellarSep41AssetDetailsStruct, + StellarUnknownAssetDetailsStruct, +]); + export const StellarAssetDiffStruct = type({ asset: StellarAssetDetailsStruct, asset_type: string(), @@ -130,42 +205,10 @@ export type StellarTransactionScanResponse = Infer< typeof StellarTransactionScanResponseStruct >; -export type TransactionScanStatus = 'SUCCESS' | 'ERROR'; - -/** Direction of an estimated balance change relative to the signer. */ -export enum AssetChangeDirection { - In = 'in', - Out = 'out', -} - -export type TransactionScanAssetChange = { - type: AssetChangeDirection; - value: number | null; - price: number | null; - symbol: string; - name: string; - logo: string | null; -}; - -export type TransactionScanEstimatedChanges = { - assets: TransactionScanAssetChange[]; -}; - -export type TransactionScanValidation = { - type: TransactionScanValidationType | null; - reason: string | null; - description: string | null; -}; - -export type TransactionScanError = { - type: string | null; - code: string | null; - message: string | null; -}; - +/** TransactionScanResult */ const TransactionScanAssetChangeStruct = type({ type: enums(Object.values(AssetChangeDirection)), - value: nullable(number()), + value: nullable(string()), price: nullable(number()), symbol: string(), name: string(), @@ -179,7 +222,9 @@ const TransactionScanValidationStruct = type({ }); const TransactionScanErrorStruct = type({ - type: nullable(string()), + type: nullable( + enums(Object.values(['simulation', 'validation', 'response'])), + ), code: nullable(string()), message: nullable(string()), }); @@ -193,33 +238,19 @@ export const TransactionScanResultStruct = type({ error: nullable(TransactionScanErrorStruct), }); -export type TransactionScanResult = { - status: TransactionScanStatus; - estimatedChanges: TransactionScanEstimatedChanges; - validation: TransactionScanValidation | null; - error: TransactionScanError | null; -}; +export type TransactionScanAssetChange = Infer< + typeof TransactionScanAssetChangeStruct +>; -export const SecurityScanRequestStruct = type({ - accountAddress: string(), - origin: string(), - scope: enums(Object.values(KnownCaip2ChainId)), - transaction: string(), -}); +export type TransactionScanResult = Infer; -export type SecurityScanRequest = { - accountAddress: string; - origin: string; - scope: KnownCaip2ChainId; - transaction: string; -}; +export type TransactionScanEstimatedChanges = + TransactionScanResult['estimatedChanges']; -export const ScanTransactionRequestStruct = type({ - accountAddress: nonempty(string()), - origin: string(), - scope: enums(Object.values(KnownCaip2ChainId)), - transaction: XdrStruct, - options: optional(array(enums(Object.values(TransactionScanOption)))), -}); +export type TransactionScanStatus = TransactionScanResult['status']; -export type ScanTransactionRequest = Infer; +export type TransactionScanError = Infer; + +export type TransactionScanValidation = Infer< + typeof TransactionScanValidationStruct +>; diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx index 29331863..0f7b05c0 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx @@ -32,7 +32,7 @@ function collectImageSources(node: unknown): unknown[] { describe('EstimatedChanges', () => { const xlmOut = { type: AssetChangeDirection.Out, - value: 10, + value: '10', price: null, symbol: 'XLM', name: 'XLM', @@ -40,7 +40,7 @@ describe('EstimatedChanges', () => { }; const usdcIn = { type: AssetChangeDirection.In, - value: 5, + value: '5', price: null, symbol: 'USDC', name: 'USDC', diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx index 69013de9..49986925 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -11,7 +11,6 @@ import { Text as SnapText, Tooltip, } from '@metamask/snaps-sdk/jsx'; -import { BigNumber } from 'bignumber.js'; import { NATIVE_ASSET_SYMBOL } from '../../../../constants'; import { AssetChangeDirection } from '../../../../services/transaction-scan'; @@ -31,16 +30,6 @@ type EstimatedChangesProps = { scanFetchStatus: FetchStatus; }; -/** - * Formats an estimated-change value without scientific notation. - * - * @param value - The human-readable asset amount, or null. - * @returns The amount as a plain decimal string. - */ -function formatValue(value: number | null): string { - return new BigNumber(value ?? 0).toFixed(); -} - /** * Resolves the text color for an estimated-change row. * @@ -128,7 +117,7 @@ const AssetChangeRow = ({ const isUnknownValue = asset.value === null; const label = isUnknownValue ? `– ${asset.symbol}` - : `${isOut ? '-' : '+'}${formatValue(asset.value)} ${asset.symbol}`; + : `${isOut ? '-' : '+'}${asset.value} ${asset.symbol}`; const color = resolveRowColor(isUnknownValue, isOut); return ( diff --git a/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx b/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx index 2e578c19..63b1acfe 100644 --- a/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx +++ b/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx @@ -41,7 +41,7 @@ describe('TransactionAlert', () => { expect(getType(component)).toBe('Banner'); expect(getProps(component)).toMatchObject({ severity: 'warning', - title: 'This transaction was reverted during simulation.', + title: 'This transaction is expected to fail.', }); }); @@ -181,6 +181,24 @@ describe('TransactionAlert', () => { expect(component).toBeNull(); }); + it('renders a localized message for transaction expired simulation errors', () => { + const component = TransactionAlert({ + preferences: { + ...preferences, + useSecurityAlerts: false, + }, + validation: null, + error: { + type: 'simulation', + code: 'transactionexpired', + message: 'Transaction expired', + }, + scanFetchStatus: FetchStatus.Fetched, + }); + + expect(JSON.stringify(component)).toContain('Transaction expired'); + }); + it('renders scan errors before validation severity findings', () => { const component = TransactionAlert({ preferences, @@ -200,7 +218,7 @@ describe('TransactionAlert', () => { expect(getType(component)).toBe('Banner'); expect(getProps(component)).toMatchObject({ severity: 'warning', - title: 'This transaction was reverted during simulation.', + title: 'This transaction is expected to fail.', }); }); }); diff --git a/packages/snap/src/ui/confirmation/components/TransactionAlert.tsx b/packages/snap/src/ui/confirmation/components/TransactionAlert.tsx index 609ed437..90093357 100644 --- a/packages/snap/src/ui/confirmation/components/TransactionAlert.tsx +++ b/packages/snap/src/ui/confirmation/components/TransactionAlert.tsx @@ -46,11 +46,14 @@ const VALIDATION_TYPE_TO_ALERT: Partial< }, }; +// Keys are normalized API codes (lowercase, no punctuation); see TransactionScanErrorId. const ERROR_MESSAGE_IDS: Record = { insufficientbalance: 'transactionScan.errors.insufficientBalance', insufficientfunds: 'transactionScan.errors.insufficientFunds', invalidtransaction: 'transactionScan.errors.invalidTransaction', invalidaddress: 'transactionScan.errors.invalidAddress', + notrustline: 'transactionScan.errors.noTrustline', + transactionexpired: 'transactionScan.errors.transactionExpired', unsupportedeip712message: 'transactionScan.errors.unsupportedEIP712Message', }; From 7dc3d914fadf04ff66ff0d8a7aab61464f87504c Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Fri, 26 Jun 2026 17:16:06 +0800 Subject: [PATCH 2/2] fix: comment --- .../TransactionScanService.test.ts | 314 +++++++++++------- .../TransactionScanService.ts | 45 ++- 2 files changed, 220 insertions(+), 139 deletions(-) diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts index e147a993..3dc67c3c 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts @@ -365,58 +365,65 @@ describe('TransactionScanService', () => { }); describe('preflight validation', () => { - it('returns a transaction expired error when the time bound has passed', async () => { - const { service, securityAlertsApiClient } = setup(); - const mockNow = 1_700_000_000_000; + const mockNow = 1_700_000_000_000; + + beforeEach(() => { jest.useFakeTimers(); jest.setSystemTime(mockNow); + }); - try { - const expiredTransaction = buildMockClassicTransaction( - [ - { - type: 'payment', - params: { - destination: FIXTURE_ACCOUNT_ADDRESS, - asset: 'native', - amount: '1', - }, - }, - ], - { networkPassphrase: Networks.PUBLIC, timeout: 1 }, - ); - jest.advanceTimersByTime(2000); - - securityAlertsApiClient.scanTransaction.mockResolvedValue({ - simulation: { - status: 'Success', - account_summary: { - account_assets_diffs: [], + afterEach(() => { + jest.useRealTimers(); + }); + + function buildExpiredTransactionXdr(): string { + const expiredTransaction = buildMockClassicTransaction( + [ + { + type: 'payment', + params: { + destination: FIXTURE_ACCOUNT_ADDRESS, + asset: 'native', + amount: '1', }, - assets_diffs: {}, - }, - validation: { - status: 'Success', - result_type: TransactionScanValidationType.Benign, }, - }); - - const result = await service.scanTransactionSafe({ - ...scanParams, - transaction: expiredTransaction.getRaw().toXDR(), - }); - - expect(result).toMatchObject({ - status: 'ERROR', - error: { - type: 'simulation', - code: TransactionScanErrorId.TransactionExpired, - message: 'Transaction expired', + ], + { networkPassphrase: Networks.PUBLIC, timeout: 1 }, + ); + jest.advanceTimersByTime(2000); + + return expiredTransaction.getRaw().toXDR(); + } + + it('returns a transaction expired error when the time bound has passed', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [], }, - }); - } finally { - jest.useRealTimers(); - } + assets_diffs: {}, + }, + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: buildExpiredTransactionXdr(), + }); + + expect(result).toMatchObject({ + status: 'ERROR', + error: { + type: 'simulation', + code: TransactionScanErrorId.TransactionExpired, + message: 'Transaction expired', + }, + }); }); it('does not block scan when XDR cannot be parsed locally', async () => { @@ -446,95 +453,99 @@ describe('TransactionScanService', () => { }); }); + it('skips preflight when simulation is not requested', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: null, + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: buildExpiredTransactionXdr(), + options: [TransactionScanOption.Validation], + }); + + expect(result).toMatchObject({ + status: 'SUCCESS', + error: null, + }); + }); + it('prioritizes simulation revert over preflight expiration error', async () => { const { service, securityAlertsApiClient } = setup(); - const mockNow = 1_700_000_000_000; - jest.useFakeTimers(); - jest.setSystemTime(mockNow); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + insufficientBalanceResponse as StellarTransactionScanResponse, + ); - try { - const expiredTransaction = buildMockClassicTransaction( - [ - { - type: 'payment', - params: { - destination: FIXTURE_ACCOUNT_ADDRESS, - asset: 'native', - amount: '1', - }, - }, - ], - { networkPassphrase: Networks.PUBLIC, timeout: 1 }, - ); - jest.advanceTimersByTime(2000); + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: buildExpiredTransactionXdr(), + }); - securityAlertsApiClient.scanTransaction.mockResolvedValue( - insufficientBalanceResponse as StellarTransactionScanResponse, - ); + expect(result?.error).toMatchObject({ + type: 'simulation', + code: TransactionScanErrorId.InsufficientBalance, + }); + }); - const result = await service.scanTransactionSafe({ - ...scanParams, - transaction: expiredTransaction.getRaw().toXDR(), - }); + it('prioritizes preflight expiration over validation error', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [], + }, + assets_diffs: {}, + }, + validation: { + status: 'Error', + error: 'Simulation failed', + }, + }); - expect(result?.error).toMatchObject({ - type: 'simulation', - code: TransactionScanErrorId.InsufficientBalance, - }); - } finally { - jest.useRealTimers(); - } + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: buildExpiredTransactionXdr(), + }); + + expect(result?.error).toMatchObject({ + type: 'simulation', + code: TransactionScanErrorId.TransactionExpired, + message: 'Transaction expired', + }); }); it('returns estimated changes when preflight reports expiration but simulation succeeds', async () => { const { service, securityAlertsApiClient } = setup(); - const mockNow = 1_700_000_000_000; - jest.useFakeTimers(); - jest.setSystemTime(mockNow); + securityAlertsApiClient.scanTransaction.mockResolvedValue( + successPaymentXLMResponse as StellarTransactionScanResponse, + ); - try { - const expiredTransaction = buildMockClassicTransaction( - [ + const result = await service.scanTransactionSafe({ + ...scanParams, + transaction: buildExpiredTransactionXdr(), + }); + + expect(result).toMatchObject({ + status: 'ERROR', + error: { + code: TransactionScanErrorId.TransactionExpired, + }, + estimatedChanges: { + assets: [ { - type: 'payment', - params: { - destination: FIXTURE_ACCOUNT_ADDRESS, - asset: 'native', - amount: '1', - }, + type: AssetChangeDirection.Out, + symbol: 'XLM', + value: '1', }, ], - { networkPassphrase: Networks.PUBLIC, timeout: 1 }, - ); - jest.advanceTimersByTime(2000); - - securityAlertsApiClient.scanTransaction.mockResolvedValue( - successPaymentXLMResponse as StellarTransactionScanResponse, - ); - - const result = await service.scanTransactionSafe({ - ...scanParams, - transaction: expiredTransaction.getRaw().toXDR(), - }); - - expect(result).toMatchObject({ - status: 'ERROR', - error: { - code: TransactionScanErrorId.TransactionExpired, - }, - estimatedChanges: { - assets: [ - { - type: AssetChangeDirection.Out, - symbol: 'XLM', - value: '1', - }, - ], - }, - }); - } finally { - jest.useRealTimers(); - } + }, + }); }); }); @@ -693,5 +704,66 @@ describe('TransactionScanService', () => { }, }); }); + + it('maps both in and out when present on the same asset diff entry', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [], + }, + assets_diffs: { + [FIXTURE_ACCOUNT_ADDRESS]: [ + { + asset: { + type: 'NATIVE', + code: 'XLM', + }, + asset_type: 'NATIVE', + in: { + usd_price: 0.18, + summary: 'Received 1 XLM', + value: 1, + raw_value: 10000000, + }, + out: { + usd_price: 0.36, + summary: 'Sent 2 XLM', + value: 2, + raw_value: 20000000, + }, + }, + ], + }, + }, + validation: { + status: 'Success', + result_type: TransactionScanValidationType.Benign, + }, + } as StellarTransactionScanResponse); + + const result = await service.scanTransactionSafe(scanParams); + + expect(result).toMatchObject({ + status: 'SUCCESS', + estimatedChanges: { + assets: [ + { + type: AssetChangeDirection.Out, + symbol: 'XLM', + value: '2', + price: 0.36, + }, + { + type: AssetChangeDirection.In, + symbol: 'XLM', + value: '1', + price: 0.18, + }, + ], + }, + }); + }); }); }); diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.ts index 41172ec5..6f65669f 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -63,10 +63,11 @@ export class TransactionScanService { options: TransactionScanOption[]; }): Promise { try { - const preflightValidationErrorResult = this.#preflightValidation( - transaction, - scope, - ); + const preflightValidationErrorResult = options.includes( + TransactionScanOption.Simulation, + ) + ? this.#preflightValidation(transaction, scope) + : null; const result = await this.#securityAlertsApiClient.scanTransaction({ accountAddress, @@ -167,14 +168,14 @@ export class TransactionScanService { options, }); // Prefer the simulation revert: it carries the actionable reason (e.g. - // "insufficient balance"). A validation `Error` only means Blockaid could - // not produce a verdict — never that the transaction is malicious (that - // comes from a validation `Success` with a malicious/warning result_type) — - // so it should not mask why the transaction would actually fail. + // "insufficient balance"). Preflight expiration is next — a local time-bound + // failure the user can act on. A validation `Error` only means Blockaid + // could not produce a verdict, so it must not mask simulation or preflight + // failures. const error = simulationError ?? - validationError ?? preflightValidationError ?? + validationError ?? missingResultError; return { @@ -222,14 +223,22 @@ export class TransactionScanService { scope: KnownCaip2ChainId, ): TransactionScanAssetChange[] { return assetDiffs - .filter((assetDiff) => assetDiff.out ?? assetDiff.in) - .map((assetDiff) => - this.#mapAssetChange( - assetDiff, - assetDiff.out ? AssetChangeDirection.Out : AssetChangeDirection.In, - scope, - ), - ) + .flatMap((assetDiff) => { + const changes: (TransactionScanAssetChange | null)[] = []; + + if (assetDiff.out) { + changes.push( + this.#mapAssetChange(assetDiff, AssetChangeDirection.Out, scope), + ); + } + if (assetDiff.in) { + changes.push( + this.#mapAssetChange(assetDiff, AssetChangeDirection.In, scope), + ); + } + + return changes; + }) .filter( (change): change is TransactionScanAssetChange => change !== null, ); @@ -304,7 +313,7 @@ export class TransactionScanService { } // If the asset is unknown, return null. - // We dont support unknown assets yet. + // We don't support unknown assets yet. return null; }