diff --git a/packages/site/.env.development b/packages/site/.env.development index 621ef3c8..a5e6bf2e 100644 --- a/packages/site/.env.development +++ b/packages/site/.env.development @@ -1 +1,2 @@ -#SNAP_ORIGIN=npm:@metamask/stellar-wallet-snap \ No newline at end of file +#SNAP_ORIGIN=npm:@metamask/stellar-wallet-snap +SNAP_ORIGIN=local:http://localhost:8080 diff --git a/packages/snap/locales/en.json b/packages/snap/locales/en.json index fb210737..9502ad74 100644 --- a/packages/snap/locales/en.json +++ b/packages/snap/locales/en.json @@ -152,7 +152,7 @@ "message": "It may have expired or your account balance changed. Close this request and try again." }, "confirmation.validationScanErrorTitle": { - "message": "Security validation failed" + "message": "Security check unavailable" }, "confirmation.validationScanErrorSubtitle": { "message": "{reason}" diff --git a/packages/snap/messages.json b/packages/snap/messages.json index 94458813..5e3af967 100644 --- a/packages/snap/messages.json +++ b/packages/snap/messages.json @@ -150,7 +150,7 @@ "message": "It may have expired or your account balance changed. Close this request and try again." }, "confirmation.validationScanErrorTitle": { - "message": "Security validation failed" + "message": "Security check unavailable" }, "confirmation.validationScanErrorSubtitle": { "message": "{reason}" diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 9f57f315..8177ac6d 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-stellar-wallet.git" }, "source": { - "shasum": "fcXiAJRViO4Lb+5+PZ8Nhi9yA6B3/kafroMv9FYTe3A=", + "shasum": "Inf8VxNlsHTyLYST3nsK1X0sdkSiAbkFciLrCTsYZNY=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/constants.ts b/packages/snap/src/constants.ts index 5eae70f2..d0577193 100644 --- a/packages/snap/src/constants.ts +++ b/packages/snap/src/constants.ts @@ -110,7 +110,6 @@ export const ACCOUNT_REQUIRES_MEMO = 'MQ=='; * Maximum native XLM threshold for an incoming * payment to be treated as dust spam. * - * Incoming native XLM payments at or below this value are omitted from activity - * history. The threshold matches the 0.001 value used on TRON and Solana. + * Incoming native XLM payments at or below this value are omitted from activity history. */ export const DUST_XLM_AMOUNT = '0.001'; diff --git a/packages/snap/src/handlers/clientRequest/changeTrustOpt.test.ts b/packages/snap/src/handlers/clientRequest/changeTrustOpt.test.ts index 7da3779a..ab63197f 100644 --- a/packages/snap/src/handlers/clientRequest/changeTrustOpt.test.ts +++ b/packages/snap/src/handlers/clientRequest/changeTrustOpt.test.ts @@ -270,8 +270,8 @@ describe('ChangeTrustOptHandler', () => { }, renderOptions: { loadPrice: true, - scanTxn: true, - validateTxn: true, + securityScanning: true, + localSimulation: true, }, securityScanRequest: { accountAddress: account.address, @@ -464,8 +464,8 @@ describe('ChangeTrustOptHandler', () => { interfaceKey: ConfirmationInterfaceKey.ChangeTrustlineOptOut, renderOptions: { loadPrice: true, - scanTxn: true, - validateTxn: true, + securityScanning: true, + localSimulation: true, }, securityScanRequest: { accountAddress: account.address, diff --git a/packages/snap/src/handlers/clientRequest/changeTrustOpt.ts b/packages/snap/src/handlers/clientRequest/changeTrustOpt.ts index d08af1c2..31c98f36 100644 --- a/packages/snap/src/handlers/clientRequest/changeTrustOpt.ts +++ b/packages/snap/src/handlers/clientRequest/changeTrustOpt.ts @@ -369,10 +369,13 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler< }, fee, interfaceKey: confirmationInterfaceKey, + // localSimulation drives re-validation only; a trustline op moves no + // balance, so there are no estimated changes to seed or display (we + // intentionally pass no initialScan). renderOptions: { loadPrice: true, - scanTxn: true, - validateTxn: true, + securityScanning: true, + localSimulation: true, }, securityScanRequest: { accountAddress: account.address, diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts index 4a21e2b6..05f392bb 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts @@ -49,6 +49,7 @@ import { TransactionValidationException, } from '../../services/transaction/exceptions'; import { KeyringTransactionType } from '../../services/transaction/KeyringTransactionBuilder'; +import { AssetChangeDirection } from '../../services/transaction-scan'; import { WalletService } from '../../services/wallet'; import { getTestWallet } from '../../services/wallet/__mocks__/wallet.fixtures'; import { ConfirmationInterfaceKey } from '../../ui/confirmation/api'; @@ -290,19 +291,34 @@ describe('ConfirmSendHandler', () => { origin: METAMASK_ORIGIN, renderContext: { account, - assetMetadata, toAddress: destinationAddress, - amount: '1', }, renderOptions: { loadPrice: true, - scanTxn: true, - validateTxn: true, + securityScanning: true, + localSimulation: true, }, securityScanRequest: { accountAddress: account.address, transaction: unsignedScanXdr, }, + initialScan: { + status: 'SUCCESS', + estimatedChanges: { + assets: [ + { + type: AssetChangeDirection.Out, + value: 1, + price: null, + symbol: assetMetadata.symbol, + name: assetMetadata.name, + logo: assetMetadata.iconUrl, + }, + ], + }, + validation: null, + error: null, + }, transactionValidationRequest: { accountId: account.id, transaction: unsignedScanXdr, diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.ts b/packages/snap/src/handlers/clientRequest/confirmSend.ts index f5d0e0a0..c4bcb22e 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.ts @@ -33,6 +33,7 @@ import type { ContextWithPrices } from '../../ui/confirmation/api'; import { ConfirmationInterfaceKey } from '../../ui/confirmation/api'; import { hasDecimals, + isSlip44Id, toSmallestUnit, trackTransactionAdded, trackTransactionApproved, @@ -46,6 +47,8 @@ import type { } from '../accountResolver'; import { BaseClientRequestHandler } from './base'; import { AccountNotActivatedException } from '../../services/network'; +import { AssetChangeDirection } from '../../services/transaction-scan'; +import type { TransactionScanEstimatedChanges } from '../../services/transaction-scan'; import type { ConfirmationUXController } from '../../ui/confirmation/controller'; import { TrackTransactionHandler } from '../cronjob/trackTransaction'; @@ -294,6 +297,12 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< const { request, account, assetMetadata, fee, scope, transaction } = params; const { toAddress, amount, assetId } = request.params; const xdr = transaction.getRaw().toXDR(); + // The send asset and amount are known from the request, so the estimated + // changes are just a single outgoing row — no local simulation needed. + const estimatedChanges = this.#buildEstimatedChanges({ + amount, + assetMetadata, + }); return ( (await this.#confirmationUIController.renderConfirmationDialog({ @@ -301,21 +310,25 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< origin: METAMASK_ORIGIN, renderContext: { account, - assetMetadata, toAddress, - amount, }, fee: fee.toString(), interfaceKey: ConfirmationInterfaceKey.ConfirmSendTransaction, renderOptions: { loadPrice: true, - scanTxn: true, - validateTxn: true, + securityScanning: true, + localSimulation: true, }, securityScanRequest: { accountAddress: account.address, transaction: xdr, }, + initialScan: { + status: 'SUCCESS', + estimatedChanges, + validation: null, + error: null, + }, transactionValidationRequest: { accountId: account.id, transaction: xdr, @@ -328,6 +341,40 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< ); } + /** + * Builds the estimated balance changes for the send confirmation: a single + * outgoing row for the known send asset and amount. The network fee is + * surfaced separately, so it is excluded here. + * + * @param params - The parameters. + * @param params.amount - The send amount in human-readable units. + * @param params.assetMetadata - The asset metadata for the row. + * @returns The estimated changes to seed the confirmation. + */ + #buildEstimatedChanges({ + amount, + assetMetadata, + }: { + amount: string; + assetMetadata: StellarAssetMetadata; + }): TransactionScanEstimatedChanges { + const { assetId, symbol, iconUrl, name } = assetMetadata; + const logo = isSlip44Id(assetId) ? null : (iconUrl ?? null); + + return { + assets: [ + { + type: AssetChangeDirection.Out, + value: Number(amount), + price: null, + symbol, + name: name ?? symbol, + logo, + }, + ], + }; + } + /** * Override the base handler to return invalid when the account is not activated. * Instead of showing the account not activated alert, it returns an invalid response. diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts index e140767f..6f239960 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts @@ -4,6 +4,7 @@ import { ConfirmationScanRefresher } from './scanRefresher'; import { KnownCaip2ChainId } from '../../../api'; import type { TransactionScanService } from '../../../services/transaction-scan'; import { + AssetChangeDirection, TransactionScanOption, TransactionScanValidationType, } from '../../../services/transaction-scan'; @@ -23,7 +24,18 @@ describe('ConfirmationScanRefresher', () => { }; const scanResult = { status: 'SUCCESS' as const, - estimatedChanges: { assets: [] }, + estimatedChanges: { + assets: [ + { + type: AssetChangeDirection.Out, + value: 2, + price: null, + symbol: 'USDC', + name: 'USD Coin', + logo: null, + }, + ], + }, validation: { type: TransactionScanValidationType.Benign, reason: null, @@ -57,13 +69,16 @@ describe('ConfirmationScanRefresher', () => { simulateOnChainActions: true, }, securityScanRequest, + // Sign-transaction-like defaults: remote validation + remote simulation. + securityScanning: true, + remoteSimulation: true, scan: null, scanFetchStatus: FetchStatus.Fetching, ...overrides, }); } - it('includes simulation and validation for sign-transaction', async () => { + it('requests simulation and validation when both intents and both scan preferences are enabled', async () => { const { refresher, transactionScanService } = setup(); const result = await refresher.refresh(createScanContext()); @@ -84,45 +99,170 @@ describe('ConfirmationScanRefresher', () => { }); }); - it.each([ - ConfirmationInterfaceKey.ConfirmSendTransaction, - ConfirmationInterfaceKey.ChangeTrustlineOptIn, - ConfirmationInterfaceKey.ChangeTrustlineOptOut, - ])( - 'skips remote simulation for %s even when simulateOnChainActions is enabled', - async (interfaceKey) => { - const { refresher, transactionScanService } = setup(); - - await refresher.refresh(createScanContext({ interfaceKey })); - - expect(transactionScanService.scanTransactionSafe).toHaveBeenCalledWith({ - ...securityScanRequest, - options: [TransactionScanOption.Validation], - }); - }, - ); - - it('omits simulation for sign-transaction when simulateOnChainActions is disabled', async () => { + it('requests simulation when remote simulation is enabled and on-chain simulation is allowed', async () => { const { refresher, transactionScanService } = setup(); await refresher.refresh( createScanContext({ + remoteSimulation: true, + securityScanning: true, preferences: { - useSecurityAlerts: true, - simulateOnChainActions: false, + useSecurityAlerts: false, + simulateOnChainActions: true, }, }), ); + expect(transactionScanService.scanTransactionSafe).toHaveBeenCalledWith({ + ...securityScanRequest, + options: [TransactionScanOption.Simulation], + }); + }); + + it('requests only validation when remote simulation is disabled (local-simulation flow)', async () => { + const { refresher, transactionScanService } = setup(); + + await refresher.refresh(createScanContext({ remoteSimulation: false })); + expect(transactionScanService.scanTransactionSafe).toHaveBeenCalledWith({ ...securityScanRequest, options: [TransactionScanOption.Validation], }); }); - it('returns error status when scan returns null', async () => { + it('uses Blockaid estimated changes when remote simulation returns asset rows', async () => { + const { refresher } = setup(); + const localEstimatedChanges = { + assets: [ + { + type: AssetChangeDirection.Out, + value: 1, + price: null, + symbol: 'XLM', + name: 'Stellar Lumens', + logo: null, + }, + ], + }; + + const result = await refresher.refresh( + createScanContext({ + scan: { + status: 'SUCCESS', + estimatedChanges: localEstimatedChanges, + validation: null, + error: null, + }, + }), + ); + + expect(result).toStrictEqual({ + result: { + scan: scanResult, + scanFetchStatus: FetchStatus.Fetched, + }, + reschedule: true, + }); + }); + + it('falls back to locally-derived estimated changes when Blockaid returns no asset rows', async () => { + const { refresher, transactionScanService } = setup(); + const localEstimatedChanges = { + assets: [ + { + type: AssetChangeDirection.Out, + value: 12.5, + price: null, + symbol: 'XLM', + name: 'Stellar Lumens', + logo: null, + }, + ], + }; + const emptyRemoteScan = { + ...scanResult, + estimatedChanges: { assets: [] }, + }; + transactionScanService.scanTransactionSafe.mockResolvedValueOnce( + emptyRemoteScan, + ); + + const result = await refresher.refresh( + createScanContext({ + scan: { + status: 'SUCCESS', + estimatedChanges: localEstimatedChanges, + validation: null, + error: null, + }, + }), + ); + + expect(result).toStrictEqual({ + result: { + scan: { + ...emptyRemoteScan, + estimatedChanges: localEstimatedChanges, + }, + scanFetchStatus: FetchStatus.Fetched, + }, + reschedule: true, + }); + }); + + it('keeps the locally-seeded estimated changes for local-simulation flows even when the scan returns rows', async () => { + const { refresher } = setup(); + const localEstimatedChanges = { + assets: [ + { + type: AssetChangeDirection.Out, + value: 7, + price: null, + symbol: 'XLM', + name: 'Stellar Lumens', + logo: null, + }, + ], + }; + + // Send / change-trust never opt into remote simulation; a validation-only + // scan can still carry simulation diffs, which must not override the seed. + const result = await refresher.refresh( + createScanContext({ + remoteSimulation: false, + scan: { + status: 'SUCCESS', + estimatedChanges: localEstimatedChanges, + validation: null, + error: null, + }, + }), + ); + + expect(result).toStrictEqual({ + result: { + scan: { ...scanResult, estimatedChanges: localEstimatedChanges }, + scanFetchStatus: FetchStatus.Fetched, + }, + reschedule: true, + }); + }); + + it('returns error status preserving estimated changes when scan returns null', async () => { const { refresher, transactionScanService } = setup(); transactionScanService.scanTransactionSafe.mockResolvedValueOnce(null); + const localEstimatedChanges = { + assets: [ + { + type: AssetChangeDirection.Out, + value: 1, + price: null, + symbol: 'XLM', + name: 'Stellar Lumens', + logo: null, + }, + ], + }; const result = await refresher.refresh( createScanContext({ @@ -130,18 +270,61 @@ describe('ConfirmationScanRefresher', () => { useSecurityAlerts: true, simulateOnChainActions: false, }, + scan: { + status: 'SUCCESS', + estimatedChanges: localEstimatedChanges, + validation: null, + error: null, + }, }), ); expect(result).toStrictEqual({ result: { - scan: null, + scan: { + status: 'ERROR', + estimatedChanges: localEstimatedChanges, + validation: null, + error: null, + }, scanFetchStatus: FetchStatus.Error, }, reschedule: false, }); }); + it('fetches when only simulateOnChainActions is enabled and remote simulation is requested', () => { + const { refresher } = setup(); + + expect( + refresher.shouldFetch( + createScanContext({ + remoteSimulation: true, + preferences: { + useSecurityAlerts: false, + simulateOnChainActions: true, + }, + }), + ), + ).toBe(true); + }); + + it('does not fetch when simulateOnChainActions is enabled but remote simulation is not requested', () => { + const { refresher } = setup(); + + expect( + refresher.shouldFetch( + createScanContext({ + remoteSimulation: false, + preferences: { + useSecurityAlerts: false, + simulateOnChainActions: true, + }, + }), + ), + ).toBe(false); + }); + it('does not fetch when securityScanRequest is missing', () => { const { refresher } = setup(); diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts index d701bdbb..4d3b9b5b 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts @@ -6,11 +6,14 @@ import { type ConfirmationDataContext, type IConfirmationContextRefresher, } from './api'; -import type { TransactionScanService } from '../../../services/transaction-scan'; +import type { + TransactionScanEstimatedChanges, + TransactionScanResult, + TransactionScanService, +} from '../../../services/transaction-scan'; import { TransactionScanOption } from '../../../services/transaction-scan'; import type { ContextWithSecurityScan } from '../../../ui/confirmation/api'; import { - ConfirmationInterfaceKey, ContextWithSecurityScanStruct, FetchStatus, } from '../../../ui/confirmation/api'; @@ -20,7 +23,7 @@ import { createPrefixedLogger } from '../../../utils/logger'; type SecurityScanContext = ConfirmationDataContext & ContextWithSecurityScan; /** - * Refreshes Blockaid security scan / simulation results in the confirmation dialog context. + * Refreshes the Blockaid scan in the confirmation dialog context. */ export class ConfirmationScanRefresher implements IConfirmationContextRefresher { readonly key = ConfirmationContextRefresherKey.Scan; @@ -64,7 +67,8 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher const optionsEnabled = this.#getScanOptions(scanCtx).length > 0; return { result: { - scan: null, + // Keep any locally-seeded estimate visible if the remote scan cannot recover. + scan: scanCtx.scan ?? null, scanFetchStatus: optionsEnabled ? FetchStatus.Error : FetchStatus.Fetched, @@ -81,6 +85,17 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher SecurityScanContext['securityScanRequest'] >; const options = this.#getScanOptions(scanCtx); + // Locally-seeded estimated changes (send / change-trust hold the known + // outgoing amount here). For those flows we never let the remote scan + // override them; sign-txn has no seed (`{ assets: [] }`) and adopts the + // remote estimate. + const localEstimatedChanges = scanCtx.scan?.estimatedChanges ?? { + assets: [], + }; + // Only sign-transaction opts into remote simulation, so only it may surface + // Blockaid's estimated changes. A validation-only scan can still carry + // simulation diffs in its payload, which must not replace the local seed. + const preferRemoteEstimatedChanges = Boolean(scanCtx.remoteSimulation); try { const scan = await this.#transactionScanService.scanTransactionSafe({ @@ -90,7 +105,11 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher return { result: { - scan, + scan: this.#resolveEstimatedChanges( + scan, + localEstimatedChanges, + preferRemoteEstimatedChanges, + ), scanFetchStatus: scan ? FetchStatus.Fetched : FetchStatus.Error, }, reschedule: scan !== null, @@ -99,7 +118,11 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher this.#logger.error('Error refreshing confirmation security scan:', error); return { result: { - scan: null, + scan: this.#resolveEstimatedChanges( + null, + localEstimatedChanges, + preferRemoteEstimatedChanges, + ), scanFetchStatus: FetchStatus.Error, }, reschedule: false, @@ -111,21 +134,57 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher return ContextWithSecurityScanStruct.is(ctx); } + /** + * Resolves which estimated changes a scan result should carry. + * + * For remote-simulation flows (sign-transaction) Blockaid's estimated changes + * win when it returns displayable rows; otherwise the locally-seeded estimate + * is kept. For local-simulation flows (send / change-trust) the local seed + * always wins — a validation-only scan must never override it. + * + * @param scan - The Blockaid scan result, or null when none was returned. + * @param localEstimatedChanges - The locally-seeded estimated changes. + * @param preferRemoteEstimatedChanges - Whether the flow opted into remote simulation. + * @returns A scan result carrying the resolved estimated changes. + */ + #resolveEstimatedChanges( + scan: TransactionScanResult | null, + localEstimatedChanges: TransactionScanEstimatedChanges, + preferRemoteEstimatedChanges: boolean, + ): TransactionScanResult { + if (scan) { + const estimatedChanges = + preferRemoteEstimatedChanges && + this.#hasEstimatedChanges(scan.estimatedChanges) + ? scan.estimatedChanges + : localEstimatedChanges; + + return { ...scan, estimatedChanges }; + } + return { + status: 'ERROR', + estimatedChanges: localEstimatedChanges, + validation: null, + error: null, + }; + } + + #hasEstimatedChanges( + estimatedChanges: TransactionScanEstimatedChanges, + ): boolean { + return estimatedChanges.assets.length > 0; + } + #getScanOptions(ctx: SecurityScanContext): TransactionScanOption[] { const options: TransactionScanOption[] = []; - // Remote simulation is only used where there is no local re-validation to - // cover submittability — the dapp sign-transaction flow. Send and - // change-trust rely on local re-validation, so we skip remote simulation - // (it proved unreliable on TRON). - if ( - ctx.preferences.simulateOnChainActions && - ctx.interfaceKey === ConfirmationInterfaceKey.SignTransaction - ) { + // Remote simulation is requested only by flows that opted into it (sign + // transaction) and only when the user enabled on-chain action simulation. + if (ctx.remoteSimulation && ctx.preferences.simulateOnChainActions) { options.push(TransactionScanOption.Simulation); } - if (ctx.preferences.useSecurityAlerts) { + if (ctx.securityScanning && ctx.preferences.useSecurityAlerts) { options.push(TransactionScanOption.Validation); } diff --git a/packages/snap/src/handlers/keyring/signTransaction.test.ts b/packages/snap/src/handlers/keyring/signTransaction.test.ts index d2e9b284..0444a58f 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.test.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.test.ts @@ -88,8 +88,7 @@ describe('SignTransactionHandler', () => { } /** - * Builds a mainnet payment transaction whose source is the wallet so it - * passes `assertAccountInvolvesTransaction`. + * Builds a mainnet payment transaction whose source is the wallet. * * @param walletAddress - Wallet's Stellar public key (`G…`). * @returns Mock transaction built with `Networks.PUBLIC`. @@ -145,7 +144,8 @@ describe('SignTransactionHandler', () => { expect.objectContaining({ renderOptions: { loadPrice: true, - scanTxn: true, + securityScanning: true, + remoteSimulation: true, }, securityScanRequest: { accountAddress: mockAccount.address, @@ -225,38 +225,6 @@ describe('SignTransactionHandler', () => { fromXdrSpy.mockRestore(); }); - it('returns error -3 when the wallet does not participate in the transaction', async () => { - const { handler, mockAccount, renderConfirmationDialog } = setupHandler(); - - const strangerTx = buildMockClassicTransaction( - [ - { - type: 'payment', - params: { - destination: Keypair.random().publicKey(), - asset: 'native', - amount: '1', - }, - }, - ], - { - networkPassphrase: Networks.PUBLIC, - source: { - accountId: Keypair.random().publicKey(), - sequence: '1', - }, - }, - ); - const result = await handler.handle( - buildRequest(mockAccount.id, strangerTx.getRaw().toXDR()), - ); - - expect(result).toMatchObject({ - error: { code: Sep43ErrorCode.InvalidRequest }, - }); - expect(renderConfirmationDialog).not.toHaveBeenCalled(); - }); - it('returns error -3 when scope is testnet', async () => { const { handler, mockAccount, renderConfirmationDialog } = setupHandler(); @@ -286,52 +254,6 @@ describe('SignTransactionHandler', () => { expect(renderConfirmationDialog).not.toHaveBeenCalled(); }); - it('returns error -3 when the transaction has expired', async () => { - const { handler, mockAccount, renderConfirmationDialog } = setupHandler(); - const USDC_ISSUER = - 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN'; - const MOCK_USDC_ASSET = { code: 'USDC', issuer: USDC_ISSUER } as const; - const mockNow = 1700000000000; - jest.useFakeTimers(); - jest.setSystemTime(mockNow); - - try { - const tx = buildMockClassicTransaction( - [ - { - type: 'pathPaymentStrictSend', - params: { - source: mockAccount.address, - sendAsset: MOCK_USDC_ASSET, - sendAmount: '40', - destination: mockAccount.address, - destAsset: MOCK_USDC_ASSET, - destMin: '35', - }, - }, - ], - { - networkPassphrase: Networks.PUBLIC, - source: { accountId: mockAccount.address, sequence: '1' }, - timeout: 1, - }, - ); - - jest.advanceTimersByTime(2000); - const result = await handler.handle( - buildRequest(mockAccount.id, tx.getRaw().toXDR(), { - opts: { networkPassphrase: Networks.PUBLIC }, - }), - ); - expect(result).toMatchObject({ - error: { code: Sep43ErrorCode.InvalidRequest }, - }); - expect(renderConfirmationDialog).not.toHaveBeenCalled(); - } finally { - jest.useRealTimers(); - } - }); - it.each([ ['opts.submit', { submit: true }], ['opts.submitUrl', { submitUrl: 'https://horizon.stellar.org' }], diff --git a/packages/snap/src/handlers/keyring/signTransaction.ts b/packages/snap/src/handlers/keyring/signTransaction.ts index ecce4e69..b3d9e9f3 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.ts @@ -12,9 +12,7 @@ import type { StellarKeyringAccount } from '../../services/account'; import type { TransactionService } from '../../services/transaction'; import { OperationMapper, Transaction } from '../../services/transaction'; import { - assertAccountInvolvesTransaction, assertTransactionScope, - assertTransactionTimeBound, collectTransactionAssetCaipIds, } from '../../services/transaction/utils'; import type { Wallet } from '../../services/wallet'; @@ -78,13 +76,6 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< // verify the transaction scope matches the requested scope assertTransactionScope(transaction, scope); - // The signer may not be the tx source of the transaction, - // but it must participate as fee source (fee bump), or op source. - // We gate signing to envelopes that involve this wallet. - assertAccountInvolvesTransaction(transaction, wallet.address); - - // Ensure the transaction has not expired - assertTransactionTimeBound(transaction); // Computing fee will inject the fee into the transaction const transactionWithFee = @@ -132,6 +123,8 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< ), ) as ContextWithPrices['tokenPrices']; + // Sign-transaction estimated changes come entirely from remote Blockaid + // simulation; the scan refresher fills them in once the scan returns. return ( (await this.#confirmationUIController.renderConfirmationDialog({ scope: request.scope, @@ -142,7 +135,11 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< readableTransaction, account, }, - renderOptions: { loadPrice: true, scanTxn: true }, + renderOptions: { + loadPrice: true, + securityScanning: true, + remoteSimulation: true, + }, securityScanRequest: { accountAddress: account.address, transaction: transaction.getRaw().toXDR(), diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts index 6d4165f8..49497b08 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts @@ -1,4 +1,8 @@ -import { TransactionScanOption, TransactionScanValidationType } from './api'; +import { + AssetChangeDirection, + TransactionScanOption, + TransactionScanValidationType, +} from './api'; import type { SecurityAlertsApiClient } from './SecurityAlertsApiClient'; import { TransactionScanService } from './TransactionScanService'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -72,7 +76,7 @@ describe('TransactionScanService', () => { estimatedChanges: { assets: [ { - type: 'out', + type: AssetChangeDirection.Out, symbol: 'XLM', name: 'XLM', logo: null, @@ -90,6 +94,71 @@ describe('TransactionScanService', () => { }); }); + 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]: [ + { + asset: { type: 'NATIVE', code: 'XLM' }, + asset_type: 'NATIVE', + in: null, + out: { raw_value: 5000000, value: 0, summary: 'Sent 0.5 XLM' }, + }, + ], + }, + }, + validation: null, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + options: [TransactionScanOption.Simulation], + }); + + 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); + }); + + 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: [ + { + asset: { code: 'USDC', issuer, type: 'ASSET' }, + asset_type: 'ASSET', + out: { raw_value: 1000000, value: 0.1, usd_price: 0.1 }, + }, + ], + }, + }, + validation: null, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + options: [TransactionScanOption.Simulation], + }); + + 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); + }); + it('maps API simulation errors', async () => { const { service, securityAlertsApiClient } = setup(); securityAlertsApiClient.scanTransaction.mockResolvedValue({ @@ -112,7 +181,7 @@ describe('TransactionScanService', () => { }); }); - it('prioritizes validation errors over simulation errors', async () => { + it('prioritizes the simulation revert over a validation error', async () => { const { service, securityAlertsApiClient } = setup(); securityAlertsApiClient.scanTransaction.mockResolvedValue({ simulation: { @@ -133,12 +202,15 @@ describe('TransactionScanService', () => { ], }); + // 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: { - type: 'validation', - code: 'known_attacker', - message: 'known_attacker', + type: 'simulation', + code: 'insufficient_balance', + message: 'insufficient_balance', }, }); }); @@ -225,4 +297,102 @@ describe('TransactionScanService', () => { const result = await service.scanTransactionSafe(scanParams); expect(result).toBeNull(); }); + + describe('estimated changes decimal precision', () => { + it('computes display value from raw_value for fractional native XLM', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [ + { + asset: { type: 'NATIVE', code: 'XLM' }, + asset_type: 'NATIVE', + out: { + raw_value: 5000000, + value: 0, + usd_price: 0.11, + }, + }, + ], + }, + }, + validation: null, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + options: [TransactionScanOption.Simulation], + }); + + expect(result?.estimatedChanges.assets[0]?.value).toBe(0.5); + }); + + it('computes display value from raw_value when value is rounded', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [ + { + asset: { type: 'NATIVE', code: 'XLM' }, + asset_type: 'NATIVE', + out: { + raw_value: 15000000, + value: 2, + usd_price: 0.33, + }, + }, + ], + }, + }, + validation: null, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + options: [TransactionScanOption.Simulation], + }); + + expect(result?.estimatedChanges.assets[0]?.value).toBe(1.5); + }); + + it('falls back to value when asset decimals are unknown', async () => { + const { service, securityAlertsApiClient } = setup(); + securityAlertsApiClient.scanTransaction.mockResolvedValue({ + simulation: { + status: 'Success', + account_summary: { + account_assets_diffs: [ + { + asset: { + type: 'CONTRACT', + address: + 'CASUP2OPFVEHCWGP2XLBXOV7DQIQIT42AQISG4MXAZGNLVFFN63X7WRT', + symbol: 'USDC', + name: 'USD Coin', + }, + asset_type: 'CONTRACT', + out: { + raw_value: 1500000, + value: 1.5, + usd_price: 1.5, + }, + }, + ], + }, + }, + validation: null, + }); + + const result = await service.scanTransactionSafe({ + ...scanParams, + options: [TransactionScanOption.Simulation], + }); + + expect(result?.estimatedChanges.assets[0]?.value).toBe(1.5); + }); + }); }); diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.ts index edd507a5..44e3b31b 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -1,4 +1,6 @@ -import { TransactionScanOption } from './api'; +import { BigNumber } from 'bignumber.js'; + +import { AssetChangeDirection, TransactionScanOption } from './api'; import type { StellarAssetDiff, StellarTransactionScanResponse, @@ -10,8 +12,16 @@ import type { } from './api'; import type { SecurityAlertsApiClient } from './SecurityAlertsApiClient'; import type { KnownCaip2ChainId } from '../../api'; +import { STELLAR_DECIMAL_PLACES } from '../../constants'; import type { ILogger } from '../../utils'; -import { createPrefixedLogger, trackError } from '../../utils'; +import { + createPrefixedLogger, + toCaip19ClassicAssetId, + toCaip19Sep41AssetId, + trackError, +} from '../../utils'; +import { normalizeAmount } from '../../utils/currency'; +import { getIconUrl } from '../asset-metadata/utils'; export class TransactionScanService { readonly #securityAlertsApiClient: SecurityAlertsApiClient; @@ -51,7 +61,7 @@ export class TransactionScanService { options, }); - return this.#mapScan(result, options); + return this.#mapScan(result, options, scope, accountAddress); } catch (error: unknown) { this.#logger.warn('Error scanning Stellar transaction', { reason: error, @@ -66,6 +76,8 @@ export class TransactionScanService { #mapScan( result: StellarTransactionScanResponse, options: TransactionScanOption[], + scope: KnownCaip2ChainId, + accountAddress: string, ): TransactionScanResult { const simulation = result.simulation ?? null; const validation = result.validation ?? null; @@ -82,7 +94,12 @@ export class TransactionScanService { validation, options, }); - const error = validationError ?? simulationError ?? missingResultError; + // 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. + const error = simulationError ?? validationError ?? missingResultError; return { status: error ? 'ERROR' : 'SUCCESS', @@ -90,7 +107,8 @@ export class TransactionScanService { assets: simulation?.status === 'Success' ? this.#mapAssetChanges( - simulation.account_summary.account_assets_diffs ?? [], + this.#getSignerAssetDiffs(simulation, accountAddress), + scope, ) : [], }, @@ -102,16 +120,47 @@ export class TransactionScanService { }; } + /** + * Returns the signer's asset diffs from a successful simulation. + * + * 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. + * + * @param simulation - The successful Blockaid simulation result. + * @param accountAddress - The signer (scanned) account address. + * @returns The signer's asset diffs. + */ + #getSignerAssetDiffs( + simulation: Extract< + NonNullable, + { status: 'Success' } + >, + accountAddress: string, + ): StellarAssetDiff[] { + return ( + simulation.assets_diffs?.[accountAddress] ?? + simulation.account_summary.account_assets_diffs ?? + [] + ); + } + #mapAssetChanges( assetDiffs: StellarAssetDiff[], + scope: KnownCaip2ChainId, ): TransactionScanAssetChange[] { return assetDiffs.flatMap((assetDiff) => { const changes: TransactionScanAssetChange[] = []; if (assetDiff.out) { - changes.push(this.#mapAssetChange(assetDiff, 'out')); + changes.push( + this.#mapAssetChange(assetDiff, AssetChangeDirection.Out, scope), + ); } if (assetDiff.in) { - changes.push(this.#mapAssetChange(assetDiff, 'in')); + changes.push( + this.#mapAssetChange(assetDiff, AssetChangeDirection.In, scope), + ); } return changes; }); @@ -119,7 +168,8 @@ export class TransactionScanService { #mapAssetChange( assetDiff: StellarAssetDiff, - type: 'in' | 'out', + type: AssetChangeDirection, + scope: KnownCaip2ChainId, ): TransactionScanAssetChange { const transfer = assetDiff[type]; const symbol = @@ -129,12 +179,99 @@ export class TransactionScanService { type, symbol, name: assetDiff.asset.name ?? symbol, - logo: null, - value: transfer?.value ?? null, + logo: this.#resolveLogo(assetDiff, scope), + value: this.#computeDisplayValue(transfer, assetDiff), price: transfer?.usd_price ?? 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. + * + * @param assetDiff - The asset diff from Blockaid. + * @param scope - The CAIP-2 chain of the transaction. + * @returns The icon URL, or null when it cannot be derived. + */ + #resolveLogo( + assetDiff: StellarAssetDiff, + 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; + } + + /** + * 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; + } + + const decimals = this.#resolveAssetDecimals(assetDiff); + if (decimals !== undefined && transfer.raw_value !== undefined) { + return normalizeAmount( + new BigNumber(transfer.raw_value), + decimals, + ).toNumber(); + } + + 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; + } + + return undefined; + } + #mapValidation( validation: Extract< NonNullable, diff --git a/packages/snap/src/services/transaction-scan/api.ts b/packages/snap/src/services/transaction-scan/api.ts index 951fc3fd..2455978f 100644 --- a/packages/snap/src/services/transaction-scan/api.ts +++ b/packages/snap/src/services/transaction-scan/api.ts @@ -132,8 +132,14 @@ export type StellarTransactionScanResponse = Infer< 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: 'in' | 'out'; + type: AssetChangeDirection; value: number | null; price: number | null; symbol: string; @@ -158,7 +164,7 @@ export type TransactionScanError = { }; const TransactionScanAssetChangeStruct = type({ - type: enums(['in', 'out']), + type: enums(Object.values(AssetChangeDirection)), value: nullable(number()), price: nullable(number()), symbol: string(), diff --git a/packages/snap/src/ui/confirmation/api.ts b/packages/snap/src/ui/confirmation/api.ts index 1027ede1..44a45acd 100644 --- a/packages/snap/src/ui/confirmation/api.ts +++ b/packages/snap/src/ui/confirmation/api.ts @@ -77,6 +77,12 @@ export const ContextWithSecurityScanStruct = type({ scan: optional(nullable(TransactionScanResultStruct)), scanFetchStatus: enums(Object.values(FetchStatus)), securityScanRequest: optional(SecurityScanRequestStruct), + // Render-time intents the scan refresher uses to pick Blockaid options, + // decoupled from the interface key: + // - securityScanning: request remote validation (gated by useSecurityAlerts) + // - remoteSimulation: request remote simulation (gated by simulateOnChainActions) + securityScanning: boolean(), + remoteSimulation: boolean(), }); export type ContextWithSecurityScan = Infer< diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx new file mode 100644 index 00000000..29331863 --- /dev/null +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx @@ -0,0 +1,140 @@ +import { EstimatedChanges } from './EstimatedChanges'; +import { AssetChangeDirection } from '../../../../services/transaction-scan'; +import { xlmIcon } from '../../../images'; +import { + defaultPreferences as preferences, + getType, +} from '../../__fixtures__/confirmation.fixtures'; +import { FetchStatus } from '../../api'; + +function collectImageSources(node: unknown): unknown[] { + if (!node || typeof node !== 'object') { + return []; + } + + const element = node as { + type?: string; + props?: { src?: unknown; children?: unknown }; + }; + const sources = element.type === 'Image' ? [element.props?.src] : []; + const children = element.props?.children; + + if (Array.isArray(children)) { + return [ + ...sources, + ...children.flatMap((child) => collectImageSources(child)), + ]; + } + + return [...sources, ...collectImageSources(children)]; +} + +describe('EstimatedChanges', () => { + const xlmOut = { + type: AssetChangeDirection.Out, + value: 10, + price: null, + symbol: 'XLM', + name: 'XLM', + logo: null, + }; + const usdcIn = { + type: AssetChangeDirection.In, + value: 5, + price: null, + symbol: 'USDC', + name: 'USDC', + logo: 'https://example.com/usdc.png', + }; + + it('renders a skeleton while the remote scan is fetching and nothing is seeded', () => { + const component = EstimatedChanges({ + changes: { assets: [] }, + preferences, + scanFetchStatus: FetchStatus.Fetching, + }); + + expect(JSON.stringify(component)).toContain('"type":"Skeleton"'); + expect(JSON.stringify(component)).not.toContain('-10 XLM'); + }); + + it('keeps locally-seeded rows visible while the remote scan is fetching', () => { + const component = EstimatedChanges({ + changes: { assets: [xlmOut] }, + preferences, + scanFetchStatus: FetchStatus.Fetching, + }); + + const serialized = JSON.stringify(component); + expect(serialized).not.toContain('"type":"Skeleton"'); + expect(serialized).toContain('-10 XLM'); + }); + + it('shows not available when the remote scan errors and nothing is seeded', () => { + const component = EstimatedChanges({ + changes: { assets: [] }, + preferences, + scanFetchStatus: FetchStatus.Error, + }); + + expect(JSON.stringify(component)).toContain( + 'Estimated changes are not available', + ); + }); + + it('keeps locally-seeded rows visible when the remote scan errors', () => { + const component = EstimatedChanges({ + changes: { assets: [xlmOut] }, + preferences, + scanFetchStatus: FetchStatus.Error, + }); + + const serialized = JSON.stringify(component); + expect(serialized).not.toContain('Estimated changes are not available'); + expect(serialized).toContain('-10 XLM'); + }); + + it('renders a neutral placeholder for an unknown (null) amount', () => { + const component = EstimatedChanges({ + changes: { + assets: [{ ...xlmOut, value: null }], + }, + preferences, + scanFetchStatus: FetchStatus.Fetched, + }); + + const serialized = JSON.stringify(component); + expect(serialized).toContain('– XLM'); + expect(serialized).toContain('"color":"alternative"'); + }); + + it('shows no changes when fetched with empty assets', () => { + const component = EstimatedChanges({ + changes: { assets: [] }, + preferences, + scanFetchStatus: FetchStatus.Fetched, + }); + + expect(JSON.stringify(component)).toContain('No estimated changes'); + }); + + it('renders a section containing the send and receive assets once fetched', () => { + const component = EstimatedChanges({ + changes: { assets: [xlmOut, usdcIn] }, + preferences, + scanFetchStatus: FetchStatus.Fetched, + }); + + expect(getType(component)).toBe('Section'); + + const serialized = JSON.stringify(component); + const imageSources = collectImageSources(component); + // Outflows render in red with a leading "-", inflows in green with "+". + expect(serialized).toContain('-10 XLM'); + expect(serialized).toContain('+5 USDC'); + expect(imageSources).toContain(xlmIcon); + expect(serialized).toContain(usdcIn.logo); + expect(serialized).toContain('"color":"error"'); + expect(serialized).toContain('"color":"success"'); + }); +}); diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx new file mode 100644 index 00000000..69013de9 --- /dev/null +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -0,0 +1,237 @@ +import type { + ComponentOrElement, + GetPreferencesResult, +} from '@metamask/snaps-sdk'; +import { + Box, + Icon, + Image, + Section, + Skeleton, + 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'; +import type { + TransactionScanAssetChange, + TransactionScanEstimatedChanges, +} from '../../../../services/transaction-scan'; +import type { Locale } from '../../../../utils'; +import { i18n } from '../../../../utils'; +import { xlmIcon } from '../../../images'; +import { FetchStatus } from '../../api'; +import { isFetchInProgress } from '../../utils'; + +type EstimatedChangesProps = { + changes: TransactionScanEstimatedChanges | null; + preferences: GetPreferencesResult; + 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. + * + * @param isUnknownValue - True when the amount could not be determined. + * @param isOut - True for an outflow row. + * @returns The SnapText color: neutral for unknown, red for out, green for in. + */ +function resolveRowColor( + isUnknownValue: boolean, + isOut: boolean, +): 'alternative' | 'error' | 'success' { + if (isUnknownValue) { + return 'alternative'; + } + return isOut ? 'error' : 'success'; +} + +/** + * Builds a stable list key for an estimated-change row. + * + * @param asset - The asset change row. + * @param index - The row index within its send/receive group. + * @returns A stable key for JSX list rendering. + */ +function getAssetChangeKey( + asset: TransactionScanAssetChange, + index: number, +): string { + return `${asset.type}-${asset.symbol}-${asset.value ?? 'unknown'}-${index}`; +} + +const EstimatedChangesHeader = ({ + preferences, +}: { + preferences: GetPreferencesResult; +}): ComponentOrElement => { + const t = i18n(preferences.locale as Locale); + + return ( + + + {t('confirmation.estimatedChanges.title')} + + + + + + ); +}; + +const EstimatedChangesSkeleton = ({ + preferences, +}: { + preferences: GetPreferencesResult; +}): ComponentOrElement => { + return ( +
+ + + + + +
+ ); +}; + +/** + * Renders one asset-change row. + * + * @param props - The component props. + * @param props.asset - The asset change to render. + * @returns The row element. + */ +const AssetChangeRow = ({ + asset, +}: { + asset: TransactionScanAssetChange; +}): ComponentOrElement => { + const isOut = asset.type === AssetChangeDirection.Out; + const iconSrc = + asset.logo ?? (asset.symbol === NATIVE_ASSET_SYMBOL ? xlmIcon : null); + + // A null value means the amount is unknown (e.g. a contract token Blockaid + // cannot quantify); render a neutral placeholder rather than a misleading 0. + const isUnknownValue = asset.value === null; + const label = isUnknownValue + ? `– ${asset.symbol}` + : `${isOut ? '-' : '+'}${formatValue(asset.value)} ${asset.symbol}`; + const color = resolveRowColor(isUnknownValue, isOut); + + return ( + + {iconSrc ? ( + + ) : null} + {label} + + ); +}; + +/** + * Renders the signer's estimated balance changes (send / receive breakdown). + * Shows a loading skeleton while the remote scan is in flight, then the best + * available estimate (Blockaid when displayable, otherwise the local fallback). + * + * @param props - The component props. + * @param props.changes - The estimated changes, or null when unavailable. + * @param props.preferences - Snap preferences (used for locale). + * @param props.scanFetchStatus - Latest remote scan fetch status. + * @returns The estimated-changes section. + */ +export const EstimatedChanges = ({ + changes, + preferences, + scanFetchStatus, +}: EstimatedChangesProps): ComponentOrElement => { + const t = i18n(preferences.locale as Locale); + const isFetching = isFetchInProgress(scanFetchStatus); + const isFetched = scanFetchStatus === FetchStatus.Fetched; + const isFetchError = scanFetchStatus === FetchStatus.Error; + // Locally-seeded rows (send flow) are final regardless of the remote scan, so + // keep them visible instead of replacing them with the loading/error chrome + // that the remote scan status would otherwise drive. + const hasSeededRows = (changes?.assets.length ?? 0) > 0; + + if (isFetching && !hasSeededRows) { + return ; + } + + if (isFetchError && !hasSeededRows) { + return ( +
+ + + {t('confirmation.estimatedChanges.notAvailable')} + +
+ ); + } + + const send = + changes?.assets.filter( + (asset) => asset.type === AssetChangeDirection.Out, + ) ?? []; + const receive = + changes?.assets.filter((asset) => asset.type === AssetChangeDirection.In) ?? + []; + const hasChanges = send.length > 0 || receive.length > 0; + + if (isFetched && !hasChanges) { + return ( +
+ + + {t('confirmation.estimatedChanges.noChanges')} + +
+ ); + } + + return ( +
+ + {send.length > 0 ? ( + + + {t('confirmation.estimatedChanges.send')} + + + {send.map((asset, index) => ( + + + + ))} + + + ) : null} + {receive.length > 0 ? ( + + + {t('confirmation.estimatedChanges.receive')} + + + {receive.map((asset, index) => ( + + + + ))} + + + ) : null} +
+ ); +}; diff --git a/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx b/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx index 751ba7e0..2e578c19 100644 --- a/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx +++ b/packages/snap/src/ui/confirmation/components/TransactionAlert.test.tsx @@ -63,7 +63,7 @@ describe('TransactionAlert', () => { expect(getType(component)).toBe('Banner'); expect(getProps(component)).toMatchObject({ severity: 'warning', - title: 'Security validation failed', + title: 'Security check unavailable', }); }); diff --git a/packages/snap/src/ui/confirmation/components/index.ts b/packages/snap/src/ui/confirmation/components/index.ts index 09ced0da..644ed769 100644 --- a/packages/snap/src/ui/confirmation/components/index.ts +++ b/packages/snap/src/ui/confirmation/components/index.ts @@ -1,6 +1,7 @@ export * from './Fee'; export * from './AssetIcon'; export * from './Asset'; +export * from './EstimatedChanges/EstimatedChanges'; export * from './TransactionAlert'; export * from './TransactionValidationAlert'; export * from './ConfirmationAlerts'; diff --git a/packages/snap/src/ui/confirmation/controller.test.tsx b/packages/snap/src/ui/confirmation/controller.test.tsx index 4a2f9769..66befb84 100644 --- a/packages/snap/src/ui/confirmation/controller.test.tsx +++ b/packages/snap/src/ui/confirmation/controller.test.tsx @@ -13,7 +13,7 @@ describe('ConfirmationUXController', () => { interfaceKey: ConfirmationInterfaceKey.SignTransaction, fee: '100', renderContext: {}, - renderOptions: { scanTxn: true }, + renderOptions: { securityScanning: true }, }), ).rejects.toThrow( 'Cannot scan a transaction confirmation without a security scan request.', diff --git a/packages/snap/src/ui/confirmation/controller.tsx b/packages/snap/src/ui/confirmation/controller.tsx index 2b8792ee..c919b29c 100644 --- a/packages/snap/src/ui/confirmation/controller.tsx +++ b/packages/snap/src/ui/confirmation/controller.tsx @@ -1,15 +1,11 @@ import type { DialogResult } from '@metamask/snaps-sdk'; -import { - FetchStatus, - type ConfirmationInterfaceKey, - type ContextWithPrices, -} from './api'; +import { FetchStatus } from './api'; +import type { ConfirmationInterfaceKey, ContextWithPrices } from './api'; import { formatFeeData, formatOrigin, getPreferencesWithFallback, - hasEnabledTransactionScan, } from './utils'; import type { KnownCaip2ChainId } from '../../api'; import { METAMASK_ORIGIN } from '../../constants'; @@ -17,7 +13,10 @@ import type { ChangeTrustOptJsonRpcRequest, ConfirmSendJsonRpcRequest, } from '../../handlers/clientRequest/api'; -import type { SecurityScanRequest } from '../../services/transaction-scan'; +import type { + SecurityScanRequest, + TransactionScanResult, +} from '../../services/transaction-scan'; import type { ILogger, Locale } from '../../utils'; import { createInterface, @@ -38,9 +37,15 @@ import { } from '../../handlers/cronjob/refreshConfirmationContext'; type ConfirmationRenderOptions = { + // Fetch external token spot prices. loadPrice?: boolean; - scanTxn?: boolean; - validateTxn?: boolean; + // Remote Blockaid validation (security alerts). + securityScanning?: boolean; + // Local on-chain simulation: estimated changes + per-cycle re-validation + // (send / change-trust). + localSimulation?: boolean; + // Remote Blockaid simulation for estimated changes (sign-transaction). + remoteSimulation?: boolean; }; /** @@ -62,6 +67,12 @@ type RenderConfirmationDialogCommon = { securityScanRequest?: Omit; transactionValidationRequest?: TransactionValidationRequest; tokenPrices?: ContextWithPrices['tokenPrices']; + /** + * Seeds the initial scan result so locally-derived data (e.g. estimated + * balance changes from on-chain simulation) renders immediately on dialog + * open, before any remote security scan completes. + */ + initialScan?: TransactionScanResult; }; type ConfirmationDialogWithFee = @@ -93,8 +104,9 @@ export class ConfirmationUXController { readonly #defaultRenderOptions: ConfirmationRenderOptions = { loadPrice: false, - scanTxn: false, - validateTxn: false, + securityScanning: false, + localSimulation: false, + remoteSimulation: false, }; constructor({ logger }: { logger: ILogger }) { @@ -133,18 +145,21 @@ export class ConfirmationUXController { ...params.renderOptions, }; - if (renderOptions.scanTxn && params.securityScanRequest === undefined) { + if ( + (renderOptions.securityScanning || renderOptions.remoteSimulation) && + params.securityScanRequest === undefined + ) { throw new Error( 'Cannot scan a transaction confirmation without a security scan request.', ); } if ( - renderOptions.validateTxn && + renderOptions.localSimulation && params.transactionValidationRequest === undefined ) { throw new Error( - 'Cannot re-validate a transaction confirmation without a transaction validation request.', + 'Cannot run local simulation on a transaction confirmation without a transaction validation request.', ); } @@ -162,7 +177,7 @@ export class ConfirmationUXController { }; /** - * Enazble Price Fetching if: + * Enable Price Fetching if: * - Pricing Loading is enabled * - External Pricing Preferences is enabled * - Token Prices mapping is provided @@ -172,13 +187,23 @@ export class ConfirmationUXController { preferences.useExternalPricingData && tokenPrices !== undefined; + // Remote Blockaid scan runs when either remote source is both requested by + // the flow and enabled by the user: validation (security alerts) or + // simulation (on-chain action simulation). + const wantsRemoteValidation = + Boolean(renderOptions.securityScanning) && + preferences.useSecurityAlerts; + const wantsRemoteSimulation = + Boolean(renderOptions.remoteSimulation) && + preferences.simulateOnChainActions; const enableSecurityScan = - renderOptions.scanTxn && - hasEnabledTransactionScan(preferences) && - params.securityScanRequest !== undefined; + params.securityScanRequest !== undefined && + (wantsRemoteValidation || wantsRemoteSimulation); - const enableTransactionValidation = - renderOptions.validateTxn && + // Local simulation (estimated changes + per-cycle submittability + // re-validation) is driven by the Transaction refresher. + const enableLocalSimulation = + renderOptions.localSimulation && params.transactionValidationRequest !== undefined; const defaultContext = { @@ -196,7 +221,7 @@ export class ConfirmationUXController { currency: preferences.currency, scope, feeData: fee ? formatFeeData(scope, fee) : {}, - scan: null, + scan: params.initialScan ?? null, scanFetchStatus: enableSecurityScan ? FetchStatus.Fetching : FetchStatus.Fetched, @@ -207,12 +232,16 @@ export class ConfirmationUXController { origin, scope, }, + // Persist the flow's scan intents so the refresher picks Blockaid + // options without keying off the interface key. + securityScanning: Boolean(renderOptions.securityScanning), + remoteSimulation: Boolean(renderOptions.remoteSimulation), } : {}), // Optimistic: tx was validated at build time, so keep confirm enabled; the // refresher flips to Error if it later drifts (submission rejects invalid txs too). // TODO(follow-up): re-validate synchronously right before signing. - ...(renderOptions.validateTxn && params.transactionValidationRequest + ...(enableLocalSimulation && params.transactionValidationRequest ? { transactionsFetchStatus: FetchStatus.Fetched, accountId: params.transactionValidationRequest.accountId, @@ -256,7 +285,7 @@ export class ConfirmationUXController { if (enableSecurityScan) { refresherKeys.push(ConfirmationContextRefresherKey.Scan); } - if (enableTransactionValidation) { + if (enableLocalSimulation) { refresherKeys.push(ConfirmationContextRefresherKey.Transaction); } diff --git a/packages/snap/src/ui/confirmation/utils.test.ts b/packages/snap/src/ui/confirmation/utils.test.ts index 05f02c38..ed0e0e98 100644 --- a/packages/snap/src/ui/confirmation/utils.test.ts +++ b/packages/snap/src/ui/confirmation/utils.test.ts @@ -5,6 +5,7 @@ import { import { FetchStatus } from './api'; import { ConfirmationBanner, + isFetchInProgress, isLocalTransactionValidationFailed, isRemoteTransactionScanLoading, requiresMaliciousAcknowledgement, @@ -23,6 +24,22 @@ const warningScan = { }; describe('confirmation utils', () => { + describe('isFetchInProgress', () => { + it.each([FetchStatus.Initial, FetchStatus.Fetching])( + 'returns true for %s', + (status) => { + expect(isFetchInProgress(status)).toBe(true); + }, + ); + + it.each([FetchStatus.Fetched, FetchStatus.Error])( + 'returns false for %s', + (status) => { + expect(isFetchInProgress(status)).toBe(false); + }, + ); + }); + describe('isRemoteTransactionScanLoading', () => { it('disables confirm while scan is fetching', () => { expect( diff --git a/packages/snap/src/ui/confirmation/utils.ts b/packages/snap/src/ui/confirmation/utils.ts index 55c641d2..c411d2ad 100644 --- a/packages/snap/src/ui/confirmation/utils.ts +++ b/packages/snap/src/ui/confirmation/utils.ts @@ -168,6 +168,17 @@ export function formatFeeData( }; } +/** + * Returns true while a fetch has not settled yet — either it hasn't started + * (`Initial`) or it is actively in flight (`Fetching`). + * + * @param status - The fetch status to inspect. + * @returns Whether the status represents an in-progress fetch. + */ +export function isFetchInProgress(status: FetchStatus): boolean { + return status === FetchStatus.Initial || status === FetchStatus.Fetching; +} + /** * Determines whether the remote (Blockaid) transaction scan is still loading. * diff --git a/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx b/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx index 2d0d9301..2b669e4a 100644 --- a/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx +++ b/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx @@ -10,14 +10,11 @@ import { Text as SnapText, Tooltip, } from '@metamask/snaps-sdk/jsx'; -import { parseCaipAssetType } from '@metamask/utils'; import { ConfirmSendTransactionFormNames } from './events'; import type { StellarKeyringAccount } from '../../../../services/account'; -import type { StellarAssetMetadata } from '../../../../services/asset-metadata'; import type { Locale } from '../../../../utils'; -import { isSlip44Id, i18n } from '../../../../utils'; -import { xlmIcon } from '../../../images'; +import { i18n } from '../../../../utils'; import type { ContextWithPrices, ConfirmationBaseProps, @@ -25,17 +22,15 @@ import type { } from '../../api'; import { FetchStatus } from '../../api'; import { - Asset, ConfirmationAlerts, ConfirmationFooter, + EstimatedChanges, FeeRow, } from '../../components'; import { NetworkRow } from '../../components/Network'; import { getAccountExplorerUrl, getAccountName, - getClassicAssetExplorerUrl, - getSepAssetExplorerUrl, requiresMaliciousAcknowledgement, shouldDisableConfirmation, } from '../../utils'; @@ -43,19 +38,14 @@ import { export type ConfirmSendTransactionProps = ConfirmationBaseProps & ContextWithPrices & { account: StellarKeyringAccount; - assetMetadata: StellarAssetMetadata; feeData: FeeData; - } & { toAddress: string; - amount: string; }; export const ConfirmSendTransaction = ({ account, toAddress, - amount, scope, - assetMetadata, locale, networkImage, feeData, @@ -69,21 +59,10 @@ export const ConfirmSendTransaction = ({ }: ConfirmSendTransactionProps): ComponentOrElement => { const t = i18n(locale); const { address } = account; - const { assetId, symbol } = assetMetadata; const shouldDisableConfirmButton = shouldDisableConfirmation({ scanFetchStatus, transactionsFetchStatus, }); - const parsedAsset = parseCaipAssetType(assetId); - let assetLink: string | undefined; - if (!isSlip44Id(assetId)) { - assetLink = - parsedAsset.assetNamespace === 'sep41' - ? getSepAssetExplorerUrl(parsedAsset.assetReference) - : getClassicAssetExplorerUrl(parsedAsset.assetReference); - } - const assetIconUrl = isSlip44Id(assetId) ? xlmIcon : assetMetadata.iconUrl; - const assetPrice = tokenPrices?.[assetId] ?? null; return ( @@ -100,6 +79,16 @@ export const ConfirmSendTransaction = ({ {null} + {/* Always shown: the rows are seeded locally from the known send + amount, so this is the only place the user sees what they're + approving. Unlike sign-transaction (remote simulation, gated by the + simulate-on-chain-actions preference), it must not be hidden. */} + +
{origin ? ( @@ -142,23 +131,6 @@ export const ConfirmSendTransaction = ({ /> - - - {t('confirmation.estimatedChanges.send')} - - - {/* Network */} {null} + {preferences.simulateOnChainActions ? ( + + ) : null} +
{origin ? (