From 67da18e987b5fffa0ac16917aab0d0e9dea59ebf Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Tue, 16 Jun 2026 17:02:31 +0200 Subject: [PATCH 01/13] feat: local-simulation fund flow on sign-transaction confirmation --- packages/snap/snap.manifest.json | 2 +- packages/snap/src/context.ts | 1 + .../scanRefresher.test.ts | 89 +++++++++- .../scanRefresher.ts | 55 ++++++- .../src/handlers/keyring/signTransaction.ts | 60 +++++++ .../transaction/TransactionService.ts | 68 +++++++- .../transaction/TransactionSimulator.ts | 30 ++++ .../__mocks__/transaction.fixtures.ts | 3 + .../snap/src/services/transaction/index.ts | 1 + .../mapSimulationToEstimatedChanges.test.ts | 127 ++++++++++++++ .../mapSimulationToEstimatedChanges.ts | 155 ++++++++++++++++++ .../EstimatedChanges/EstimatedChanges.tsx | 114 +++++++++++++ .../src/ui/confirmation/components/index.ts | 1 + .../snap/src/ui/confirmation/controller.tsx | 13 +- .../ConfirmSignTransaction.tsx | 6 + 15 files changed, 705 insertions(+), 20 deletions(-) create mode 100644 packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts create mode 100644 packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts create mode 100644 packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index d282f7db..e2f3cbf5 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": "0bXKXi1JKlz9Tl+t7sHnw2HInhPpNtkrTIrUdRD/n4I=", + "shasum": "gdcX1HR0NKNSQ2pu4VN0voRBoHNy6d5m4lWG+wxxZqk=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/context.ts b/packages/snap/src/context.ts index bd9c5d7f..e7ffc078 100644 --- a/packages/snap/src/context.ts +++ b/packages/snap/src/context.ts @@ -114,6 +114,7 @@ const transactionService = new TransactionService({ networkService, transactionBuilder, accountService, + assetMetadataService, }); const priceService = new PriceService({ diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts index 586e1fca..6c1a480e 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts @@ -59,17 +59,16 @@ describe('ConfirmationScanRefresher', () => { }); } - it('returns fetched scan data and reschedules on success', async () => { + it('requests validation only and never remote simulation', async () => { const { refresher, transactionScanService } = setup(); const result = await refresher.refresh(createScanContext()); + // Remote simulation is intentionally omitted; estimated changes come from + // the local on-chain simulation instead. expect(transactionScanService.scanTransaction).toHaveBeenCalledWith({ ...securityScanRequest, - options: [ - TransactionScanOption.Simulation, - TransactionScanOption.Validation, - ], + options: [TransactionScanOption.Validation], }); expect(result).toStrictEqual({ result: { @@ -80,9 +79,59 @@ describe('ConfirmationScanRefresher', () => { }); }); - it('returns error status when scan returns null', async () => { + it('preserves locally-derived estimated changes over the Blockaid result', async () => { + const { refresher } = setup(); + const localEstimatedChanges = { + assets: [ + { + type: 'out' as const, + value: 12.5, + 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, + estimatedChanges: localEstimatedChanges, + }, + scanFetchStatus: FetchStatus.Fetched, + }, + reschedule: true, + }); + }); + + it('returns error status preserving estimated changes when scan returns null', async () => { const { refresher, transactionScanService } = setup(); transactionScanService.scanTransaction.mockResolvedValueOnce(null); + const localEstimatedChanges = { + assets: [ + { + type: 'out' as const, + value: 1, + price: null, + symbol: 'XLM', + name: 'Stellar Lumens', + logo: null, + }, + ], + }; const result = await refresher.refresh( createScanContext({ @@ -90,18 +139,44 @@ 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('does not fetch when only simulateOnChainActions is enabled', () => { + const { refresher } = setup(); + + expect( + refresher.shouldFetch( + createScanContext({ + 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 7854ef8d..2745aa66 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts @@ -6,7 +6,11 @@ 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 { @@ -21,7 +25,11 @@ type SecurityScanContext = ConfirmationDataContext & ContextWithSecurityScan; type SecurityScanPreferences = ContextWithSecurityScan['preferences']; /** - * Refreshes Blockaid security scan / simulation results in the confirmation dialog context. + * Refreshes the Blockaid security validation in the confirmation dialog context. + * + * Estimated balance changes are owned by the local on-chain simulation (seeded + * at dialog open), so this refresher only requests Blockaid `Validation` and + * never overwrites the locally-derived `estimatedChanges`. */ export class ConfirmationScanRefresher implements IConfirmationContextRefresher { readonly key = ConfirmationContextRefresherKey.Scan; @@ -65,7 +73,9 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher const optionsEnabled = this.#getScanOptions(scanCtx.preferences).length > 0; return { result: { - scan: null, + // Preserve the locally-derived estimated changes; only the scan fetch + // status reflects the (validation) failure. + scan: scanCtx.scan ?? null, scanFetchStatus: optionsEnabled ? FetchStatus.Error : FetchStatus.Fetched, @@ -82,6 +92,9 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher SecurityScanContext['securityScanRequest'] >; const options = this.#getScanOptions(scanCtx.preferences); + const localEstimatedChanges = scanCtx.scan?.estimatedChanges ?? { + assets: [], + }; try { const scan = await this.#transactionScanService.scanTransaction({ @@ -91,7 +104,9 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher return { result: { - scan, + // Blockaid only contributes validation/error; the locally-derived + // estimated changes always take precedence. + scan: this.#scanWithLocalChanges(scan, localEstimatedChanges), scanFetchStatus: scan ? FetchStatus.Fetched : FetchStatus.Error, }, reschedule: scan !== null, @@ -100,7 +115,7 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher this.#logger.error('Error refreshing confirmation security scan:', error); return { result: { - scan: null, + scan: this.#scanWithLocalChanges(null, localEstimatedChanges), scanFetchStatus: FetchStatus.Error, }, reschedule: false, @@ -112,15 +127,37 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher return ContextWithSecurityScanStruct.is(ctx); } + /** + * Merges a Blockaid scan result with the locally-derived estimated changes, + * preserving the fund-flow breakdown regardless of the remote scan outcome. + * + * @param scan - The Blockaid scan result, or null when none was returned. + * @param localEstimatedChanges - The estimated changes from local simulation. + * @returns A scan result carrying the local estimated changes. + */ + #scanWithLocalChanges( + scan: TransactionScanResult | null, + localEstimatedChanges: TransactionScanEstimatedChanges, + ): TransactionScanResult { + if (scan) { + return { ...scan, estimatedChanges: localEstimatedChanges }; + } + return { + status: 'ERROR', + estimatedChanges: localEstimatedChanges, + validation: null, + error: null, + }; + } + #getScanOptions( preferences: SecurityScanPreferences, ): TransactionScanOption[] { const options: TransactionScanOption[] = []; - if (preferences.simulateOnChainActions) { - options.push(TransactionScanOption.Simulation); - } - + // Remote simulation is intentionally not requested: estimated balance + // changes are derived from the local on-chain simulation. Blockaid is used + // for security validation only. if (preferences.useSecurityAlerts) { options.push(TransactionScanOption.Validation); } diff --git a/packages/snap/src/handlers/keyring/signTransaction.ts b/packages/snap/src/handlers/keyring/signTransaction.ts index ecce4e69..62b91834 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.ts @@ -6,6 +6,7 @@ import { SignTransactionResponseStruct, } from './api'; import type { AccountResolver } from '../accountResolver'; +import { ResolveAccountSource } from '../accountResolver'; import { BaseSep43KeyringHandler } from './base'; import type { Sep43Error } from './exceptions'; import type { StellarKeyringAccount } from '../../services/account'; @@ -17,6 +18,7 @@ import { assertTransactionTimeBound, collectTransactionAssetCaipIds, } from '../../services/transaction/utils'; +import type { TransactionScanEstimatedChanges } from '../../services/transaction-scan'; import type { Wallet } from '../../services/wallet'; import type { ContextWithPrices } from '../../ui/confirmation/api'; import { ConfirmationInterfaceKey } from '../../ui/confirmation/api'; @@ -41,6 +43,8 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< readonly #confirmationUIController: ConfirmationUXController; + readonly #accountResolver: AccountResolver; + constructor({ logger, accountResolver, @@ -61,6 +65,7 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< }); this.#transactionService = transactionService; this.#confirmationUIController = confirmationUIController; + this.#accountResolver = accountResolver; } protected async execute( @@ -132,6 +137,15 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< ), ) as ContextWithPrices['tokenPrices']; + // Estimated balance changes come from local on-chain simulation (not the + // remote security scan), so they render immediately on dialog open. The + // remote scan only contributes security validation. + const estimatedChanges = await this.#deriveEstimatedChanges( + request, + transaction, + account, + ); + return ( (await this.#confirmationUIController.renderConfirmationDialog({ scope: request.scope, @@ -147,8 +161,54 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< accountAddress: account.address, transaction: transaction.getRaw().toXDR(), }, + initialScan: { + status: 'SUCCESS', + estimatedChanges, + validation: null, + error: null, + }, tokenPrices, })) === true ); } + + /** + * Best-effort local simulation of the signer's balance changes. Resolves the + * on-chain account and runs the local simulator; any failure (account not + * activated, unsupported/Soroban transaction, network error) resolves to an + * empty result so the confirmation simply hides the estimated-changes section. + * + * @param request - The validated sign-transaction request. + * @param transaction - The transaction with fee already applied. + * @param account - The resolved keyring account. + * @returns The estimated changes, or `{ assets: [] }` when unavailable. + */ + async #deriveEstimatedChanges( + request: SignTransactionRequest, + transaction: Transaction, + account: StellarKeyringAccount, + ): Promise { + try { + const { onChainAccount } = await this.#accountResolver.resolveAccount({ + accountId: account.id, + scope: request.scope, + options: { + onChainAccount: { load: true, source: ResolveAccountSource.OnChain }, + wallet: false, + }, + }); + + return await this.#transactionService.deriveEstimatedChanges({ + transaction, + onChainAccount, + signerAddress: account.address, + }); + } catch (error) { + this.logger.logErrorWithDetails( + 'Failed to derive estimated changes for sign transaction', + error, + ); + return { assets: [] }; + } + } } diff --git a/packages/snap/src/services/transaction/TransactionService.ts b/packages/snap/src/services/transaction/TransactionService.ts index 002e8ca5..ea729686 100644 --- a/packages/snap/src/services/transaction/TransactionService.ts +++ b/packages/snap/src/services/transaction/TransactionService.ts @@ -8,6 +8,7 @@ import { groupBy } from 'lodash'; import { InsufficientBalanceException } from './exceptions'; import type { KeyringTransactionRequest } from './KeyringTransactionBuilder'; import { KeyringTransactionBuilder } from './KeyringTransactionBuilder'; +import { mapSimulationToEstimatedChanges } from './mapSimulationToEstimatedChanges'; import { Transaction } from './Transaction'; import type { TransactionBuilder } from './TransactionBuilder'; import { TransactionMapper } from './TransactionMapper'; @@ -30,7 +31,10 @@ import { getSnapProvider, isSep41Id, isSlip44Id } from '../../utils'; import type { ILogger } from '../../utils/logger'; import { createPrefixedLogger } from '../../utils/logger'; import type { AccountService } from '../account'; -import type { StellarAssetMetadata } from '../asset-metadata'; +import type { + AssetMetadataService, + StellarAssetMetadata, +} from '../asset-metadata'; import type { NetworkService } from '../network'; import { AccountNotActivatedException, @@ -38,6 +42,7 @@ import { } from '../network/exceptions'; import type { OnChainAccount } from '../on-chain-account/OnChainAccount'; import type { ActivatedAccountPair } from '../sync/api'; +import type { TransactionScanEstimatedChanges } from '../transaction-scan'; import type { Wallet } from '../wallet'; export class TransactionService { @@ -53,23 +58,28 @@ export class TransactionService { readonly #transactionSynchronizeService: TransactionSynchronizeService; + readonly #assetMetadataService: AssetMetadataService; + constructor({ logger, transactionRepository, networkService, transactionBuilder, accountService, + assetMetadataService, }: { logger: ILogger; transactionRepository: TransactionRepository; networkService: NetworkService; transactionBuilder: TransactionBuilder; accountService: AccountService; + assetMetadataService: AssetMetadataService; }) { this.#logger = createPrefixedLogger(logger, '[๐Ÿงพ TransactionService]'); this.#transactionRepository = transactionRepository; this.#networkService = networkService; this.#transactionBuilder = transactionBuilder; + this.#assetMetadataService = assetMetadataService; this.#keyringTransactionBuilder = new KeyringTransactionBuilder(); const transactionMapper = new TransactionMapper({ keyringTransactionBuilder: this.#keyringTransactionBuilder, @@ -522,6 +532,62 @@ export class TransactionService { simulator.simulate(transaction, onChainAccount, options); } + /** + * Derives the signer's estimated balance changes from a local on-chain + * simulation, mapped into the {@link TransactionScanEstimatedChanges} shape the + * confirmation UI renders. Used to seed the `sign-transaction` confirmation + * fund-flow breakdown without relying on a remote (Blockaid) simulation. + * + * The network fee is excluded from the per-asset rows (the diff baselines on + * the post-fee simulation snapshot); it is surfaced separately as the fee row. + * + * Best-effort: any failure (unsupported operation, unknown destination + * account, Soroban invoke producing no modeled deltas, etc.) resolves to an + * empty result so the UI simply hides the section. + * + * @param params - The parameters. + * @param params.transaction - The transaction with fee already applied. + * @param params.onChainAccount - The loaded signing account. + * @param params.signerAddress - The Stellar address whose changes are surfaced. + * @returns The estimated changes, or `{ assets: [] }` when they cannot be derived. + */ + async deriveEstimatedChanges(params: { + transaction: Transaction; + onChainAccount: OnChainAccount; + signerAddress: string; + }): Promise { + const { transaction, onChainAccount, signerAddress } = params; + try { + const preloadedAccounts = await this.#getPreloadedAccounts( + transaction, + onChainAccount, + ); + + const simulator = new TransactionSimulator(); + const { initialState, finalState } = simulator.simulateEndpoints( + transaction, + onChainAccount, + { preloadedAccounts }, + ); + + const assets = await mapSimulationToEstimatedChanges({ + initialState, + finalState, + signerAddress, + scope: transaction.scope, + assetMetadataService: this.#assetMetadataService, + }); + + return { assets }; + } catch (error) { + this.#logger.logErrorWithDetails( + 'Failed to derive estimated balance changes', + error, + ); + return { assets: [] }; + } + } + /** * Submits a signed transaction. * When the transaction fails with `txBadSeq`, reloads the account sequence, rebuilds, re-signs once, and retries diff --git a/packages/snap/src/services/transaction/TransactionSimulator.ts b/packages/snap/src/services/transaction/TransactionSimulator.ts index 6b59c06e..ffa7527f 100644 --- a/packages/snap/src/services/transaction/TransactionSimulator.ts +++ b/packages/snap/src/services/transaction/TransactionSimulator.ts @@ -116,6 +116,36 @@ export class TransactionSimulator { }); } + /** + * Runs {@link simulate} and returns only the simulation endpoints used to + * derive estimated balance changes for the confirmation UI. + * + * `initialState` is the post-fee snapshot (fee already deducted from the fee + * source) and `finalState` is the snapshot after all operations have been + * applied. Diffing these two excludes the network fee from the per-asset + * rows (it is surfaced separately as its own fee row), matching the + * EVM/Tron confirmation experience. + * + * @param transaction - Wrapped Stellar transaction. + * @param account - Loaded signing account (Horizon-shaped raw for balances). + * @param options - Optional `expectedOPTypes` and `preloadedAccounts`. + * @returns The post-fee initial state and the final state after all operations. + * @throws {TransactionValidationException} When simulation produces no states. + */ + simulateEndpoints( + transaction: Transaction, + account: OnChainAccount, + options?: TransactionSimulatorOptions, + ): { initialState: SimulationState; finalState: SimulationState } { + const states = this.simulate(transaction, account, options); + const initialState = states[0]; + const finalState = states[states.length - 1]; + if (initialState === undefined || finalState === undefined) { + throw new TransactionValidationException('Simulation produced no states'); + } + return { initialState, finalState }; + } + #run(params: { operations: SupportedOPType[]; transaction: Transaction; diff --git a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts index 30c5cbe3..0d72140a 100644 --- a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts +++ b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts @@ -16,6 +16,7 @@ import type { KnownCaip19AssetIdOrSlip44Id } from '../../../api'; import { KnownCaip2ChainId } from '../../../api'; import { getSlip44AssetId, logger } from '../../../utils'; import { mockAccountService } from '../../account/__mocks__/account.fixtures'; +import { createMockAssetMetadataService } from '../../asset-metadata/__mocks__/assets.fixtures'; import { createMemoryCache } from '../../cache/__mocks__/cache.fixtures'; import { NetworkService } from '../../network'; import { State } from '../../state/State'; @@ -30,6 +31,7 @@ export const createMockTransactionService = () => { const networkService = new NetworkService({ logger, cache }); const transactionBuilder = new TransactionBuilder({ logger }); const { accountService } = mockAccountService(); + const { service: assetMetadataService } = createMockAssetMetadataService(); const transactionService = new TransactionService({ logger, transactionRepository: new TransactionRepository( @@ -44,6 +46,7 @@ export const createMockTransactionService = () => { networkService, transactionBuilder, accountService, + assetMetadataService, }); const transactionRepositorySaveSpy = jest.spyOn( diff --git a/packages/snap/src/services/transaction/index.ts b/packages/snap/src/services/transaction/index.ts index f1d557c0..12535efc 100644 --- a/packages/snap/src/services/transaction/index.ts +++ b/packages/snap/src/services/transaction/index.ts @@ -6,4 +6,5 @@ export * from './TransactionRepository'; export * from './TransactionService'; export * from './TransactionSimulator'; export * from './KeyringTransactionBuilder'; +export * from './mapSimulationToEstimatedChanges'; export * from './api'; diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts new file mode 100644 index 00000000..7f4e1982 --- /dev/null +++ b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts @@ -0,0 +1,127 @@ +import { BigNumber } from 'bignumber.js'; + +import { mapSimulationToEstimatedChanges } from './mapSimulationToEstimatedChanges'; +import type { AccountState, SimulationState } from './simulation'; +import { KnownCaip2ChainId } from '../../api'; +import { toCaip19ClassicAssetId } from '../../utils'; +import type { AssetMetadataService } from '../asset-metadata'; + +describe('mapSimulationToEstimatedChanges', () => { + const scope = KnownCaip2ChainId.Mainnet; + const signerAddress = + 'GDPMFLKUGASUTWBN2XGYYKD27QGHCYH4BUFUTER4L23INYQ4JHDWFOIE'; + const usdcIssuer = 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN'; + + const assetMetadataService = { + resolve: jest.fn(), + } as unknown as AssetMetadataService; + + function buildAccountState(overrides: Partial): AccountState { + return { + nativeRawBalance: new BigNumber(0), + subentryCount: 0, + numSponsoring: 0, + numSponsored: 0, + requiresMemo: false, + trustlines: new Map(), + sep41Balances: new Map(), + ...overrides, + }; + } + + function buildState(accountState: AccountState): SimulationState { + return { accounts: new Map([[signerAddress, accountState]]) }; + } + + it('maps a native XLM outflow excluding the fee (post-fee baseline)', async () => { + // Both snapshots are post-fee, so the only diff is the payment amount. + const result = await mapSimulationToEstimatedChanges({ + initialState: buildState( + buildAccountState({ nativeRawBalance: new BigNumber('1000000000') }), + ), + finalState: buildState( + buildAccountState({ nativeRawBalance: new BigNumber('900000000') }), + ), + signerAddress, + scope, + assetMetadataService, + }); + + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ + type: 'out', + value: 10, + price: null, + symbol: 'XLM', + }); + expect(assetMetadataService.resolve).not.toHaveBeenCalled(); + }); + + it('maps a classic asset payment from the signer trustline', async () => { + const usdc = toCaip19ClassicAssetId(scope, 'USDC', usdcIssuer); + const trustline = { + limit: new BigNumber('1000000000000'), + authorized: true, + sponsored: false, + }; + + const result = await mapSimulationToEstimatedChanges({ + initialState: buildState( + buildAccountState({ + trustlines: new Map([ + [usdc, { ...trustline, balance: new BigNumber('5000000') }], + ]), + }), + ), + finalState: buildState( + buildAccountState({ + trustlines: new Map([ + [usdc, { ...trustline, balance: new BigNumber('3000000') }], + ]), + }), + ), + signerAddress, + scope, + assetMetadataService, + }); + + expect(result).toStrictEqual([ + { + type: 'out', + value: 0.2, + price: null, + symbol: 'USDC', + name: 'USDC', + logo: expect.any(String), + }, + ]); + }); + + it('returns an empty array when there are no balance changes', async () => { + const accountState = buildAccountState({ + nativeRawBalance: new BigNumber('1000000000'), + }); + + const result = await mapSimulationToEstimatedChanges({ + initialState: buildState(accountState), + finalState: buildState(accountState), + signerAddress, + scope, + assetMetadataService, + }); + + expect(result).toStrictEqual([]); + }); + + it('returns an empty array when the signer is absent from a snapshot', async () => { + const result = await mapSimulationToEstimatedChanges({ + initialState: { accounts: new Map() }, + finalState: { accounts: new Map() }, + signerAddress, + scope, + assetMetadataService, + }); + + expect(result).toStrictEqual([]); + }); +}); diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts new file mode 100644 index 00000000..31acf2ef --- /dev/null +++ b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts @@ -0,0 +1,155 @@ +import { parseCaipAssetType } from '@metamask/utils'; +import { BigNumber } from 'bignumber.js'; + +import type { SimulationState } from './simulation'; +import type { + KnownCaip19ClassicAssetId, + KnownCaip19Sep41AssetId, + KnownCaip2ChainId, +} from '../../api'; +import { STELLAR_DECIMAL_PLACES } from '../../constants'; +import { parseClassicAssetCodeIssuer } from '../../utils'; +import { normalizeAmount } from '../../utils/currency'; +import type { AssetMetadataService } from '../asset-metadata'; +import { getIconUrl, getNativeAssetMetadata } from '../asset-metadata/utils'; +import type { TransactionScanAssetChange } from '../transaction-scan'; + +/** + * Builds a single estimated-change row from a signed balance delta. + * + * @param params - Row inputs. + * @param params.delta - Signed balance delta in smallest units (negative = outflow). + * @param params.decimals - Asset decimals used to normalize the raw delta. + * @param params.symbol - Display ticker. + * @param params.name - Display name. + * @param params.logo - Icon URL, or null when unknown. + * @returns A {@link TransactionScanAssetChange} with a human-readable value. + */ +function buildChange(params: { + delta: BigNumber; + decimals: number; + symbol: string; + name: string; + logo: string | null; +}): TransactionScanAssetChange { + const { delta, decimals, symbol, name, logo } = params; + return { + type: delta.isNegative() ? 'out' : 'in', + value: normalizeAmount(delta.abs(), decimals).toNumber(), + price: null, + symbol, + name, + logo, + }; +} + +/** + * Diffs the signer's account between two local simulation snapshots and maps the + * non-zero balance deltas into the {@link TransactionScanAssetChange} shape the + * confirmation UI renders. + * + * Native XLM and classic (trustline) assets are diffed synchronously; SEP-41 + * contract tokens resolve their metadata via {@link AssetMetadataService}. The + * caller is expected to pass the post-fee `initialState` so the network fee is + * excluded from the resulting rows. + * + * @param params - Mapping inputs. + * @param params.initialState - Post-fee simulation snapshot (baseline). + * @param params.finalState - Snapshot after all operations are applied. + * @param params.signerAddress - Stellar address whose balance changes are surfaced. + * @param params.scope - CAIP-2 chain of the transaction. + * @param params.assetMetadataService - Resolver for SEP-41 token metadata. + * @returns The non-zero asset changes for the signer, or an empty array when the + * signer is not present in both snapshots. + */ +export async function mapSimulationToEstimatedChanges(params: { + initialState: SimulationState; + finalState: SimulationState; + signerAddress: string; + scope: KnownCaip2ChainId; + assetMetadataService: AssetMetadataService; +}): Promise { + const { + initialState, + finalState, + signerAddress, + scope, + assetMetadataService, + } = params; + + const initialAccount = initialState.accounts.get(signerAddress); + const finalAccount = finalState.accounts.get(signerAddress); + if (initialAccount === undefined || finalAccount === undefined) { + return []; + } + + const changes: TransactionScanAssetChange[] = []; + + // Native XLM. + const nativeDelta = finalAccount.nativeRawBalance.minus( + initialAccount.nativeRawBalance, + ); + if (!nativeDelta.isZero()) { + const meta = getNativeAssetMetadata(scope); + changes.push( + buildChange({ + delta: nativeDelta, + decimals: meta.units[0].decimals, + symbol: meta.symbol, + name: meta.name ?? meta.symbol, + logo: meta.iconUrl ?? null, + }), + ); + } + + // Classic (trustline) assets. + const classicAssetIds = new Set([ + ...initialAccount.trustlines.keys(), + ...finalAccount.trustlines.keys(), + ]); + for (const assetId of classicAssetIds) { + const before = initialAccount.trustlines.get(assetId)?.balance ?? null; + const after = finalAccount.trustlines.get(assetId)?.balance ?? null; + const delta = (after ?? new BigNumber(0)).minus(before ?? new BigNumber(0)); + if (delta.isZero()) { + continue; + } + const { assetReference } = parseCaipAssetType(assetId); + const { assetCode } = parseClassicAssetCodeIssuer(assetReference); + changes.push( + buildChange({ + delta, + decimals: STELLAR_DECIMAL_PLACES, + symbol: assetCode, + name: assetCode, + logo: getIconUrl(assetId), + }), + ); + } + + // SEP-41 contract tokens. + const sep41AssetIds = new Set([ + ...initialAccount.sep41Balances.keys(), + ...finalAccount.sep41Balances.keys(), + ]); + for (const assetId of sep41AssetIds) { + const before = initialAccount.sep41Balances.get(assetId) ?? null; + const after = finalAccount.sep41Balances.get(assetId) ?? null; + const delta = (after ?? new BigNumber(0)).minus(before ?? new BigNumber(0)); + if (delta.isZero()) { + continue; + } + const meta = await assetMetadataService.resolve(assetId); + changes.push( + buildChange({ + delta, + decimals: meta.units[0].decimals, + symbol: meta.symbol, + name: meta.name ?? meta.symbol, + logo: meta.iconUrl ?? null, + }), + ); + } + + return changes; +} 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..f5f4d165 --- /dev/null +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -0,0 +1,114 @@ +import type { + ComponentOrElement, + GetPreferencesResult, +} from '@metamask/snaps-sdk'; +import { + Box, + Icon, + Image, + Section, + Text as SnapText, + Tooltip, +} from '@metamask/snaps-sdk/jsx'; +import { BigNumber } from 'bignumber.js'; + +import type { + TransactionScanAssetChange, + TransactionScanEstimatedChanges, +} from '../../../../services/transaction-scan'; +import type { Locale } from '../../../../utils'; +import { i18n } from '../../../../utils'; + +type EstimatedChangesProps = { + changes: TransactionScanEstimatedChanges | null; + preferences: GetPreferencesResult; +}; + +/** + * 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(); +} + +const AssetChangeRow = ({ + asset, +}: { + asset: TransactionScanAssetChange; +}): ComponentOrElement => { + const isOut = asset.type === 'out'; + return ( + + {asset.logo ? ( + + ) : null} + + {`${isOut ? '-' : '+'}${formatValue(asset.value)} ${asset.symbol}`} + + + ); +}; + +/** + * Renders the signer's estimated balance changes (send / receive breakdown) + * derived from the local on-chain simulation. Hidden entirely when there are no + * modeled changes (for example Soroban invokes or unsupported transactions). + * + * @param props - The component props. + * @param props.changes - The estimated changes, or null when unavailable. + * @param props.preferences - Snap preferences (used for locale). + * @returns The estimated-changes section, or null when there is nothing to show. + */ +export const EstimatedChanges = ({ + changes, + preferences, +}: EstimatedChangesProps): ComponentOrElement | null => { + const assets = changes?.assets ?? []; + if (assets.length === 0) { + return null; + } + + const t = i18n(preferences.locale as Locale); + const send = assets.filter((asset) => asset.type === 'out'); + const receive = assets.filter((asset) => asset.type === 'in'); + + return ( +
+ + + {t('confirmation.estimatedChanges.title')} + + + + + + {send.length > 0 ? ( + + + {t('confirmation.estimatedChanges.send')} + + + {send.map((asset) => ( + + ))} + + + ) : null} + {receive.length > 0 ? ( + + + {t('confirmation.estimatedChanges.receive')} + + + {receive.map((asset) => ( + + ))} + + + ) : null} +
+ ); +}; 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.tsx b/packages/snap/src/ui/confirmation/controller.tsx index c608b34d..1f16d5e3 100644 --- a/packages/snap/src/ui/confirmation/controller.tsx +++ b/packages/snap/src/ui/confirmation/controller.tsx @@ -17,7 +17,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, @@ -62,6 +65,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 = @@ -196,7 +205,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, diff --git a/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx index 3a49be8c..33c365be 100644 --- a/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx +++ b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx @@ -25,6 +25,7 @@ import type { ConfirmationBaseProps, FeeData } from '../../api'; import { FetchStatus } from '../../api'; import { Asset } from '../../components/Asset'; import { ConfirmationFooter } from '../../components/ConfirmationFooter'; +import { EstimatedChanges } from '../../components/EstimatedChanges/EstimatedChanges'; import { FeeRow } from '../../components/Fee'; import { TransactionAlert } from '../../components/TransactionAlert'; import { @@ -250,6 +251,11 @@ export const ConfirmSignTransaction = ({ ))} + +
{readableTransaction.operations.map((operationJson, index) => ( Date: Wed, 17 Jun 2026 13:30:42 +0200 Subject: [PATCH 02/13] fix(confirmation): only schedule security scan when useSecurityAlerts is enabled --- packages/snap/snap.manifest.json | 2 +- .../transaction/TransactionService.test.ts | 81 ++++++++++++++++++- .../transaction/TransactionSimulator.test.ts | 45 +++++++++++ .../EstimatedChanges.test.tsx | 50 ++++++++++++ .../EstimatedChanges/EstimatedChanges.tsx | 7 ++ .../snap/src/ui/confirmation/controller.tsx | 8 +- 6 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 0dd45ebe..20b55e36 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": "CqDIlTXtEriT+o4D0iXXxi6MJxD5ULjyctXKZZnAoFY=", + "shasum": "m/4e9buaSSuHrpyWKxSbwFA/JK4VR5/4rUOBYgcThfU=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/services/transaction/TransactionService.test.ts b/packages/snap/src/services/transaction/TransactionService.test.ts index 59bf4da9..3622732d 100644 --- a/packages/snap/src/services/transaction/TransactionService.test.ts +++ b/packages/snap/src/services/transaction/TransactionService.test.ts @@ -39,7 +39,10 @@ import { DEFAULT_MOCK_ACCOUNT_WITH_BALANCES, horizonSource, } from '../on-chain-account/__mocks__/onChainAccount.fixtures'; -import { getTestWallet } from '../wallet/__mocks__/wallet.fixtures'; +import { + generateStellarAddress, + getTestWallet, +} from '../wallet/__mocks__/wallet.fixtures'; import type { Wallet } from '../wallet/Wallet'; jest.mock('../../utils/logger'); @@ -252,6 +255,82 @@ describe('TransactionService', () => { }); }); + describe('deriveEstimatedChanges', () => { + const mainnet = KnownCaip2ChainId.Mainnet; + + const buildOnChainAccount = (accountId: string, nativeBalance: number) => { + const account = createMockAccountWithBalances(accountId, '1', { + nativeBalance, + subentryCount: 0, + assets: [], + }); + return new OnChainAccount( + account, + mainnet, + horizonSource(account, mainnet), + ); + }; + + const buildNativePayment = (source: string, destination: string) => + buildMockClassicTransaction( + [ + { + type: 'payment', + params: { source, destination, asset: 'native', amount: '10' }, + }, + ], + { + networkPassphrase: Networks.PUBLIC, + source: { accountId: source, sequence: '1' }, + baseFeePerOperation: '100', + timeout: 30, + }, + ); + + it('derives the signer XLM outflow from local simulation', async () => { + const { transactionService } = createMockTransactionService(); + const wallet = getTestWallet(); + const destination = generateStellarAddress(); + + jest + .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .mockResolvedValue([buildOnChainAccount(destination, 50)]); + + const result = await transactionService.deriveEstimatedChanges({ + transaction: buildNativePayment(wallet.address, destination), + onChainAccount: buildOnChainAccount(wallet.address, 500), + signerAddress: wallet.address, + }); + + expect(result.assets).toHaveLength(1); + expect(result.assets[0]).toMatchObject({ + type: 'out', + value: 10, + symbol: 'XLM', + price: null, + }); + }); + + it('returns empty assets when the local simulation fails', async () => { + const { transactionService } = createMockTransactionService(); + const wallet = getTestWallet(); + const destination = generateStellarAddress(); + + // Destination is not loaded, so simulation throws and the section hides. + jest + .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .mockResolvedValue([]); + + const result = await transactionService.deriveEstimatedChanges({ + transaction: buildNativePayment(wallet.address, destination), + onChainAccount: buildOnChainAccount(wallet.address, 500), + signerAddress: wallet.address, + }); + + expect(result).toStrictEqual({ assets: [] }); + }); + }); + describe('savePendingKeyringTransactionSafe', () => { it('returns saved transaction when savePendingKeyringTransaction succeeds', async () => { const { transactionService } = createMockTransactionService(); diff --git a/packages/snap/src/services/transaction/TransactionSimulator.test.ts b/packages/snap/src/services/transaction/TransactionSimulator.test.ts index 98b26d85..4dcb146c 100644 --- a/packages/snap/src/services/transaction/TransactionSimulator.test.ts +++ b/packages/snap/src/services/transaction/TransactionSimulator.test.ts @@ -280,6 +280,51 @@ const destinationAddress = generateStellarAddress(); describe('TransactionSimulator', () => { const simulator = new TransactionSimulator(); + describe('simulateEndpoints', () => { + it('returns the post-fee initial state and a final state excluding the fee', () => { + const wallet = getTestWallet(); + const onChainAccount = onChainFromMockBalances(wallet.address, '1', { + nativeBalance: 500, + subentryCount: 0, + assets: [], + }); + + const tx = buildMockClassicTransaction( + [ + { + type: 'payment', + params: { + source: wallet.address, + destination: destinationAddress, + asset: 'native', + amount: '10', + }, + }, + ], + mainnetSimulatorTxOptions(wallet.address, '1'), + ); + + const { initialState, finalState } = simulator.simulateEndpoints( + tx, + onChainAccount, + { preloadedAccounts: [destOnChainAccount(destinationAddress)] }, + ); + + const initialBalance = initialState.accounts.get( + wallet.address, + )?.nativeRawBalance; + const finalBalance = finalState.accounts.get( + wallet.address, + )?.nativeRawBalance; + + // initialState is the post-fee snapshot: 500 XLM minus the 100-stroop fee. + expect(initialBalance?.toFixed()).toBe('4999999900'); + // finalState applies the 10 XLM (1e8 stroops) payment on top of that, so + // the signer delta excludes the fee (already baked into the baseline). + expect(finalBalance?.toFixed()).toBe('4899999900'); + }); + }); + describe('preflight validation', () => { it('throws when account scope does not match transaction network', () => { const wallet = getTestWallet(); 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..5277c075 --- /dev/null +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx @@ -0,0 +1,50 @@ +import { EstimatedChanges } from './EstimatedChanges'; +import { + defaultPreferences as preferences, + getType, +} from '../../__fixtures__/confirmation.fixtures'; + +describe('EstimatedChanges', () => { + const xlmOut = { + type: 'out' as const, + value: 10, + price: null, + symbol: 'XLM', + name: 'XLM', + logo: null, + }; + const usdcIn = { + type: 'in' as const, + value: 5, + price: null, + symbol: 'USDC', + name: 'USDC', + logo: 'https://example.com/usdc.png', + }; + + it('returns null when there are no asset changes', () => { + expect( + EstimatedChanges({ changes: { assets: [] }, preferences }), + ).toBeNull(); + }); + + it('returns null when changes is null', () => { + expect(EstimatedChanges({ changes: null, preferences })).toBeNull(); + }); + + it('renders a section containing the send and receive assets', () => { + const component = EstimatedChanges({ + changes: { assets: [xlmOut, usdcIn] }, + preferences, + }); + + expect(getType(component)).toBe('Section'); + + const serialized = JSON.stringify(component); + // Outflows render in red with a leading "-", inflows in green with "+". + expect(serialized).toContain('-10 XLM'); + expect(serialized).toContain('+5 USDC'); + 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 index f5f4d165..1f1d8a9f 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -34,6 +34,13 @@ function formatValue(value: number | null): string { return new BigNumber(value ?? 0).toFixed(); } +/** + * 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, }: { diff --git a/packages/snap/src/ui/confirmation/controller.tsx b/packages/snap/src/ui/confirmation/controller.tsx index 1f16d5e3..2cc79787 100644 --- a/packages/snap/src/ui/confirmation/controller.tsx +++ b/packages/snap/src/ui/confirmation/controller.tsx @@ -9,7 +9,6 @@ import { formatFeeData, formatOrigin, getPreferencesWithFallback, - hasEnabledTransactionScan, } from './utils'; import type { KnownCaip2ChainId } from '../../api'; import { METAMASK_ORIGIN } from '../../constants'; @@ -181,9 +180,14 @@ export class ConfirmationUXController { preferences.useExternalPricingData && tokenPrices !== undefined; + // The remote scan is validation-only (estimated changes come from local + // simulation), so only schedule it when security alerts are on. Gating on + // `hasEnabledTransactionScan` here would also fire for + // `simulateOnChainActions`, scheduling a refresher with no work to do and + // briefly disabling the confirm button for nothing. const enableSecurityScan = renderOptions.scanTxn && - hasEnabledTransactionScan(preferences) && + preferences.useSecurityAlerts && params.securityScanRequest !== undefined; const enableTransactionValidation = From f4296a59958e7a28bc5ac2a22fcfb3d78ba8c43c Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Wed, 17 Jun 2026 15:45:37 +0200 Subject: [PATCH 03/13] fix: support operation-source estimated changes --- packages/snap/snap.manifest.json | 2 +- .../transaction/TransactionService.test.ts | 80 +++++++++++++++++++ .../transaction/TransactionService.ts | 2 +- .../transaction/TransactionSimulator.ts | 24 ++++-- 4 files changed, 99 insertions(+), 9 deletions(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 20b55e36..5a252499 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": "m/4e9buaSSuHrpyWKxSbwFA/JK4VR5/4rUOBYgcThfU=", + "shasum": "slqOiy7T5yCiuLp8SgW5Z+8meELKUbrjJ7HZYA/PdNM=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/services/transaction/TransactionService.test.ts b/packages/snap/src/services/transaction/TransactionService.test.ts index 3622732d..328fc326 100644 --- a/packages/snap/src/services/transaction/TransactionService.test.ts +++ b/packages/snap/src/services/transaction/TransactionService.test.ts @@ -287,6 +287,31 @@ describe('TransactionService', () => { }, ); + const buildNativePaymentWithTxSource = (params: { + txSource: string; + operationSource: string; + destination: string; + }) => + buildMockClassicTransaction( + [ + { + type: 'payment', + params: { + source: params.operationSource, + destination: params.destination, + asset: 'native', + amount: '10', + }, + }, + ], + { + networkPassphrase: Networks.PUBLIC, + source: { accountId: params.txSource, sequence: '1' }, + baseFeePerOperation: '100', + timeout: 30, + }, + ); + it('derives the signer XLM outflow from local simulation', async () => { const { transactionService } = createMockTransactionService(); const wallet = getTestWallet(); @@ -311,6 +336,61 @@ describe('TransactionService', () => { }); }); + it('derives changes when the signer is only the operation source', async () => { + const { transactionService } = createMockTransactionService(); + const wallet = getTestWallet(); + const txSource = generateStellarAddress(); + const destination = generateStellarAddress(); + + jest + .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .mockResolvedValue([ + buildOnChainAccount(txSource, 500), + buildOnChainAccount(destination, 50), + ]); + + const result = await transactionService.deriveEstimatedChanges({ + transaction: buildNativePaymentWithTxSource({ + txSource, + operationSource: wallet.address, + destination, + }), + onChainAccount: buildOnChainAccount(wallet.address, 500), + signerAddress: wallet.address, + }); + + expect(result.assets).toHaveLength(1); + expect(result.assets[0]).toMatchObject({ + type: 'out', + value: 10, + symbol: 'XLM', + price: null, + }); + }); + + it('returns empty assets when an operation-source transaction cannot load the fee source', async () => { + const { transactionService } = createMockTransactionService(); + const wallet = getTestWallet(); + const txSource = generateStellarAddress(); + const destination = generateStellarAddress(); + + jest + .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .mockResolvedValue([buildOnChainAccount(destination, 50)]); + + const result = await transactionService.deriveEstimatedChanges({ + transaction: buildNativePaymentWithTxSource({ + txSource, + operationSource: wallet.address, + destination, + }), + onChainAccount: buildOnChainAccount(wallet.address, 500), + signerAddress: wallet.address, + }); + + expect(result).toStrictEqual({ assets: [] }); + }); + it('returns empty assets when the local simulation fails', async () => { const { transactionService } = createMockTransactionService(); const wallet = getTestWallet(); diff --git a/packages/snap/src/services/transaction/TransactionService.ts b/packages/snap/src/services/transaction/TransactionService.ts index ea729686..3c09c9c1 100644 --- a/packages/snap/src/services/transaction/TransactionService.ts +++ b/packages/snap/src/services/transaction/TransactionService.ts @@ -567,7 +567,7 @@ export class TransactionService { const { initialState, finalState } = simulator.simulateEndpoints( transaction, onChainAccount, - { preloadedAccounts }, + { preloadedAccounts, allowOperationSourceAccount: true }, ); const assets = await mapSimulationToEstimatedChanges({ diff --git a/packages/snap/src/services/transaction/TransactionSimulator.ts b/packages/snap/src/services/transaction/TransactionSimulator.ts index ffa7527f..770311f3 100644 --- a/packages/snap/src/services/transaction/TransactionSimulator.ts +++ b/packages/snap/src/services/transaction/TransactionSimulator.ts @@ -24,6 +24,7 @@ import { } from './simulation'; import type { Transaction } from './Transaction'; import { + assertAccountInvolvesTransaction, assertInvokeHostFunctionSoleOperation, assertTransactionTimeBound, assertTransactionScope, @@ -64,6 +65,12 @@ export enum SupportedOperations { */ export type TransactionSimulatorOptions = { expectedOPTypes?: SupportedOperations[]; + /** + * Allow simulation when the loaded account is only an operation source. Keep + * this scoped to sign-transaction estimated changes; normal validation still + * requires the wallet account to be the transaction or fee source. + */ + allowOperationSourceAccount?: boolean; /** * Extra accounts merged into simulation (e.g. payment destinations). Ignored when simulation path does not apply. */ @@ -205,16 +212,19 @@ export class TransactionSimulator { transaction: Transaction, options?: TransactionSimulatorOptions, ): asserts ops is SupportedOPType[] { - const { expectedOPTypes = [] } = options ?? {}; + const { expectedOPTypes = [], allowOperationSourceAccount = false } = + options ?? {}; // Ensure the transaction is not expired assertTransactionTimeBound(transaction); // Ensure the transaction scope matches the account scope. assertTransactionScope(transaction, account.scope); - // Envelope must involve this wallet as source or fee source (API XDR or in-app builds). - // TODO: we may need to relax it in future when we support fee payment by other account. - assertTransactionSourceAccount(transaction, account.accountId); + if (allowOperationSourceAccount) { + assertAccountInvolvesTransaction(transaction, account.accountId); + } else { + assertTransactionSourceAccount(transaction, account.accountId); + } // Soroban `invokeHostFunction` is only allowed as a single-op tx and is a no-op for state. assertInvokeHostFunctionSoleOperation(transaction); @@ -327,9 +337,9 @@ export class TransactionSimulator { }): SimulationState { // Assume the state is cloned beforehand const { state, feeSource, fee } = params; - // it is possible that the transaction fee source is different than the wallet user, - // if the transaction is passed from external, we dont support it yet, - // hence `getAccount` will throw an error. + // The fee source must be present in the simulation state. For + // sign-transaction estimated changes, external fee sources are preloaded + // from the transaction's participating accounts. const feePayer = getAccount(state, feeSource); const spendable = getSpendableNative(feePayer); From 8c7a978e8cc436b799f7cf16876088f876bb1946 Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Wed, 17 Jun 2026 17:02:01 +0200 Subject: [PATCH 04/13] fix: hide native icon in estimated changes --- packages/site/.env.development | 3 ++- packages/snap/snap.manifest.json | 2 +- .../transaction/mapSimulationToEstimatedChanges.test.ts | 1 + .../services/transaction/mapSimulationToEstimatedChanges.ts | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) 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/snap.manifest.json b/packages/snap/snap.manifest.json index 5a252499..23a5cddb 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": "slqOiy7T5yCiuLp8SgW5Z+8meELKUbrjJ7HZYA/PdNM=", + "shasum": "JWlk1FdrqkD/utgXyXVruYKYVA/AGvUs4JirFygwG/A=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts index 7f4e1982..32daa6c2 100644 --- a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts +++ b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts @@ -53,6 +53,7 @@ describe('mapSimulationToEstimatedChanges', () => { value: 10, price: null, symbol: 'XLM', + logo: null, }); expect(assetMetadataService.resolve).not.toHaveBeenCalled(); }); diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts index 31acf2ef..b0a5d284 100644 --- a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts +++ b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts @@ -97,7 +97,7 @@ export async function mapSimulationToEstimatedChanges(params: { decimals: meta.units[0].decimals, symbol: meta.symbol, name: meta.name ?? meta.symbol, - logo: meta.iconUrl ?? null, + logo: null, }), ); } From 5590841443b2a6c28a556c254065a4ae88d51a34 Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Fri, 19 Jun 2026 12:54:21 +0200 Subject: [PATCH 05/13] feat: use blockaid simulation for estimated changes on sign-tx and send --- .gitignore | 1 + packages/snap/snap.manifest.json | 2 +- .../clientRequest/confirmSend.test.ts | 19 ++- .../src/handlers/clientRequest/confirmSend.ts | 38 +++++- .../scanRefresher.test.ts | 116 ++++++++++++++++-- .../scanRefresher.ts | 64 ++++++---- .../src/handlers/keyring/signTransaction.ts | 6 +- .../TransactionScanService.test.ts | 98 +++++++++++++++ .../TransactionScanService.ts | 58 ++++++++- .../EstimatedChanges.test.tsx | 65 ++++++++-- .../EstimatedChanges/EstimatedChanges.tsx | 107 ++++++++++++---- .../snap/src/ui/confirmation/controller.tsx | 13 +- .../snap/src/ui/confirmation/utils.test.ts | 17 +++ packages/snap/src/ui/confirmation/utils.ts | 10 ++ .../ConfirmSendTransaction.tsx | 50 ++------ .../ConfirmSignTransaction.tsx | 11 +- 16 files changed, 549 insertions(+), 126 deletions(-) diff --git a/.gitignore b/.gitignore index 839047c2..4ce92bc2 100644 --- a/.gitignore +++ b/.gitignore @@ -65,6 +65,7 @@ node_modules/ # dotenv environment variables file .env .env.test +.env.development # Stores VSCode versions used for testing VSCode extensions .vscode-test diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 23a5cddb..8e6a037b 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": "JWlk1FdrqkD/utgXyXVruYKYVA/AGvUs4JirFygwG/A=", + "shasum": "xdjrJ7krCauJTRy3HdZu7vcyYJGh0RA5APGn11rLwXk=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts index 4a21e2b6..2b1ec7a1 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts @@ -290,9 +290,7 @@ describe('ConfirmSendHandler', () => { origin: METAMASK_ORIGIN, renderContext: { account, - assetMetadata, toAddress: destinationAddress, - amount: '1', }, renderOptions: { loadPrice: true, @@ -303,6 +301,23 @@ describe('ConfirmSendHandler', () => { accountAddress: account.address, transaction: unsignedScanXdr, }, + initialScan: { + status: 'SUCCESS', + estimatedChanges: { + assets: [ + { + type: '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..e12576d0 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,7 @@ import type { } from '../accountResolver'; import { BaseClientRequestHandler } from './base'; import { AccountNotActivatedException } from '../../services/network'; +import type { TransactionScanEstimatedChanges } from '../../services/transaction-scan'; import type { ConfirmationUXController } from '../../ui/confirmation/controller'; import { TrackTransactionHandler } from '../cronjob/trackTransaction'; @@ -294,6 +296,10 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< const { request, account, assetMetadata, fee, scope, transaction } = params; const { toAddress, amount, assetId } = request.params; const xdr = transaction.getRaw().toXDR(); + const estimatedChanges = this.#buildEstimatedChangesFallback({ + amount, + assetMetadata, + }); return ( (await this.#confirmationUIController.renderConfirmationDialog({ @@ -301,9 +307,7 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< origin: METAMASK_ORIGIN, renderContext: { account, - assetMetadata, toAddress, - amount, }, fee: fee.toString(), interfaceKey: ConfirmationInterfaceKey.ConfirmSendTransaction, @@ -316,6 +320,12 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< accountAddress: account.address, transaction: xdr, }, + initialScan: { + status: 'SUCCESS', + estimatedChanges, + validation: null, + error: null, + }, transactionValidationRequest: { accountId: account.id, transaction: xdr, @@ -328,6 +338,30 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< ); } + #buildEstimatedChangesFallback({ + amount, + assetMetadata, + }: { + amount: string; + assetMetadata: StellarAssetMetadata; + }): TransactionScanEstimatedChanges { + const { assetId, symbol, iconUrl, name } = assetMetadata; + const logo = isSlip44Id(assetId) ? null : (iconUrl ?? null); + + return { + assets: [ + { + type: '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 06c72397..a815f361 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts @@ -23,7 +23,18 @@ describe('ConfirmationScanRefresher', () => { }; const scanResult = { status: 'SUCCESS' as const, - estimatedChanges: { assets: [] }, + estimatedChanges: { + assets: [ + { + type: 'out' as const, + value: 2, + price: null, + symbol: 'USDC', + name: 'USD Coin', + logo: null, + }, + ], + }, validation: { type: TransactionScanValidationType.Benign, reason: null, @@ -63,16 +74,17 @@ describe('ConfirmationScanRefresher', () => { }); } - it('requests validation only and never remote simulation', async () => { + it('requests simulation and validation for sign transaction when both scan preferences are enabled', async () => { const { refresher, transactionScanService } = setup(); const result = await refresher.refresh(createScanContext()); - // Remote simulation is intentionally omitted; estimated changes come from - // the local on-chain simulation instead. expect(transactionScanService.scanTransaction).toHaveBeenCalledWith({ ...securityScanRequest, - options: [TransactionScanOption.Validation], + options: [ + TransactionScanOption.Simulation, + TransactionScanOption.Validation, + ], }); expect(result).toStrictEqual({ result: { @@ -86,24 +98,79 @@ describe('ConfirmationScanRefresher', () => { it.each([ ConfirmationInterfaceKey.SignTransaction, ConfirmationInterfaceKey.ConfirmSendTransaction, - ConfirmationInterfaceKey.ChangeTrustlineOptIn, - ConfirmationInterfaceKey.ChangeTrustlineOptOut, ])( - 'never requests remote simulation for %s even when simulateOnChainActions is enabled', + 'requests simulation for %s when estimated changes are enabled', async (interfaceKey) => { const { refresher, transactionScanService } = setup(); - await refresher.refresh(createScanContext({ interfaceKey })); + await refresher.refresh( + createScanContext({ + interfaceKey, + preferences: { + useSecurityAlerts: false, + simulateOnChainActions: true, + }, + }), + ); expect(transactionScanService.scanTransaction).toHaveBeenCalledWith({ ...securityScanRequest, - options: [TransactionScanOption.Validation], + options: [TransactionScanOption.Simulation], }); }, ); - it('preserves locally-derived estimated changes over the Blockaid result', async () => { + it.each([ + ConfirmationInterfaceKey.ChangeTrustlineOptIn, + ConfirmationInterfaceKey.ChangeTrustlineOptOut, + ])('does not request simulation for %s', async (interfaceKey) => { + const { refresher, transactionScanService } = setup(); + + await refresher.refresh(createScanContext({ interfaceKey })); + + expect(transactionScanService.scanTransaction).toHaveBeenCalledWith({ + ...securityScanRequest, + options: [TransactionScanOption.Validation], + }); + }); + + it('uses Blockaid estimated changes when remote simulation returns asset rows', async () => { const { refresher } = setup(); + const localEstimatedChanges = { + assets: [ + { + type: 'out' as const, + 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: [ { @@ -116,6 +183,13 @@ describe('ConfirmationScanRefresher', () => { }, ], }; + const emptyRemoteScan = { + ...scanResult, + estimatedChanges: { assets: [] }, + }; + transactionScanService.scanTransaction.mockResolvedValueOnce( + emptyRemoteScan, + ); const result = await refresher.refresh( createScanContext({ @@ -131,7 +205,7 @@ describe('ConfirmationScanRefresher', () => { expect(result).toStrictEqual({ result: { scan: { - ...scanResult, + ...emptyRemoteScan, estimatedChanges: localEstimatedChanges, }, scanFetchStatus: FetchStatus.Fetched, @@ -185,12 +259,28 @@ describe('ConfirmationScanRefresher', () => { }); }); - it('does not fetch when only simulateOnChainActions is enabled', () => { + it('fetches when only simulateOnChainActions is enabled for sign transaction', () => { + const { refresher } = setup(); + + expect( + refresher.shouldFetch( + createScanContext({ + preferences: { + useSecurityAlerts: false, + simulateOnChainActions: true, + }, + }), + ), + ).toBe(true); + }); + + it('does not fetch when only simulateOnChainActions is enabled for change trust', () => { const { refresher } = setup(); expect( refresher.shouldFetch( createScanContext({ + interfaceKey: ConfirmationInterfaceKey.ChangeTrustlineOptIn, preferences: { useSecurityAlerts: false, simulateOnChainActions: true, diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts index 7a3f42be..c1a7571a 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts @@ -14,6 +14,7 @@ import type { import { TransactionScanOption } from '../../../services/transaction-scan'; import type { ContextWithSecurityScan } from '../../../ui/confirmation/api'; import { + ConfirmationInterfaceKey, ContextWithSecurityScanStruct, FetchStatus, } from '../../../ui/confirmation/api'; @@ -23,11 +24,7 @@ import { createPrefixedLogger } from '../../../utils/logger'; type SecurityScanContext = ConfirmationDataContext & ContextWithSecurityScan; /** - * Refreshes the Blockaid security validation in the confirmation dialog context. - * - * Estimated balance changes are owned by the local on-chain simulation (seeded - * at dialog open), so this refresher only requests Blockaid `Validation` and - * never overwrites the locally-derived `estimatedChanges`. + * Refreshes the Blockaid scan in the confirmation dialog context. */ export class ConfirmationScanRefresher implements IConfirmationContextRefresher { readonly key = ConfirmationContextRefresherKey.Scan; @@ -71,8 +68,7 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher const optionsEnabled = this.#getScanOptions(scanCtx).length > 0; return { result: { - // Preserve the locally-derived estimated changes; only the scan fetch - // status reflects the (validation) failure. + // Keep any locally-seeded estimate visible if the remote scan cannot recover. scan: scanCtx.scan ?? null, scanFetchStatus: optionsEnabled ? FetchStatus.Error @@ -90,7 +86,7 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher SecurityScanContext['securityScanRequest'] >; const options = this.#getScanOptions(scanCtx); - const localEstimatedChanges = scanCtx.scan?.estimatedChanges ?? { + const fallbackEstimatedChanges = scanCtx.scan?.estimatedChanges ?? { assets: [], }; @@ -102,9 +98,10 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher return { result: { - // Blockaid only contributes validation/error; the locally-derived - // estimated changes always take precedence. - scan: this.#scanWithLocalChanges(scan, localEstimatedChanges), + scan: this.#scanWithEstimatedChangesFallback( + scan, + fallbackEstimatedChanges, + ), scanFetchStatus: scan ? FetchStatus.Fetched : FetchStatus.Error, }, reschedule: scan !== null, @@ -113,7 +110,10 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher this.#logger.error('Error refreshing confirmation security scan:', error); return { result: { - scan: this.#scanWithLocalChanges(null, localEstimatedChanges), + scan: this.#scanWithEstimatedChangesFallback( + null, + fallbackEstimatedChanges, + ), scanFetchStatus: FetchStatus.Error, }, reschedule: false, @@ -126,36 +126,50 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher } /** - * Merges a Blockaid scan result with the locally-derived estimated changes, - * preserving the fund-flow breakdown regardless of the remote scan outcome. + * Merges a Blockaid scan result with the locally-seeded estimated changes. + * Remote estimated changes are preferred when Blockaid returns displayable + * asset rows; otherwise the locally-derived fallback stays on screen. * * @param scan - The Blockaid scan result, or null when none was returned. - * @param localEstimatedChanges - The estimated changes from local simulation. - * @returns A scan result carrying the local estimated changes. + * @param fallbackEstimatedChanges - The locally-seeded estimated changes. + * @returns A scan result carrying the best available estimated changes. */ - #scanWithLocalChanges( + #scanWithEstimatedChangesFallback( scan: TransactionScanResult | null, - localEstimatedChanges: TransactionScanEstimatedChanges, + fallbackEstimatedChanges: TransactionScanEstimatedChanges, ): TransactionScanResult { if (scan) { - return { ...scan, estimatedChanges: localEstimatedChanges }; + const estimatedChanges = this.#hasEstimatedChanges(scan.estimatedChanges) + ? scan.estimatedChanges + : fallbackEstimatedChanges; + + return { ...scan, estimatedChanges }; } return { status: 'ERROR', - estimatedChanges: localEstimatedChanges, + estimatedChanges: fallbackEstimatedChanges, validation: null, error: null, }; } + #hasEstimatedChanges( + estimatedChanges: TransactionScanEstimatedChanges, + ): boolean { + return estimatedChanges.assets.length > 0; + } + #getScanOptions(ctx: SecurityScanContext): TransactionScanOption[] { const options: TransactionScanOption[] = []; - // Remote simulation is intentionally not requested: estimated balance - // changes are derived from the local on-chain simulation. Blockaid is used - // for security validation only. (This completes the flow-aware change from - // PR #112: sign-transaction no longer needs remote simulation either, now - // that the local simulation drives the fund-flow breakdown.) + if ( + ctx.preferences.simulateOnChainActions && + (ctx.interfaceKey === ConfirmationInterfaceKey.SignTransaction || + ctx.interfaceKey === ConfirmationInterfaceKey.ConfirmSendTransaction) + ) { + options.push(TransactionScanOption.Simulation); + } + if (ctx.preferences.useSecurityAlerts) { options.push(TransactionScanOption.Validation); } diff --git a/packages/snap/src/handlers/keyring/signTransaction.ts b/packages/snap/src/handlers/keyring/signTransaction.ts index 62b91834..41dd3fb8 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.ts @@ -137,9 +137,9 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< ), ) as ContextWithPrices['tokenPrices']; - // Estimated balance changes come from local on-chain simulation (not the - // remote security scan), so they render immediately on dialog open. The - // remote scan only contributes security validation. + // Seed a local estimate so the dialog can render immediately. If Blockaid + // later returns displayable simulation results, the scan refresher replaces + // this fallback with the remote estimate. const estimatedChanges = await this.#deriveEstimatedChanges( request, transaction, diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts index 0a9130b7..113db271 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts @@ -225,4 +225,102 @@ describe('TransactionScanService', () => { const result = await service.scanTransaction(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.scanTransaction({ + ...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.scanTransaction({ + ...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.scanTransaction({ + ...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 634d6209..7da29ea1 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -1,3 +1,5 @@ +import { BigNumber } from 'bignumber.js'; + import { TransactionScanOption } from './api'; import type { StellarAssetDiff, @@ -10,8 +12,10 @@ 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 } from '../../utils'; +import { normalizeAmount } from '../../utils/currency'; export class TransactionScanService { readonly #securityAlertsApiClient: SecurityAlertsApiClient; @@ -128,11 +132,63 @@ export class TransactionScanService { symbol, name: assetDiff.asset.name ?? symbol, logo: null, - value: transfer?.value ?? null, + value: this.#computeDisplayValue(transfer, assetDiff), price: transfer?.usd_price ?? null, }; } + /** + * Computes the human-readable amount for an asset transfer. + * Prefers {@link StellarAssetTransferDetails.raw_value} with known decimals + * (Tron parity) because Blockaid's `value` can be imprecise for fractional + * native XLM amounts. + * + * @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. + * + * @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/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx index 5277c075..bf1ed4aa 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx @@ -1,8 +1,32 @@ import { EstimatedChanges } from './EstimatedChanges'; +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 = { @@ -22,28 +46,55 @@ describe('EstimatedChanges', () => { logo: 'https://example.com/usdc.png', }; - it('returns null when there are no asset changes', () => { - expect( - EstimatedChanges({ changes: { assets: [] }, preferences }), - ).toBeNull(); + it('renders a skeleton while the remote scan is fetching', () => { + const component = EstimatedChanges({ + changes: { assets: [xlmOut] }, + preferences, + scanFetchStatus: FetchStatus.Fetching, + }); + + expect(JSON.stringify(component)).toContain('"type":"Skeleton"'); + expect(JSON.stringify(component)).not.toContain('-10 XLM'); }); - it('returns null when changes is null', () => { - expect(EstimatedChanges({ changes: null, preferences })).toBeNull(); + it('shows not available when the remote scan errors', () => { + const component = EstimatedChanges({ + changes: { assets: [xlmOut] }, + preferences, + scanFetchStatus: FetchStatus.Error, + }); + + expect(JSON.stringify(component)).toContain( + 'Estimated changes are not available', + ); + }); + + 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', () => { + 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 index 1f1d8a9f..7fbd3d96 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -7,21 +7,27 @@ import { 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 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 { isFetchStatusLoadingOrFetching } from '../../utils'; type EstimatedChangesProps = { changes: TransactionScanEstimatedChanges | null; preferences: GetPreferencesResult; + scanFetchStatus: FetchStatus; }; /** @@ -34,6 +40,41 @@ function formatValue(value: number | null): string { return new BigNumber(value ?? 0).toFixed(); } +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. * @@ -47,10 +88,13 @@ const AssetChangeRow = ({ asset: TransactionScanAssetChange; }): ComponentOrElement => { const isOut = asset.type === 'out'; + const iconSrc = + asset.logo ?? (asset.symbol === NATIVE_ASSET_SYMBOL ? xlmIcon : null); + return ( - {asset.logo ? ( - + {iconSrc ? ( + ) : null} {`${isOut ? '-' : '+'}${formatValue(asset.value)} ${asset.symbol}`} @@ -60,38 +104,59 @@ const AssetChangeRow = ({ }; /** - * Renders the signer's estimated balance changes (send / receive breakdown) - * derived from the local on-chain simulation. Hidden entirely when there are no - * modeled changes (for example Soroban invokes or unsupported transactions). + * 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). - * @returns The estimated-changes section, or null when there is nothing to show. + * @param props.scanFetchStatus - Latest remote scan fetch status. + * @returns The estimated-changes section. */ export const EstimatedChanges = ({ changes, preferences, -}: EstimatedChangesProps): ComponentOrElement | null => { - const assets = changes?.assets ?? []; - if (assets.length === 0) { - return null; + scanFetchStatus, +}: EstimatedChangesProps): ComponentOrElement => { + const t = i18n(preferences.locale as Locale); + const isFetching = isFetchStatusLoadingOrFetching(scanFetchStatus); + const isFetched = scanFetchStatus === FetchStatus.Fetched; + const isFetchError = scanFetchStatus === FetchStatus.Error; + + if (isFetching) { + return ; } - const t = i18n(preferences.locale as Locale); - const send = assets.filter((asset) => asset.type === 'out'); - const receive = assets.filter((asset) => asset.type === 'in'); + if (isFetchError) { + return ( +
+ + + {t('confirmation.estimatedChanges.notAvailable')} + +
+ ); + } + + const send = changes?.assets.filter((asset) => asset.type === 'out') ?? []; + const receive = changes?.assets.filter((asset) => asset.type === 'in') ?? []; + const hasChanges = send.length > 0 || receive.length > 0; + + if (isFetched && !hasChanges) { + return ( +
+ + + {t('confirmation.estimatedChanges.noChanges')} + +
+ ); + } return (
- - - {t('confirmation.estimatedChanges.title')} - - - - - + {send.length > 0 ? ( diff --git a/packages/snap/src/ui/confirmation/controller.tsx b/packages/snap/src/ui/confirmation/controller.tsx index 2cc79787..c5a76ea9 100644 --- a/packages/snap/src/ui/confirmation/controller.tsx +++ b/packages/snap/src/ui/confirmation/controller.tsx @@ -1,8 +1,8 @@ import type { DialogResult } from '@metamask/snaps-sdk'; import { + ConfirmationInterfaceKey, FetchStatus, - type ConfirmationInterfaceKey, type ContextWithPrices, } from './api'; import { @@ -180,14 +180,13 @@ export class ConfirmationUXController { preferences.useExternalPricingData && tokenPrices !== undefined; - // The remote scan is validation-only (estimated changes come from local - // simulation), so only schedule it when security alerts are on. Gating on - // `hasEnabledTransactionScan` here would also fire for - // `simulateOnChainActions`, scheduling a refresher with no work to do and - // briefly disabling the confirm button for nothing. + const canUseRemoteSimulation = + interfaceKey === ConfirmationInterfaceKey.SignTransaction || + interfaceKey === ConfirmationInterfaceKey.ConfirmSendTransaction; const enableSecurityScan = renderOptions.scanTxn && - preferences.useSecurityAlerts && + (preferences.useSecurityAlerts || + (preferences.simulateOnChainActions && canUseRemoteSimulation)) && params.securityScanRequest !== undefined; const enableTransactionValidation = diff --git a/packages/snap/src/ui/confirmation/utils.test.ts b/packages/snap/src/ui/confirmation/utils.test.ts index 05f02c38..a13a911a 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, + isFetchStatusLoadingOrFetching, isLocalTransactionValidationFailed, isRemoteTransactionScanLoading, requiresMaliciousAcknowledgement, @@ -23,6 +24,22 @@ const warningScan = { }; describe('confirmation utils', () => { + describe('isFetchStatusLoadingOrFetching', () => { + it.each([FetchStatus.Initial, FetchStatus.Fetching])( + 'returns true for %s', + (status) => { + expect(isFetchStatusLoadingOrFetching(status)).toBe(true); + }, + ); + + it.each([FetchStatus.Fetched, FetchStatus.Error])( + 'returns false for %s', + (status) => { + expect(isFetchStatusLoadingOrFetching(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..5b5cc561 100644 --- a/packages/snap/src/ui/confirmation/utils.ts +++ b/packages/snap/src/ui/confirmation/utils.ts @@ -168,6 +168,16 @@ export function formatFeeData( }; } +/** + * Returns true while a fetch is still in flight (initial or actively fetching). + * + * @param status - The fetch status to inspect. + * @returns Whether the status represents an in-progress fetch. + */ +export function isFetchStatusLoadingOrFetching(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 53b7e1d9..6cbc8e36 100644 --- a/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx +++ b/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx @@ -11,13 +11,10 @@ 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 { isSlip44Id, i18n } from '../../../../utils'; -import { xlmIcon } from '../../../images'; +import { i18n } from '../../../../utils'; import { STELLAR_IMAGE } from '../../../images/icon'; import type { ContextWithPrices, @@ -26,17 +23,15 @@ import type { } from '../../api'; import { FetchStatus } from '../../api'; import { - Asset, ConfirmationAlerts, ConfirmationFooter, + EstimatedChanges, FeeRow, } from '../../components'; import { getAccountExplorerUrl, getAccountName, - getClassicAssetExplorerUrl, getNetworkName, - getSepAssetExplorerUrl, requiresMaliciousAcknowledgement, shouldDisableConfirmation, } from '../../utils'; @@ -44,19 +39,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, @@ -70,21 +60,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 ( @@ -101,6 +80,14 @@ export const ConfirmSendTransaction = ({ {null} + {preferences.simulateOnChainActions ? ( + + ) : null} +
{origin ? ( @@ -143,23 +130,6 @@ export const ConfirmSendTransaction = ({ /> - - - {t('confirmation.estimatedChanges.send')} - - - {/* Network */} diff --git a/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx index 33c365be..4c9a5092 100644 --- a/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx +++ b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx @@ -251,10 +251,13 @@ export const ConfirmSignTransaction = ({ ))}
- + {preferences.simulateOnChainActions ? ( + + ) : null}
{readableTransaction.operations.map((operationJson, index) => ( From ed711ffb6923a85e0f72088180b4192a748216db Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Fri, 19 Jun 2026 15:44:58 +0200 Subject: [PATCH 06/13] fix: fix comments --- packages/site/.env.development | 2 -- packages/snap/snap.manifest.json | 2 +- .../EstimatedChanges/EstimatedChanges.tsx | 26 ++++++++++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) delete mode 100644 packages/site/.env.development diff --git a/packages/site/.env.development b/packages/site/.env.development deleted file mode 100644 index a5e6bf2e..00000000 --- a/packages/site/.env.development +++ /dev/null @@ -1,2 +0,0 @@ -#SNAP_ORIGIN=npm:@metamask/stellar-wallet-snap -SNAP_ORIGIN=local:http://localhost:8080 diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 8e6a037b..66bcd893 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": "xdjrJ7krCauJTRy3HdZu7vcyYJGh0RA5APGn11rLwXk=", + "shasum": "hFzSRh2JGZWkBDueapnD9AEDhi3asx+O9Sk6J9+/lEk=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx index 7fbd3d96..d9dd50fc 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -40,6 +40,20 @@ function formatValue(value: number | null): string { return new BigNumber(value ?? 0).toFixed(); } +/** + * 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, }: { @@ -163,8 +177,10 @@ export const EstimatedChanges = ({ {t('confirmation.estimatedChanges.send')} - {send.map((asset) => ( - + {send.map((asset, index) => ( + + + ))} @@ -175,8 +191,10 @@ export const EstimatedChanges = ({ {t('confirmation.estimatedChanges.receive')} - {receive.map((asset) => ( - + {receive.map((asset, index) => ( + + + ))} From 6353666d1f9f312deff957682805fc2ad7a27b78 Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Mon, 22 Jun 2026 11:40:55 +0200 Subject: [PATCH 07/13] fix: update deriveEstimatedChanges tests to use loadOnChainAccountsSafe --- .../src/services/transaction/TransactionService.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/snap/src/services/transaction/TransactionService.test.ts b/packages/snap/src/services/transaction/TransactionService.test.ts index 4c7785f5..ae3e9849 100644 --- a/packages/snap/src/services/transaction/TransactionService.test.ts +++ b/packages/snap/src/services/transaction/TransactionService.test.ts @@ -318,7 +318,7 @@ describe('TransactionService', () => { const destination = generateStellarAddress(); jest - .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') .mockResolvedValue([buildOnChainAccount(destination, 50)]); const result = await transactionService.deriveEstimatedChanges({ @@ -343,7 +343,7 @@ describe('TransactionService', () => { const destination = generateStellarAddress(); jest - .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') .mockResolvedValue([ buildOnChainAccount(txSource, 500), buildOnChainAccount(destination, 50), @@ -375,7 +375,7 @@ describe('TransactionService', () => { const destination = generateStellarAddress(); jest - .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') .mockResolvedValue([buildOnChainAccount(destination, 50)]); const result = await transactionService.deriveEstimatedChanges({ @@ -398,7 +398,7 @@ describe('TransactionService', () => { // Destination is not loaded, so simulation throws and the section hides. jest - .spyOn(NetworkService.prototype, 'loadOnChainAccounts') + .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') .mockResolvedValue([]); const result = await transactionService.deriveEstimatedChanges({ From 74587d39adfb5755c1b4c6e52e1ad3f64158ecd9 Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Wed, 24 Jun 2026 15:39:32 +0200 Subject: [PATCH 08/13] feat: estimated balance changes in confirmation dialog --- .gitignore | 1 - packages/snap/snap.manifest.json | 2 +- .../clientRequest/changeTrustOpt.test.ts | 8 +-- .../handlers/clientRequest/changeTrustOpt.ts | 7 +- .../clientRequest/confirmSend.test.ts | 4 +- .../src/handlers/clientRequest/confirmSend.ts | 67 ++++++++++++++++-- .../scanRefresher.test.ts | 58 ++++++++-------- .../scanRefresher.ts | 14 ++-- .../handlers/keyring/signTransaction.test.ts | 3 +- .../src/handlers/keyring/signTransaction.ts | 68 ++----------------- .../TransactionScanService.ts | 26 ++++--- .../snap/src/services/transaction-scan/api.ts | 10 ++- .../transaction/TransactionService.test.ts | 34 ++-------- .../transaction/TransactionService.ts | 28 +++++--- .../transaction/TransactionSimulator.test.ts | 16 ++--- .../transaction/TransactionSimulator.ts | 48 ++----------- .../mapSimulationToEstimatedChanges.ts | 5 +- packages/snap/src/ui/confirmation/api.ts | 6 ++ .../EstimatedChanges.test.tsx | 5 +- .../EstimatedChanges/EstimatedChanges.tsx | 12 +++- .../src/ui/confirmation/controller.test.tsx | 2 +- .../snap/src/ui/confirmation/controller.tsx | 65 +++++++++++------- .../ConfirmSignTransaction.tsx | 4 +- 23 files changed, 239 insertions(+), 254 deletions(-) diff --git a/.gitignore b/.gitignore index 4ce92bc2..839047c2 100644 --- a/.gitignore +++ b/.gitignore @@ -65,7 +65,6 @@ node_modules/ # dotenv environment variables file .env .env.test -.env.development # Stores VSCode versions used for testing VSCode extensions .vscode-test diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 368d4a78..76662883 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": "MP9Bw7a1brpokYRhspRu5JKbaDhoO6IHi7/FvQb1fic=", + "shasum": "MgbBvdiovrTeeU6hChXcMKYtNjEv5GLC5elEFz+Atd0=", "location": { "npm": { "filePath": "dist/bundle.js", 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 2b1ec7a1..6bc9b2f9 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts @@ -294,8 +294,8 @@ describe('ConfirmSendHandler', () => { }, renderOptions: { loadPrice: true, - scanTxn: true, - validateTxn: true, + securityScanning: true, + localSimulation: true, }, securityScanRequest: { accountAddress: account.address, diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.ts b/packages/snap/src/handlers/clientRequest/confirmSend.ts index e12576d0..9418f27e 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.ts @@ -47,6 +47,7 @@ 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'; @@ -152,6 +153,7 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< scope, fee: transaction.totalFee, transaction, + onChainAccount, })) ) { await trackTransactionRejected({ @@ -292,11 +294,23 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< scope: KnownCaip2ChainId; fee: BigNumber; transaction: Transaction; + onChainAccount: ResolvedActivatedAccount['onChainAccount']; }): Promise { - const { request, account, assetMetadata, fee, scope, transaction } = params; + const { + request, + account, + assetMetadata, + fee, + scope, + transaction, + onChainAccount, + } = params; const { toAddress, amount, assetId } = request.params; const xdr = transaction.getRaw().toXDR(); - const estimatedChanges = this.#buildEstimatedChangesFallback({ + const estimatedChanges = await this.#deriveEstimatedChanges({ + transaction, + onChainAccount, + signerAddress: account.address, amount, assetMetadata, }); @@ -313,8 +327,8 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< interfaceKey: ConfirmationInterfaceKey.ConfirmSendTransaction, renderOptions: { loadPrice: true, - scanTxn: true, - validateTxn: true, + securityScanning: true, + localSimulation: true, }, securityScanRequest: { accountAddress: account.address, @@ -338,6 +352,49 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< ); } + /** + * Estimated balance changes for the send confirmation, derived from a local + * on-chain simulation (send/change-trust never use remote simulation). Falls + * back to the known send amount as a single outgoing row when the simulation + * yields nothing, so the user always sees what they are sending. + * + * @param params - The parameters. + * @param params.transaction - The built, validated send transaction. + * @param params.onChainAccount - The live on-chain sender snapshot. + * @param params.signerAddress - The sender's Stellar address. + * @param params.amount - The send amount (human-readable units), for the fallback row. + * @param params.assetMetadata - The asset metadata, for the fallback row. + * @returns The estimated changes to seed the confirmation. + */ + async #deriveEstimatedChanges(params: { + transaction: Transaction; + onChainAccount: ResolvedActivatedAccount['onChainAccount']; + signerAddress: string; + amount: string; + assetMetadata: StellarAssetMetadata; + }): Promise { + const { + transaction, + onChainAccount, + signerAddress, + amount, + assetMetadata, + } = params; + + const estimatedChanges = + await this.#transactionService.deriveEstimatedChanges({ + transaction, + onChainAccount, + signerAddress, + }); + + if (estimatedChanges.assets.length > 0) { + return estimatedChanges; + } + + return this.#buildEstimatedChangesFallback({ amount, assetMetadata }); + } + #buildEstimatedChangesFallback({ amount, assetMetadata, @@ -351,7 +408,7 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< return { assets: [ { - type: 'out', + type: AssetChangeDirection.Out, value: Number(amount), price: null, symbol, diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts index c82f5aca..54b5fd9b 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts @@ -68,13 +68,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('requests simulation and validation for sign transaction when both scan preferences are enabled', 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()); @@ -95,38 +98,30 @@ describe('ConfirmationScanRefresher', () => { }); }); - it.each([ - ConfirmationInterfaceKey.SignTransaction, - ConfirmationInterfaceKey.ConfirmSendTransaction, - ])( - 'requests simulation for %s when estimated changes are enabled', - async (interfaceKey) => { - const { refresher, transactionScanService } = setup(); + it('requests simulation when remote simulation is enabled and on-chain simulation is allowed', async () => { + const { refresher, transactionScanService } = setup(); - await refresher.refresh( - createScanContext({ - interfaceKey, - preferences: { - useSecurityAlerts: false, - simulateOnChainActions: true, - }, - }), - ); + await refresher.refresh( + createScanContext({ + remoteSimulation: true, + securityScanning: true, + preferences: { + useSecurityAlerts: false, + simulateOnChainActions: true, + }, + }), + ); - expect(transactionScanService.scanTransactionSafe).toHaveBeenCalledWith({ - ...securityScanRequest, - options: [TransactionScanOption.Simulation], - }); - }, - ); + expect(transactionScanService.scanTransactionSafe).toHaveBeenCalledWith({ + ...securityScanRequest, + options: [TransactionScanOption.Simulation], + }); + }); - it.each([ - ConfirmationInterfaceKey.ChangeTrustlineOptIn, - ConfirmationInterfaceKey.ChangeTrustlineOptOut, - ])('does not request simulation for %s', async (interfaceKey) => { + it('requests only validation when remote simulation is disabled (local-simulation flow)', async () => { const { refresher, transactionScanService } = setup(); - await refresher.refresh(createScanContext({ interfaceKey })); + await refresher.refresh(createScanContext({ remoteSimulation: false })); expect(transactionScanService.scanTransactionSafe).toHaveBeenCalledWith({ ...securityScanRequest, @@ -259,12 +254,13 @@ describe('ConfirmationScanRefresher', () => { }); }); - it('fetches when only simulateOnChainActions is enabled for sign transaction', () => { + 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, @@ -274,13 +270,13 @@ describe('ConfirmationScanRefresher', () => { ).toBe(true); }); - it('does not fetch when only simulateOnChainActions is enabled for change trust', () => { + it('does not fetch when simulateOnChainActions is enabled but remote simulation is not requested', () => { const { refresher } = setup(); expect( refresher.shouldFetch( createScanContext({ - interfaceKey: ConfirmationInterfaceKey.ChangeTrustlineOptIn, + remoteSimulation: false, preferences: { useSecurityAlerts: false, simulateOnChainActions: true, diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts index 2062bda8..82ebac90 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts @@ -14,7 +14,6 @@ import type { import { TransactionScanOption } from '../../../services/transaction-scan'; import type { ContextWithSecurityScan } from '../../../ui/confirmation/api'; import { - ConfirmationInterfaceKey, ContextWithSecurityScanStruct, FetchStatus, } from '../../../ui/confirmation/api'; @@ -86,6 +85,9 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher SecurityScanContext['securityScanRequest'] >; const options = this.#getScanOptions(scanCtx); + // Send / change-trust seed locally-simulated estimated changes here, which + // we keep when the validation-only remote scan returns no rows. Sign-txn has + // no local seed, so this is `{ assets: [] }` until remote simulation returns. const fallbackEstimatedChanges = scanCtx.scan?.estimatedChanges ?? { assets: [], }; @@ -162,15 +164,13 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher #getScanOptions(ctx: SecurityScanContext): TransactionScanOption[] { const options: TransactionScanOption[] = []; - if ( - ctx.preferences.simulateOnChainActions && - (ctx.interfaceKey === ConfirmationInterfaceKey.SignTransaction || - ctx.interfaceKey === ConfirmationInterfaceKey.ConfirmSendTransaction) - ) { + // 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..03835478 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.test.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.test.ts @@ -145,7 +145,8 @@ describe('SignTransactionHandler', () => { expect.objectContaining({ renderOptions: { loadPrice: true, - scanTxn: true, + securityScanning: true, + remoteSimulation: true, }, securityScanRequest: { accountAddress: mockAccount.address, diff --git a/packages/snap/src/handlers/keyring/signTransaction.ts b/packages/snap/src/handlers/keyring/signTransaction.ts index 41dd3fb8..cc7174d2 100644 --- a/packages/snap/src/handlers/keyring/signTransaction.ts +++ b/packages/snap/src/handlers/keyring/signTransaction.ts @@ -6,7 +6,6 @@ import { SignTransactionResponseStruct, } from './api'; import type { AccountResolver } from '../accountResolver'; -import { ResolveAccountSource } from '../accountResolver'; import { BaseSep43KeyringHandler } from './base'; import type { Sep43Error } from './exceptions'; import type { StellarKeyringAccount } from '../../services/account'; @@ -18,7 +17,6 @@ import { assertTransactionTimeBound, collectTransactionAssetCaipIds, } from '../../services/transaction/utils'; -import type { TransactionScanEstimatedChanges } from '../../services/transaction-scan'; import type { Wallet } from '../../services/wallet'; import type { ContextWithPrices } from '../../ui/confirmation/api'; import { ConfirmationInterfaceKey } from '../../ui/confirmation/api'; @@ -43,8 +41,6 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< readonly #confirmationUIController: ConfirmationUXController; - readonly #accountResolver: AccountResolver; - constructor({ logger, accountResolver, @@ -65,7 +61,6 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< }); this.#transactionService = transactionService; this.#confirmationUIController = confirmationUIController; - this.#accountResolver = accountResolver; } protected async execute( @@ -137,15 +132,8 @@ export class SignTransactionHandler extends BaseSep43KeyringHandler< ), ) as ContextWithPrices['tokenPrices']; - // Seed a local estimate so the dialog can render immediately. If Blockaid - // later returns displayable simulation results, the scan refresher replaces - // this fallback with the remote estimate. - const estimatedChanges = await this.#deriveEstimatedChanges( - request, - transaction, - account, - ); - + // 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, @@ -156,59 +144,17 @@ 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(), }, - initialScan: { - status: 'SUCCESS', - estimatedChanges, - validation: null, - error: null, - }, tokenPrices, })) === true ); } - - /** - * Best-effort local simulation of the signer's balance changes. Resolves the - * on-chain account and runs the local simulator; any failure (account not - * activated, unsupported/Soroban transaction, network error) resolves to an - * empty result so the confirmation simply hides the estimated-changes section. - * - * @param request - The validated sign-transaction request. - * @param transaction - The transaction with fee already applied. - * @param account - The resolved keyring account. - * @returns The estimated changes, or `{ assets: [] }` when unavailable. - */ - async #deriveEstimatedChanges( - request: SignTransactionRequest, - transaction: Transaction, - account: StellarKeyringAccount, - ): Promise { - try { - const { onChainAccount } = await this.#accountResolver.resolveAccount({ - accountId: account.id, - scope: request.scope, - options: { - onChainAccount: { load: true, source: ResolveAccountSource.OnChain }, - wallet: false, - }, - }); - - return await this.#transactionService.deriveEstimatedChanges({ - transaction, - onChainAccount, - signerAddress: account.address, - }); - } catch (error) { - this.logger.logErrorWithDetails( - 'Failed to derive estimated changes for sign transaction', - error, - ); - return { assets: [] }; - } - } } diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.ts index 730e663e..ca1fa1f7 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -1,6 +1,6 @@ import { BigNumber } from 'bignumber.js'; -import { TransactionScanOption } from './api'; +import { AssetChangeDirection, TransactionScanOption } from './api'; import type { StellarAssetDiff, StellarTransactionScanResponse, @@ -112,10 +112,10 @@ export class TransactionScanService { return assetDiffs.flatMap((assetDiff) => { const changes: TransactionScanAssetChange[] = []; if (assetDiff.out) { - changes.push(this.#mapAssetChange(assetDiff, 'out')); + changes.push(this.#mapAssetChange(assetDiff, AssetChangeDirection.Out)); } if (assetDiff.in) { - changes.push(this.#mapAssetChange(assetDiff, 'in')); + changes.push(this.#mapAssetChange(assetDiff, AssetChangeDirection.In)); } return changes; }); @@ -123,7 +123,7 @@ export class TransactionScanService { #mapAssetChange( assetDiff: StellarAssetDiff, - type: 'in' | 'out', + type: AssetChangeDirection, ): TransactionScanAssetChange { const transfer = assetDiff[type]; const symbol = @@ -170,21 +170,19 @@ export class TransactionScanService { /** * 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. + * + * Native and classic Stellar assets use 7 decimal places; contract tokens do + * not expose decimals in the Blockaid payload today. We key off the canonical + * top-level `asset_type` (`NATIVE` / `ASSET`) โ€” the nested `asset.type` is the + * same classification and adds no information. * * @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' - ) { + const { asset_type: assetType } = assetDiff; + + if (assetType === 'NATIVE' || assetType === 'ASSET') { return STELLAR_DECIMAL_PLACES; } 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/services/transaction/TransactionService.test.ts b/packages/snap/src/services/transaction/TransactionService.test.ts index ae3e9849..0b99a290 100644 --- a/packages/snap/src/services/transaction/TransactionService.test.ts +++ b/packages/snap/src/services/transaction/TransactionService.test.ts @@ -336,7 +336,7 @@ describe('TransactionService', () => { }); }); - it('derives changes when the signer is only the operation source', async () => { + it('returns empty assets when the signer is not the transaction or fee source', async () => { const { transactionService } = createMockTransactionService(); const wallet = getTestWallet(); const txSource = generateStellarAddress(); @@ -349,35 +349,9 @@ describe('TransactionService', () => { buildOnChainAccount(destination, 50), ]); - const result = await transactionService.deriveEstimatedChanges({ - transaction: buildNativePaymentWithTxSource({ - txSource, - operationSource: wallet.address, - destination, - }), - onChainAccount: buildOnChainAccount(wallet.address, 500), - signerAddress: wallet.address, - }); - - expect(result.assets).toHaveLength(1); - expect(result.assets[0]).toMatchObject({ - type: 'out', - value: 10, - symbol: 'XLM', - price: null, - }); - }); - - it('returns empty assets when an operation-source transaction cannot load the fee source', async () => { - const { transactionService } = createMockTransactionService(); - const wallet = getTestWallet(); - const txSource = generateStellarAddress(); - const destination = generateStellarAddress(); - - jest - .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') - .mockResolvedValue([buildOnChainAccount(destination, 50)]); - + // Local simulation (send / change-trust) requires the wallet to be the + // transaction or fee source; an operation-source-only signer is rejected + // and the estimated-changes section stays hidden. const result = await transactionService.deriveEstimatedChanges({ transaction: buildNativePaymentWithTxSource({ txSource, diff --git a/packages/snap/src/services/transaction/TransactionService.ts b/packages/snap/src/services/transaction/TransactionService.ts index 01279378..f4201e77 100644 --- a/packages/snap/src/services/transaction/TransactionService.ts +++ b/packages/snap/src/services/transaction/TransactionService.ts @@ -5,7 +5,10 @@ import { import { emitSnapKeyringEvent } from '@metamask/keyring-snap-sdk'; import { groupBy } from 'lodash'; -import { InsufficientBalanceException } from './exceptions'; +import { + InsufficientBalanceException, + TransactionValidationException, +} from './exceptions'; import type { KeyringTransactionRequest } from './KeyringTransactionBuilder'; import { KeyringTransactionBuilder } from './KeyringTransactionBuilder'; import { mapSimulationToEstimatedChanges } from './mapSimulationToEstimatedChanges'; @@ -545,8 +548,9 @@ export class TransactionService { /** * Derives the signer's estimated balance changes from a local on-chain * simulation, mapped into the {@link TransactionScanEstimatedChanges} shape the - * confirmation UI renders. Used to seed the `sign-transaction` confirmation - * fund-flow breakdown without relying on a remote (Blockaid) simulation. + * confirmation UI renders. Seeds the send / change-trust confirmation + * fund-flow breakdown locally (these flows never use remote Blockaid + * simulation). * * The network fee is excluded from the per-asset rows (the diff baselines on * the post-fee simulation snapshot); it is surfaced separately as the fee row. @@ -574,11 +578,19 @@ export class TransactionService { ); const simulator = new TransactionSimulator(); - const { initialState, finalState } = simulator.simulateEndpoints( - transaction, - onChainAccount, - { preloadedAccounts, allowOperationSourceAccount: true }, - ); + // The simulation stack is the post-fee snapshot first, then one entry per + // operation; diffing the endpoints excludes the network fee from per-asset + // rows (it is surfaced separately). + const states = simulator.simulate(transaction, onChainAccount, { + preloadedAccounts, + }); + const initialState = states[0]; + const finalState = states[states.length - 1]; + if (initialState === undefined || finalState === undefined) { + throw new TransactionValidationException( + 'Simulation produced no states', + ); + } const assets = await mapSimulationToEstimatedChanges({ initialState, diff --git a/packages/snap/src/services/transaction/TransactionSimulator.test.ts b/packages/snap/src/services/transaction/TransactionSimulator.test.ts index 4dcb146c..a9ba745a 100644 --- a/packages/snap/src/services/transaction/TransactionSimulator.test.ts +++ b/packages/snap/src/services/transaction/TransactionSimulator.test.ts @@ -280,7 +280,7 @@ const destinationAddress = generateStellarAddress(); describe('TransactionSimulator', () => { const simulator = new TransactionSimulator(); - describe('simulateEndpoints', () => { + describe('simulate endpoints', () => { it('returns the post-fee initial state and a final state excluding the fee', () => { const wallet = getTestWallet(); const onChainAccount = onChainFromMockBalances(wallet.address, '1', { @@ -304,16 +304,16 @@ describe('TransactionSimulator', () => { mainnetSimulatorTxOptions(wallet.address, '1'), ); - const { initialState, finalState } = simulator.simulateEndpoints( - tx, - onChainAccount, - { preloadedAccounts: [destOnChainAccount(destinationAddress)] }, - ); + const states = simulator.simulate(tx, onChainAccount, { + preloadedAccounts: [destOnChainAccount(destinationAddress)], + }); + const initialState = states[0]; + const finalState = states[states.length - 1]; - const initialBalance = initialState.accounts.get( + const initialBalance = initialState?.accounts.get( wallet.address, )?.nativeRawBalance; - const finalBalance = finalState.accounts.get( + const finalBalance = finalState?.accounts.get( wallet.address, )?.nativeRawBalance; diff --git a/packages/snap/src/services/transaction/TransactionSimulator.ts b/packages/snap/src/services/transaction/TransactionSimulator.ts index 770311f3..adc108be 100644 --- a/packages/snap/src/services/transaction/TransactionSimulator.ts +++ b/packages/snap/src/services/transaction/TransactionSimulator.ts @@ -24,7 +24,6 @@ import { } from './simulation'; import type { Transaction } from './Transaction'; import { - assertAccountInvolvesTransaction, assertInvokeHostFunctionSoleOperation, assertTransactionTimeBound, assertTransactionScope, @@ -65,12 +64,6 @@ export enum SupportedOperations { */ export type TransactionSimulatorOptions = { expectedOPTypes?: SupportedOperations[]; - /** - * Allow simulation when the loaded account is only an operation source. Keep - * this scoped to sign-transaction estimated changes; normal validation still - * requires the wallet account to be the transaction or fee source. - */ - allowOperationSourceAccount?: boolean; /** * Extra accounts merged into simulation (e.g. payment destinations). Ignored when simulation path does not apply. */ @@ -123,36 +116,6 @@ export class TransactionSimulator { }); } - /** - * Runs {@link simulate} and returns only the simulation endpoints used to - * derive estimated balance changes for the confirmation UI. - * - * `initialState` is the post-fee snapshot (fee already deducted from the fee - * source) and `finalState` is the snapshot after all operations have been - * applied. Diffing these two excludes the network fee from the per-asset - * rows (it is surfaced separately as its own fee row), matching the - * EVM/Tron confirmation experience. - * - * @param transaction - Wrapped Stellar transaction. - * @param account - Loaded signing account (Horizon-shaped raw for balances). - * @param options - Optional `expectedOPTypes` and `preloadedAccounts`. - * @returns The post-fee initial state and the final state after all operations. - * @throws {TransactionValidationException} When simulation produces no states. - */ - simulateEndpoints( - transaction: Transaction, - account: OnChainAccount, - options?: TransactionSimulatorOptions, - ): { initialState: SimulationState; finalState: SimulationState } { - const states = this.simulate(transaction, account, options); - const initialState = states[0]; - const finalState = states[states.length - 1]; - if (initialState === undefined || finalState === undefined) { - throw new TransactionValidationException('Simulation produced no states'); - } - return { initialState, finalState }; - } - #run(params: { operations: SupportedOPType[]; transaction: Transaction; @@ -212,19 +175,16 @@ export class TransactionSimulator { transaction: Transaction, options?: TransactionSimulatorOptions, ): asserts ops is SupportedOPType[] { - const { expectedOPTypes = [], allowOperationSourceAccount = false } = - options ?? {}; + const { expectedOPTypes = [] } = options ?? {}; // Ensure the transaction is not expired assertTransactionTimeBound(transaction); // Ensure the transaction scope matches the account scope. assertTransactionScope(transaction, account.scope); - if (allowOperationSourceAccount) { - assertAccountInvolvesTransaction(transaction, account.accountId); - } else { - assertTransactionSourceAccount(transaction, account.accountId); - } + // The wallet account must be the transaction or fee source for the local + // simulation paths (send / change-trust). + assertTransactionSourceAccount(transaction, account.accountId); // Soroban `invokeHostFunction` is only allowed as a single-op tx and is a no-op for state. assertInvokeHostFunctionSoleOperation(transaction); diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts index b0a5d284..95a448f3 100644 --- a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts +++ b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts @@ -12,6 +12,7 @@ import { parseClassicAssetCodeIssuer } from '../../utils'; import { normalizeAmount } from '../../utils/currency'; import type { AssetMetadataService } from '../asset-metadata'; import { getIconUrl, getNativeAssetMetadata } from '../asset-metadata/utils'; +import { AssetChangeDirection } from '../transaction-scan'; import type { TransactionScanAssetChange } from '../transaction-scan'; /** @@ -34,7 +35,9 @@ function buildChange(params: { }): TransactionScanAssetChange { const { delta, decimals, symbol, name, logo } = params; return { - type: delta.isNegative() ? 'out' : 'in', + type: delta.isNegative() + ? AssetChangeDirection.Out + : AssetChangeDirection.In, value: normalizeAmount(delta.abs(), decimals).toNumber(), price: null, symbol, 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 index bf1ed4aa..a3d1c4c4 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx @@ -1,4 +1,5 @@ import { EstimatedChanges } from './EstimatedChanges'; +import { AssetChangeDirection } from '../../../../services/transaction-scan'; import { xlmIcon } from '../../../images'; import { defaultPreferences as preferences, @@ -30,7 +31,7 @@ function collectImageSources(node: unknown): unknown[] { describe('EstimatedChanges', () => { const xlmOut = { - type: 'out' as const, + type: AssetChangeDirection.Out, value: 10, price: null, symbol: 'XLM', @@ -38,7 +39,7 @@ describe('EstimatedChanges', () => { logo: null, }; const usdcIn = { - type: 'in' as const, + type: AssetChangeDirection.In, value: 5, price: null, symbol: 'USDC', diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx index d9dd50fc..3387cbee 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -14,6 +14,7 @@ import { import { BigNumber } from 'bignumber.js'; import { NATIVE_ASSET_SYMBOL } from '../../../../constants'; +import { AssetChangeDirection } from '../../../../services/transaction-scan'; import type { TransactionScanAssetChange, TransactionScanEstimatedChanges, @@ -101,7 +102,7 @@ const AssetChangeRow = ({ }: { asset: TransactionScanAssetChange; }): ComponentOrElement => { - const isOut = asset.type === 'out'; + const isOut = asset.type === AssetChangeDirection.Out; const iconSrc = asset.logo ?? (asset.symbol === NATIVE_ASSET_SYMBOL ? xlmIcon : null); @@ -153,8 +154,13 @@ export const EstimatedChanges = ({ ); } - const send = changes?.assets.filter((asset) => asset.type === 'out') ?? []; - const receive = changes?.assets.filter((asset) => asset.type === 'in') ?? []; + 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) { 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 d99f0afd..c919b29c 100644 --- a/packages/snap/src/ui/confirmation/controller.tsx +++ b/packages/snap/src/ui/confirmation/controller.tsx @@ -1,10 +1,7 @@ import type { DialogResult } from '@metamask/snaps-sdk'; -import { - ConfirmationInterfaceKey, - FetchStatus, - type ContextWithPrices, -} from './api'; +import { FetchStatus } from './api'; +import type { ConfirmationInterfaceKey, ContextWithPrices } from './api'; import { formatFeeData, formatOrigin, @@ -40,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; }; /** @@ -101,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 }) { @@ -141,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.', ); } @@ -170,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 @@ -180,17 +187,23 @@ export class ConfirmationUXController { preferences.useExternalPricingData && tokenPrices !== undefined; - const canUseRemoteSimulation = - interfaceKey === ConfirmationInterfaceKey.SignTransaction || - interfaceKey === ConfirmationInterfaceKey.ConfirmSendTransaction; + // 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 && - (preferences.useSecurityAlerts || - (preferences.simulateOnChainActions && canUseRemoteSimulation)) && - 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 = { @@ -219,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, @@ -268,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/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx index cc36f1dd..bc261f39 100644 --- a/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx +++ b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx @@ -176,8 +176,8 @@ export const ConfirmSignTransaction = ({ const addressCaip10 = getAccountName(scope, address); const priceLoading = tokenPricesFetchStatus === FetchStatus.Fetching; const feePrice = tokenPrices?.[feeData.assetId] ?? null; - // Sign-transaction has no local re-validation (no validateTxn step), so only - // the remote-scan-loading guard applies here. + // Sign-transaction has no local simulation/re-validation step, so only the + // remote-scan-loading guard applies here. const shouldDisableConfirmButton = shouldDisableConfirmation({ scanFetchStatus, }); From ab41c555e7930a738fb42472fedebf9140dac96c Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Wed, 24 Jun 2026 16:22:12 +0200 Subject: [PATCH 09/13] fix: copilot comments --- packages/snap/snap.manifest.json | 2 +- .../clientRequest/confirmSend.test.ts | 3 +- .../scanRefresher.test.ts | 9 ++-- .../TransactionScanService.test.ts | 8 +++- .../transaction/TransactionService.test.ts | 3 +- .../mapSimulationToEstimatedChanges.test.ts | 5 ++- .../EstimatedChanges.test.tsx | 44 +++++++++++++++++-- .../EstimatedChanges/EstimatedChanges.tsx | 37 +++++++++++++--- .../ConfirmSendTransaction.tsx | 16 ++++--- 9 files changed, 101 insertions(+), 26 deletions(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 76662883..6431cea3 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": "MgbBvdiovrTeeU6hChXcMKYtNjEv5GLC5elEFz+Atd0=", + "shasum": "4E/U2/fwcv8cWdo04i0VKpbsNv3sYPjwoDzJLDXAA6A=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.test.ts b/packages/snap/src/handlers/clientRequest/confirmSend.test.ts index 6bc9b2f9..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'; @@ -306,7 +307,7 @@ describe('ConfirmSendHandler', () => { estimatedChanges: { assets: [ { - type: 'out', + type: AssetChangeDirection.Out, value: 1, price: null, symbol: assetMetadata.symbol, diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts index 54b5fd9b..469af853 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'; @@ -26,7 +27,7 @@ describe('ConfirmationScanRefresher', () => { estimatedChanges: { assets: [ { - type: 'out' as const, + type: AssetChangeDirection.Out, value: 2, price: null, symbol: 'USDC', @@ -134,7 +135,7 @@ describe('ConfirmationScanRefresher', () => { const localEstimatedChanges = { assets: [ { - type: 'out' as const, + type: AssetChangeDirection.Out, value: 1, price: null, symbol: 'XLM', @@ -169,7 +170,7 @@ describe('ConfirmationScanRefresher', () => { const localEstimatedChanges = { assets: [ { - type: 'out' as const, + type: AssetChangeDirection.Out, value: 12.5, price: null, symbol: 'XLM', @@ -215,7 +216,7 @@ describe('ConfirmationScanRefresher', () => { const localEstimatedChanges = { assets: [ { - type: 'out' as const, + type: AssetChangeDirection.Out, value: 1, price: null, symbol: 'XLM', diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts index 6eb32704..364bf7a7 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, diff --git a/packages/snap/src/services/transaction/TransactionService.test.ts b/packages/snap/src/services/transaction/TransactionService.test.ts index 0b99a290..e405a1c1 100644 --- a/packages/snap/src/services/transaction/TransactionService.test.ts +++ b/packages/snap/src/services/transaction/TransactionService.test.ts @@ -39,6 +39,7 @@ import { DEFAULT_MOCK_ACCOUNT_WITH_BALANCES, horizonSource, } from '../on-chain-account/__mocks__/onChainAccount.fixtures'; +import { AssetChangeDirection } from '../transaction-scan'; import { generateStellarAddress, getTestWallet, @@ -329,7 +330,7 @@ describe('TransactionService', () => { expect(result.assets).toHaveLength(1); expect(result.assets[0]).toMatchObject({ - type: 'out', + type: AssetChangeDirection.Out, value: 10, symbol: 'XLM', price: null, diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts index 32daa6c2..14d9ab76 100644 --- a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts +++ b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts @@ -5,6 +5,7 @@ import type { AccountState, SimulationState } from './simulation'; import { KnownCaip2ChainId } from '../../api'; import { toCaip19ClassicAssetId } from '../../utils'; import type { AssetMetadataService } from '../asset-metadata'; +import { AssetChangeDirection } from '../transaction-scan'; describe('mapSimulationToEstimatedChanges', () => { const scope = KnownCaip2ChainId.Mainnet; @@ -49,7 +50,7 @@ describe('mapSimulationToEstimatedChanges', () => { expect(result).toHaveLength(1); expect(result[0]).toMatchObject({ - type: 'out', + type: AssetChangeDirection.Out, value: 10, price: null, symbol: 'XLM', @@ -88,7 +89,7 @@ describe('mapSimulationToEstimatedChanges', () => { expect(result).toStrictEqual([ { - type: 'out', + type: AssetChangeDirection.Out, value: 0.2, price: null, symbol: 'USDC', 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 a3d1c4c4..29331863 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.test.tsx @@ -47,9 +47,9 @@ describe('EstimatedChanges', () => { logo: 'https://example.com/usdc.png', }; - it('renders a skeleton while the remote scan is fetching', () => { + it('renders a skeleton while the remote scan is fetching and nothing is seeded', () => { const component = EstimatedChanges({ - changes: { assets: [xlmOut] }, + changes: { assets: [] }, preferences, scanFetchStatus: FetchStatus.Fetching, }); @@ -58,10 +58,22 @@ describe('EstimatedChanges', () => { expect(JSON.stringify(component)).not.toContain('-10 XLM'); }); - it('shows not available when the remote scan errors', () => { + 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, }); @@ -70,6 +82,32 @@ describe('EstimatedChanges', () => { ); }); + 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: [] }, diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx index 3387cbee..4e677cb2 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -41,6 +41,23 @@ 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. * @@ -106,14 +123,20 @@ const AssetChangeRow = ({ 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} - - {`${isOut ? '-' : '+'}${formatValue(asset.value)} ${asset.symbol}`} - + {label} ); }; @@ -138,12 +161,16 @@ export const EstimatedChanges = ({ const isFetching = isFetchStatusLoadingOrFetching(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) { + if (isFetching && !hasSeededRows) { return ; } - if (isFetchError) { + if (isFetchError && !hasSeededRows) { return (
diff --git a/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx b/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx index 198d447a..2b669e4a 100644 --- a/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx +++ b/packages/snap/src/ui/confirmation/views/ConfirmSendTransaction/ConfirmSendTransaction.tsx @@ -79,13 +79,15 @@ export const ConfirmSendTransaction = ({ {null} - {preferences.simulateOnChainActions ? ( - - ) : 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 ? ( From fa705dede5542d0df3070e9598f976191458bd3a Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Thu, 25 Jun 2026 13:51:08 +0200 Subject: [PATCH 10/13] fix: comments --- packages/snap/snap.manifest.json | 2 +- packages/snap/src/context.ts | 1 - .../src/handlers/clientRequest/confirmSend.ts | 64 ++----- .../transaction/TransactionService.test.ts | 136 +-------------- .../transaction/TransactionService.ts | 82 +-------- .../transaction/TransactionSimulator.ts | 10 +- .../__mocks__/transaction.fixtures.ts | 3 - .../snap/src/services/transaction/index.ts | 1 - .../mapSimulationToEstimatedChanges.test.ts | 129 -------------- .../mapSimulationToEstimatedChanges.ts | 158 ------------------ 10 files changed, 19 insertions(+), 567 deletions(-) delete mode 100644 packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts delete mode 100644 packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 6431cea3..9399bb97 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": "4E/U2/fwcv8cWdo04i0VKpbsNv3sYPjwoDzJLDXAA6A=", + "shasum": "UqNPBfbFj9FXqhvCI3x3BF9O4vfIDiT1N9Rp2g9rbp4=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/context.ts b/packages/snap/src/context.ts index fe7a8c98..fd29ea8c 100644 --- a/packages/snap/src/context.ts +++ b/packages/snap/src/context.ts @@ -114,7 +114,6 @@ const transactionService = new TransactionService({ networkService, transactionBuilder, accountService, - assetMetadataService, }); const priceService = new PriceService({ diff --git a/packages/snap/src/handlers/clientRequest/confirmSend.ts b/packages/snap/src/handlers/clientRequest/confirmSend.ts index 9418f27e..c4bcb22e 100644 --- a/packages/snap/src/handlers/clientRequest/confirmSend.ts +++ b/packages/snap/src/handlers/clientRequest/confirmSend.ts @@ -153,7 +153,6 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< scope, fee: transaction.totalFee, transaction, - onChainAccount, })) ) { await trackTransactionRejected({ @@ -294,23 +293,13 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< scope: KnownCaip2ChainId; fee: BigNumber; transaction: Transaction; - onChainAccount: ResolvedActivatedAccount['onChainAccount']; }): Promise { - const { - request, - account, - assetMetadata, - fee, - scope, - transaction, - onChainAccount, - } = params; + const { request, account, assetMetadata, fee, scope, transaction } = params; const { toAddress, amount, assetId } = request.params; const xdr = transaction.getRaw().toXDR(); - const estimatedChanges = await this.#deriveEstimatedChanges({ - transaction, - onChainAccount, - signerAddress: account.address, + // 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, }); @@ -353,49 +342,16 @@ export class ConfirmSendHandler extends BaseClientRequestHandler< } /** - * Estimated balance changes for the send confirmation, derived from a local - * on-chain simulation (send/change-trust never use remote simulation). Falls - * back to the known send amount as a single outgoing row when the simulation - * yields nothing, so the user always sees what they are sending. + * 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.transaction - The built, validated send transaction. - * @param params.onChainAccount - The live on-chain sender snapshot. - * @param params.signerAddress - The sender's Stellar address. - * @param params.amount - The send amount (human-readable units), for the fallback row. - * @param params.assetMetadata - The asset metadata, for the fallback row. + * @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. */ - async #deriveEstimatedChanges(params: { - transaction: Transaction; - onChainAccount: ResolvedActivatedAccount['onChainAccount']; - signerAddress: string; - amount: string; - assetMetadata: StellarAssetMetadata; - }): Promise { - const { - transaction, - onChainAccount, - signerAddress, - amount, - assetMetadata, - } = params; - - const estimatedChanges = - await this.#transactionService.deriveEstimatedChanges({ - transaction, - onChainAccount, - signerAddress, - }); - - if (estimatedChanges.assets.length > 0) { - return estimatedChanges; - } - - return this.#buildEstimatedChangesFallback({ amount, assetMetadata }); - } - - #buildEstimatedChangesFallback({ + #buildEstimatedChanges({ amount, assetMetadata, }: { diff --git a/packages/snap/src/services/transaction/TransactionService.test.ts b/packages/snap/src/services/transaction/TransactionService.test.ts index e405a1c1..bfcffb74 100644 --- a/packages/snap/src/services/transaction/TransactionService.test.ts +++ b/packages/snap/src/services/transaction/TransactionService.test.ts @@ -39,11 +39,7 @@ import { DEFAULT_MOCK_ACCOUNT_WITH_BALANCES, horizonSource, } from '../on-chain-account/__mocks__/onChainAccount.fixtures'; -import { AssetChangeDirection } from '../transaction-scan'; -import { - generateStellarAddress, - getTestWallet, -} from '../wallet/__mocks__/wallet.fixtures'; +import { getTestWallet } from '../wallet/__mocks__/wallet.fixtures'; import type { Wallet } from '../wallet/Wallet'; jest.mock('../../utils/logger'); @@ -256,136 +252,6 @@ describe('TransactionService', () => { }); }); - describe('deriveEstimatedChanges', () => { - const mainnet = KnownCaip2ChainId.Mainnet; - - const buildOnChainAccount = (accountId: string, nativeBalance: number) => { - const account = createMockAccountWithBalances(accountId, '1', { - nativeBalance, - subentryCount: 0, - assets: [], - }); - return new OnChainAccount( - account, - mainnet, - horizonSource(account, mainnet), - ); - }; - - const buildNativePayment = (source: string, destination: string) => - buildMockClassicTransaction( - [ - { - type: 'payment', - params: { source, destination, asset: 'native', amount: '10' }, - }, - ], - { - networkPassphrase: Networks.PUBLIC, - source: { accountId: source, sequence: '1' }, - baseFeePerOperation: '100', - timeout: 30, - }, - ); - - const buildNativePaymentWithTxSource = (params: { - txSource: string; - operationSource: string; - destination: string; - }) => - buildMockClassicTransaction( - [ - { - type: 'payment', - params: { - source: params.operationSource, - destination: params.destination, - asset: 'native', - amount: '10', - }, - }, - ], - { - networkPassphrase: Networks.PUBLIC, - source: { accountId: params.txSource, sequence: '1' }, - baseFeePerOperation: '100', - timeout: 30, - }, - ); - - it('derives the signer XLM outflow from local simulation', async () => { - const { transactionService } = createMockTransactionService(); - const wallet = getTestWallet(); - const destination = generateStellarAddress(); - - jest - .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') - .mockResolvedValue([buildOnChainAccount(destination, 50)]); - - const result = await transactionService.deriveEstimatedChanges({ - transaction: buildNativePayment(wallet.address, destination), - onChainAccount: buildOnChainAccount(wallet.address, 500), - signerAddress: wallet.address, - }); - - expect(result.assets).toHaveLength(1); - expect(result.assets[0]).toMatchObject({ - type: AssetChangeDirection.Out, - value: 10, - symbol: 'XLM', - price: null, - }); - }); - - it('returns empty assets when the signer is not the transaction or fee source', async () => { - const { transactionService } = createMockTransactionService(); - const wallet = getTestWallet(); - const txSource = generateStellarAddress(); - const destination = generateStellarAddress(); - - jest - .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') - .mockResolvedValue([ - buildOnChainAccount(txSource, 500), - buildOnChainAccount(destination, 50), - ]); - - // Local simulation (send / change-trust) requires the wallet to be the - // transaction or fee source; an operation-source-only signer is rejected - // and the estimated-changes section stays hidden. - const result = await transactionService.deriveEstimatedChanges({ - transaction: buildNativePaymentWithTxSource({ - txSource, - operationSource: wallet.address, - destination, - }), - onChainAccount: buildOnChainAccount(wallet.address, 500), - signerAddress: wallet.address, - }); - - expect(result).toStrictEqual({ assets: [] }); - }); - - it('returns empty assets when the local simulation fails', async () => { - const { transactionService } = createMockTransactionService(); - const wallet = getTestWallet(); - const destination = generateStellarAddress(); - - // Destination is not loaded, so simulation throws and the section hides. - jest - .spyOn(NetworkService.prototype, 'loadOnChainAccountsSafe') - .mockResolvedValue([]); - - const result = await transactionService.deriveEstimatedChanges({ - transaction: buildNativePayment(wallet.address, destination), - onChainAccount: buildOnChainAccount(wallet.address, 500), - signerAddress: wallet.address, - }); - - expect(result).toStrictEqual({ assets: [] }); - }); - }); - describe('savePendingKeyringTransactionSafe', () => { it('returns saved transaction when savePendingKeyringTransaction succeeds', async () => { const { transactionService } = createMockTransactionService(); diff --git a/packages/snap/src/services/transaction/TransactionService.ts b/packages/snap/src/services/transaction/TransactionService.ts index f4201e77..4d6ebee4 100644 --- a/packages/snap/src/services/transaction/TransactionService.ts +++ b/packages/snap/src/services/transaction/TransactionService.ts @@ -5,13 +5,9 @@ import { import { emitSnapKeyringEvent } from '@metamask/keyring-snap-sdk'; import { groupBy } from 'lodash'; -import { - InsufficientBalanceException, - TransactionValidationException, -} from './exceptions'; +import { InsufficientBalanceException } from './exceptions'; import type { KeyringTransactionRequest } from './KeyringTransactionBuilder'; import { KeyringTransactionBuilder } from './KeyringTransactionBuilder'; -import { mapSimulationToEstimatedChanges } from './mapSimulationToEstimatedChanges'; import { Transaction } from './Transaction'; import type { TransactionBuilder } from './TransactionBuilder'; import { TransactionMapper } from './TransactionMapper'; @@ -34,10 +30,7 @@ import { getSnapProvider, isSep41Id, isSlip44Id } from '../../utils'; import type { ILogger } from '../../utils/logger'; import { createPrefixedLogger } from '../../utils/logger'; import type { AccountService } from '../account'; -import type { - AssetMetadataService, - StellarAssetMetadata, -} from '../asset-metadata'; +import type { StellarAssetMetadata } from '../asset-metadata'; import type { NetworkService } from '../network'; import { AccountNotActivatedException, @@ -45,7 +38,6 @@ import { } from '../network/exceptions'; import type { OnChainAccount } from '../on-chain-account/OnChainAccount'; import type { ActivatedAccountPair } from '../sync/api'; -import type { TransactionScanEstimatedChanges } from '../transaction-scan'; import type { Wallet } from '../wallet'; export class TransactionService { @@ -61,28 +53,23 @@ export class TransactionService { readonly #transactionSynchronizeService: TransactionSynchronizeService; - readonly #assetMetadataService: AssetMetadataService; - constructor({ logger, transactionRepository, networkService, transactionBuilder, accountService, - assetMetadataService, }: { logger: ILogger; transactionRepository: TransactionRepository; networkService: NetworkService; transactionBuilder: TransactionBuilder; accountService: AccountService; - assetMetadataService: AssetMetadataService; }) { this.#logger = createPrefixedLogger(logger, '[๐Ÿงพ TransactionService]'); this.#transactionRepository = transactionRepository; this.#networkService = networkService; this.#transactionBuilder = transactionBuilder; - this.#assetMetadataService = assetMetadataService; this.#keyringTransactionBuilder = new KeyringTransactionBuilder(); const transactionMapper = new TransactionMapper({ keyringTransactionBuilder: this.#keyringTransactionBuilder, @@ -545,71 +532,6 @@ export class TransactionService { simulator.simulate(transaction, onChainAccount, options); } - /** - * Derives the signer's estimated balance changes from a local on-chain - * simulation, mapped into the {@link TransactionScanEstimatedChanges} shape the - * confirmation UI renders. Seeds the send / change-trust confirmation - * fund-flow breakdown locally (these flows never use remote Blockaid - * simulation). - * - * The network fee is excluded from the per-asset rows (the diff baselines on - * the post-fee simulation snapshot); it is surfaced separately as the fee row. - * - * Best-effort: any failure (unsupported operation, unknown destination - * account, Soroban invoke producing no modeled deltas, etc.) resolves to an - * empty result so the UI simply hides the section. - * - * @param params - The parameters. - * @param params.transaction - The transaction with fee already applied. - * @param params.onChainAccount - The loaded signing account. - * @param params.signerAddress - The Stellar address whose changes are surfaced. - * @returns The estimated changes, or `{ assets: [] }` when they cannot be derived. - */ - async deriveEstimatedChanges(params: { - transaction: Transaction; - onChainAccount: OnChainAccount; - signerAddress: string; - }): Promise { - const { transaction, onChainAccount, signerAddress } = params; - try { - const preloadedAccounts = await this.#getPreloadedAccounts( - transaction, - onChainAccount, - ); - - const simulator = new TransactionSimulator(); - // The simulation stack is the post-fee snapshot first, then one entry per - // operation; diffing the endpoints excludes the network fee from per-asset - // rows (it is surfaced separately). - const states = simulator.simulate(transaction, onChainAccount, { - preloadedAccounts, - }); - const initialState = states[0]; - const finalState = states[states.length - 1]; - if (initialState === undefined || finalState === undefined) { - throw new TransactionValidationException( - 'Simulation produced no states', - ); - } - - const assets = await mapSimulationToEstimatedChanges({ - initialState, - finalState, - signerAddress, - scope: transaction.scope, - assetMetadataService: this.#assetMetadataService, - }); - - return { assets }; - } catch (error) { - this.#logger.logErrorWithDetails( - 'Failed to derive estimated balance changes', - error, - ); - return { assets: [] }; - } - } - /** * Submits a signed transaction. * When the transaction fails with `txBadSeq`, reloads the account sequence, rebuilds, re-signs once, and retries diff --git a/packages/snap/src/services/transaction/TransactionSimulator.ts b/packages/snap/src/services/transaction/TransactionSimulator.ts index adc108be..6b59c06e 100644 --- a/packages/snap/src/services/transaction/TransactionSimulator.ts +++ b/packages/snap/src/services/transaction/TransactionSimulator.ts @@ -182,8 +182,8 @@ export class TransactionSimulator { // Ensure the transaction scope matches the account scope. assertTransactionScope(transaction, account.scope); - // The wallet account must be the transaction or fee source for the local - // simulation paths (send / change-trust). + // Envelope must involve this wallet as source or fee source (API XDR or in-app builds). + // TODO: we may need to relax it in future when we support fee payment by other account. assertTransactionSourceAccount(transaction, account.accountId); // Soroban `invokeHostFunction` is only allowed as a single-op tx and is a no-op for state. @@ -297,9 +297,9 @@ export class TransactionSimulator { }): SimulationState { // Assume the state is cloned beforehand const { state, feeSource, fee } = params; - // The fee source must be present in the simulation state. For - // sign-transaction estimated changes, external fee sources are preloaded - // from the transaction's participating accounts. + // it is possible that the transaction fee source is different than the wallet user, + // if the transaction is passed from external, we dont support it yet, + // hence `getAccount` will throw an error. const feePayer = getAccount(state, feeSource); const spendable = getSpendableNative(feePayer); diff --git a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts index 0d72140a..30c5cbe3 100644 --- a/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts +++ b/packages/snap/src/services/transaction/__mocks__/transaction.fixtures.ts @@ -16,7 +16,6 @@ import type { KnownCaip19AssetIdOrSlip44Id } from '../../../api'; import { KnownCaip2ChainId } from '../../../api'; import { getSlip44AssetId, logger } from '../../../utils'; import { mockAccountService } from '../../account/__mocks__/account.fixtures'; -import { createMockAssetMetadataService } from '../../asset-metadata/__mocks__/assets.fixtures'; import { createMemoryCache } from '../../cache/__mocks__/cache.fixtures'; import { NetworkService } from '../../network'; import { State } from '../../state/State'; @@ -31,7 +30,6 @@ export const createMockTransactionService = () => { const networkService = new NetworkService({ logger, cache }); const transactionBuilder = new TransactionBuilder({ logger }); const { accountService } = mockAccountService(); - const { service: assetMetadataService } = createMockAssetMetadataService(); const transactionService = new TransactionService({ logger, transactionRepository: new TransactionRepository( @@ -46,7 +44,6 @@ export const createMockTransactionService = () => { networkService, transactionBuilder, accountService, - assetMetadataService, }); const transactionRepositorySaveSpy = jest.spyOn( diff --git a/packages/snap/src/services/transaction/index.ts b/packages/snap/src/services/transaction/index.ts index 12535efc..f1d557c0 100644 --- a/packages/snap/src/services/transaction/index.ts +++ b/packages/snap/src/services/transaction/index.ts @@ -6,5 +6,4 @@ export * from './TransactionRepository'; export * from './TransactionService'; export * from './TransactionSimulator'; export * from './KeyringTransactionBuilder'; -export * from './mapSimulationToEstimatedChanges'; export * from './api'; diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts deleted file mode 100644 index 14d9ab76..00000000 --- a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.test.ts +++ /dev/null @@ -1,129 +0,0 @@ -import { BigNumber } from 'bignumber.js'; - -import { mapSimulationToEstimatedChanges } from './mapSimulationToEstimatedChanges'; -import type { AccountState, SimulationState } from './simulation'; -import { KnownCaip2ChainId } from '../../api'; -import { toCaip19ClassicAssetId } from '../../utils'; -import type { AssetMetadataService } from '../asset-metadata'; -import { AssetChangeDirection } from '../transaction-scan'; - -describe('mapSimulationToEstimatedChanges', () => { - const scope = KnownCaip2ChainId.Mainnet; - const signerAddress = - 'GDPMFLKUGASUTWBN2XGYYKD27QGHCYH4BUFUTER4L23INYQ4JHDWFOIE'; - const usdcIssuer = 'GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN'; - - const assetMetadataService = { - resolve: jest.fn(), - } as unknown as AssetMetadataService; - - function buildAccountState(overrides: Partial): AccountState { - return { - nativeRawBalance: new BigNumber(0), - subentryCount: 0, - numSponsoring: 0, - numSponsored: 0, - requiresMemo: false, - trustlines: new Map(), - sep41Balances: new Map(), - ...overrides, - }; - } - - function buildState(accountState: AccountState): SimulationState { - return { accounts: new Map([[signerAddress, accountState]]) }; - } - - it('maps a native XLM outflow excluding the fee (post-fee baseline)', async () => { - // Both snapshots are post-fee, so the only diff is the payment amount. - const result = await mapSimulationToEstimatedChanges({ - initialState: buildState( - buildAccountState({ nativeRawBalance: new BigNumber('1000000000') }), - ), - finalState: buildState( - buildAccountState({ nativeRawBalance: new BigNumber('900000000') }), - ), - signerAddress, - scope, - assetMetadataService, - }); - - expect(result).toHaveLength(1); - expect(result[0]).toMatchObject({ - type: AssetChangeDirection.Out, - value: 10, - price: null, - symbol: 'XLM', - logo: null, - }); - expect(assetMetadataService.resolve).not.toHaveBeenCalled(); - }); - - it('maps a classic asset payment from the signer trustline', async () => { - const usdc = toCaip19ClassicAssetId(scope, 'USDC', usdcIssuer); - const trustline = { - limit: new BigNumber('1000000000000'), - authorized: true, - sponsored: false, - }; - - const result = await mapSimulationToEstimatedChanges({ - initialState: buildState( - buildAccountState({ - trustlines: new Map([ - [usdc, { ...trustline, balance: new BigNumber('5000000') }], - ]), - }), - ), - finalState: buildState( - buildAccountState({ - trustlines: new Map([ - [usdc, { ...trustline, balance: new BigNumber('3000000') }], - ]), - }), - ), - signerAddress, - scope, - assetMetadataService, - }); - - expect(result).toStrictEqual([ - { - type: AssetChangeDirection.Out, - value: 0.2, - price: null, - symbol: 'USDC', - name: 'USDC', - logo: expect.any(String), - }, - ]); - }); - - it('returns an empty array when there are no balance changes', async () => { - const accountState = buildAccountState({ - nativeRawBalance: new BigNumber('1000000000'), - }); - - const result = await mapSimulationToEstimatedChanges({ - initialState: buildState(accountState), - finalState: buildState(accountState), - signerAddress, - scope, - assetMetadataService, - }); - - expect(result).toStrictEqual([]); - }); - - it('returns an empty array when the signer is absent from a snapshot', async () => { - const result = await mapSimulationToEstimatedChanges({ - initialState: { accounts: new Map() }, - finalState: { accounts: new Map() }, - signerAddress, - scope, - assetMetadataService, - }); - - expect(result).toStrictEqual([]); - }); -}); diff --git a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts b/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts deleted file mode 100644 index 95a448f3..00000000 --- a/packages/snap/src/services/transaction/mapSimulationToEstimatedChanges.ts +++ /dev/null @@ -1,158 +0,0 @@ -import { parseCaipAssetType } from '@metamask/utils'; -import { BigNumber } from 'bignumber.js'; - -import type { SimulationState } from './simulation'; -import type { - KnownCaip19ClassicAssetId, - KnownCaip19Sep41AssetId, - KnownCaip2ChainId, -} from '../../api'; -import { STELLAR_DECIMAL_PLACES } from '../../constants'; -import { parseClassicAssetCodeIssuer } from '../../utils'; -import { normalizeAmount } from '../../utils/currency'; -import type { AssetMetadataService } from '../asset-metadata'; -import { getIconUrl, getNativeAssetMetadata } from '../asset-metadata/utils'; -import { AssetChangeDirection } from '../transaction-scan'; -import type { TransactionScanAssetChange } from '../transaction-scan'; - -/** - * Builds a single estimated-change row from a signed balance delta. - * - * @param params - Row inputs. - * @param params.delta - Signed balance delta in smallest units (negative = outflow). - * @param params.decimals - Asset decimals used to normalize the raw delta. - * @param params.symbol - Display ticker. - * @param params.name - Display name. - * @param params.logo - Icon URL, or null when unknown. - * @returns A {@link TransactionScanAssetChange} with a human-readable value. - */ -function buildChange(params: { - delta: BigNumber; - decimals: number; - symbol: string; - name: string; - logo: string | null; -}): TransactionScanAssetChange { - const { delta, decimals, symbol, name, logo } = params; - return { - type: delta.isNegative() - ? AssetChangeDirection.Out - : AssetChangeDirection.In, - value: normalizeAmount(delta.abs(), decimals).toNumber(), - price: null, - symbol, - name, - logo, - }; -} - -/** - * Diffs the signer's account between two local simulation snapshots and maps the - * non-zero balance deltas into the {@link TransactionScanAssetChange} shape the - * confirmation UI renders. - * - * Native XLM and classic (trustline) assets are diffed synchronously; SEP-41 - * contract tokens resolve their metadata via {@link AssetMetadataService}. The - * caller is expected to pass the post-fee `initialState` so the network fee is - * excluded from the resulting rows. - * - * @param params - Mapping inputs. - * @param params.initialState - Post-fee simulation snapshot (baseline). - * @param params.finalState - Snapshot after all operations are applied. - * @param params.signerAddress - Stellar address whose balance changes are surfaced. - * @param params.scope - CAIP-2 chain of the transaction. - * @param params.assetMetadataService - Resolver for SEP-41 token metadata. - * @returns The non-zero asset changes for the signer, or an empty array when the - * signer is not present in both snapshots. - */ -export async function mapSimulationToEstimatedChanges(params: { - initialState: SimulationState; - finalState: SimulationState; - signerAddress: string; - scope: KnownCaip2ChainId; - assetMetadataService: AssetMetadataService; -}): Promise { - const { - initialState, - finalState, - signerAddress, - scope, - assetMetadataService, - } = params; - - const initialAccount = initialState.accounts.get(signerAddress); - const finalAccount = finalState.accounts.get(signerAddress); - if (initialAccount === undefined || finalAccount === undefined) { - return []; - } - - const changes: TransactionScanAssetChange[] = []; - - // Native XLM. - const nativeDelta = finalAccount.nativeRawBalance.minus( - initialAccount.nativeRawBalance, - ); - if (!nativeDelta.isZero()) { - const meta = getNativeAssetMetadata(scope); - changes.push( - buildChange({ - delta: nativeDelta, - decimals: meta.units[0].decimals, - symbol: meta.symbol, - name: meta.name ?? meta.symbol, - logo: null, - }), - ); - } - - // Classic (trustline) assets. - const classicAssetIds = new Set([ - ...initialAccount.trustlines.keys(), - ...finalAccount.trustlines.keys(), - ]); - for (const assetId of classicAssetIds) { - const before = initialAccount.trustlines.get(assetId)?.balance ?? null; - const after = finalAccount.trustlines.get(assetId)?.balance ?? null; - const delta = (after ?? new BigNumber(0)).minus(before ?? new BigNumber(0)); - if (delta.isZero()) { - continue; - } - const { assetReference } = parseCaipAssetType(assetId); - const { assetCode } = parseClassicAssetCodeIssuer(assetReference); - changes.push( - buildChange({ - delta, - decimals: STELLAR_DECIMAL_PLACES, - symbol: assetCode, - name: assetCode, - logo: getIconUrl(assetId), - }), - ); - } - - // SEP-41 contract tokens. - const sep41AssetIds = new Set([ - ...initialAccount.sep41Balances.keys(), - ...finalAccount.sep41Balances.keys(), - ]); - for (const assetId of sep41AssetIds) { - const before = initialAccount.sep41Balances.get(assetId) ?? null; - const after = finalAccount.sep41Balances.get(assetId) ?? null; - const delta = (after ?? new BigNumber(0)).minus(before ?? new BigNumber(0)); - if (delta.isZero()) { - continue; - } - const meta = await assetMetadataService.resolve(assetId); - changes.push( - buildChange({ - delta, - decimals: meta.units[0].decimals, - symbol: meta.symbol, - name: meta.name ?? meta.symbol, - logo: meta.iconUrl ?? null, - }), - ); - } - - return changes; -} From 26fd368af4d2dade07545d6f5fb8a4d1a904f324 Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Thu, 25 Jun 2026 18:26:43 +0200 Subject: [PATCH 11/13] fix: fix comments --- packages/site/.env.development | 2 + packages/snap/snap.manifest.json | 2 +- .../scanRefresher.test.ts | 38 +++++++++ .../scanRefresher.ts | 52 +++++++----- .../handlers/keyring/signTransaction.test.ts | 81 +------------------ .../src/handlers/keyring/signTransaction.ts | 9 --- .../TransactionScanService.test.ts | 32 ++++++++ .../TransactionScanService.ts | 69 +++++++++++++--- 8 files changed, 166 insertions(+), 119 deletions(-) create mode 100644 packages/site/.env.development diff --git a/packages/site/.env.development b/packages/site/.env.development new file mode 100644 index 00000000..a5e6bf2e --- /dev/null +++ b/packages/site/.env.development @@ -0,0 +1,2 @@ +#SNAP_ORIGIN=npm:@metamask/stellar-wallet-snap +SNAP_ORIGIN=local:http://localhost:8080 diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 9399bb97..00630424 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": "UqNPBfbFj9FXqhvCI3x3BF9O4vfIDiT1N9Rp2g9rbp4=", + "shasum": "GyDO49yo1JCEBWrmkYPUnERiRay6fnB82Qd57f2jDTU=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts index 469af853..6f239960 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.test.ts @@ -210,6 +210,44 @@ describe('ConfirmationScanRefresher', () => { }); }); + 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); diff --git a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts index 82ebac90..4d3b9b5b 100644 --- a/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts +++ b/packages/snap/src/handlers/cronjob/refreshConfirmationContext/scanRefresher.ts @@ -85,12 +85,17 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher SecurityScanContext['securityScanRequest'] >; const options = this.#getScanOptions(scanCtx); - // Send / change-trust seed locally-simulated estimated changes here, which - // we keep when the validation-only remote scan returns no rows. Sign-txn has - // no local seed, so this is `{ assets: [] }` until remote simulation returns. - const fallbackEstimatedChanges = scanCtx.scan?.estimatedChanges ?? { + // 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({ @@ -100,9 +105,10 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher return { result: { - scan: this.#scanWithEstimatedChangesFallback( + scan: this.#resolveEstimatedChanges( scan, - fallbackEstimatedChanges, + localEstimatedChanges, + preferRemoteEstimatedChanges, ), scanFetchStatus: scan ? FetchStatus.Fetched : FetchStatus.Error, }, @@ -112,9 +118,10 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher this.#logger.error('Error refreshing confirmation security scan:', error); return { result: { - scan: this.#scanWithEstimatedChangesFallback( + scan: this.#resolveEstimatedChanges( null, - fallbackEstimatedChanges, + localEstimatedChanges, + preferRemoteEstimatedChanges, ), scanFetchStatus: FetchStatus.Error, }, @@ -128,28 +135,35 @@ export class ConfirmationScanRefresher implements IConfirmationContextRefresher } /** - * Merges a Blockaid scan result with the locally-seeded estimated changes. - * Remote estimated changes are preferred when Blockaid returns displayable - * asset rows; otherwise the locally-derived fallback stays on screen. + * 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 fallbackEstimatedChanges - The locally-seeded estimated changes. - * @returns A scan result carrying the best available estimated changes. + * @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. */ - #scanWithEstimatedChangesFallback( + #resolveEstimatedChanges( scan: TransactionScanResult | null, - fallbackEstimatedChanges: TransactionScanEstimatedChanges, + localEstimatedChanges: TransactionScanEstimatedChanges, + preferRemoteEstimatedChanges: boolean, ): TransactionScanResult { if (scan) { - const estimatedChanges = this.#hasEstimatedChanges(scan.estimatedChanges) - ? scan.estimatedChanges - : fallbackEstimatedChanges; + const estimatedChanges = + preferRemoteEstimatedChanges && + this.#hasEstimatedChanges(scan.estimatedChanges) + ? scan.estimatedChanges + : localEstimatedChanges; return { ...scan, estimatedChanges }; } return { status: 'ERROR', - estimatedChanges: fallbackEstimatedChanges, + estimatedChanges: localEstimatedChanges, validation: null, error: null, }; diff --git a/packages/snap/src/handlers/keyring/signTransaction.test.ts b/packages/snap/src/handlers/keyring/signTransaction.test.ts index 03835478..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`. @@ -226,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(); @@ -287,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 cc7174d2..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 = diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts index 364bf7a7..0c269a8c 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts @@ -94,6 +94,38 @@ describe('TransactionScanService', () => { }); }); + 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({ diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.ts index ca1fa1f7..81e7f916 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -14,8 +14,14 @@ 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; @@ -55,7 +61,7 @@ export class TransactionScanService { options, }); - return this.#mapScan(result, options); + return this.#mapScan(result, options, scope); } catch (error: unknown) { this.#logger.warn('Error scanning Stellar transaction', { reason: error, @@ -70,6 +76,7 @@ export class TransactionScanService { #mapScan( result: StellarTransactionScanResponse, options: TransactionScanOption[], + scope: KnownCaip2ChainId, ): TransactionScanResult { const simulation = result.simulation ?? null; const validation = result.validation ?? null; @@ -95,6 +102,7 @@ export class TransactionScanService { simulation?.status === 'Success' ? this.#mapAssetChanges( simulation.account_summary.account_assets_diffs ?? [], + scope, ) : [], }, @@ -108,14 +116,19 @@ export class TransactionScanService { #mapAssetChanges( assetDiffs: StellarAssetDiff[], + scope: KnownCaip2ChainId, ): TransactionScanAssetChange[] { return assetDiffs.flatMap((assetDiff) => { const changes: TransactionScanAssetChange[] = []; if (assetDiff.out) { - changes.push(this.#mapAssetChange(assetDiff, AssetChangeDirection.Out)); + changes.push( + this.#mapAssetChange(assetDiff, AssetChangeDirection.Out, scope), + ); } if (assetDiff.in) { - changes.push(this.#mapAssetChange(assetDiff, AssetChangeDirection.In)); + changes.push( + this.#mapAssetChange(assetDiff, AssetChangeDirection.In, scope), + ); } return changes; }); @@ -124,6 +137,7 @@ export class TransactionScanService { #mapAssetChange( assetDiff: StellarAssetDiff, type: AssetChangeDirection, + scope: KnownCaip2ChainId, ): TransactionScanAssetChange { const transfer = assetDiff[type]; const symbol = @@ -133,12 +147,41 @@ export class TransactionScanService { type, symbol, name: assetDiff.asset.name ?? symbol, - logo: 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. * Prefers {@link StellarAssetTransferDetails.raw_value} with known decimals @@ -172,17 +215,23 @@ export class TransactionScanService { * 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. We key off the canonical - * top-level `asset_type` (`NATIVE` / `ASSET`) โ€” the nested `asset.type` is the - * same classification and adds no information. + * 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 } = assetDiff; + const { asset_type: assetType, asset } = assetDiff; - if (assetType === 'NATIVE' || assetType === 'ASSET') { + if ( + assetType === 'NATIVE' || + asset.type === 'NATIVE' || + assetType === 'ASSET' || + asset.type === 'ASSET' + ) { return STELLAR_DECIMAL_PLACES; } From bf9dd37327f12bf86a14c0219dbbce725d9749fb Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Thu, 25 Jun 2026 21:42:11 +0200 Subject: [PATCH 12/13] fix: surface Blockaid estimated changes and clarify scan error copy --- packages/snap/locales/en.json | 2 +- packages/snap/messages.json | 2 +- packages/snap/snap.manifest.json | 2 +- .../TransactionScanService.test.ts | 44 +++++++++++++++++-- .../TransactionScanService.ts | 38 ++++++++++++++-- .../components/TransactionAlert.test.tsx | 2 +- 6 files changed, 79 insertions(+), 11 deletions(-) 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 00630424..7100130e 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": "GyDO49yo1JCEBWrmkYPUnERiRay6fnB82Qd57f2jDTU=", + "shasum": "lz/x+P2KtK7HCEJkv5F/DK8bbia13d44IUCyqWNlO7s=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts index 0c269a8c..49497b08 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.test.ts @@ -94,6 +94,39 @@ 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'; @@ -148,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: { @@ -169,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', }, }); }); diff --git a/packages/snap/src/services/transaction-scan/TransactionScanService.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.ts index 81e7f916..0de1fd80 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -61,7 +61,7 @@ export class TransactionScanService { options, }); - return this.#mapScan(result, options, scope); + return this.#mapScan(result, options, scope, accountAddress); } catch (error: unknown) { this.#logger.warn('Error scanning Stellar transaction', { reason: error, @@ -77,6 +77,7 @@ export class TransactionScanService { result: StellarTransactionScanResponse, options: TransactionScanOption[], scope: KnownCaip2ChainId, + accountAddress: string, ): TransactionScanResult { const simulation = result.simulation ?? null; const validation = result.validation ?? null; @@ -93,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', @@ -101,7 +107,7 @@ export class TransactionScanService { assets: simulation?.status === 'Success' ? this.#mapAssetChanges( - simulation.account_summary.account_assets_diffs ?? [], + this.#getSignerAssetDiffs(simulation, accountAddress), scope, ) : [], @@ -114,6 +120,32 @@ 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, 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', }); }); From 6fc41be8fe2b4f26b8133fab14b58c113e1263ba Mon Sep 17 00:00:00 2001 From: Amine Harty Date: Thu, 25 Jun 2026 21:51:31 +0200 Subject: [PATCH 13/13] fix: comments --- packages/snap/snap.manifest.json | 2 +- packages/snap/src/constants.ts | 3 +- .../TransactionScanService.ts | 8 ++-- .../transaction/TransactionSimulator.test.ts | 45 ------------------- .../EstimatedChanges/EstimatedChanges.tsx | 4 +- .../snap/src/ui/confirmation/utils.test.ts | 8 ++-- packages/snap/src/ui/confirmation/utils.ts | 5 ++- .../ConfirmSignTransaction.tsx | 16 +++---- 8 files changed, 24 insertions(+), 67 deletions(-) diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 7100130e..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": "lz/x+P2KtK7HCEJkv5F/DK8bbia13d44IUCyqWNlO7s=", + "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/services/transaction-scan/TransactionScanService.ts b/packages/snap/src/services/transaction-scan/TransactionScanService.ts index 0de1fd80..44e3b31b 100644 --- a/packages/snap/src/services/transaction-scan/TransactionScanService.ts +++ b/packages/snap/src/services/transaction-scan/TransactionScanService.ts @@ -216,9 +216,11 @@ export class TransactionScanService { /** * Computes the human-readable amount for an asset transfer. - * Prefers {@link StellarAssetTransferDetails.raw_value} with known decimals - * (Tron parity) because Blockaid's `value` can be imprecise for fractional - * native XLM amounts. + * + * 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). diff --git a/packages/snap/src/services/transaction/TransactionSimulator.test.ts b/packages/snap/src/services/transaction/TransactionSimulator.test.ts index a9ba745a..98b26d85 100644 --- a/packages/snap/src/services/transaction/TransactionSimulator.test.ts +++ b/packages/snap/src/services/transaction/TransactionSimulator.test.ts @@ -280,51 +280,6 @@ const destinationAddress = generateStellarAddress(); describe('TransactionSimulator', () => { const simulator = new TransactionSimulator(); - describe('simulate endpoints', () => { - it('returns the post-fee initial state and a final state excluding the fee', () => { - const wallet = getTestWallet(); - const onChainAccount = onChainFromMockBalances(wallet.address, '1', { - nativeBalance: 500, - subentryCount: 0, - assets: [], - }); - - const tx = buildMockClassicTransaction( - [ - { - type: 'payment', - params: { - source: wallet.address, - destination: destinationAddress, - asset: 'native', - amount: '10', - }, - }, - ], - mainnetSimulatorTxOptions(wallet.address, '1'), - ); - - const states = simulator.simulate(tx, onChainAccount, { - preloadedAccounts: [destOnChainAccount(destinationAddress)], - }); - const initialState = states[0]; - const finalState = states[states.length - 1]; - - const initialBalance = initialState?.accounts.get( - wallet.address, - )?.nativeRawBalance; - const finalBalance = finalState?.accounts.get( - wallet.address, - )?.nativeRawBalance; - - // initialState is the post-fee snapshot: 500 XLM minus the 100-stroop fee. - expect(initialBalance?.toFixed()).toBe('4999999900'); - // finalState applies the 10 XLM (1e8 stroops) payment on top of that, so - // the signer delta excludes the fee (already baked into the baseline). - expect(finalBalance?.toFixed()).toBe('4899999900'); - }); - }); - describe('preflight validation', () => { it('throws when account scope does not match transaction network', () => { const wallet = getTestWallet(); diff --git a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx index 4e677cb2..69013de9 100644 --- a/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx +++ b/packages/snap/src/ui/confirmation/components/EstimatedChanges/EstimatedChanges.tsx @@ -23,7 +23,7 @@ import type { Locale } from '../../../../utils'; import { i18n } from '../../../../utils'; import { xlmIcon } from '../../../images'; import { FetchStatus } from '../../api'; -import { isFetchStatusLoadingOrFetching } from '../../utils'; +import { isFetchInProgress } from '../../utils'; type EstimatedChangesProps = { changes: TransactionScanEstimatedChanges | null; @@ -158,7 +158,7 @@ export const EstimatedChanges = ({ scanFetchStatus, }: EstimatedChangesProps): ComponentOrElement => { const t = i18n(preferences.locale as Locale); - const isFetching = isFetchStatusLoadingOrFetching(scanFetchStatus); + 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 diff --git a/packages/snap/src/ui/confirmation/utils.test.ts b/packages/snap/src/ui/confirmation/utils.test.ts index a13a911a..ed0e0e98 100644 --- a/packages/snap/src/ui/confirmation/utils.test.ts +++ b/packages/snap/src/ui/confirmation/utils.test.ts @@ -5,7 +5,7 @@ import { import { FetchStatus } from './api'; import { ConfirmationBanner, - isFetchStatusLoadingOrFetching, + isFetchInProgress, isLocalTransactionValidationFailed, isRemoteTransactionScanLoading, requiresMaliciousAcknowledgement, @@ -24,18 +24,18 @@ const warningScan = { }; describe('confirmation utils', () => { - describe('isFetchStatusLoadingOrFetching', () => { + describe('isFetchInProgress', () => { it.each([FetchStatus.Initial, FetchStatus.Fetching])( 'returns true for %s', (status) => { - expect(isFetchStatusLoadingOrFetching(status)).toBe(true); + expect(isFetchInProgress(status)).toBe(true); }, ); it.each([FetchStatus.Fetched, FetchStatus.Error])( 'returns false for %s', (status) => { - expect(isFetchStatusLoadingOrFetching(status)).toBe(false); + expect(isFetchInProgress(status)).toBe(false); }, ); }); diff --git a/packages/snap/src/ui/confirmation/utils.ts b/packages/snap/src/ui/confirmation/utils.ts index 5b5cc561..c411d2ad 100644 --- a/packages/snap/src/ui/confirmation/utils.ts +++ b/packages/snap/src/ui/confirmation/utils.ts @@ -169,12 +169,13 @@ export function formatFeeData( } /** - * Returns true while a fetch is still in flight (initial or actively fetching). + * 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 isFetchStatusLoadingOrFetching(status: FetchStatus): boolean { +export function isFetchInProgress(status: FetchStatus): boolean { return status === FetchStatus.Initial || status === FetchStatus.Fetching; } diff --git a/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx index bc261f39..1bd9de4d 100644 --- a/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx +++ b/packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx @@ -199,6 +199,14 @@ export const ConfirmSignTransaction = ({ {null} + {preferences.simulateOnChainActions ? ( + + ) : null} +
{origin ? ( @@ -243,14 +251,6 @@ export const ConfirmSignTransaction = ({ ))}
- {preferences.simulateOnChainActions ? ( - - ) : null} -
{readableTransaction.operations.map((operationJson, index) => (