From f241d4df730b8691588d4936e29ca70e30e82384 Mon Sep 17 00:00:00 2001 From: Andrew Taran Date: Wed, 4 Nov 2026 18:43:09 +0100 Subject: [PATCH 1/2] fix(tron): align security warnings and scan preferences --- packages/tron-wallet-snap/CHANGELOG.md | 5 + packages/tron-wallet-snap/locales/en.json | 6 + packages/tron-wallet-snap/locales/es.json | 6 + packages/tron-wallet-snap/messages.json | 6 + .../src/handlers/cronjob/cronjob.test.tsx | 44 ++++++ .../src/handlers/cronjob/cronjob.tsx | 69 +++++---- .../utils/getTransactionAlertProps.ts | 7 +- .../ConfirmSignTransaction.test.tsx | 52 +++++-- .../ConfirmSignTransaction/render.test.tsx | 144 +++++++++++++++++- .../views/ConfirmSignTransaction/render.tsx | 59 +++---- .../ConfirmTransactionRequest.test.tsx | 81 ++++++++-- .../ConfirmTransactionRequest/render.test.tsx | 76 ++++++++- .../ConfirmTransactionRequest/render.tsx | 30 ++-- 13 files changed, 491 insertions(+), 94 deletions(-) diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index 5960cd5e5..e405fbac6 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -7,10 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Align transaction security warning copy and scan-loading behavior with user preferences. ([#557](https://github.com/MetaMask/internal-snaps/pull/557)) + ## [4.3.1] ### Fixed +- Align transaction security warning copy and scan-loading behavior with user preferences. ([#557](https://github.com/MetaMask/internal-snaps/pull/557)) - Allow transaction confirmation after remote simulation failures while retaining security warnings and checks for locally invalid transactions. ([#552](https://github.com/MetaMask/internal-snaps/pull/552)) ## [4.3.0] diff --git a/packages/tron-wallet-snap/locales/en.json b/packages/tron-wallet-snap/locales/en.json index fe137fb7e..8b034253b 100644 --- a/packages/tron-wallet-snap/locales/en.json +++ b/packages/tron-wallet-snap/locales/en.json @@ -85,6 +85,12 @@ "confirmation.validationErrorSubtitle": { "message": "If you approve this request, a third party known for scams will take all your assets." }, + "confirmation.validationWarningTitle": { + "message": "This request may be risky" + }, + "confirmation.validationWarningSubtitle": { + "message": "Only continue if you trust this site and/or every address involved." + }, "confirmation.validationErrorLearnMore": { "message": "Learn more" }, diff --git a/packages/tron-wallet-snap/locales/es.json b/packages/tron-wallet-snap/locales/es.json index 6533189db..91222369f 100644 --- a/packages/tron-wallet-snap/locales/es.json +++ b/packages/tron-wallet-snap/locales/es.json @@ -85,6 +85,12 @@ "confirmation.validationErrorSubtitle": { "message": "Si aprueba esta solicitud, un tercero conocido por estafas tomará todos sus activos." }, + "confirmation.validationWarningTitle": { + "message": "This request may be risky" + }, + "confirmation.validationWarningSubtitle": { + "message": "Only continue if you trust this site and/or every address involved." + }, "confirmation.validationErrorLearnMore": { "message": "Más información" }, diff --git a/packages/tron-wallet-snap/messages.json b/packages/tron-wallet-snap/messages.json index 8ab63c300..4c784c7fe 100644 --- a/packages/tron-wallet-snap/messages.json +++ b/packages/tron-wallet-snap/messages.json @@ -83,6 +83,12 @@ "confirmation.validationErrorSubtitle": { "message": "If you approve this request, a third party known for scams will take all your assets." }, + "confirmation.validationWarningTitle": { + "message": "This request may be risky" + }, + "confirmation.validationWarningSubtitle": { + "message": "Only continue if you trust this site and/or every address involved." + }, "confirmation.validationErrorLearnMore": { "message": "Learn more" }, diff --git a/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.test.tsx b/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.test.tsx index 3b8a096ca..191e316b0 100644 --- a/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.test.tsx +++ b/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.test.tsx @@ -506,6 +506,50 @@ async function withCronHandler( describe('CronHandler', () => { describe('refreshConfirmationSend', () => { + it('refreshes transaction validity without scanning when both scan preferences are disabled', async () => { + const context = buildMockInterfaceContext(); + const interfaceContext = { + ...context, + preferences: { + ...context.preferences, + useSecurityAlerts: false, + simulateOnChainActions: false, + }, + scanFetchStatus: FetchStatus.Fetched, + }; + + await withCronHandler( + { interfaceContext, refreshRawData: true }, + async ({ cronHandler, mockSnapClient, mockTransactionScanService }) => { + await cronHandler.refreshConfirmationSend(); + + expect( + mockTransactionScanService.scanTransaction, + ).not.toHaveBeenCalled(); + expect(mockSnapClient.updateInterface).toHaveBeenCalledTimes(2); + expect( + mockSnapClient.updateInterface.mock.calls[0]?.[2], + ).toMatchObject({ + scanFetchStatus: FetchStatus.Fetched, + }); + expect( + mockSnapClient.updateInterface.mock.calls[1]?.[2], + ).toMatchObject({ + scanFetchStatus: FetchStatus.Fetched, + transactionRawData: { + ref_block_bytes: getRefBlockBytes(200_000), + expiration: MOCK_BLOCK_TIMESTAMP + 60_000, + timestamp: MOCK_BLOCK_TIMESTAMP, + }, + }); + expect(mockSnapClient.scheduleBackgroundEvent).toHaveBeenCalledWith({ + method: BackgroundEventMethod.RefreshConfirmationSend, + duration: 'PT20S', + }); + }, + ); + }); + it('refreshes security scan and updates interface', async () => { await withCronHandler( async ({ cronHandler, mockSnapClient, mockTransactionScanService }) => { diff --git a/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.tsx b/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.tsx index ca23a1731..badc5dc10 100644 --- a/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.tsx +++ b/packages/tron-wallet-snap/src/handlers/cronjob/cronjob.tsx @@ -311,7 +311,7 @@ export class CronHandler { } /** - * Background job to refresh the security scan for simple send confirmation dialogs. + * Refresh transaction validity and enabled scans for send confirmation dialogs. */ async refreshConfirmationSend(): Promise { this.#logger.info( @@ -347,6 +347,11 @@ export class CronHandler { } const { preferences, scope, fromAddress, origin } = interfaceContext; + + const { useSecurityAlerts, simulateOnChainActions } = preferences; + + const shouldRefreshScan = useSecurityAlerts || simulateOnChainActions; + const transactionRawData = interfaceContext.transactionRawData as JsonTransactionRawData | null; @@ -360,7 +365,9 @@ export class CronHandler { try { const fetchingContext: ConfirmTransactionRequestContext = { ...interfaceContext, - scanFetchStatus: FetchStatus.Fetching, + scanFetchStatus: shouldRefreshScan + ? FetchStatus.Fetching + : interfaceContext.scanFetchStatus, }; await this.#snapClient.updateInterface( @@ -369,16 +376,6 @@ export class CronHandler { fetchingContext, ); - const options: SecurityAlertsScanOption[] = ['simulation']; - if (preferences.useSecurityAlerts) { - options.push('validation'); - } - - const scanAccount = { - type: interfaceContext.accountType, - address: fromAddress, - } as ExtendedKeyringAccount; - let { scan } = interfaceContext; let { scanFetchStatus } = interfaceContext; let freshTransactionRawData = transactionRawData; @@ -390,16 +387,33 @@ export class CronHandler { rawData: transactionRawData, }); - scan = await this.#transactionScanService.scanTransaction({ - accountAddress: fromAddress, - transactionRawData: freshTransactionRawData, - origin, - scope, - options, - account: scanAccount, - }); - scanFetchStatus = scan ? FetchStatus.Fetched : FetchStatus.Error; - this.#logger.info('Successfully refreshed send confirmation scan'); + if (shouldRefreshScan) { + const options: SecurityAlertsScanOption[] = []; + + if (simulateOnChainActions) { + options.push('simulation'); + } + + if (useSecurityAlerts) { + options.push('validation'); + } + + const scanAccount = { + type: interfaceContext.accountType, + address: fromAddress, + } as ExtendedKeyringAccount; + + scan = await this.#transactionScanService.scanTransaction({ + accountAddress: fromAddress, + transactionRawData: freshTransactionRawData, + origin, + scope, + options, + account: scanAccount, + }); + scanFetchStatus = scan ? FetchStatus.Fetched : FetchStatus.Error; + this.#logger.info('Successfully refreshed send confirmation scan'); + } } catch (error) { this.#logger.error('Error refreshing send confirmation scan:', error); scan = null; @@ -487,8 +501,9 @@ export class CronHandler { const { preferences, scope, account, origin, transaction } = interfaceContext; - const shouldRefreshScan = - preferences.simulateOnChainActions || preferences.useSecurityAlerts; + const { useSecurityAlerts, simulateOnChainActions } = preferences; + + const shouldRefreshScan = simulateOnChainActions || useSecurityAlerts; try { const fetchingContext: ConfirmSignTransactionContext = { @@ -531,10 +546,12 @@ export class CronHandler { if (shouldRefreshScan && rawData) { const options: SecurityAlertsScanOption[] = []; - if (preferences.simulateOnChainActions) { + + if (simulateOnChainActions) { options.push('simulation'); } - if (preferences.useSecurityAlerts) { + + if (useSecurityAlerts) { options.push('validation'); } diff --git a/packages/tron-wallet-snap/src/ui/confirmation/utils/getTransactionAlertProps.ts b/packages/tron-wallet-snap/src/ui/confirmation/utils/getTransactionAlertProps.ts index a0ffe66c0..52223589e 100644 --- a/packages/tron-wallet-snap/src/ui/confirmation/utils/getTransactionAlertProps.ts +++ b/packages/tron-wallet-snap/src/ui/confirmation/utils/getTransactionAlertProps.ts @@ -4,7 +4,6 @@ import type { TransactionScanResult } from '../../../services/transaction-scan/t import { FetchStatus } from '../../../types/snap'; import type { Preferences } from '../../../types/snap'; import { i18n } from '../../../utils/i18n'; -import { isFetchStatusLoadingOrFetching } from '../../../utils/isFetchStatusLoadingOrFetching'; import { getErrorMessage } from './getErrorMessage'; /** @@ -40,14 +39,14 @@ export function getTransactionAlertProps({ scanFailedMessage: translate('confirmation.simulationMessageAPIError'), maliciousTitle: translate('confirmation.validationErrorTitle'), maliciousMessage: translate('confirmation.validationErrorSubtitle'), - warningTitle: translate('confirmation.validationErrorTitle'), - warningMessage: translate('confirmation.validationErrorSubtitle'), + warningTitle: translate('confirmation.validationWarningTitle'), + warningMessage: translate('confirmation.validationWarningSubtitle'), learnMore: translate('confirmation.validationErrorLearnMore'), securityAdvisedBy: translate( 'confirmation.validationErrorSecurityAdviced', ), }, - isFetching: isFetchStatusLoadingOrFetching(scanFetchStatus), + isFetching: scanFetchStatus === FetchStatus.Loading, isFetchError: scanFetchStatus === FetchStatus.Error, error: scanError ? { diff --git a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.test.tsx b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.test.tsx index 668153bec..341213580 100644 --- a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.test.tsx +++ b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.test.tsx @@ -301,15 +301,18 @@ describe('ConfirmSignTransaction', () => { return { texts: collectTexts(tree), serialized: JSON.stringify(tree) }; }; - it.each([FetchStatus.Loading, FetchStatus.Fetching])( - 'renders the scan-in-progress info banner while the scan is %s', - (scanFetchStatus) => { + it.each([ + [FetchStatus.Loading, true], + [FetchStatus.Fetching, false], + ])( + 'sets loading banner visibility when the scan is %s', + (scanFetchStatus, visible) => { const { texts, serialized } = render( buildContext({ scanFetchStatus, scan: null }), ); - expect(texts).toContain(SCAN_IN_PROGRESS_TITLE); - expect(serialized).toContain('"severity":"info"'); + expect(texts.includes(SCAN_IN_PROGRESS_TITLE)).toBe(visible); + expect(serialized.includes('"severity":"info"')).toBe(visible); }, ); @@ -336,7 +339,7 @@ describe('ConfirmSignTransaction', () => { expect(serialized).toContain('"severity":"danger"'); }); - it('renders warning validations as a warning banner', () => { + it('renders the target warning', () => { const { texts, serialized } = render( buildContext({ scan: buildScanResult({ @@ -345,10 +348,34 @@ describe('ConfirmSignTransaction', () => { }), ); - expect(texts).toContain(MALICIOUS_TITLE); + expect(texts).toContain('This request may be risky'); + expect(texts).toContain( + 'Only continue if you trust this site and/or every address involved.', + ); + expect(serialized).toContain('"severity":"warning"'); + expect(serialized).not.toContain('"disabled":true'); }); + it.each([ + ['Warning', 'This request may be risky'], + ['Malicious', MALICIOUS_TITLE], + ] as const)( + 'preserves the %s presentation during background refresh', + (type, title) => { + const { texts, serialized } = render( + buildContext({ + scanFetchStatus: FetchStatus.Fetching, + scan: buildScanResult({ validation: { type, reason: null } }), + }), + ); + + expect(texts).not.toContain(SCAN_IN_PROGRESS_TITLE); + expect(serialized).not.toContain('"disabled":true'); + expect(texts).toContain(title); + }, + ); + it('renders scan errors before validation findings', () => { const { texts } = render( buildContext({ @@ -363,11 +390,14 @@ describe('ConfirmSignTransaction', () => { expect(texts).not.toContain(MALICIOUS_TITLE); }); - it('renders no banner for benign validations', () => { - const { serialized } = render(buildContext()); + it.each([FetchStatus.Fetched, FetchStatus.Fetching])( + 'renders no banner for benign validations when the scan is %s', + (scanFetchStatus) => { + const { serialized } = render(buildContext({ scanFetchStatus })); - expect(serialized).not.toContain('"type":"Banner"'); - }); + expect(serialized).not.toContain('"type":"Banner"'); + }, + ); }); it('shows unsupported-contract copy for skipped simulations of unknown contracts', () => { diff --git a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.test.tsx b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.test.tsx index b0e35c2ab..d4ce72899 100644 --- a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.test.tsx +++ b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.test.tsx @@ -241,9 +241,143 @@ async function withSnapContext( } describe('ConfirmSignTransaction render', () => { + it.each([ + [true, true, FetchStatus.Loading], + [true, false, FetchStatus.Loading], + [false, true, FetchStatus.Loading], + [false, false, FetchStatus.Fetched], + ])( + 'sets initial scan state with security alerts %s and simulation %s', + async (useSecurityAlerts, simulateOnChainActions, scanFetchStatus) => { + await withSnapContext(async ({ mockSnapClient }) => { + mockSnapClient.getPreferences.mockResolvedValue({ + ...mockPreferences, + useSecurityAlerts, + simulateOnChainActions, + }); + mockSnapClient.createInterface.mockImplementation( + async (ui, context) => { + expect(context).toMatchObject({ scanFetchStatus }); + expect(JSON.stringify(ui).includes('"disabled":true')).toBe( + scanFetchStatus === FetchStatus.Loading, + ); + return 'interface-id-123'; + }, + ); + + await render( + { + id: '00000000-0000-4000-8000-000000000001', + origin: 'https://example.com', + account: mockAccount.id, + scope: Network.Mainnet, + request: { + method: TronMultichainMethod.SignTransaction, + params: { + address: mockAccount.address, + transaction: { + rawDataHex: toHex('transaction'), + type: 'TransferContract', + }, + }, + }, + }, + mockAccount, + { + contract: [], + ref_block_bytes: '0000', + ref_block_hash: '0000000000000000', + expiration: 60000, + timestamp: 0, + }, + ); + + expect(mockSnapClient.createInterface).toHaveBeenCalledTimes(1); + }); + }, + ); + + it.each(['expiry', 'insufficient balance'])( + 'keeps local %s blocking on the initial screen with scans disabled', + async (blocker) => { + await withSnapContext( + async ({ + mockSnapClient, + mockTransactionExpirationRefresherService, + mockAssetsService, + }) => { + mockSnapClient.getPreferences.mockResolvedValue({ + ...mockPreferences, + useSecurityAlerts: false, + simulateOnChainActions: false, + }); + mockTransactionExpirationRefresherService.isTransactionExpired.mockResolvedValue( + blocker === 'expiry', + ); + if (blocker === 'insufficient balance') { + mockAssetsService.getAccountAssetsByIDs.mockResolvedValue([ + { ...mockAssets[0], rawAmount: '0' } as AssetEntity, + ...mockAssets.slice(1), + ]); + } + mockSnapClient.createInterface.mockImplementation( + async (ui, context) => { + expect(context).toMatchObject({ + scanFetchStatus: FetchStatus.Fetched, + }); + expect(JSON.stringify(ui)).toContain('"disabled":true'); + expect(JSON.stringify(ui)).not.toContain( + 'Checking for security issues', + ); + expect(context).toMatchObject( + blocker === 'expiry' + ? { scan: { error: { type: 'TransactionTaposExpired' } } } + : { isInsufficientBalance: true }, + ); + return 'interface-id-123'; + }, + ); + + await render( + { + id: '00000000-0000-4000-8000-000000000001', + origin: 'https://example.com', + account: mockAccount.id, + scope: Network.Mainnet, + request: { + method: TronMultichainMethod.SignTransaction, + params: { + address: mockAccount.address, + transaction: { + rawDataHex: toHex('transaction'), + type: 'TransferContract', + }, + }, + }, + }, + mockAccount, + { + contract: [], + ref_block_bytes: '0000', + ref_block_hash: '0000000000000000', + expiration: 60000, + timestamp: 0, + }, + ); + + expect(mockSnapClient.createInterface).toHaveBeenCalledTimes(1); + }, + ); + }, + ); + it('renders the confirmation dialog with security scan', async () => { await withSnapContext( - async ({ mockSnapClient, mockTransactionScanService }) => { + async ({ + mockSnapClient, + mockTransactionScanService, + mockTransactionExpirationRefresherService, + }) => { const testOrigin = 'https://example.com'; const testTransaction = toHex('mock-transaction-data'); @@ -305,6 +439,14 @@ describe('ConfirmSignTransaction render', () => { // Verify interface was updated with scan results expect(mockSnapClient.updateInterface).toHaveBeenCalled(); + // A transaction can expire while a remote scan is in flight. + expect( + mockTransactionExpirationRefresherService.isTransactionExpired.mock + .invocationCallOrder[0], + ).toBeGreaterThan( + mockTransactionScanService.scanTransaction.mock + .invocationCallOrder[0] ?? 0, + ); }, ); }); diff --git a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.tsx b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.tsx index 28c3e7d99..7be9966ab 100644 --- a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.tsx +++ b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmSignTransaction/render.tsx @@ -12,7 +12,6 @@ import { Network, Networks, ZERO } from '../../../../constants'; import snapContext from '../../../../context'; import { BackgroundEventMethod } from '../../../../handlers/cronjob/cronjob'; import { EXPIRED_TRANSACTION_SCAN } from '../../../../services/transaction-scan/buildExpiredScanResult'; -import type { TransactionScanResult } from '../../../../services/transaction-scan/types'; import { TRX_IMAGE_SVG } from '../../../../static/tron-logo'; import { FetchStatus } from '../../../../types/snap'; import { sunToTrx } from '../../../../utils/conversion'; @@ -179,7 +178,34 @@ export async function render( context.tokenPricesFetchStatus = FetchStatus.Error; } - // Create initial interface with loading state + const securityScanEnabled = + Boolean(transactionScanService) && + (useSecurityAlerts || simulateOnChainActions); + + const checkTransactionExpiration = async (): Promise => { + try { + return await snapContext.transactionExpirationRefresherService.isTransactionExpired( + { + scope, + rawData, + }, + ); + } catch (error) { + logger.error('Error checking transaction expiration:', error); + await snapClient.trackError(error as Error); + return false; + } + }; + + if (!securityScanEnabled) { + // Check local validity before exposing an enabled Confirm button when + // scans are disabled. Expiry remains independent of scan preferences. + const isExpired = await checkTransactionExpiration(); + context.scan = isExpired ? EXPIRED_TRANSACTION_SCAN : null; + context.scanFetchStatus = FetchStatus.Fetched; + } + + // Create the initial interface, waiting for a scan only when enabled. const id = await snapClient.createInterface( , context, @@ -193,12 +219,8 @@ export async function render( id, ); - const securityScanEnabled = - Boolean(transactionScanService) && - (useSecurityAlerts || simulateOnChainActions); - try { - let scan: TransactionScanResult | null = null; + let { scan } = context; if (securityScanEnabled) { const options: SecurityAlertsScanOption[] = []; @@ -224,26 +246,9 @@ export async function render( } } - // Local TAPOS-expiry check. The security-API simulation does not validate - // Tron TAPOS fields (expiration / ref block), so the snap surfaces protocol - // expiry itself. This is independent of the security preferences: an expired - // transaction won't broadcast regardless of the contract simulation result, - // so it takes precedence over a benign scan. - try { - const expired = - await snapContext.transactionExpirationRefresherService.isTransactionExpired( - { - scope, - rawData, - }, - ); - - if (expired) { - scan = EXPIRED_TRANSACTION_SCAN; - } - } catch (error) { - logger.error('Error checking transaction expiration:', error); - await snapClient.trackError(error as Error); + // A local TAPOS expiry takes precedence over any remote scan result. + if (securityScanEnabled && (await checkTransactionExpiration())) { + scan = EXPIRED_TRANSACTION_SCAN; } context.scan = scan; diff --git a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.test.tsx b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.test.tsx index 5a62974c6..a101f982c 100644 --- a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.test.tsx +++ b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest.test.tsx @@ -98,17 +98,78 @@ describe('ConfirmTransactionRequest', () => { ); }); - it('renders the scan-in-progress banner while the scan is fetching', () => { - const context: ConfirmTransactionRequestContext = { - ...baseContext, - scanFetchStatus: FetchStatus.Fetching, - scan: null, - }; + it.each([ + [FetchStatus.Loading, true], + [FetchStatus.Fetching, false], + ])( + 'sets loading banner visibility when the scan is %s', + (scanFetchStatus, visible) => { + const context: ConfirmTransactionRequestContext = { + ...baseContext, + scanFetchStatus, + scan: null, + }; - const serialized = JSON.stringify(ConfirmTransactionRequest({ context })); + const serialized = JSON.stringify(ConfirmTransactionRequest({ context })); - expect(serialized).toContain('confirmation.securityScanInProgressTitle'); - expect(serialized).toContain('"severity":"info"'); + expect( + serialized.includes('confirmation.securityScanInProgressTitle'), + ).toBe(visible); + expect(serialized.includes('"severity":"info"')).toBe(visible); + }, + ); + + it.each([ + ['Warning', 'confirmation.validationWarningTitle'], + ['Malicious', 'confirmation.validationErrorTitle'], + ] as const)( + 'preserves the %s presentation during background refresh', + (type, title) => { + const context: ConfirmTransactionRequestContext = { + ...baseContext, + scanFetchStatus: FetchStatus.Fetching, + scan: { ...mockScanResult, validation: { type, reason: null } }, + }; + + const serialized = JSON.stringify(ConfirmTransactionRequest({ context })); + + expect(serialized).not.toContain( + 'confirmation.securityScanInProgressTitle', + ); + expect(serialized).not.toContain('"disabled":true'); + expect(serialized).toContain(title); + }, + ); + + it('renders no banner for a benign scan during background refresh', () => { + const serialized = JSON.stringify( + ConfirmTransactionRequest({ + context: { ...baseContext, scanFetchStatus: FetchStatus.Fetching }, + }), + ); + + expect(serialized).not.toContain('"type":"Banner"'); + expect(serialized).not.toContain('"disabled":true'); + }); + + it('uses warning copy separately from malicious copy', () => { + const serialized = JSON.stringify( + ConfirmTransactionRequest({ + context: { + ...baseContext, + scan: { + ...mockScanResult, + validation: { type: 'Warning', reason: null }, + }, + }, + }), + ); + + expect(serialized).toContain('confirmation.validationWarningTitle'); + expect(serialized).toContain('confirmation.validationWarningSubtitle'); + expect(serialized).not.toContain('confirmation.validationErrorTitle'); + expect(serialized).toContain('"severity":"warning"'); + expect(serialized).not.toContain('"disabled":true'); }); it('renders the security alert banner for malicious validations', () => { @@ -181,7 +242,7 @@ describe('ConfirmTransactionRequest', () => { it.each([ [FetchStatus.Fetched, 'confirmation.simulationErrorTitle'], - [FetchStatus.Fetching, 'confirmation.securityScanInProgressTitle'], + [FetchStatus.Fetching, 'confirmation.simulationErrorTitle'], [FetchStatus.Error, 'confirmation.simulationTitleAPIError'], ])( 'enables confirm after a remote simulation failure when the scan is %s', diff --git a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.test.tsx b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.test.tsx index a357f1bc8..b8bd394b5 100644 --- a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.test.tsx +++ b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.test.tsx @@ -1,5 +1,6 @@ import { DEFAULT_METAMASK_ORIGIN } from '@metamask/snap-networks-utils'; import type { IStateManager } from '@metamask/snap-networks-utils'; +import { InvalidParamsError } from '@metamask/snaps-sdk'; import { Types as TronwebTypes } from 'tronweb'; import { @@ -243,6 +244,65 @@ async function withRender( // --------------------------------------------------------------------------- describe('ConfirmTransactionRequest render', () => { + it('rejects a malformed transaction before reading preferences or opening confirmation', async () => { + await withRender( + async ({ callRender, mockSnapClient, mockTransactionScanService }) => { + await expect( + callRender({ + transactionRawData: { + ...defaultTransactionRawData, + contract: [ + ...defaultTransactionRawData.contract, + ...defaultTransactionRawData.contract, + ], + }, + }), + ).rejects.toThrow(InvalidParamsError); + + expect(mockSnapClient.getPreferences).not.toHaveBeenCalled(); + expect(mockSnapClient.createInterface).not.toHaveBeenCalled(); + expect( + mockTransactionScanService.scanTransaction, + ).not.toHaveBeenCalled(); + }, + ); + }); + + it.each([ + [true, true, FetchStatus.Loading], + [true, false, FetchStatus.Loading], + [false, true, FetchStatus.Loading], + [false, false, FetchStatus.Fetched], + ])( + 'sets initial scan state with security alerts %s and simulation %s', + async (useSecurityAlerts, simulateOnChainActions, scanFetchStatus) => { + await withRender(async ({ callRender, mockSnapClient }) => { + mockSnapClient.getPreferences.mockResolvedValue({ + ...defaultPreferences, + useSecurityAlerts, + simulateOnChainActions, + }); + // Inspect the initial context before render mutates it with scan results. + mockSnapClient.createInterface.mockImplementation( + async (ui, context) => { + expect(context).toMatchObject({ scanFetchStatus }); + expect(JSON.stringify(ui).includes('"disabled":true')).toBe( + scanFetchStatus === FetchStatus.Loading, + ); + expect( + JSON.stringify(ui).includes('Checking for security issues'), + ).toBe(useSecurityAlerts); + return 'interface-id-123'; + }, + ); + + await callRender(); + + expect(mockSnapClient.createInterface).toHaveBeenCalledTimes(1); + }); + }, + ); + it('triggers security scan when preferences enable it', async () => { await withRender( async ({ callRender, mockSnapClient, mockTransactionScanService }) => { @@ -270,7 +330,7 @@ describe('ConfirmTransactionRequest render', () => { ); }); - it('always triggers scan even when security preferences are disabled', async () => { + it('skips the scan when security alerts and simulation are disabled', async () => { await withRender( async ({ callRender, mockSnapClient, mockTransactionScanService }) => { mockSnapClient.getPreferences.mockResolvedValue({ @@ -281,9 +341,15 @@ describe('ConfirmTransactionRequest render', () => { await callRender(); - expect(mockTransactionScanService.scanTransaction).toHaveBeenCalledWith( + expect( + mockTransactionScanService.scanTransaction, + ).not.toHaveBeenCalled(); + expect(mockSnapClient.updateInterface).toHaveBeenCalledWith( + 'interface-id-123', + expect.anything(), expect.objectContaining({ - options: ['simulation'], + scan: null, + scanFetchStatus: FetchStatus.Fetched, }), ); expect(mockSnapClient.scheduleBackgroundEvent).toHaveBeenCalledWith({ @@ -363,7 +429,7 @@ describe('ConfirmTransactionRequest render', () => { ); }); - it('includes both simulation and validation when useSecurityAlerts is true', async () => { + it('requests only validation when security alerts are enabled and simulation is disabled', async () => { await withRender( async ({ callRender, mockSnapClient, mockTransactionScanService }) => { mockSnapClient.getPreferences.mockResolvedValue({ @@ -376,7 +442,7 @@ describe('ConfirmTransactionRequest render', () => { expect(mockTransactionScanService.scanTransaction).toHaveBeenCalledWith( expect.objectContaining({ - options: ['simulation', 'validation'], + options: ['validation'], }), ); }, diff --git a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.tsx b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.tsx index 07eb05506..f15e78ad7 100644 --- a/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.tsx +++ b/packages/tron-wallet-snap/src/ui/confirmation/views/ConfirmTransactionRequest/render.tsx @@ -16,6 +16,7 @@ import type { ComputeFeeResult } from '../../../../services/send/types'; import type { UnencryptedStateValue } from '../../../../services/state/stateTypes'; import { TRX_IMAGE_SVG } from '../../../../static/tron-logo'; import { FetchStatus } from '../../../../types/snap'; +import { assertTransactionStructure } from '../../../../validation/transaction'; import { getIconUrlForKnownAsset } from '../../utils/getIconUrlForKnownAsset'; import { ConfirmTransactionRequest } from './ConfirmTransactionRequest'; import { CONFIRM_TRANSACTION_INTERFACE_NAME } from './types'; @@ -95,6 +96,8 @@ export async function render( const { transactionRawData } = incomingContext; + assertTransactionStructure(transactionRawData); + // 1. Initial context with loading state const context: ConfirmTransactionRequestContext = { ...DEFAULT_CONFIRMATION_CONTEXT, @@ -110,7 +113,15 @@ export async function render( // keep defaults } - const { useSecurityAlerts } = context.preferences; + const { useSecurityAlerts, simulateOnChainActions } = context.preferences; + + const securityScanEnabled = + Boolean(transactionScanService) && + (useSecurityAlerts || simulateOnChainActions); + + if (!securityScanEnabled) { + context.scanFetchStatus = FetchStatus.Fetched; + } /** * Resolve icon URLs for fee assets from known asset metadata. @@ -132,11 +143,13 @@ export async function render( id, ); - // 3. Perform security scan (always needed for estimated changes simulation) - if (transactionScanService) { - // Always request simulation for estimated changes; - // conditionally add validation based on user preference - const options: SecurityAlertsScanOption[] = ['simulation']; + // 3. Scan only when security alerts or simulation are enabled. + if (securityScanEnabled) { + const options: SecurityAlertsScanOption[] = []; + + if (simulateOnChainActions) { + options.push('simulation'); + } if (useSecurityAlerts) { options.push('validation'); @@ -164,9 +177,6 @@ export async function render( context.scan = null; context.scanFetchStatus = FetchStatus.Error; } - } else { - // No scan service available, mark as fetched immediately - context.scanFetchStatus = FetchStatus.Fetched; } // Ensure interface ID is stored before updating @@ -193,7 +203,7 @@ export async function render( }); } - // Schedule security scan background refresh (every 20 seconds) + // Refresh transaction validity and any enabled scans every 20 seconds. if (transactionScanService) { await snapClient.scheduleBackgroundEvent({ method: BackgroundEventMethod.RefreshConfirmationSend, From 4b7fb6ebd16a156ca25d84cbe6d9cf250bcb5e84 Mon Sep 17 00:00:00 2001 From: Andrew Taran Date: Thu, 5 Nov 2026 11:12:04 +0100 Subject: [PATCH 2/2] fix: changelog and es translation --- packages/tron-wallet-snap/CHANGELOG.md | 1 - packages/tron-wallet-snap/locales/es.json | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/tron-wallet-snap/CHANGELOG.md b/packages/tron-wallet-snap/CHANGELOG.md index e405fbac6..4e8a66938 100644 --- a/packages/tron-wallet-snap/CHANGELOG.md +++ b/packages/tron-wallet-snap/CHANGELOG.md @@ -15,7 +15,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Align transaction security warning copy and scan-loading behavior with user preferences. ([#557](https://github.com/MetaMask/internal-snaps/pull/557)) - Allow transaction confirmation after remote simulation failures while retaining security warnings and checks for locally invalid transactions. ([#552](https://github.com/MetaMask/internal-snaps/pull/552)) ## [4.3.0] diff --git a/packages/tron-wallet-snap/locales/es.json b/packages/tron-wallet-snap/locales/es.json index 91222369f..c2aa06951 100644 --- a/packages/tron-wallet-snap/locales/es.json +++ b/packages/tron-wallet-snap/locales/es.json @@ -86,10 +86,10 @@ "message": "Si aprueba esta solicitud, un tercero conocido por estafas tomará todos sus activos." }, "confirmation.validationWarningTitle": { - "message": "This request may be risky" + "message": "Esta solicitud puede ser arriesgada" }, "confirmation.validationWarningSubtitle": { - "message": "Only continue if you trust this site and/or every address involved." + "message": "Solo continúe si confía en este sitio y/o en todas las direcciones involucradas." }, "confirmation.validationErrorLearnMore": { "message": "Más información"