From 59d9cca66fb16b1de77c4eb1f8dc40df4ff5e843 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 29 Oct 2025 13:15:33 +0000 Subject: [PATCH 1/2] fix: Gas station improvements --- .../gas-fee-token-icon/gas-fee-token-icon.tsx | 43 ++++-- .../useAutomaticGasFeeTokenSelect.test.ts | 146 ++++++++++++++++-- .../hooks/useAutomaticGasFeeTokenSelect.ts | 3 +- 3 files changed, 163 insertions(+), 29 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/shared/gas-fee-token-icon/gas-fee-token-icon.tsx b/ui/pages/confirmations/components/confirm/info/shared/gas-fee-token-icon/gas-fee-token-icon.tsx index 4a96cf83e4e2..04cf1bc3a5cb 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/gas-fee-token-icon/gas-fee-token-icon.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/gas-fee-token-icon/gas-fee-token-icon.tsx @@ -1,19 +1,23 @@ -import React from 'react'; -import { Hex } from '@metamask/utils'; +import { AvatarAccountSize } from '@metamask/design-system-react'; import { TransactionMeta } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import React from 'react'; import { useSelector } from 'react-redux'; - -import { NATIVE_TOKEN_ADDRESS } from '../../../../../../../../shared/constants/transaction'; -import { useConfirmContext } from '../../../../../context/confirm'; -import { selectNetworkConfigurationByChainId } from '../../../../../../../selectors'; -import Identicon from '../../../../../../../components/ui/identicon'; import { CHAIN_ID_TOKEN_IMAGE_MAP } from '../../../../../../../../shared/constants/network'; +import { NATIVE_TOKEN_ADDRESS } from '../../../../../../../../shared/constants/transaction'; +import { PreferredAvatar } from '../../../../../../../components/app/preferred-avatar'; import { AvatarToken, AvatarTokenSize, Box, } from '../../../../../../../components/component-library'; +import Identicon from '../../../../../../../components/ui/identicon'; import { BackgroundColor } from '../../../../../../../helpers/constants/design-system'; +import { + selectERC20TokensByChain, + selectNetworkConfigurationByChainId, +} from '../../../../../../../selectors'; +import { useConfirmContext } from '../../../../../context/confirm'; export enum GasFeeTokenIconSize { Sm = 'sm', @@ -36,13 +40,30 @@ export function GasFeeTokenIcon({ selectNetworkConfigurationByChainId(state, chainId), ); + const erc20TokensByChain = useSelector(selectERC20TokensByChain); + const variation = chainId; + const { iconUrl: image } = + erc20TokensByChain?.[variation]?.data?.[tokenAddress] ?? {}; + if (tokenAddress !== NATIVE_TOKEN_ADDRESS) { return ( - + {image ? ( + + ) : ( + + )} ); } diff --git a/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts b/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts index c1e96bc421d6..a0f9851baaf6 100644 --- a/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts +++ b/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts @@ -6,6 +6,7 @@ import { NATIVE_TOKEN_ADDRESS } from '../../../../shared/constants/transaction'; import { genUnapprovedContractInteractionConfirmation } from '../../../../test/data/confirmations/contract-interaction'; import { getMockConfirmStateForTransaction } from '../../../../test/data/confirmations/helper'; import { renderHookWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers'; +import { flushPromises } from '../../../../test/lib/timer-helpers'; import { updateSelectedGasFeeToken } from '../../../store/controller-actions/transaction-controller'; import { Alert } from '../../../ducks/confirm-alerts/confirm-alerts'; import { forceUpdateMetamaskState } from '../../../store/actions'; @@ -46,6 +47,12 @@ function runHook({ return { ...result, state }; } +async function flushAsyncUpdates() { + await act(async () => { + await flushPromises(); + }); +} + describe('useAutomaticGasFeeTokenSelect', () => { const updateSelectedGasFeeTokenMock = jest.mocked(updateSelectedGasFeeToken); const forceUpdateMetamaskStateMock = jest.mocked(forceUpdateMetamaskState); @@ -67,31 +74,56 @@ describe('useAutomaticGasFeeTokenSelect', () => { }); }); - it('selects first gas fee token', () => { - runHook(); + it('selects first gas fee token', async () => { + const { store } = runHook(); + + await flushAsyncUpdates(); + + if (!store) { + throw new Error('Expected store to be defined'); + } expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1); expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith( expect.any(String), GAS_FEE_TOKEN_MOCK.tokenAddress, ); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch); }); - it('does not select first gas fee token if gas fee token already selected', () => { + it('does not select first gas fee token if gas fee token already selected', async () => { runHook({ selectedGasFeeToken: GAS_FEE_TOKEN_MOCK.tokenAddress }); + + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); }); - it('does not select first gas fee token if no gas fee tokens', () => { + it('does not select first gas fee token if no gas fee tokens', async () => { runHook({ gasFeeTokens: [] }); + + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); }); - it('does not select first gas fee token if not first load', () => { - const { rerender, state } = runHook({ + it('selects first gas fee token on rerender when selection becomes eligible', async () => { + const { rerender, state, store } = runHook({ selectedGasFeeToken: GAS_FEE_TOKEN_MOCK.tokenAddress, }); + if (!store) { + throw new Error('Expected store to be defined'); + } + + await flushAsyncUpdates(); + + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); + const transactionMeta = state.metamask .transactions[0] as unknown as TransactionMeta; @@ -101,10 +133,18 @@ describe('useAutomaticGasFeeTokenSelect', () => { rerender(); - expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + await flushAsyncUpdates(); + + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith( + expect.any(String), + GAS_FEE_TOKEN_MOCK.tokenAddress, + ); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch); }); - it('does not select first gas fee token if gasless not supported', () => { + it('does not select first gas fee token if gasless not supported', async () => { useIsGaslessSupportedMock.mockReturnValue({ isSupported: false, isSmartTransaction: false, @@ -112,30 +152,81 @@ describe('useAutomaticGasFeeTokenSelect', () => { runHook(); + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); }); - it('does not select first gas fee token if sufficient balance', () => { + it('does not select first gas fee token if sufficient balance', async () => { useInsufficientBalanceAlertsMock.mockReturnValue([]); runHook(); + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); }); - it('does not select first gas fee token after firstCheck is set to false', () => { - const { rerender, state } = runHook(); + it('selects first gas fee token when insufficient balance appears after first render', async () => { + let alerts: Alert[] = []; + useInsufficientBalanceAlertsMock.mockImplementation(() => alerts); + + const { rerender, store } = runHook({ + selectedGasFeeToken: undefined, + }); + + if (!store) { + throw new Error('Expected store to be defined'); + } + + await flushAsyncUpdates(); + + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); + + alerts = [{} as Alert]; + + rerender(); + + await flushAsyncUpdates(); + + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith( + expect.any(String), + GAS_FEE_TOKEN_MOCK.tokenAddress, + ); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch); + }); + + it('does not select first gas fee token after firstCheck is set to false', async () => { + const { rerender, state, store } = runHook(); + + if (!store) { + throw new Error('Expected store to be defined'); + } + + await flushAsyncUpdates(); + // Simulate a rerender with new state that would otherwise trigger selection act(() => { ( state.metamask.transactions[0] as unknown as TransactionMeta ).selectedGasFeeToken = undefined; }); + rerender(); + + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1); // Only first run + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch); }); - it('does not select if transactionId is falsy', () => { + it('does not select if transactionId is falsy', async () => { const state = getMockConfirmStateForTransaction( genUnapprovedContractInteractionConfirmation({ gasFeeTokens: [GAS_FEE_TOKEN_MOCK], @@ -145,15 +236,23 @@ describe('useAutomaticGasFeeTokenSelect', () => { // Remove transactionId state.metamask.transactions = []; renderHookWithConfirmContextProvider(useAutomaticGasFeeTokenSelect, state); + + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); }); - it('does not select if gasFeeTokens is falsy', () => { + it('does not select if gasFeeTokens is falsy', async () => { runHook({ gasFeeTokens: [] }); + + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); }); - it('does not select first gas fee token if 7702 and future native token', () => { + it('does not select first gas fee token if 7702 and future native token', async () => { useIsGaslessSupportedMock.mockReturnValue({ isSupported: true, isSmartTransaction: false, @@ -168,16 +267,19 @@ describe('useAutomaticGasFeeTokenSelect', () => { ], }); + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); }); - it('selects second gas fee token if 7702 and future native token', () => { + it('selects second gas fee token if 7702 and future native token', async () => { useIsGaslessSupportedMock.mockReturnValue({ isSupported: true, isSmartTransaction: false, }); - runHook({ + const { store } = runHook({ gasFeeTokens: [ { ...GAS_FEE_TOKEN_MOCK, @@ -187,6 +289,18 @@ describe('useAutomaticGasFeeTokenSelect', () => { ], }); + if (!store) { + throw new Error('Expected store to be defined'); + } + + await flushAsyncUpdates(); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(1); + expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledWith( + expect.any(String), + GAS_FEE_TOKEN_MOCK.tokenAddress, + ); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(1); + expect(forceUpdateMetamaskStateMock).toHaveBeenCalledWith(store.dispatch); }); }); diff --git a/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.ts b/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.ts index 5969fc19378d..6f0904133969 100644 --- a/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.ts +++ b/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.ts @@ -51,10 +51,9 @@ export function useAutomaticGasFeeTokenSelect() { return; } - setFirstCheck(false); - if (shouldSelect) { await selectFirstToken(); + setFirstCheck(false); } }, [shouldSelect, selectFirstToken, firstCheck, gasFeeTokens, transactionId]); } From 1b1e3828a13a01d57c9aed2c8f20e6ab9adff8d4 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 30 Oct 2025 16:52:22 +0000 Subject: [PATCH 2/2] fix --- .../pages/confirmations/redesign/transaction-confirmation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/page-objects/pages/confirmations/redesign/transaction-confirmation.ts b/test/e2e/page-objects/pages/confirmations/redesign/transaction-confirmation.ts index 37d5fcf202f8..0ac37ea0ff2e 100644 --- a/test/e2e/page-objects/pages/confirmations/redesign/transaction-confirmation.ts +++ b/test/e2e/page-objects/pages/confirmations/redesign/transaction-confirmation.ts @@ -304,7 +304,7 @@ class TransactionConfirmation extends Confirmation { async closeGasFeeToastMessage() { // the toast message automatically disappears after some seconds, so we need to use clickElementSafe to prevent race conditions - await this.driver.clickElementSafe(this.gasFeeCloseToastMessage, 5000); + await this.driver.clickElementSafe(this.gasFeeCloseToastMessage, 10000); } /**