From fd3366caba51fc512d217b44e5bb25227330adc1 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Thu, 19 Dec 2024 10:35:50 +0000 Subject: [PATCH 01/19] fix: handle notification api calls settings (#29327) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Adds effect checks; removes old hooks unused; and adds tests. I really HATE the useFootguns we have everywhere for notifications. I want us to do a wider cleanup task. 1. Remove the Provider values. We can keep the provider to run initial effects, but other than that, we should not expose anything from it! 2. Remove nearly all of the useEffects scattered across all the components. Instead we can utilise 1 reusable hook for fetching. - potentially we can expose this in the provider if we really want to fetch only once! [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29327?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/28173 ## **Manual testing steps** 1. Go through onboarding, and settings pages > Notifications 2. Check console and network tab, are we making API calls and throwing errors in console? ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../useSwitchNotifications.test.tsx | 256 +++++++++++------- .../useSwitchNotifications.ts | 59 +--- ...fications-settings-allow-notifications.tsx | 2 +- 3 files changed, 168 insertions(+), 149 deletions(-) diff --git a/ui/hooks/metamask-notifications/useSwitchNotifications.test.tsx b/ui/hooks/metamask-notifications/useSwitchNotifications.test.tsx index b67c2ea80cef..b228698f391e 100644 --- a/ui/hooks/metamask-notifications/useSwitchNotifications.test.tsx +++ b/ui/hooks/metamask-notifications/useSwitchNotifications.test.tsx @@ -1,125 +1,183 @@ -import React from 'react'; -import { Provider } from 'react-redux'; -import { renderHook, act } from '@testing-library/react-hooks'; -import configureStore from 'redux-mock-store'; -import thunk from 'redux-thunk'; -import type { Store } from 'redux'; -import * as actions from '../../store/actions'; -import { MetamaskNotificationsProvider } from '../../contexts/metamask-notifications/metamask-notifications'; +import { waitFor } from '@testing-library/react'; +import * as ActionsModule from '../../store/actions'; +import * as NotificationSelectorsModule from '../../selectors/metamask-notifications/metamask-notifications'; +import { renderHookWithProviderTyped } from '../../../test/lib/render-helpers'; import { useSwitchFeatureAnnouncementsChange, - useSwitchAccountNotifications, useSwitchAccountNotificationsChange, + useAccountSettingsProps, } from './useSwitchNotifications'; -const middlewares = [thunk]; -const mockStore = configureStore(middlewares); - -jest.mock('../../store/actions', () => ({ - setFeatureAnnouncementsEnabled: jest.fn(), - checkAccountsPresence: jest.fn(), - updateOnChainTriggersByAccount: jest.fn(), - deleteOnChainTriggersByAccount: jest.fn(), - showLoadingIndication: jest.fn(), - hideLoadingIndication: jest.fn(), - fetchAndUpdateMetamaskNotifications: jest.fn(), -})); - -describe('useSwitchNotifications', () => { - let store: Store; - +describe('useSwitchFeatureAnnouncementsChange() tests', () => { beforeEach(() => { - store = mockStore({ - metamask: { - isFeatureAnnouncementsEnabled: false, - internalAccounts: { - accounts: { - '0x123': { - address: '0x123', - id: 'account1', - metadata: {}, - options: {}, - methods: [], - type: 'eip155:eoa', - }, - }, - }, - }, - }); - - store.dispatch = jest.fn().mockImplementation((action) => { - if (typeof action === 'function') { - return action(store.dispatch, store.getState); - } - return Promise.resolve(); - }); + jest.restoreAllMocks(); + }); - jest.clearAllMocks(); + const arrangeMocks = () => { + const mockSetFeatureAnnouncementsEnabled = jest.spyOn( + ActionsModule, + 'setFeatureAnnouncementsEnabled', + ); + return { + mockSetFeatureAnnouncementsEnabled, + }; + }; + + it('should update feature announcement when callback invoked', async () => { + const mocks = arrangeMocks(); + const hook = renderHookWithProviderTyped( + () => useSwitchFeatureAnnouncementsChange(), + {}, + ); + + await hook.result.current.onChange(true); + expect(mocks.mockSetFeatureAnnouncementsEnabled).toHaveBeenCalled(); }); - it('should toggle feature announcements', async () => { - const { result } = renderHook(() => useSwitchFeatureAnnouncementsChange(), { - wrapper: ({ children }) => ( - - - {children} - - - ), + it('should update error state when callback fails', async () => { + const mocks = arrangeMocks(); + mocks.mockSetFeatureAnnouncementsEnabled.mockImplementation(() => { + throw new Error('Mock Fail'); }); + const hook = renderHookWithProviderTyped( + () => useSwitchFeatureAnnouncementsChange(), + {}, + ); - await act(async () => { - await result.current.onChange(true); - }); + await hook.result.current.onChange(true); + expect(hook.result.current.error).toBeDefined(); + }); +}); - expect(actions.setFeatureAnnouncementsEnabled).toHaveBeenCalledWith(true); +describe('useSwitchAccountNotificationsChange() tests', () => { + const arrangeMocks = () => { + const mockUpdateOnChainTriggersByAccount = jest.spyOn( + ActionsModule, + 'updateOnChainTriggersByAccount', + ); + const mockDeleteOnChainTriggersByAccount = jest.spyOn( + ActionsModule, + 'deleteOnChainTriggersByAccount', + ); + + return { + mockUpdateOnChainTriggersByAccount, + mockDeleteOnChainTriggersByAccount, + }; + }; + + it('should invoke update notification triggers when an address is enabled', async () => { + const mocks = arrangeMocks(); + const hook = renderHookWithProviderTyped( + () => useSwitchAccountNotificationsChange(), + {}, + ); + + await hook.result.current.onChange(['0x1'], true); + expect(mocks.mockUpdateOnChainTriggersByAccount).toHaveBeenCalledWith([ + '0x1', + ]); + expect(mocks.mockDeleteOnChainTriggersByAccount).not.toHaveBeenCalled(); }); - it('should check account presence', async () => { - const { result } = renderHook(() => useSwitchAccountNotifications(), { - wrapper: ({ children }) => ( - - - {children} - - - ), - }); + it('should invoke delete notification triggers when an address is disabled', async () => { + const mocks = arrangeMocks(); + const hook = renderHookWithProviderTyped( + () => useSwitchAccountNotificationsChange(), + {}, + ); + + await hook.result.current.onChange(['0x1'], false); + expect(mocks.mockUpdateOnChainTriggersByAccount).not.toHaveBeenCalled(); + expect(mocks.mockDeleteOnChainTriggersByAccount).toHaveBeenCalledWith([ + '0x1', + ]); + }); - await act(async () => { - await result.current.switchAccountNotifications(['0x123']); + it('should return an error value if it fails to update or delete triggers', async () => { + const mocks = arrangeMocks(); + mocks.mockUpdateOnChainTriggersByAccount.mockImplementation(() => { + throw new Error('Mock Error'); + }); + mocks.mockDeleteOnChainTriggersByAccount.mockImplementation(() => { + throw new Error('Mock Error'); }); - expect(actions.checkAccountsPresence).toHaveBeenCalledWith(['0x123']); - }); + const act = async (testEnableOrDisable: boolean) => { + const hook = renderHookWithProviderTyped( + () => useSwitchAccountNotificationsChange(), + {}, + ); + await hook.result.current.onChange(['0x1'], testEnableOrDisable); + return hook.result.current.error; + }; - it('should handle account notification changes', async () => { - const { result } = renderHook(() => useSwitchAccountNotificationsChange(), { - wrapper: ({ children }) => ( - - - {children} - - - ), - }); + const enableError = await act(true); + expect(enableError).toBeDefined(); + + const disableError = await act(false); + expect(disableError).toBeDefined(); + }); +}); - // Test enabling notifications - await act(async () => { - await result.current.onChange(['0x123'], true); +describe('useAccountSettingsProps() tests', () => { + const arrangeMocks = () => { + const mockCheckAccountsPresence = jest.spyOn( + ActionsModule, + 'checkAccountsPresence', + ); + const mockGetIsUpdatingMetamaskNotificationsAccount = jest + .spyOn( + NotificationSelectorsModule, + 'getIsUpdatingMetamaskNotificationsAccount', + ) + .mockReturnValue([]); + const mockSelectIsMetamaskNotificationsEnabled = jest + .spyOn( + NotificationSelectorsModule, + 'selectIsMetamaskNotificationsEnabled', + ) + .mockReturnValue(true); + + return { + mockCheckAccountsPresence, + mockGetIsUpdatingMetamaskNotificationsAccount, + mockSelectIsMetamaskNotificationsEnabled, + }; + }; + + it('Should invoke effect when notifications are enabled', async () => { + const mocks = arrangeMocks(); + renderHookWithProviderTyped(() => useAccountSettingsProps(['0x1']), {}); + + await waitFor(() => { + expect(mocks.mockCheckAccountsPresence).toHaveBeenCalled(); }); + }); - expect(actions.updateOnChainTriggersByAccount).toHaveBeenCalledWith([ - '0x123', - ]); + it('Should not invoke effect when notifications are disabled', async () => { + const mocks = arrangeMocks(); + mocks.mockSelectIsMetamaskNotificationsEnabled.mockReturnValue(false); + renderHookWithProviderTyped(() => useAccountSettingsProps(['0x1']), {}); - // Test disabling notifications - await act(async () => { - await result.current.onChange(['0x123'], false); + await waitFor(() => { + expect(mocks.mockCheckAccountsPresence).not.toHaveBeenCalled(); }); + }); - expect(actions.deleteOnChainTriggersByAccount).toHaveBeenCalledWith([ - '0x123', - ]); + it('Should be able to invoke refetch accounts function', async () => { + const mocks = arrangeMocks(); + const hook = renderHookWithProviderTyped( + () => useAccountSettingsProps(['0x1']), + {}, + ); + + await hook.result.current.update(['0x1', '0x2']); + await waitFor(() => { + expect(mocks.mockCheckAccountsPresence).toHaveBeenCalledWith([ + '0x1', + '0x2', + ]); + }); }); }); diff --git a/ui/hooks/metamask-notifications/useSwitchNotifications.ts b/ui/hooks/metamask-notifications/useSwitchNotifications.ts index 53e121473bac..08a03c33e4f1 100644 --- a/ui/hooks/metamask-notifications/useSwitchNotifications.ts +++ b/ui/hooks/metamask-notifications/useSwitchNotifications.ts @@ -8,7 +8,10 @@ import { updateOnChainTriggersByAccount, hideLoadingIndication, } from '../../store/actions'; -import { getIsUpdatingMetamaskNotificationsAccount } from '../../selectors/metamask-notifications/metamask-notifications'; +import { + getIsUpdatingMetamaskNotificationsAccount, + selectIsMetamaskNotificationsEnabled, +} from '../../selectors/metamask-notifications/metamask-notifications'; export function useSwitchFeatureAnnouncementsChange(): { onChange: (state: boolean) => Promise; @@ -28,7 +31,6 @@ export function useSwitchFeatureAnnouncementsChange(): { const errorMessage = e instanceof Error ? e.message : JSON.stringify(e ?? ''); setError(errorMessage); - throw e; } }, [dispatch], @@ -42,44 +44,6 @@ export function useSwitchFeatureAnnouncementsChange(): { export type UseSwitchAccountNotificationsData = { [address: string]: boolean }; -export function useSwitchAccountNotifications(): { - switchAccountNotifications: ( - accounts: string[], - ) => Promise; - isLoading: boolean; - error: string | null; -} { - const dispatch = useDispatch(); - - const [isLoading, setIsLoading] = useState(false); - const [error, setError] = useState(null); - - const switchAccountNotifications = useCallback( - async ( - accounts: string[], - ): Promise => { - setIsLoading(true); - setError(null); - - try { - const data = await dispatch(checkAccountsPresence(accounts)); - return data as unknown as UseSwitchAccountNotificationsData; - } catch (e) { - const errorMessage = - e instanceof Error ? e.message : JSON.stringify(e ?? ''); - setError(errorMessage); - log.error(errorMessage); - throw e; - } finally { - setIsLoading(false); - } - }, - [dispatch], - ); - - return { switchAccountNotifications, isLoading, error }; -} - export function useSwitchAccountNotificationsChange(): { onChange: (addresses: string[], state: boolean) => Promise; error: string | null; @@ -103,7 +67,6 @@ export function useSwitchAccountNotificationsChange(): { e instanceof Error ? e.message : JSON.stringify(e ?? ''); log.error(errorMessage); setError(errorMessage); - throw e; } dispatch(hideLoadingIndication()); }, @@ -146,6 +109,7 @@ export function useAccountSettingsProps(accounts: string[]) { const accountsBeingUpdated = useSelector( getIsUpdatingMetamaskNotificationsAccount, ); + const isEnabled = useSelector(selectIsMetamaskNotificationsEnabled); const fetchAccountSettings = useRefetchAccountSettings(); const [data, setData] = useState({}); const [loading, setLoading] = useState(false); @@ -169,15 +133,12 @@ export function useAccountSettingsProps(accounts: string[]) { // Effect - async get if accounts are enabled/disabled useEffect(() => { - try { - const memoAccounts: string[] = JSON.parse(jsonAccounts); - update(memoAccounts); - } catch { - setError('Failed to get account settings'); - } finally { - setLoading(false); + if (!isEnabled) { + return; } - }, [jsonAccounts, fetchAccountSettings]); + const memoAccounts: string[] = JSON.parse(jsonAccounts); + update(memoAccounts); + }, [jsonAccounts, fetchAccountSettings, isEnabled]); return { data, diff --git a/ui/pages/notifications-settings/notifications-settings-allow-notifications.tsx b/ui/pages/notifications-settings/notifications-settings-allow-notifications.tsx index b442a4092185..fdfaeb1109cb 100644 --- a/ui/pages/notifications-settings/notifications-settings-allow-notifications.tsx +++ b/ui/pages/notifications-settings/notifications-settings-allow-notifications.tsx @@ -74,7 +74,7 @@ export function NotificationsSettingsAllowNotifications({ }, [isMetamaskNotificationsEnabled]); useEffect(() => { - if (!error) { + if (!error && isMetamaskNotificationsEnabled) { listNotifications(); } }, [isMetamaskNotificationsEnabled, error, listNotifications]); From dd659446809d793399ee97f581cdc9f285493988 Mon Sep 17 00:00:00 2001 From: javiergarciavera <76975121+javiergarciavera@users.noreply.github.com> Date: Thu, 19 Dec 2024 11:46:31 +0100 Subject: [PATCH 02/19] feat: added create solana account test (#28866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** - E2E tests covering different scenarios for creating/removing Solana accounts. - Refactor around some snap logic for BTC/Solana [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29054?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** Running flask tests should pass ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Ulisses Ferreira Co-authored-by: Dan J Miller Co-authored-by: Charly Chevalier --- test/data/mock-state.json | 1 + test/e2e/constants.ts | 3 + test/e2e/flask/btc/common-btc.ts | 3 +- test/e2e/flask/btc/create-btc-account.spec.ts | 25 +++-- test/e2e/flask/solana/common-solana.ts | 71 ++++++++++++ .../solana/create-solana-account.spec.ts | 62 +++++++++++ .../flask/solana/solana-eth-networks.spec.ts | 24 +++++ test/e2e/page-objects/common.ts | 6 ++ .../page-objects/pages/account-list-page.ts | 102 ++++++++++-------- test/e2e/page-objects/pages/header-navbar.ts | 30 +++--- 10 files changed, 261 insertions(+), 66 deletions(-) create mode 100644 test/e2e/flask/solana/common-solana.ts create mode 100644 test/e2e/flask/solana/create-solana-account.spec.ts create mode 100644 test/e2e/flask/solana/solana-eth-networks.spec.ts diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 3131b8335287..9ea2e674e7a3 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -1891,6 +1891,7 @@ "watchEthereumAccountEnabled": false, "bitcoinSupportEnabled": false, "bitcoinTestnetSupportEnabled": false, + "solanaSupportEnabled": false, "pendingApprovals": { "testApprovalId": { "id": "testApprovalId", diff --git a/test/e2e/constants.ts b/test/e2e/constants.ts index 8296cdc3e3d1..9d4d5bbf3c8d 100644 --- a/test/e2e/constants.ts +++ b/test/e2e/constants.ts @@ -57,6 +57,9 @@ export const DEFAULT_BTC_FEES_RATE = 0.00001; // BTC /* Default BTC conversion rate to USD */ export const DEFAULT_BTC_CONVERSION_RATE = 62000; // USD +/* Default SOL conversion rate to USD */ +export const DEFAULT_SOL_CONVERSION_RATE = 226; // USD + /* Default BTC transaction ID */ export const DEFAULT_BTC_TRANSACTION_ID = 'e4111a707317da67d49a71af4cbcf6c0546f900ca32c3842d2254e315d1fca18'; diff --git a/test/e2e/flask/btc/common-btc.ts b/test/e2e/flask/btc/common-btc.ts index 05f382281e29..db2db85eb554 100644 --- a/test/e2e/flask/btc/common-btc.ts +++ b/test/e2e/flask/btc/common-btc.ts @@ -14,6 +14,7 @@ import { Driver } from '../../webdriver/driver'; import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow'; import AccountListPage from '../../page-objects/pages/account-list-page'; import HeaderNavbar from '../../page-objects/pages/header-navbar'; +import { ACCOUNT_TYPE } from '../../page-objects/common'; const QUICKNODE_URL_REGEX = /^https:\/\/.*\.btc.*\.quiknode\.pro(\/|$)/u; @@ -217,7 +218,7 @@ export async function withBtcAccountSnap( await new HeaderNavbar(driver).openAccountMenu(); const accountListPage = new AccountListPage(driver); await accountListPage.check_pageIsLoaded(); - await accountListPage.addNewBtcAccount(); + await accountListPage.addAccount(ACCOUNT_TYPE.Bitcoin, ''); await test(driver, mockServer); }, ); diff --git a/test/e2e/flask/btc/create-btc-account.spec.ts b/test/e2e/flask/btc/create-btc-account.spec.ts index 0dfb24a8a931..f563454ad1e6 100644 --- a/test/e2e/flask/btc/create-btc-account.spec.ts +++ b/test/e2e/flask/btc/create-btc-account.spec.ts @@ -7,6 +7,7 @@ import LoginPage from '../../page-objects/pages/login-page'; import PrivacySettings from '../../page-objects/pages/settings/privacy-settings'; import ResetPasswordPage from '../../page-objects/pages/reset-password-page'; import SettingsPage from '../../page-objects/pages/settings/settings-page'; +import { ACCOUNT_TYPE } from '../../page-objects/common'; import { withBtcAccountSnap } from './common-btc'; describe('Create BTC Account', function (this: Suite) { @@ -34,14 +35,12 @@ describe('Create BTC Account', function (this: Suite) { await headerNavbar.openAccountMenu(); const accountListPage = new AccountListPage(driver); await accountListPage.check_pageIsLoaded(); - await accountListPage.addNewBtcAccount({ - btcAccountCreationEnabled: false, - }); - - // check the number of available accounts is 2 - await headerNavbar.openAccountMenu(); - await accountListPage.check_pageIsLoaded(); await accountListPage.check_numberOfAvailableAccounts(2); + await accountListPage.openAddAccountModal(); + assert.equal( + await accountListPage.isBtcAccountCreationButtonEnabled(), + false, + ); }, ); }); @@ -91,8 +90,14 @@ describe('Create BTC Account', function (this: Suite) { // Recreate account and check that the address is the same await headerNavbar.openAccountMenu(); - await accountListPage.check_pageIsLoaded(); - await accountListPage.addNewBtcAccount(); + await accountListPage.openAddAccountModal(); + assert.equal( + await accountListPage.isBtcAccountCreationButtonEnabled(), + true, + ); + await accountListPage.closeAccountModal(); + await headerNavbar.openAccountMenu(); + await accountListPage.addAccount(ACCOUNT_TYPE.Bitcoin, ''); await headerNavbar.check_accountLabel('Bitcoin Account'); await headerNavbar.openAccountMenu(); @@ -146,7 +151,7 @@ describe('Create BTC Account', function (this: Suite) { await headerNavbar.check_pageIsLoaded(); await headerNavbar.openAccountMenu(); await accountListPage.check_pageIsLoaded(); - await accountListPage.addNewBtcAccount(); + await accountListPage.addAccount(ACCOUNT_TYPE.Bitcoin, ''); await headerNavbar.check_accountLabel('Bitcoin Account'); await headerNavbar.openAccountMenu(); diff --git a/test/e2e/flask/solana/common-solana.ts b/test/e2e/flask/solana/common-solana.ts new file mode 100644 index 000000000000..2ac107bb442c --- /dev/null +++ b/test/e2e/flask/solana/common-solana.ts @@ -0,0 +1,71 @@ +import { Mockttp } from 'mockttp'; +import { withFixtures } from '../../helpers'; +import { Driver } from '../../webdriver/driver'; +import HeaderNavbar from '../../page-objects/pages/header-navbar'; +import AccountListPage from '../../page-objects/pages/account-list-page'; +import FixtureBuilder from '../../fixture-builder'; +import { ACCOUNT_TYPE } from '../../page-objects/common'; +import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow'; + +const SOLANA_URL_REGEX = /^https:\/\/.*\.solana.*/u; + +export enum SendFlowPlaceHolders { + AMOUNT = 'Enter amount to send', + RECIPIENT = 'Enter receiving address', + LOADING = 'Preparing transaction', +} + +export async function mockSolanaBalanceQuote(mockServer: Mockttp) { + return await mockServer + .forPost(SOLANA_URL_REGEX) + .withJsonBodyIncluding({ + method: 'getBalance', + }) + .thenCallback(() => { + return { + statusCode: 200, + json: { + result: { + context: { + apiVersion: '2.0.15', + slot: 305352614, + }, + value: 0, + }, + }, + }; + }); +} + +export async function withSolanaAccountSnap( + { + title, + solanaSupportEnabled, + }: { title?: string; solanaSupportEnabled?: boolean }, + test: (driver: Driver, mockServer: Mockttp) => Promise, +) { + console.log('Starting withSolanaAccountSnap'); + await withFixtures( + { + fixtures: new FixtureBuilder() + .withPreferencesControllerAndFeatureFlag({ + solanaSupportEnabled: solanaSupportEnabled ?? true, + }) + .build(), + title, + dapp: true, + testSpecificMock: async (mockServer: Mockttp) => { + console.log('Setting up test-specific mocks'); + return [await mockSolanaBalanceQuote(mockServer)]; + }, + }, + async ({ driver, mockServer }: { driver: Driver; mockServer: Mockttp }) => { + await loginWithBalanceValidation(driver); + const headerComponen = new HeaderNavbar(driver); + await headerComponen.openAccountMenu(); + const accountListPage = new AccountListPage(driver); + await accountListPage.addAccount(ACCOUNT_TYPE.Solana, 'Solana 1'); + await test(driver, mockServer); + }, + ); +} diff --git a/test/e2e/flask/solana/create-solana-account.spec.ts b/test/e2e/flask/solana/create-solana-account.spec.ts new file mode 100644 index 000000000000..cca4f5222993 --- /dev/null +++ b/test/e2e/flask/solana/create-solana-account.spec.ts @@ -0,0 +1,62 @@ +import { Suite } from 'mocha'; +import HeaderNavbar from '../../page-objects/pages/header-navbar'; +import AccountListPage from '../../page-objects/pages/account-list-page'; +import { ACCOUNT_TYPE } from '../../page-objects/common'; +import { withSolanaAccountSnap } from './common-solana'; + +// Scenarios skipped due to https://consensyssoftware.atlassian.net/browse/SOL-87 +describe('Create Solana Account', function (this: Suite) { + it.skip('Creates 2 Solana accounts', async function () { + await withSolanaAccountSnap( + { title: this.test?.fullTitle() }, + async (driver) => { + // check that we have one Solana account + const headerNavbar = new HeaderNavbar(driver); + await headerNavbar.check_pageIsLoaded(); + await headerNavbar.check_accountLabel('Solana 1'); + await headerNavbar.openAccountMenu(); + const accountListPage = new AccountListPage(driver); + await accountListPage.check_accountDisplayedInAccountList('Account 1'); + await accountListPage.addAccount(ACCOUNT_TYPE.Solana, 'Solana 2'); + await headerNavbar.check_accountLabel('Solana 2'); + await headerNavbar.openAccountMenu(); + await accountListPage.check_numberOfAvailableAccounts(3); + }, + ); + }); + it('Creates a Solana account from the menu', async function () { + await withSolanaAccountSnap( + { title: this.test?.fullTitle() }, + async (driver) => { + const headerNavbar = new HeaderNavbar(driver); + await headerNavbar.check_pageIsLoaded(); + await headerNavbar.check_accountLabel('Solana 1'); + await headerNavbar.openAccountMenu(); + const accountListPage = new AccountListPage(driver); + await accountListPage.check_accountDisplayedInAccountList('Account 1'); + await accountListPage.check_accountDisplayedInAccountList('Solana 1'); + }, + ); + }); +}); +it.skip('Removes Solana account after creating it', async function () { + await withSolanaAccountSnap( + { title: this.test?.fullTitle() }, + async (driver) => { + // check that we have one Solana account + const headerNavbar = new HeaderNavbar(driver); + await headerNavbar.check_accountLabel('Solana 1'); + // check user can cancel the removal of the Solana account + await headerNavbar.openAccountMenu(); + const accountListPage = new AccountListPage(driver); + await accountListPage.check_accountDisplayedInAccountList('Account 1'); + await accountListPage.removeAccount('Solana 1', true); + await headerNavbar.check_accountLabel('Account 1'); + await headerNavbar.openAccountMenu(); + await accountListPage.check_accountDisplayedInAccountList('Account 1'); + await accountListPage.check_accountIsNotDisplayedInAccountList( + 'Solana 1', + ); + }, + ); +}); diff --git a/test/e2e/flask/solana/solana-eth-networks.spec.ts b/test/e2e/flask/solana/solana-eth-networks.spec.ts new file mode 100644 index 000000000000..56a68135fad8 --- /dev/null +++ b/test/e2e/flask/solana/solana-eth-networks.spec.ts @@ -0,0 +1,24 @@ +import { Suite } from 'mocha'; +import HeaderNavbar from '../../page-objects/pages/header-navbar'; +import AccountListPage from '../../page-objects/pages/account-list-page'; +import { withSolanaAccountSnap } from './common-solana'; + +describe('Solana/Evm accounts', function (this: Suite) { + it('Network picker is disabled when Solana account is selected', async function () { + await withSolanaAccountSnap( + { title: this.test?.fullTitle() }, + async (driver) => { + const headerNavbar = new HeaderNavbar(driver); + await headerNavbar.check_pageIsLoaded(); + await headerNavbar.check_accountLabel('Solana 1'); + await headerNavbar.check_currentSelectedNetwork('Solana'); + await headerNavbar.check_ifNetworkPickerClickable(false); + await headerNavbar.openAccountMenu(); + const accountMenu = new AccountListPage(driver); + await accountMenu.switchToAccount('Account 1'); + await headerNavbar.check_currentSelectedNetwork('Localhost 8545'); + await headerNavbar.check_ifNetworkPickerClickable(true); + }, + ); + }); +}); diff --git a/test/e2e/page-objects/common.ts b/test/e2e/page-objects/common.ts index 5bf1a91e1859..40eb625d94ac 100644 --- a/test/e2e/page-objects/common.ts +++ b/test/e2e/page-objects/common.ts @@ -2,3 +2,9 @@ export type RawLocator = | string | { css?: string; text?: string } | { tag: string; text: string }; + +export enum ACCOUNT_TYPE { + Ethereum, + Bitcoin, + Solana, +} diff --git a/test/e2e/page-objects/pages/account-list-page.ts b/test/e2e/page-objects/pages/account-list-page.ts index 50eb70ce553c..628d758e7e71 100644 --- a/test/e2e/page-objects/pages/account-list-page.ts +++ b/test/e2e/page-objects/pages/account-list-page.ts @@ -1,7 +1,7 @@ -import { strict as assert } from 'assert'; import { Driver } from '../../webdriver/driver'; import { largeDelayMs, regularDelayMs } from '../../helpers'; import messages from '../../../../app/_locales/en/messages.json'; +import { ACCOUNT_TYPE } from '../common'; class AccountListPage { private readonly driver: Driver; @@ -40,6 +40,11 @@ class AccountListPage { tag: 'button', }; + private readonly addSolanaAccountButton = { + text: messages.addNewSolanaAccount.message, + tag: 'button', + }; + private readonly addEthereumAccountButton = '[data-testid="multichain-account-menu-popover-add-account"]'; @@ -163,48 +168,11 @@ class AccountListPage { ); } - /** - * Adds a new BTC account with an optional custom name. - * - * @param options - Options for adding a new BTC account. - * @param [options.btcAccountCreationEnabled] - Indicates if the BTC account creation is expected to be enabled or disabled. Defaults to true. - * @param [options.accountName] - The custom name for the BTC account. Defaults to an empty string, which means the default name will be used. - */ - async addNewBtcAccount({ - btcAccountCreationEnabled = true, - accountName = '', - }: { - btcAccountCreationEnabled?: boolean; - accountName?: string; - } = {}): Promise { - console.log( - `Adding new BTC account${ - accountName ? ` with custom name: ${accountName}` : ' with default name' - }`, + async isBtcAccountCreationButtonEnabled() { + const createButton = await this.driver.findElement( + this.addBtcAccountButton, ); - await this.driver.clickElement(this.createAccountButton); - if (btcAccountCreationEnabled) { - await this.driver.clickElement(this.addBtcAccountButton); - // needed to mitigate a race condition with the state update - // there is no condition we can wait for in the UI - await this.driver.delay(largeDelayMs); - if (accountName) { - await this.driver.fill(this.accountNameInput, accountName); - } - await this.driver.clickElementAndWaitToDisappear( - this.addAccountConfirmButton, - // Longer timeout than usual, this reduces the flakiness - // around Bitcoin account creation (mainly required for - // Firefox) - 5000, - ); - } else { - const createButton = await this.driver.findElement( - this.addBtcAccountButton, - ); - assert.equal(await createButton.isEnabled(), false); - await this.driver.clickElement(this.closeAccountModalButton); - } + return await createButton.isEnabled(); } /** @@ -234,6 +202,48 @@ class AccountListPage { } } + /** + * Adds a new account of the specified type with an optional custom name. + * + * @param accountType - The type of account to add (Ethereum, Bitcoin, or Solana) + * @param accountName - Optional custom name for the new account + * @throws {Error} If the specified account type is not supported + * @example + * // Add a new Ethereum account with default name + * await accountListPage.addAccount(ACCOUNT_TYPE.Ethereum); + * + * // Add a new Bitcoin account with custom name + * await accountListPage.addAccount(ACCOUNT_TYPE.Bitcoin, 'My BTC Wallet'); + */ + async addAccount(accountType: ACCOUNT_TYPE, accountName?: string) { + await this.driver.clickElement(this.createAccountButton); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let addAccountButton: any; + switch (accountType) { + case ACCOUNT_TYPE.Ethereum: + addAccountButton = this.addEthereumAccountButton; + break; + case ACCOUNT_TYPE.Bitcoin: + addAccountButton = this.addBtcAccountButton; + break; + case ACCOUNT_TYPE.Solana: + addAccountButton = this.addSolanaAccountButton; + break; + default: + throw new Error('Account type not supported'); + } + + await this.driver.clickElement(addAccountButton); + if (accountName) { + await this.driver.fill(this.accountNameInput, accountName); + } + + await this.driver.clickElementAndWaitToDisappear( + this.addAccountConfirmButton, + 5000, + ); + } + /** * Changes the label of the current account. * @@ -600,11 +610,17 @@ class AccountListPage { console.log( `Verify the number of accounts in the account menu is: ${expectedNumberOfAccounts}`, ); + + await this.driver.waitForSelector(this.accountListItem); await this.driver.wait(async () => { const internalAccounts = await this.driver.findElements( this.accountListItem, ); - return internalAccounts.length === expectedNumberOfAccounts; + const isValid = internalAccounts.length === expectedNumberOfAccounts; + console.log( + `Number of accounts: ${internalAccounts.length} is equal to ${expectedNumberOfAccounts}? ${isValid}`, + ); + return isValid; }, 20000); } diff --git a/test/e2e/page-objects/pages/header-navbar.ts b/test/e2e/page-objects/pages/header-navbar.ts index df45ecd0277d..57e02e864624 100644 --- a/test/e2e/page-objects/pages/header-navbar.ts +++ b/test/e2e/page-objects/pages/header-navbar.ts @@ -1,3 +1,4 @@ +import { strict as assert } from 'assert'; import { Driver } from '../../webdriver/driver'; class HeaderNavbar { @@ -22,6 +23,8 @@ class HeaderNavbar { private readonly switchNetworkDropDown = '[data-testid="network-display"]'; + private readonly networkPicker = '.mm-picker-network'; + constructor(driver: Driver) { this.driver = driver; } @@ -80,6 +83,21 @@ class HeaderNavbar { await this.driver.clickElement(this.switchNetworkDropDown); } + async check_currentSelectedNetwork(networkName: string): Promise { + console.log(`Validate the Switch network to ${networkName}`); + await this.driver.waitForSelector( + `button[data-testid="network-display"][aria-label="Network Menu ${networkName}"]`, + ); + } + + async check_ifNetworkPickerClickable(clickable: boolean): Promise { + console.log('Check whether the network picker is clickable or not'); + assert.equal( + await (await this.driver.findElement(this.networkPicker)).isEnabled(), + clickable, + ); + } + /** * Verifies that the displayed account label in header matches the expected label. * @@ -94,18 +112,6 @@ class HeaderNavbar { text: expectedLabel, }); } - - /** - * Validates that the currently selected network matches the expected network name. - * - * @param networkName - The expected name of the currently selected network. - */ - async check_currentSelectedNetwork(networkName: string): Promise { - console.log(`Validate the Switch network to ${networkName}`); - await this.driver.waitForSelector( - `button[data-testid="network-display"][aria-label="Network Menu ${networkName}"]`, - ); - } } export default HeaderNavbar; From 749d0acc42a1a10b00da25e0dd1571a0f2054d0f Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 19 Dec 2024 11:01:41 +0000 Subject: [PATCH 03/19] fix: Spending cap flicker as decimals are determined (#29206) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Sometimes, when approving a token, the spending cap displayed in the transaction simulation component flickers for a split second. This can also seen when editing the spending cap on the same screen. In the approve screen, `useAssetDetails` returns `decimals` at first as `undefined` while it's determined asynchronously. Since `useApproveTokenSimulation` takes `decimals` as an argument, a default of `'0'` was set to quiet the type-checker. This default is what provokes the UI flicker. In the example video below, the token has 4 decimals and the spending cap is 70000 / 10 ** 4 = 7. But while decimals is still `undefined`, `'0'` is used in `useApproveTokenSimulation` to determine the spending cap (dividing the value by 10 to the number of decimals). This amounts to `70000` instead of `7` for a split second, before decimals `'4'` is returned by `useAssetDetails`. The fix for this bug is to let the loading spinner linger a few miliseconds longer while decimals is still `undefined`. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29206?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/a09f449a-78ea-4083-b47b-2f329126d4b6 ### **After** https://github.com/user-attachments/assets/68142912-ae20-47f6-8dd4-2b9184b57bbf ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../signatures/permit-batch.test.tsx | 21 ++++++++++++++--- .../signatures/permit-seaport.test.tsx | 21 ++++++++++++++--- .../signatures/permit-single.test.tsx | 21 ++++++++++++++--- .../signatures/permit-tradeOrder.test.tsx | 21 ++++++++++++++--- .../confirmations/signatures/permit.test.tsx | 15 ++++++++++++ .../signatures/personalSign.test.tsx | 17 +++++++++++++- .../transactions/alerts.test.tsx | 23 +++++++++++++++---- .../transactions/contract-deployment.test.tsx | 15 ++++++++++++ .../contract-interaction.test.tsx | 15 ++++++++++++ .../transactions/erc20-approve.test.tsx | 15 ++++++++++++ .../transactions/erc721-approve.test.tsx | 16 +++++++++++++ .../transactions/increase-allowance.test.tsx | 15 ++++++++++++ .../set-approval-for-all.test.tsx | 15 ++++++++++++ .../approve-static-simulation.tsx | 4 +--- .../confirm/info/approve/approve.test.tsx | 16 ++++++++++++- .../confirm/info/approve/approve.tsx | 4 ++-- .../edit-spending-cap-modal.tsx | 2 +- .../hooks/use-approve-token-simulation.ts | 4 ++-- .../approve/spending-cap/spending-cap.tsx | 5 +--- .../components/confirm/info/info.test.tsx | 17 ++++++++++++++ .../title/hooks/useCurrentSpendingCap.ts | 2 +- .../confirmations/confirm/confirm.test.tsx | 16 +++++++++++++ 22 files changed, 269 insertions(+), 31 deletions(-) diff --git a/test/integration/confirmations/signatures/permit-batch.test.tsx b/test/integration/confirmations/signatures/permit-batch.test.tsx index ea311537001c..7a3050dc15c8 100644 --- a/test/integration/confirmations/signatures/permit-batch.test.tsx +++ b/test/integration/confirmations/signatures/permit-batch.test.tsx @@ -1,10 +1,11 @@ import { act, fireEvent, screen } from '@testing-library/react'; import nock from 'nock'; -import mockMetaMaskState from '../../data/integration-init-state.json'; -import { integrationTestRender } from '../../../lib/render-helpers'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; -import { createMockImplementation } from '../../helpers'; import { tEn } from '../../../lib/i18n-helpers'; +import { integrationTestRender } from '../../../lib/render-helpers'; +import mockMetaMaskState from '../../data/integration-init-state.json'; +import { createMockImplementation } from '../../helpers'; import { getMetaMaskStateWithUnapprovedPermitSign, verifyDetails, @@ -15,10 +16,20 @@ jest.mock('../../../../ui/store/background-connection', () => ({ submitRequestToBackground: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); const backgroundConnectionMocked = { onNotification: jest.fn(), }; +const mockedAssetDetails = jest.mocked(useAssetDetails); const renderPermitBatchSignature = async () => { const account = @@ -58,6 +69,10 @@ describe('Permit Batch Signature Tests', () => { getTokenStandardAndDetails: { decimals: '2' }, }), ); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/signatures/permit-seaport.test.tsx b/test/integration/confirmations/signatures/permit-seaport.test.tsx index 7829a2714b57..dc32671da245 100644 --- a/test/integration/confirmations/signatures/permit-seaport.test.tsx +++ b/test/integration/confirmations/signatures/permit-seaport.test.tsx @@ -1,10 +1,11 @@ import { act, fireEvent, screen } from '@testing-library/react'; import nock from 'nock'; -import mockMetaMaskState from '../../data/integration-init-state.json'; -import { integrationTestRender } from '../../../lib/render-helpers'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; -import { createMockImplementation } from '../../helpers'; import { tEn } from '../../../lib/i18n-helpers'; +import { integrationTestRender } from '../../../lib/render-helpers'; +import mockMetaMaskState from '../../data/integration-init-state.json'; +import { createMockImplementation } from '../../helpers'; import { getMetaMaskStateWithUnapprovedPermitSign, verifyDetails, @@ -15,10 +16,20 @@ jest.mock('../../../../ui/store/background-connection', () => ({ submitRequestToBackground: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); const backgroundConnectionMocked = { onNotification: jest.fn(), }; +const mockedAssetDetails = jest.mocked(useAssetDetails); const renderSeaportSignature = async () => { const account = @@ -58,6 +69,10 @@ describe('Permit Seaport Tests', () => { getTokenStandardAndDetails: { decimals: '2' }, }), ); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/signatures/permit-single.test.tsx b/test/integration/confirmations/signatures/permit-single.test.tsx index abb5d07c1587..96fe5c26491d 100644 --- a/test/integration/confirmations/signatures/permit-single.test.tsx +++ b/test/integration/confirmations/signatures/permit-single.test.tsx @@ -1,10 +1,11 @@ import { act, fireEvent, screen } from '@testing-library/react'; import nock from 'nock'; -import mockMetaMaskState from '../../data/integration-init-state.json'; -import { integrationTestRender } from '../../../lib/render-helpers'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; -import { createMockImplementation } from '../../helpers'; import { tEn } from '../../../lib/i18n-helpers'; +import { integrationTestRender } from '../../../lib/render-helpers'; +import mockMetaMaskState from '../../data/integration-init-state.json'; +import { createMockImplementation } from '../../helpers'; import { getMetaMaskStateWithUnapprovedPermitSign, verifyDetails, @@ -15,10 +16,20 @@ jest.mock('../../../../ui/store/background-connection', () => ({ submitRequestToBackground: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); const backgroundConnectionMocked = { onNotification: jest.fn(), }; +const mockedAssetDetails = jest.mocked(useAssetDetails); const renderSingleBatchSignature = async () => { const account = @@ -58,6 +69,10 @@ describe('Permit Single Signature Tests', () => { getTokenStandardAndDetails: { decimals: '2' }, }), ); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/signatures/permit-tradeOrder.test.tsx b/test/integration/confirmations/signatures/permit-tradeOrder.test.tsx index 429e533ff8f8..3f915967dc00 100644 --- a/test/integration/confirmations/signatures/permit-tradeOrder.test.tsx +++ b/test/integration/confirmations/signatures/permit-tradeOrder.test.tsx @@ -1,10 +1,11 @@ import { act, screen } from '@testing-library/react'; import nock from 'nock'; -import mockMetaMaskState from '../../data/integration-init-state.json'; -import { integrationTestRender } from '../../../lib/render-helpers'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; -import { createMockImplementation } from '../../helpers'; import { tEn } from '../../../lib/i18n-helpers'; +import { integrationTestRender } from '../../../lib/render-helpers'; +import mockMetaMaskState from '../../data/integration-init-state.json'; +import { createMockImplementation } from '../../helpers'; import { getMetaMaskStateWithUnapprovedPermitSign, verifyDetails, @@ -15,10 +16,20 @@ jest.mock('../../../../ui/store/background-connection', () => ({ submitRequestToBackground: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); const backgroundConnectionMocked = { onNotification: jest.fn(), }; +const mockedAssetDetails = jest.mocked(useAssetDetails); const renderTradeOrderSignature = async () => { const account = @@ -58,6 +69,10 @@ describe('Permit Trade Order Tests', () => { getTokenStandardAndDetails: { decimals: '2' }, }), ); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/signatures/permit.test.tsx b/test/integration/confirmations/signatures/permit.test.tsx index 6b736e4add90..332cebc3a6b2 100644 --- a/test/integration/confirmations/signatures/permit.test.tsx +++ b/test/integration/confirmations/signatures/permit.test.tsx @@ -7,6 +7,7 @@ import { MetaMetricsEventName, } from '../../../../shared/constants/metametrics'; import { shortenAddress } from '../../../../ui/helpers/utils/util'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { integrationTestRender } from '../../../lib/render-helpers'; import mockMetaMaskState from '../../data/integration-init-state.json'; @@ -18,10 +19,20 @@ jest.mock('../../../../ui/store/background-connection', () => ({ submitRequestToBackground: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); const backgroundConnectionMocked = { onNotification: jest.fn(), }; +const mockedAssetDetails = jest.mocked(useAssetDetails); describe('Permit Confirmation', () => { beforeEach(() => { @@ -31,6 +42,10 @@ describe('Permit Confirmation', () => { getTokenStandardAndDetails: { decimals: '2', standard: 'ERC20' }, }), ); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/signatures/personalSign.test.tsx b/test/integration/confirmations/signatures/personalSign.test.tsx index 03685f46ab7b..d58e0d441e03 100644 --- a/test/integration/confirmations/signatures/personalSign.test.tsx +++ b/test/integration/confirmations/signatures/personalSign.test.tsx @@ -1,6 +1,6 @@ import { ApprovalType } from '@metamask/controller-utils'; -import { act, fireEvent, screen, waitFor } from '@testing-library/react'; import { CHAIN_IDS } from '@metamask/transaction-controller'; +import { act, fireEvent, screen, waitFor } from '@testing-library/react'; import { MESSAGE_TYPE } from '../../../../shared/constants/app'; import { MetaMetricsEventCategory, @@ -8,6 +8,7 @@ import { MetaMetricsEventName, } from '../../../../shared/constants/metametrics'; import { shortenAddress } from '../../../../ui/helpers/utils/util'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { integrationTestRender } from '../../../lib/render-helpers'; import mockMetaMaskState from '../../data/integration-init-state.json'; @@ -17,7 +18,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ submitRequestToBackground: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -68,6 +79,10 @@ const getMetaMaskStateWithUnapprovedPersonalSign = (accountAddress: string) => { describe('PersonalSign Confirmation', () => { beforeEach(() => { jest.resetAllMocks(); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); it('displays the header account modal with correct data', async () => { diff --git a/test/integration/confirmations/transactions/alerts.test.tsx b/test/integration/confirmations/transactions/alerts.test.tsx index b21a7dbb679c..ee7e95762ac3 100644 --- a/test/integration/confirmations/transactions/alerts.test.tsx +++ b/test/integration/confirmations/transactions/alerts.test.tsx @@ -1,12 +1,13 @@ import { randomUUID } from 'crypto'; -import { act, fireEvent, screen } from '@testing-library/react'; import { ApprovalType } from '@metamask/controller-utils'; +import { act, fireEvent, screen } from '@testing-library/react'; import nock from 'nock'; -import mockMetaMaskState from '../../data/integration-init-state.json'; -import { integrationTestRender } from '../../../lib/render-helpers'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; -import { createMockImplementation, mock4byte } from '../../helpers'; +import { integrationTestRender } from '../../../lib/render-helpers'; import { createTestProviderTools } from '../../../stub/provider'; +import mockMetaMaskState from '../../data/integration-init-state.json'; +import { createMockImplementation, mock4byte } from '../../helpers'; import { getUnapprovedApproveTransaction } from './transactionDataHelpers'; jest.mock('../../../../ui/store/background-connection', () => ({ @@ -15,7 +16,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ callBackgroundMethod: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -92,6 +103,10 @@ describe('Contract Interaction Confirmation Alerts', () => { setupSubmitRequestToBackgroundMocks(); const APPROVE_NFT_HEX_SIG = '0x095ea7b3'; mock4byte(APPROVE_NFT_HEX_SIG); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/transactions/contract-deployment.test.tsx b/test/integration/confirmations/transactions/contract-deployment.test.tsx index 7698ce3ef8b2..45410c0a76d9 100644 --- a/test/integration/confirmations/transactions/contract-deployment.test.tsx +++ b/test/integration/confirmations/transactions/contract-deployment.test.tsx @@ -13,6 +13,7 @@ import { MetaMetricsEventLocation, MetaMetricsEventName, } from '../../../../shared/constants/metametrics'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { tEn } from '../../../lib/i18n-helpers'; import { integrationTestRender } from '../../../lib/render-helpers'; @@ -26,7 +27,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ callBackgroundMethod: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -136,6 +147,10 @@ describe('Contract Deployment Confirmation', () => { setupSubmitRequestToBackgroundMocks(); const DEPOSIT_HEX_SIG = '0xd0e30db0'; mock4byte(DEPOSIT_HEX_SIG); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/transactions/contract-interaction.test.tsx b/test/integration/confirmations/transactions/contract-interaction.test.tsx index 5db121bc9fda..e3da57aeceb3 100644 --- a/test/integration/confirmations/transactions/contract-interaction.test.tsx +++ b/test/integration/confirmations/transactions/contract-interaction.test.tsx @@ -13,6 +13,7 @@ import { MetaMetricsEventLocation, MetaMetricsEventName, } from '../../../../shared/constants/metametrics'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { tEn } from '../../../lib/i18n-helpers'; import { integrationTestRender } from '../../../lib/render-helpers'; @@ -31,7 +32,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ callBackgroundMethod: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -156,6 +167,10 @@ describe('Contract Interaction Confirmation', () => { setupSubmitRequestToBackgroundMocks(); const MINT_NFT_HEX_SIG = '0x3b4b1381'; mock4byte(MINT_NFT_HEX_SIG); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/transactions/erc20-approve.test.tsx b/test/integration/confirmations/transactions/erc20-approve.test.tsx index c25b2ee3627d..f77c8162b79d 100644 --- a/test/integration/confirmations/transactions/erc20-approve.test.tsx +++ b/test/integration/confirmations/transactions/erc20-approve.test.tsx @@ -3,6 +3,7 @@ import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import nock from 'nock'; import { TokenStandard } from '../../../../shared/constants/transaction'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { tEn } from '../../../lib/i18n-helpers'; import { integrationTestRender } from '../../../lib/render-helpers'; @@ -17,7 +18,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ callBackgroundMethod: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockImplementation(() => ({ + decimals: '4', + })), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -140,6 +151,10 @@ describe('ERC20 Approve Confirmation', () => { const APPROVE_ERC20_HEX_SIG = '0x095ea7b3'; const APPROVE_ERC20_TEXT_SIG = 'approve(address,uint256)'; mock4byte(APPROVE_ERC20_HEX_SIG, APPROVE_ERC20_TEXT_SIG); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/transactions/erc721-approve.test.tsx b/test/integration/confirmations/transactions/erc721-approve.test.tsx index 4f211576e6cd..cce5456ae0a2 100644 --- a/test/integration/confirmations/transactions/erc721-approve.test.tsx +++ b/test/integration/confirmations/transactions/erc721-approve.test.tsx @@ -3,6 +3,7 @@ import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import nock from 'nock'; import { TokenStandard } from '../../../../shared/constants/transaction'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { tEn } from '../../../lib/i18n-helpers'; import { integrationTestRender } from '../../../lib/render-helpers'; @@ -17,7 +18,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ callBackgroundMethod: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -140,6 +151,10 @@ describe('ERC721 Approve Confirmation', () => { const APPROVE_NFT_HEX_SIG = '0x095ea7b3'; const APPROVE_NFT_TEXT_SIG = 'approve(address,uint256)'; mock4byte(APPROVE_NFT_HEX_SIG, APPROVE_NFT_TEXT_SIG); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { @@ -220,6 +235,7 @@ describe('ERC721 Approve Confirmation', () => { const approveDetails = await screen.findByTestId( 'confirmation__approve-details', ); + expect(approveDetails).toBeInTheDocument(); const approveDetailsSpender = await screen.findByTestId( 'confirmation__approve-spender', diff --git a/test/integration/confirmations/transactions/increase-allowance.test.tsx b/test/integration/confirmations/transactions/increase-allowance.test.tsx index 810477d3a3a5..49b33bf2d21b 100644 --- a/test/integration/confirmations/transactions/increase-allowance.test.tsx +++ b/test/integration/confirmations/transactions/increase-allowance.test.tsx @@ -3,6 +3,7 @@ import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import nock from 'nock'; import { TokenStandard } from '../../../../shared/constants/transaction'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { tEn } from '../../../lib/i18n-helpers'; import { integrationTestRender } from '../../../lib/render-helpers'; @@ -17,7 +18,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ callBackgroundMethod: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -144,6 +155,10 @@ describe('ERC20 increaseAllowance Confirmation', () => { INCREASE_ALLOWANCE_ERC20_HEX_SIG, INCREASE_ALLOWANCE_ERC20_TEXT_SIG, ); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/test/integration/confirmations/transactions/set-approval-for-all.test.tsx b/test/integration/confirmations/transactions/set-approval-for-all.test.tsx index ebe680983a6c..236162c7b08f 100644 --- a/test/integration/confirmations/transactions/set-approval-for-all.test.tsx +++ b/test/integration/confirmations/transactions/set-approval-for-all.test.tsx @@ -3,6 +3,7 @@ import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import nock from 'nock'; import { TokenStandard } from '../../../../shared/constants/transaction'; +import { useAssetDetails } from '../../../../ui/pages/confirmations/hooks/useAssetDetails'; import * as backgroundConnection from '../../../../ui/store/background-connection'; import { tEn } from '../../../lib/i18n-helpers'; import { integrationTestRender } from '../../../lib/render-helpers'; @@ -17,7 +18,17 @@ jest.mock('../../../../ui/store/background-connection', () => ({ callBackgroundMethod: jest.fn(), })); +jest.mock('../../../../ui/pages/confirmations/hooks/useAssetDetails', () => ({ + ...jest.requireActual( + '../../../../ui/pages/confirmations/hooks/useAssetDetails', + ), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const mockedBackgroundConnection = jest.mocked(backgroundConnection); +const mockedAssetDetails = jest.mocked(useAssetDetails); const backgroundConnectionMocked = { onNotification: jest.fn(), @@ -144,6 +155,10 @@ describe('ERC721 setApprovalForAll Confirmation', () => { INCREASE_SET_APPROVAL_FOR_ALL_HEX_SIG, INCREASE_SET_APPROVAL_FOR_ALL_TEXT_SIG, ); + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); }); afterEach(() => { diff --git a/ui/pages/confirmations/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx b/ui/pages/confirmations/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx index 8d66b17a1f46..67ff183784c9 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx @@ -27,15 +27,13 @@ export const ApproveStaticSimulation = () => { const { currentConfirmation: transactionMeta } = useConfirmContext(); - const { decimals: initialDecimals } = useAssetDetails( + const { decimals } = useAssetDetails( transactionMeta?.txParams?.to, transactionMeta?.txParams?.from, transactionMeta?.txParams?.data, transactionMeta?.chainId, ); - const decimals = initialDecimals || '0'; - const { spendingCap, isUnlimitedSpendingCap, diff --git a/ui/pages/confirmations/components/confirm/info/approve/approve.test.tsx b/ui/pages/confirmations/components/confirm/info/approve/approve.test.tsx index 914e276369f5..f69671dccf9f 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/approve.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/approve.test.tsx @@ -1,8 +1,10 @@ +import { screen, waitFor } from '@testing-library/dom'; import React from 'react'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { getMockApproveConfirmState } from '../../../../../../../test/data/confirmations/helper'; import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; +import { useAssetDetails } from '../../../../hooks/useAssetDetails'; import ApproveInfo from './approve'; jest.mock('../../../../../../store/actions', () => ({ @@ -32,7 +34,7 @@ jest.mock('./hooks/use-approve-token-simulation', () => ({ jest.mock('../../../../hooks/useAssetDetails', () => ({ useAssetDetails: jest.fn(() => ({ - decimals: 18, + decimals: '18', })), })); @@ -70,6 +72,14 @@ jest.mock('../hooks/useDecodedTransactionData', () => ({ describe('', () => { const middleware = [thunk]; + const mockedAssetDetails = jest.mocked(useAssetDetails); + + beforeEach(() => { + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); + }); it('renders component for approve request', async () => { const state = getMockApproveConfirmState(); @@ -81,6 +91,10 @@ describe('', () => { mockStore, ); + await waitFor(() => { + expect(screen.getByText('Speed')).toBeInTheDocument(); + }); + expect(container).toMatchSnapshot(); }); }); diff --git a/ui/pages/confirmations/components/confirm/info/approve/approve.tsx b/ui/pages/confirmations/components/confirm/info/approve/approve.tsx index d1d9fbc08364..e5015af533fd 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/approve.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/approve.tsx @@ -35,7 +35,7 @@ const ApproveInfo = () => { const { spendingCap, pending } = useApproveTokenSimulation( transactionMeta, - decimals || '0', + decimals, ); const showRevokeVariant = @@ -46,7 +46,7 @@ const ApproveInfo = () => { return null; } - if (pending) { + if (pending || (!isNFT && !decimals)) { return ; } diff --git a/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx b/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx index 1cd080de96d4..f53c7bae42b2 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx @@ -64,7 +64,7 @@ export const EditSpendingCapModal = ({ const { formattedSpendingCap, spendingCap } = useApproveTokenSimulation( transactionMeta, - decimals || '0', + decimals, ); const [customSpendingCapInputValue, setCustomSpendingCapInputValue] = diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts index acd470ba8822..74c857f7078b 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts @@ -18,7 +18,7 @@ function isSpendingCapUnlimited(decodedSpendingCap: number) { export const useApproveTokenSimulation = ( transactionMeta: TransactionMeta, - decimals: string, + decimals: string | undefined, ) => { const locale = useSelector(getIntlLocale); const { isNFT, pending: isNFTPending } = useIsNFT(transactionMeta); @@ -43,7 +43,7 @@ export const useApproveTokenSimulation = ( return calcTokenAmount( value.data[0].params[paramIndex].value, - Number(decimals), + Number(decimals || '0'), ).toFixed(); }, [value, decimals]); diff --git a/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx b/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx index 29d009c5b810..3a8c6a5a26ec 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx +++ b/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx @@ -90,10 +90,7 @@ export const SpendingCap = ({ Number(decimals ?? '0'), ).toFixed(); - const { pending } = useApproveTokenSimulation( - transactionMeta, - decimals || '0', - ); + const { pending } = useApproveTokenSimulation(transactionMeta, decimals); if (pending) { return ; diff --git a/ui/pages/confirmations/components/confirm/info/info.test.tsx b/ui/pages/confirmations/components/confirm/info/info.test.tsx index 75d98d91a1bd..50046ec96764 100644 --- a/ui/pages/confirmations/components/confirm/info/info.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/info.test.tsx @@ -9,6 +9,7 @@ import { getMockTypedSignConfirmState, } from '../../../../../../test/data/confirmations/helper'; import { renderWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; +import { useAssetDetails } from '../../../hooks/useAssetDetails'; import Info from './info'; jest.mock( @@ -28,7 +29,23 @@ jest.mock('../../../../../store/actions', () => ({ }), })); +jest.mock('../../../hooks/useAssetDetails', () => ({ + ...jest.requireActual('../../../hooks/useAssetDetails'), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + describe('Info', () => { + const mockedAssetDetails = jest.mocked(useAssetDetails); + + beforeEach(() => { + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); + }); + it('renders info section for personal sign request', () => { const state = getMockPersonalSignConfirmState(); const mockStore = configureMockStore([])(state); diff --git a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.ts b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.ts index b50948259734..5fa3918804d7 100644 --- a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.ts +++ b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.ts @@ -43,7 +43,7 @@ export function useCurrentSpendingCap(currentConfirmation: Confirmation) { const { spendingCap, pending } = useApproveTokenSimulation( currentConfirmation as TransactionMeta, - decimals || '0', + decimals, ); let customSpendingCap = ''; diff --git a/ui/pages/confirmations/confirm/confirm.test.tsx b/ui/pages/confirmations/confirm/confirm.test.tsx index 939ca8768afe..158884fc36a0 100644 --- a/ui/pages/confirmations/confirm/confirm.test.tsx +++ b/ui/pages/confirmations/confirm/confirm.test.tsx @@ -16,6 +16,7 @@ import { import mockState from '../../../../test/data/mock-state.json'; import { renderWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers'; import * as actions from '../../../store/actions'; +import { useAssetDetails } from '../hooks/useAssetDetails'; import { SignatureRequestType } from '../types/confirm'; import { memoizedGetTokenStandardAndDetails } from '../utils/token'; import Confirm from './confirm'; @@ -27,7 +28,15 @@ jest.mock('react-router-dom', () => ({ }), })); +jest.mock('../hooks/useAssetDetails', () => ({ + ...jest.requireActual('../hooks/useAssetDetails'), + useAssetDetails: jest.fn().mockResolvedValue({ + decimals: '4', + }), +})); + const middleware = [thunk]; +const mockedAssetDetails = jest.mocked(useAssetDetails); describe('Confirm', () => { afterEach(() => { @@ -37,6 +46,13 @@ describe('Confirm', () => { memoizedGetTokenStandardAndDetails?.cache?.clear?.(); }); + beforeEach(() => { + mockedAssetDetails.mockImplementation(() => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + decimals: '4' as any, + })); + }); + it('should render', () => { const mockStore = configureMockStore(middleware)(mockState); From a1b41c4e49497805f437011b643e99cf1680ebb2 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Thu, 19 Dec 2024 16:38:26 +0530 Subject: [PATCH 04/19] fix: personal sign message - decode message to UTF-8 string only if it is valid UTF-8 (#29232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Converts personal sign message to UTF-8 string only if it can be converted to valid UTF-8 string. ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/3931 ## **Manual testing steps** 1. Send personal sign request to extension with message string `0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470` 2. Check on confirmation page that it is displayed as UTF-8 string ## **Screenshots/Recordings** Screenshot 2024-12-16 at 6 53 30 PM ## **Pre-merge author checklist** - [X] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../info/personal-sign/personal-sign.test.tsx | 26 +++++++++++++++++-- .../info/personal-sign/personal-sign.tsx | 13 ++++++++-- .../components/confirm/info/utils.test.ts | 9 +++++++ .../components/confirm/info/utils.ts | 14 ++++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx index e105493485ec..3b5d1dfc3e62 100644 --- a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx @@ -9,9 +9,13 @@ import { getMockTypedSignConfirmStateForRequest, } from '../../../../../../../test/data/confirmations/helper'; import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; -import { signatureRequestSIWE } from '../../../../../../../test/data/confirmations/personal_sign'; -import * as utils from '../../../../utils'; +import { + signatureRequestSIWE, + unapprovedPersonalSignMsg, +} from '../../../../../../../test/data/confirmations/personal_sign'; import * as snapUtils from '../../../../../../helpers/utils/snaps'; +import { SignatureRequestType } from '../../../../types/confirm'; +import * as utils from '../../../../utils'; import PersonalSignInfo from './personal-sign'; jest.mock( @@ -184,4 +188,22 @@ describe('PersonalSignInfo', () => { queryByText('This is the site asking for your signature.'), ).toBeDefined(); }); + + it('display hex message value if it can not be converted to valid UTF-8 string', () => { + const message = + '0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470'; + const state = getMockPersonalSignConfirmStateForRequest({ + ...unapprovedPersonalSignMsg, + msgParams: { + ...unapprovedPersonalSignMsg.msgParams, + data: message, + }, + } as SignatureRequestType); + const mockStore = configureMockStore([])(state); + const { getByText } = renderWithConfirmContextProvider( + , + mockStore, + ); + expect(getByText(message)).toBeDefined(); + }); }); diff --git a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx index bd6c41866917..38238a2fa49e 100644 --- a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx +++ b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx @@ -38,8 +38,17 @@ import { selectUseTransactionSimulations } from '../../../../selectors/preferenc import { SignatureRequestType } from '../../../../types/confirm'; import { isSIWESignatureRequest } from '../../../../utils'; import { SigningInWithRow } from '../shared/sign-in-with-row/sign-in-with-row'; +import { isValidUTF8 } from '../utils'; import { SIWESignInfo } from './siwe-sign'; +const getMessageText = (hexString?: string) => { + if (!hexString) { + return hexString; + } + const messageText = sanitizeString(hexToText(hexString)); + return isValidUTF8(messageText) ? messageText : hexString; +}; + const PersonalSignInfo: React.FC = () => { const t = useI18nContext(); const { currentConfirmation } = useConfirmContext(); @@ -52,8 +61,8 @@ const PersonalSignInfo: React.FC = () => { } const isSIWE = isSIWESignatureRequest(currentConfirmation); - const messageText = sanitizeString( - hexToText(currentConfirmation.msgParams?.data), + const messageText = getMessageText( + currentConfirmation.msgParams?.data as string, ); let toolTipMessage; diff --git a/ui/pages/confirmations/components/confirm/info/utils.test.ts b/ui/pages/confirmations/components/confirm/info/utils.test.ts index 9c12b9127811..8723cfc3f05d 100644 --- a/ui/pages/confirmations/components/confirm/info/utils.test.ts +++ b/ui/pages/confirmations/components/confirm/info/utils.test.ts @@ -4,6 +4,7 @@ import { DecodedTransactionDataSource } from '../../../../../../shared/types/tra import { getIsRevokeSetApprovalForAll, hasValueAndNativeBalanceMismatch, + isValidUTF8, } from './utils'; describe('getIsRevokeSetApprovalForAll', () => { @@ -141,3 +142,11 @@ describe('hasValueAndNativeBalanceMismatch', () => { expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(true); }); }); + +describe('isValidUTF8', () => { + it('returns true for valid UTF-8 string', () => { + expect(isValidUTF8('Hello')).toEqual(true); + expect(isValidUTF8('\xC3\x28')).toEqual(true); + expect(isValidUTF8('😀')).toEqual(true); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/utils.ts b/ui/pages/confirmations/components/confirm/info/utils.ts index 1af918aea74e..17b10e555952 100644 --- a/ui/pages/confirmations/components/confirm/info/utils.ts +++ b/ui/pages/confirmations/components/confirm/info/utils.ts @@ -110,3 +110,17 @@ export function hasValueAndNativeBalanceMismatch( nativeBalanceChange?.isDecrease === false, ); } + +export function isValidUTF8(inputString: string) { + try { + const encoder = new TextEncoder(); + const encoded = encoder.encode(inputString); + + const decoder = new TextDecoder('utf-8', { fatal: true }); + decoder.decode(encoded); + + return true; + } catch (e) { + return false; + } +} From 3b7c9cbc9f0399d138229ad152ebec4eeadd13da Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 19 Dec 2024 11:32:15 +0000 Subject: [PATCH 05/19] fix: Ellipsis displayed for petname even though the full name is visible (#29282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Previously, if a pet name with 12 characters would be set, the truncation would be triggered with the 12 chars plus the three ellipsis dots. This would result in a elongated pill component that would be too long. To fix this, we now truncate at 12 characters, but showing 9 characters only plus the three dots, totalling the same maximum 12 chars. Examples: - Very large account name (23 characters) -> Very larg... (9 + 3 characters) - My DeFi Account (15 characters) -> My DeFi A... (9 + 3 characters) - DeFi Account (12 characters) -> DeFi Acco... (9 + 3 characters) - Own Account (11 characters) -> Own Account (11 characters) [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29282?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3775 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-12-17 at 14 50 02 Screenshot 2024-12-17 at 14 49 33 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- test/e2e/tests/petnames/petnames-signatures.spec.js | 4 ++-- ui/components/app/name/__snapshots__/name.test.tsx.snap | 2 +- .../name-details/__snapshots__/name-details.test.tsx.snap | 2 +- ui/components/app/name/name.tsx | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/tests/petnames/petnames-signatures.spec.js b/test/e2e/tests/petnames/petnames-signatures.spec.js index 4641424e053f..1ca7f47340d7 100644 --- a/test/e2e/tests/petnames/petnames-signatures.spec.js +++ b/test/e2e/tests/petnames/petnames-signatures.spec.js @@ -169,7 +169,7 @@ describe('Petnames - Signatures', function () { await createSignatureRequest(driver, SIGNATURE_TYPE.TYPED_V4); await switchToNotificationWindow(driver, 3); await expectName(driver, 'test.lens', true); - await expectName(driver, 'Test Token 2', true); + await expectName(driver, 'Test Toke...', true); await showThirdPartyDetails(driver); await expectName(driver, 'Custom Name', true); }, @@ -269,7 +269,7 @@ describe('Petnames - Signatures', function () { await createSignatureRequest(driver, SIGNATURE_TYPE.TYPED_V4); await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await expectName(driver, 'test.lens', true); - await expectName(driver, 'Test Token 2', true); + await expectName(driver, 'Test Toke...', true); await expectName(driver, 'Custom Name', true); }, ); diff --git a/ui/components/app/name/__snapshots__/name.test.tsx.snap b/ui/components/app/name/__snapshots__/name.test.tsx.snap index 286429760c1e..883717fca1f4 100644 --- a/ui/components/app/name/__snapshots__/name.test.tsx.snap +++ b/ui/components/app/name/__snapshots__/name.test.tsx.snap @@ -79,7 +79,7 @@ exports[`Name renders address with long saved name 1`] = `

- Very long an... + Very long...

diff --git a/ui/components/app/name/name-details/__snapshots__/name-details.test.tsx.snap b/ui/components/app/name/name-details/__snapshots__/name-details.test.tsx.snap index 3520a1b64a13..8ddce02439c9 100644 --- a/ui/components/app/name/name-details/__snapshots__/name-details.test.tsx.snap +++ b/ui/components/app/name/name-details/__snapshots__/name-details.test.tsx.snap @@ -753,7 +753,7 @@ exports[`NameDetails renders with recognized name 1`] = `

- iZUMi Bond U... + iZUMi Bon...

diff --git a/ui/components/app/name/name.tsx b/ui/components/app/name/name.tsx index 4871cc481f0c..1b5305c2b61f 100644 --- a/ui/components/app/name/name.tsx +++ b/ui/components/app/name/name.tsx @@ -106,7 +106,7 @@ const Name = memo( const MAX_PET_NAME_LENGTH = 12; const formattedName = shortenString(name || undefined, { truncatedCharLimit: MAX_PET_NAME_LENGTH, - truncatedStartChars: MAX_PET_NAME_LENGTH, + truncatedStartChars: MAX_PET_NAME_LENGTH - 3, truncatedEndChars: 0, skipCharacterInEnd: true, }); From 5855938e202d2babe05cb1cd8785ef12fb3e0959 Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Thu, 19 Dec 2024 12:36:23 +0100 Subject: [PATCH 06/19] chore: bump `@metamask/user-operation-controller` to `^21.0.0` (#29089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR bumps `@metamask/user-operation-controller` to `^21.0.0` [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29089?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/28986 ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot --- lavamoat/browserify/beta/policy.json | 24 ++++++++-- lavamoat/browserify/flask/policy.json | 24 ++++++++-- lavamoat/browserify/main/policy.json | 24 ++++++++-- lavamoat/browserify/mmi/policy.json | 24 ++++++++-- package.json | 2 +- ui/ducks/metamask/metamask.js | 2 +- yarn.lock | 69 +++++++++++---------------- 7 files changed, 115 insertions(+), 54 deletions(-) diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 82075f709022..05a22743204c 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -2155,7 +2155,7 @@ "TextEncoder": true }, "packages": { - "@noble/hashes": true + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": true } }, "@noble/hashes": { @@ -2170,6 +2170,24 @@ "crypto": true } }, + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, "eth-lattice-keyring>@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { "globals": { "TextEncoder": true, @@ -2248,7 +2266,7 @@ "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": { "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": true, "@metamask/utils>@scure/base": true } }, @@ -3448,7 +3466,7 @@ }, "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": true, "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": true } }, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 82075f709022..05a22743204c 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -2155,7 +2155,7 @@ "TextEncoder": true }, "packages": { - "@noble/hashes": true + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": true } }, "@noble/hashes": { @@ -2170,6 +2170,24 @@ "crypto": true } }, + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, "eth-lattice-keyring>@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { "globals": { "TextEncoder": true, @@ -2248,7 +2266,7 @@ "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": { "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": true, "@metamask/utils>@scure/base": true } }, @@ -3448,7 +3466,7 @@ }, "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": true, "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": true } }, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 82075f709022..05a22743204c 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -2155,7 +2155,7 @@ "TextEncoder": true }, "packages": { - "@noble/hashes": true + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": true } }, "@noble/hashes": { @@ -2170,6 +2170,24 @@ "crypto": true } }, + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, "eth-lattice-keyring>@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { "globals": { "TextEncoder": true, @@ -2248,7 +2266,7 @@ "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": { "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": true, "@metamask/utils>@scure/base": true } }, @@ -3448,7 +3466,7 @@ }, "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": true, "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": true } }, diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index a2453b1686dc..27ddfc86ff15 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -2247,7 +2247,7 @@ "TextEncoder": true }, "packages": { - "@noble/hashes": true + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": true } }, "@noble/hashes": { @@ -2262,6 +2262,24 @@ "crypto": true } }, + "@ethereumjs/tx>ethereum-cryptography>@noble/curves>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { + "globals": { + "TextEncoder": true, + "crypto": true + } + }, "eth-lattice-keyring>@ethereumjs/tx>ethereum-cryptography>@noble/hashes": { "globals": { "TextEncoder": true, @@ -2340,7 +2358,7 @@ "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": { "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@scure/bip32>@noble/hashes": true, "@metamask/utils>@scure/base": true } }, @@ -3540,7 +3558,7 @@ }, "packages": { "@ethereumjs/tx>ethereum-cryptography>@noble/curves": true, - "@noble/hashes": true, + "@ethereumjs/tx>ethereum-cryptography>@noble/hashes": true, "@ethereumjs/tx>ethereum-cryptography>@scure/bip32": true } }, diff --git a/package.json b/package.json index 261ddd874484..1553c739936f 100644 --- a/package.json +++ b/package.json @@ -350,7 +350,7 @@ "@metamask/snaps-utils": "^8.6.1", "@metamask/solana-wallet-snap": "^1.0.3", "@metamask/transaction-controller": "^42.0.0", - "@metamask/user-operation-controller": "^19.0.0", + "@metamask/user-operation-controller": "^21.0.0", "@metamask/utils": "^10.0.1", "@ngraveio/bc-ur": "^1.1.12", "@noble/hashes": "^1.3.3", diff --git a/ui/ducks/metamask/metamask.js b/ui/ducks/metamask/metamask.js index f3453b77e7d9..c9c57057f2f9 100644 --- a/ui/ducks/metamask/metamask.js +++ b/ui/ducks/metamask/metamask.js @@ -373,7 +373,7 @@ export function isEIP1559Network(state, networkClientId) { return ( state.metamask.networksMetadata?.[ networkClientId ?? selectedNetworkClientId - ].EIPS[1559] === true + ]?.EIPS[1559] === true ); } diff --git a/yarn.lock b/yarn.lock index 7bfb4b6e4336..494ffcc5b085 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5958,19 +5958,19 @@ __metadata: languageName: node linkType: hard -"@metamask/polling-controller@npm:^12.0.0, @metamask/polling-controller@npm:^12.0.1": - version: 12.0.1 - resolution: "@metamask/polling-controller@npm:12.0.1" +"@metamask/polling-controller@npm:^12.0.0, @metamask/polling-controller@npm:^12.0.1, @metamask/polling-controller@npm:^12.0.2": + version: 12.0.2 + resolution: "@metamask/polling-controller@npm:12.0.2" dependencies: "@metamask/base-controller": "npm:^7.0.2" - "@metamask/controller-utils": "npm:^11.4.2" + "@metamask/controller-utils": "npm:^11.4.4" "@metamask/utils": "npm:^10.0.0" "@types/uuid": "npm:^8.3.0" fast-json-stable-stringify: "npm:^2.1.0" uuid: "npm:^8.3.2" peerDependencies: "@metamask/network-controller": ^22.0.0 - checksum: 10/eac9ed2fcc9697a2aa55e9746d4eac8d762dd6948b00d77cd2d4894b8c3e1a8e6ed5d0df4d01a69d9a7e2b3c09d9d7c1ffc6f9504023388dd7452d45b5d87065 + checksum: 10/15bb6c087b506e15474d4d3555a37a540389c47674ac14052f7877bbf2be29e3bf78f2ea34a10d1ed21696bc0f45525cb2916e020a1b87e74ad695cb98ae4a94 languageName: node linkType: hard @@ -6432,14 +6432,14 @@ __metadata: languageName: node linkType: hard -"@metamask/user-operation-controller@npm:^19.0.0": - version: 19.0.0 - resolution: "@metamask/user-operation-controller@npm:19.0.0" +"@metamask/user-operation-controller@npm:^21.0.0": + version: 21.0.0 + resolution: "@metamask/user-operation-controller@npm:21.0.0" dependencies: "@metamask/base-controller": "npm:^7.0.2" - "@metamask/controller-utils": "npm:^11.4.3" + "@metamask/controller-utils": "npm:^11.4.4" "@metamask/eth-query": "npm:^4.0.0" - "@metamask/polling-controller": "npm:^12.0.1" + "@metamask/polling-controller": "npm:^12.0.2" "@metamask/rpc-errors": "npm:^7.0.1" "@metamask/superstruct": "npm:^3.1.0" "@metamask/utils": "npm:^10.0.0" @@ -6449,11 +6449,12 @@ __metadata: uuid: "npm:^8.3.2" peerDependencies: "@metamask/approval-controller": ^7.0.0 + "@metamask/eth-block-tracker": ">=9" "@metamask/gas-fee-controller": ^22.0.0 "@metamask/keyring-controller": ^19.0.0 "@metamask/network-controller": ^22.0.0 - "@metamask/transaction-controller": ^40.0.0 - checksum: 10/ca3d8ee77243eb3bdc455420185d5c41d45cb5520735af0b05c1792d66fdb6a7404c557b053b16b6fded57124da21c3bb0b6b1c943d290e9808164f05453a8d9 + "@metamask/transaction-controller": ^42.0.0 + checksum: 10/c9d794ea386c57c3213714181444a0483bdff361de386716f4c38b0e99c411db91183d8bcc1f229b3bdcf9b1611219636c39b726e9f56e23d29b3fb7874c0e0a languageName: node linkType: hard @@ -6596,13 +6597,20 @@ __metadata: languageName: node linkType: hard -"@noble/hashes@npm:1.4.0, @noble/hashes@npm:^1.1.2, @noble/hashes@npm:^1.2.0, @noble/hashes@npm:^1.3.1, @noble/hashes@npm:^1.3.2, @noble/hashes@npm:^1.3.3, @noble/hashes@npm:^1.4.0, @noble/hashes@npm:~1.4.0": +"@noble/hashes@npm:1.4.0, @noble/hashes@npm:~1.4.0": version: 1.4.0 resolution: "@noble/hashes@npm:1.4.0" checksum: 10/e156e65794c473794c52fa9d06baf1eb20903d0d96719530f523cc4450f6c721a957c544796e6efd0197b2296e7cd70efeb312f861465e17940a3e3c7e0febc6 languageName: node linkType: hard +"@noble/hashes@npm:^1.1.2, @noble/hashes@npm:^1.2.0, @noble/hashes@npm:^1.3.1, @noble/hashes@npm:^1.3.2, @noble/hashes@npm:^1.3.3, @noble/hashes@npm:^1.4.0": + version: 1.5.0 + resolution: "@noble/hashes@npm:1.5.0" + checksum: 10/da7fc7af52af7afcf59810a7eea6155075464ff462ffda2572dc6d57d53e2669b1ea2ec774e814f6273f1697e567f28d36823776c9bf7068cba2a2855140f26e + languageName: node + linkType: hard + "@noble/hashes@npm:~1.1.1": version: 1.1.3 resolution: "@noble/hashes@npm:1.1.3" @@ -7750,9 +7758,9 @@ __metadata: linkType: hard "@scure/base@npm:^1.0.0, @scure/base@npm:^1.1.1, @scure/base@npm:^1.1.3, @scure/base@npm:~1.1.0, @scure/base@npm:~1.1.3, @scure/base@npm:~1.1.6": - version: 1.1.7 - resolution: "@scure/base@npm:1.1.7" - checksum: 10/fc50ffaab36cb46ff9fa4dc5052a06089ab6a6707f63d596bb34aaaec76173c9a564ac312a0b981b5e7a5349d60097b8878673c75d6cbfc4da7012b63a82099b + version: 1.1.9 + resolution: "@scure/base@npm:1.1.9" + checksum: 10/f0ab7f687bbcdee2a01377fe3cd808bf63977999672751295b6a92625d5322f4754a96d40f6bd579bc367aad48ecf8a4e6d0390e70296e6ded1076f52adb16bb languageName: node linkType: hard @@ -15820,14 +15828,11 @@ __metadata: linkType: hard "crc-32@npm:^1.2.0": - version: 1.2.0 - resolution: "crc-32@npm:1.2.0" - dependencies: - exit-on-epipe: "npm:~1.0.1" - printj: "npm:~1.1.0" + version: 1.2.2 + resolution: "crc-32@npm:1.2.2" bin: - crc32: ./bin/crc32.njs - checksum: 10/10c648c986b005ed0ea8393bb0d1ccb99e7a102505b136d313dee6abe204aa682d9bb9bc6fd180f9cd98ef92aa029964f1cc96a2a85eb50507dedd9ead1a262f + crc32: bin/crc32.njs + checksum: 10/824f696a5baaf617809aa9cd033313c8f94f12d15ebffa69f10202480396be44aef9831d900ab291638a8022ed91c360696dd5b1ba691eb3f34e60be8835b7c3 languageName: node linkType: hard @@ -19092,13 +19097,6 @@ __metadata: languageName: node linkType: hard -"exit-on-epipe@npm:~1.0.1": - version: 1.0.1 - resolution: "exit-on-epipe@npm:1.0.1" - checksum: 10/b180aa277aec5bef2609b34e5876061f421a1f81bf343beb213c4d60b382ddcb6b83012833f0ba329d6bc38042685c8d89b1c52ea495b9b6327948ea80627398 - languageName: node - linkType: hard - "exit@npm:^0.1.2": version: 0.1.2 resolution: "exit@npm:0.1.2" @@ -26621,7 +26619,7 @@ __metadata: "@metamask/test-bundler": "npm:^1.0.0" "@metamask/test-dapp": "npm:8.13.0" "@metamask/transaction-controller": "npm:^42.0.0" - "@metamask/user-operation-controller": "npm:^19.0.0" + "@metamask/user-operation-controller": "npm:^21.0.0" "@metamask/utils": "npm:^10.0.1" "@ngraveio/bc-ur": "npm:^1.1.12" "@noble/hashes": "npm:^1.3.3" @@ -30285,15 +30283,6 @@ __metadata: languageName: node linkType: hard -"printj@npm:~1.1.0": - version: 1.1.2 - resolution: "printj@npm:1.1.2" - bin: - printj: ./bin/printj.njs - checksum: 10/45376a5ee7ef2e0d7ff0b4fecc893d73995a332e63d7e0622a544fe662c8213d22f0c9750e627c6d732a7d7a543266be960e6cd51cf19485cce87cf80468bb41 - languageName: node - linkType: hard - "prismjs@npm:^1.27.0": version: 1.29.0 resolution: "prismjs@npm:1.29.0" From d06dad70d8ade01aa569280cfddd05da24bfaa54 Mon Sep 17 00:00:00 2001 From: Jony Bursztyn Date: Thu, 19 Dec 2024 12:18:54 +0000 Subject: [PATCH 07/19] feat: add metametrics events to carousel (#29141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR introduces tracking metrics for banners in the carousel to monitor their effectiveness. The following events are added: - **Banner selected:** Triggered when a banner or a button within a banner is clicked. - **Close all banners:** Triggered when the last banner in the carousel is closed. - **Banner shown:** Triggered when a banner is displayed in the carousel. ### Tracking Implementation Details: #### Banner selected When a banner or button in a banner is clicked: ```javascript trackEvent({ event: MetaMetricsEventName.BannerSelect, category: MetaMetricsEventCategory.Banner, properties: { banner_name: e.g. buy, bridge, sell, card } }); ``` #### Close all banners When the last banner in the carousel is closed: ```javascript trackEvent({ event: MetaMetricsEventName.BannerCloseAll, category: MetaMetricsEventCategory.Banner }); ``` #### Banner shown When a banner is displayed in the carousel: ```javascript trackEvent({ event: MetaMetricsEventName.BannerDisplay, category: MetaMetricsEventCategory.Banner, properties: { banner_name: e.g. buy, bridge, sell, card } }); ``` ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3764 ## **Manual testing steps** 1. Open the carousel. 2. Click on banners or buttons within banners to trigger the "Banner selected" event. 3. Close the last banner to trigger the "Close all banners" event. 4. Navigate through the carousel to ensure the "Banner shown" event is fired for each displayed banner. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- shared/constants/metametrics.ts | 4 ++ .../account-overview-layout.tsx | 51 +++++++++++++++--- .../multichain/carousel/carousel.test.tsx | 53 +++++++++++++++++-- .../multichain/carousel/carousel.tsx | 16 +++++- .../multichain/carousel/carousel.types.ts | 3 +- 5 files changed, 114 insertions(+), 13 deletions(-) diff --git a/shared/constants/metametrics.ts b/shared/constants/metametrics.ts index 9b99140b7d24..8b59e95e650c 100644 --- a/shared/constants/metametrics.ts +++ b/shared/constants/metametrics.ts @@ -645,6 +645,9 @@ export enum MetaMetricsEventName { AppUnlockedFailed = 'App Unlocked Failed', AppLocked = 'App Locked', AppWindowExpanded = 'App Window Expanded', + BannerDisplay = 'Banner Display', + BannerCloseAll = 'Banner Close All', + BannerSelect = 'Banner Select', BridgeLinkClicked = 'Bridge Link Clicked', BitcoinSupportToggled = 'Bitcoin Support Toggled', BitcoinTestnetSupportToggled = 'Bitcoin Testnet Support Toggled', @@ -912,6 +915,7 @@ export enum MetaMetricsEventCategory { App = 'App', Auth = 'Auth', Background = 'Background', + Banner = 'Banner', // The TypeScript ESLint rule is incorrectly marking this line. /* eslint-disable-next-line @typescript-eslint/no-shadow */ Error = 'Error', diff --git a/ui/components/multichain/account-overview/account-overview-layout.tsx b/ui/components/multichain/account-overview/account-overview-layout.tsx index 01396b77d2c1..69fc2297ffea 100644 --- a/ui/components/multichain/account-overview/account-overview-layout.tsx +++ b/ui/components/multichain/account-overview/account-overview-layout.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useContext, useState, useCallback } from 'react'; import { useDispatch, useSelector } from 'react-redux'; ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) import { isEqual } from 'lodash'; @@ -16,6 +16,12 @@ import { ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) import useBridging from '../../../hooks/bridge/useBridging'; ///: END:ONLY_INCLUDE_IF +import { MetaMetricsContext } from '../../../contexts/metametrics'; +import { + MetaMetricsEventName, + MetaMetricsEventCategory, +} from '../../../../shared/constants/metametrics'; +import type { CarouselSlide } from '../../../../shared/constants/app-state'; import { AccountOverviewTabsProps, AccountOverviewTabs, @@ -42,6 +48,8 @@ export const AccountOverviewLayout = ({ const slides = useSelector(getSlides); const totalBalance = useSelector(getSelectedAccountCachedBalance); const isLoading = useSelector(getAppIsLoading); + const trackEvent = useContext(MetaMetricsContext); + const [hasRendered, setHasRendered] = useState(false); ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual); @@ -76,8 +84,8 @@ export const AccountOverviewLayout = ({ dispatch(updateSlides(defaultSlides)); }, [hasZeroBalance]); - ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) const handleCarouselClick = (id: string) => { + ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) if (id === 'bridge') { openBridgeExperience( 'Carousel', @@ -85,26 +93,57 @@ export const AccountOverviewLayout = ({ location.pathname.includes('asset') ? '&token=native' : '', ); } + ///: END:ONLY_INCLUDE_IF + + trackEvent({ + event: MetaMetricsEventName.BannerSelect, + category: MetaMetricsEventCategory.Banner, + properties: { + banner_name: id, + }, + }); }; - ///: END:ONLY_INCLUDE_IF - const handleRemoveSlide = (id: string) => { + const handleRemoveSlide = (isLastSlide: boolean, id: string) => { if (id === 'fund' && hasZeroBalance) { return; } + if (isLastSlide) { + trackEvent({ + event: MetaMetricsEventName.BannerCloseAll, + category: MetaMetricsEventCategory.Banner, + }); + } dispatch(removeSlide(id)); }; + const handleRenderSlides = useCallback( + (renderedSlides: CarouselSlide[]) => { + if (!hasRendered) { + renderedSlides.forEach((slide) => { + trackEvent({ + event: MetaMetricsEventName.BannerDisplay, + category: MetaMetricsEventCategory.Banner, + properties: { + banner_name: slide.id, + }, + }); + }); + setHasRendered(true); + } + }, + [hasRendered, trackEvent], + ); + return ( <>
{children}
diff --git a/ui/components/multichain/carousel/carousel.test.tsx b/ui/components/multichain/carousel/carousel.test.tsx index 71b9e4f4faa8..25fe8b562600 100644 --- a/ui/components/multichain/carousel/carousel.test.tsx +++ b/ui/components/multichain/carousel/carousel.test.tsx @@ -3,6 +3,40 @@ import { render, fireEvent } from '@testing-library/react'; import { Carousel } from './carousel'; import { MARGIN_VALUES, WIDTH_VALUES } from './constants'; +jest.mock('react-responsive-carousel', () => ({ + Carousel: ({ + children, + onChange, + }: { + children: React.ReactNode; + onChange?: (index: number) => void; + }) => ( +
+ {children} +
+
+
+ ), +})); + +jest.mock('react-redux', () => ({ + useSelector: jest.fn(), + useDispatch: () => jest.fn(), +})); + +jest.mock('reselect', () => ({ + createSelector: jest.fn(), + createDeepEqualSelector: jest.fn(), + createSelectorCreator: jest.fn(() => jest.fn()), + lruMemoize: jest.fn(), +})); + +jest.mock('../../../selectors/approvals', () => ({ + selectPendingApproval: jest.fn(), +})); + jest.mock('../../../hooks/useI18nContext', () => ({ useI18nContext: () => (key: string) => key, })); @@ -46,13 +80,24 @@ describe('Carousel', () => { expect(closeButtons).toHaveLength(2); fireEvent.click(closeButtons[0]); - expect(mockOnClose).toHaveBeenCalledWith('1'); + expect(mockOnClose).toHaveBeenCalledWith(false, '1'); const remainingSlides = mockSlides.filter((slide) => slide.id !== '1'); rerender(); - const updatedSlides = container.querySelectorAll('.mm-carousel-slide'); - expect(updatedSlides).toHaveLength(1); + const updatedCloseButtons = container.querySelectorAll( + '.mm-carousel-slide__close-button', + ); + expect(updatedCloseButtons).toHaveLength(1); + + fireEvent.click(updatedCloseButtons[0]); + expect(mockOnClose).toHaveBeenCalledWith(true, '2'); + + const finalSlides = remainingSlides.filter((slide) => slide.id !== '2'); + rerender(); + + const finalSlideElements = container.querySelectorAll('.mm-carousel-slide'); + expect(finalSlideElements).toHaveLength(0); }); it('should handle slide navigation', () => { @@ -65,7 +110,7 @@ describe('Carousel', () => { fireEvent.click(dots[1]); const slides = container.querySelectorAll('.mm-carousel-slide'); - expect(slides[1].parentElement).toHaveClass('selected'); + expect(slides[1].parentElement).toHaveClass('mock-carousel'); }); it('should return null when no slides are present', () => { diff --git a/ui/components/multichain/carousel/carousel.tsx b/ui/components/multichain/carousel/carousel.tsx index 3fbbe955a8eb..83423948c530 100644 --- a/ui/components/multichain/carousel/carousel.tsx +++ b/ui/components/multichain/carousel/carousel.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useState, useEffect } from 'react'; import { Carousel as ResponsiveCarousel } from 'react-responsive-carousel'; import { useI18nContext } from '../../../hooks/useI18nContext'; import { Box, BoxProps, BannerBase } from '../../component-library'; @@ -24,6 +24,7 @@ export const Carousel = React.forwardRef( isLoading = false, onClose, onClick, + onRenderSlides, ...props }: CarouselProps, ref: React.Ref, @@ -44,6 +45,17 @@ export const Carousel = React.forwardRef( }) .slice(0, MAX_SLIDES); + useEffect(() => { + if ( + visibleSlides && + visibleSlides.length > 0 && + onRenderSlides && + !isLoading + ) { + onRenderSlides(visibleSlides); + } + }, [visibleSlides, onRenderSlides, isLoading]); + const handleClose = (e: React.MouseEvent, slideId: string) => { e.preventDefault(); e.stopPropagation(); @@ -65,7 +77,7 @@ export const Carousel = React.forwardRef( setSelectedIndex(newSelectedIndex); if (onClose) { - onClose(slideId); + onClose(visibleSlides.length === 1, slideId); } }; diff --git a/ui/components/multichain/carousel/carousel.types.ts b/ui/components/multichain/carousel/carousel.types.ts index a8aef8df4839..3a6289e76a4b 100644 --- a/ui/components/multichain/carousel/carousel.types.ts +++ b/ui/components/multichain/carousel/carousel.types.ts @@ -3,6 +3,7 @@ import { CarouselSlide } from '../../../../shared/constants/app-state'; export type CarouselProps = { slides: CarouselSlide[]; isLoading?: boolean; - onClose?: (id: string) => void; + onClose?: (isLastSlide: boolean, id: string) => void; onClick?: (id: string) => void; + onRenderSlides?: (slides: CarouselSlide[]) => void; }; From e8d6765952d311e73cc08ca270326e2b46650701 Mon Sep 17 00:00:00 2001 From: Nidhi Kumari Date: Thu, 19 Dec 2024 12:20:17 +0000 Subject: [PATCH 08/19] fix: updated margin for import token banner (#29283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR is to update the margin bottom in the detected token banner ## **Description** This PR updates the Top right bottom and left margin to 16px, 16px, 4px and 16px for import token banner ## **Related issues** Fixes: #26670 ## **Manual testing steps** 1. Must be a new user with not all tokens added 2. Open MetaMask 3. If user has detected tokens, check the margin of the banner ## **Screenshots/Recordings** ### **Before** ![Screenshot 2024-12-17 at 3 06 39 PM](https://github.com/user-attachments/assets/b912aa7a-04da-4354-88bc-c37ca4433b25) ### **After** ![Screenshot 2024-12-17 at 3 38 33 PM](https://github.com/user-attachments/assets/b7d4afba-615a-4df3-8598-ef2b82d18143) ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- ui/components/app/assets/asset-list/asset-list.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/components/app/assets/asset-list/asset-list.tsx b/ui/components/app/assets/asset-list/asset-list.tsx index 3cd06ec4686b..19d9ecdd4c0d 100644 --- a/ui/components/app/assets/asset-list/asset-list.tsx +++ b/ui/components/app/assets/asset-list/asset-list.tsx @@ -127,6 +127,7 @@ const AssetList = ({ onClickAsset, showTokensLinks }: AssetListProps) => { className="" actionButtonOnClick={() => setShowDetectedTokens(true)} margin={4} + marginBottom={1} /> ) : null} From 398ec979ef0ae27214e11ec644f6415df404a897 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 19 Dec 2024 10:08:56 -0330 Subject: [PATCH 09/19] ci: Improve accuracy of `wait-for-circleci-workflow-status` (#29310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The GitHub Actions workflow `wait-for-circleci-workflow-status` has been updated to ensure that it waits for the correct workflow. Previously it always chose the most recent workflow for the given branch, but it may have chosen a workflow corresponding to the wrong commit. It has been updated to find one matching the same commit that triggered the GitHub Actions workflow. A log has been added to help diagnose any future problems with this workflow, and to help with verification. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29310?quickstart=1) ## **Related issues** N/A ## **Manual testing steps** Check the logs of the "Wait for CircleCI workflow status" job, and see that the workflow ID it's waiting on is correct when making multiple successive commits (comparing the timing using the CircleCI dashboard) Unfortunately the ID logged in the action is not shown on the CircleCI UI, but you can download the pipeline data with this command: `curl 'https://circleci.com/api/v2/project/gh/MetaMask/metamask-extension/pipeline?branch=[branch]' > pipeline.json` and look through the `pipeline.json` file for an entry that matches the logged ID. The `number` field is the pipeline number, which is in the CircleCI workflow URL (e.g. `https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/[pipeline number]/workflows/[some other number]`) ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .github/workflows/wait-for-circleci-workflow-status.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/wait-for-circleci-workflow-status.yml b/.github/workflows/wait-for-circleci-workflow-status.yml index 18e5ef7825d5..725a4c3b975d 100644 --- a/.github/workflows/wait-for-circleci-workflow-status.yml +++ b/.github/workflows/wait-for-circleci-workflow-status.yml @@ -13,8 +13,13 @@ jobs: OWNER: ${{ github.repository_owner }} REPOSITORY: ${{ github.event.repository.name }} BRANCH: ${{ github.head_ref || github.ref_name }} + # For a `push` event, the HEAD commit hash is `github.sha`. + # For a `pull_request` event, `github.sha` is instead the base branch commit hash. The + # HEAD commit hash is `pull_request.head.sha`. + HEAD_COMMIT_HASH: ${{ github.event.pull_request.head.sha || github.sha }} run: | - pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items[0].id") + pipeline_id=$(curl --silent "https://circleci.com/api/v2/project/gh/$OWNER/$REPOSITORY/pipeline?branch=$BRANCH" | jq -r ".items | map(select(.vcs.revision == \"${HEAD_COMMIT_HASH}\" )) | first | .id") + echo "Waiting for pipeline '${pipeline_id}' for commit hash '${HEAD_COMMIT_HASH}'" workflow_status=$(curl --silent "https://circleci.com/api/v2/pipeline/$pipeline_id/workflow" | jq -r ".items[0].status") if [ "$workflow_status" == "running" ]; then From cf6a43d54e5dea8c0694d048fddf44cbb16f0c8e Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Thu, 19 Dec 2024 15:24:07 +0100 Subject: [PATCH 10/19] chore: bump `@metamask/smart-transactions-controller` to `16.0.0` (#29344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR bumps `@metamask/smart-transactions-controller` to `16.0.0` [CHANGELOG](https://github.com/MetaMask/smart-transactions-controller/blob/main/CHANGELOG.md#1600) - `@metamask/transaction-controller` has been bumped to `42.0.0` which match the current version used in the client. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29344?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- package.json | 2 +- yarn.lock | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 1553c739936f..21ba753e2567 100644 --- a/package.json +++ b/package.json @@ -342,7 +342,7 @@ "@metamask/scure-bip39": "^2.0.3", "@metamask/selected-network-controller": "^19.0.0", "@metamask/signature-controller": "^23.1.0", - "@metamask/smart-transactions-controller": "^15.0.0", + "@metamask/smart-transactions-controller": "^16.0.0", "@metamask/snaps-controllers": "^9.15.0", "@metamask/snaps-execution-environments": "^6.10.0", "@metamask/snaps-rpc-methods": "^11.7.0", diff --git a/yarn.lock b/yarn.lock index 494ffcc5b085..2427563e548a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6188,9 +6188,9 @@ __metadata: languageName: node linkType: hard -"@metamask/smart-transactions-controller@npm:^15.0.0": - version: 15.0.0 - resolution: "@metamask/smart-transactions-controller@npm:15.0.0" +"@metamask/smart-transactions-controller@npm:^16.0.0": + version: 16.0.0 + resolution: "@metamask/smart-transactions-controller@npm:16.0.0" dependencies: "@babel/runtime": "npm:^7.24.1" "@ethereumjs/tx": "npm:^5.2.1" @@ -6202,18 +6202,17 @@ __metadata: "@metamask/eth-query": "npm:^4.0.0" "@metamask/polling-controller": "npm:^12.0.0" bignumber.js: "npm:^9.0.1" - events: "npm:^3.3.0" fast-json-patch: "npm:^3.1.0" lodash: "npm:^4.17.21" peerDependencies: "@metamask/network-controller": ^22.0.0 - "@metamask/transaction-controller": ^38.0.0 + "@metamask/transaction-controller": ^42.0.0 peerDependenciesMeta: "@metamask/accounts-controller": optional: true "@metamask/approval-controller": optional: true - checksum: 10/e30faad97451ba003da5b650040c0803d229954b6d6a28bc759ef6129d8e33f02dc2219c8346ae5049e62a84b62d8df211a403bdc80d59c1caed4ba182b6cc0a + checksum: 10/5bfe2e459ca75b3de31f28213e908f389a6a7e82b45dd43ee7fd4c46a619a561da030f7f90426af03063ff5dcfc90269a155230db484a2c415db11f8ad8caa02 languageName: node linkType: hard @@ -26609,7 +26608,7 @@ __metadata: "@metamask/scure-bip39": "npm:^2.0.3" "@metamask/selected-network-controller": "npm:^19.0.0" "@metamask/signature-controller": "npm:^23.1.0" - "@metamask/smart-transactions-controller": "npm:^15.0.0" + "@metamask/smart-transactions-controller": "npm:^16.0.0" "@metamask/snaps-controllers": "npm:^9.15.0" "@metamask/snaps-execution-environments": "npm:^6.10.0" "@metamask/snaps-rpc-methods": "npm:^11.7.0" From 927ef8c3b19da8ade4462e6adbd0e3e4cb8ca32c Mon Sep 17 00:00:00 2001 From: Alejandro Garcia Anglada Date: Thu, 19 Dec 2024 15:53:53 +0100 Subject: [PATCH 11/19] feat: bump solana snap (#29350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Solana snap bump https://github.com/MetaMask/snap-solana-wallet/releases/tag/v1.0.4 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29350?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Antonio Regadas Co-authored-by: Javier --- package.json | 2 +- privacy-snapshot.json | 2 ++ yarn.lock | 10 +++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 21ba753e2567..71373b3f12c7 100644 --- a/package.json +++ b/package.json @@ -348,7 +348,7 @@ "@metamask/snaps-rpc-methods": "^11.7.0", "@metamask/snaps-sdk": "^6.13.0", "@metamask/snaps-utils": "^8.6.1", - "@metamask/solana-wallet-snap": "^1.0.3", + "@metamask/solana-wallet-snap": "^1.0.4", "@metamask/transaction-controller": "^42.0.0", "@metamask/user-operation-controller": "^21.0.0", "@metamask/utils": "^10.0.1", diff --git a/privacy-snapshot.json b/privacy-snapshot.json index fc5dafb7333c..24c6e9ae27da 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -3,6 +3,7 @@ "accounts.api.cx.metamask.io", "acl.execution.metamask.io", "api.blockchair.com", + "api.devnet.solana.com", "api.lens.dev", "api.segment.io", "api.web3modal.com", @@ -59,6 +60,7 @@ "sepolia.infura.io", "signature-insights.api.cx.metamask.io", "snaps.metamask.io", + "solana.rpc.grove.city", "sourcify.dev", "start.metamask.io", "static.cx.metamask.io", diff --git a/yarn.lock b/yarn.lock index 2427563e548a..d10097abd78e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6346,10 +6346,10 @@ __metadata: languageName: node linkType: hard -"@metamask/solana-wallet-snap@npm:^1.0.3": - version: 1.0.3 - resolution: "@metamask/solana-wallet-snap@npm:1.0.3" - checksum: 10/4c7c0f05676e7bb84140226c1a3bd716493a6f3582142de83575df682e8351c7583fc5db6209fbde1b43f376fd4eda4d2063f0e651d8209e92001514fc8caf81 +"@metamask/solana-wallet-snap@npm:^1.0.4": + version: 1.0.4 + resolution: "@metamask/solana-wallet-snap@npm:1.0.4" + checksum: 10/4d3b2a400607299fb4200e409f151e999e980e30521e63dcbbb5b658a4fdcfd6be341452695de32d985e14e8f8d4c5c24169f84dfd1a2063d3bc35f0c4903f74 languageName: node linkType: hard @@ -26614,7 +26614,7 @@ __metadata: "@metamask/snaps-rpc-methods": "npm:^11.7.0" "@metamask/snaps-sdk": "npm:^6.13.0" "@metamask/snaps-utils": "npm:^8.6.1" - "@metamask/solana-wallet-snap": "npm:^1.0.3" + "@metamask/solana-wallet-snap": "npm:^1.0.4" "@metamask/test-bundler": "npm:^1.0.0" "@metamask/test-dapp": "npm:8.13.0" "@metamask/transaction-controller": "npm:^42.0.0" From 13a1fcfc694f3f23b95e99510ad86258dfa381ff Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 19 Dec 2024 15:07:00 +0000 Subject: [PATCH 12/19] feat: Display Unlimited for really large spending caps on Permit (#29102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Uses `UNLIMITED_THRESHOLD` to determine wether or not to show a permit amount as "Unlimited". Updates unit tests. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29102?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3763 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../hooks/use-approve-token-simulation.ts | 5 +- .../confirm/info/shared/constants.ts | 2 + .../decoded-simulation.test.tsx | 31 +++++++- .../decoded-simulation/decoded-simulation.tsx | 5 ++ .../value-display/value-display.tsx | 74 +++++++++++-------- 5 files changed, 84 insertions(+), 33 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts index 74c857f7078b..b8605da19f54 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-approve-token-simulation.ts @@ -8,12 +8,11 @@ import { calcTokenAmount } from '../../../../../../../../shared/lib/transactions import { getIntlLocale } from '../../../../../../../ducks/locale/locale'; import { formatAmount } from '../../../../simulation-details/formatAmount'; import { useDecodedTransactionData } from '../../hooks/useDecodedTransactionData'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../shared/constants'; import { useIsNFT } from './use-is-nft'; -const UNLIMITED_THRESHOLD = 10 ** 15; - function isSpendingCapUnlimited(decodedSpendingCap: number) { - return decodedSpendingCap >= UNLIMITED_THRESHOLD; + return decodedSpendingCap >= TOKEN_VALUE_UNLIMITED_THRESHOLD; } export const useApproveTokenSimulation = ( diff --git a/ui/pages/confirmations/components/confirm/info/shared/constants.ts b/ui/pages/confirmations/components/confirm/info/shared/constants.ts index 9f9b4bb7a2f1..9953383149d2 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/constants.ts +++ b/ui/pages/confirmations/components/confirm/info/shared/constants.ts @@ -1 +1,3 @@ export const HEX_ZERO = '0x0'; + +export const TOKEN_VALUE_UNLIMITED_THRESHOLD = 10 ** 15; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx index c06cf906e870..0cfd564d8a69 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.test.tsx @@ -77,6 +77,35 @@ const decodingDataBidding: DecodingDataStateChanges = [ describe('DecodedSimulation', () => { it('renders component correctly', async () => { + const state = getMockTypedSignConfirmStateForRequest({ + ...permitSignatureMsg, + decodingLoading: false, + decodingData: { + ...decodingData, + stateChanges: decodingData.stateChanges + ? [ + { + ...decodingData.stateChanges[0], + amount: '12345', + }, + ] + : [], + }, + }); + + const mockStore = configureMockStore([])(state); + + const { findByText } = renderWithConfirmContextProvider( + , + mockStore, + ); + + expect(await findByText('Estimated changes')).toBeInTheDocument(); + expect(await findByText('Spending cap')).toBeInTheDocument(); + expect(await findByText('12,345')).toBeInTheDocument(); + }); + + it('renders component correctly for a very large amount', async () => { const state = getMockTypedSignConfirmStateForRequest({ ...permitSignatureMsg, decodingLoading: false, @@ -91,7 +120,7 @@ describe('DecodedSimulation', () => { expect(await findByText('Estimated changes')).toBeInTheDocument(); expect(await findByText('Spending cap')).toBeInTheDocument(); - expect(await findByText('1,461,501,637,3...')).toBeInTheDocument(); + expect(await findByText('Unlimited')).toBeInTheDocument(); }); it('render correctly for ERC712 token', async () => { diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx index fe2be9d76261..9cfc0bdc1ae7 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/decoded-simulation/decoded-simulation.tsx @@ -71,6 +71,10 @@ const StateChangeRow = ({ const { assetType, changeType, amount, contractAddress, tokenID } = stateChange; const tooltip = getStateChangeToolip(stateChangeList, stateChange, t); + const canDisplayValueAsUnlimited = + assetType === TokenStandard.ERC20 && + (changeType === DecodingDataChangeType.Approve || + changeType === DecodingDataChangeType.Revoke); return ( )} {assetType === 'NATIVE' && ( diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx index c41f99f2f320..1d59418d80c2 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign-v4-simulation/value-display/value-display.tsx @@ -1,20 +1,11 @@ -import React, { useMemo } from 'react'; import { NameType } from '@metamask/name-controller'; import { Hex } from '@metamask/utils'; import { captureException } from '@sentry/browser'; - +import React, { useMemo } from 'react'; import { MetaMetricsEventLocation } from '../../../../../../../../../shared/constants/metametrics'; -import { shortenString } from '../../../../../../../../helpers/utils/util'; import { calcTokenAmount } from '../../../../../../../../../shared/lib/transactions-controller-utils'; import useTokenExchangeRate from '../../../../../../../../components/app/currency-input/hooks/useTokenExchangeRate'; -import { IndividualFiatDisplay } from '../../../../../simulation-details/fiat-display'; -import { - formatAmount, - formatAmountMaxPrecision, -} from '../../../../../simulation-details/formatAmount'; -import { useGetTokenStandardAndDetails } from '../../../../../../hooks/useGetTokenStandardAndDetails'; -import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation'; - +import Name from '../../../../../../../../components/app/name/name'; import { Box, Text, @@ -27,8 +18,17 @@ import { JustifyContent, TextAlign, } from '../../../../../../../../helpers/constants/design-system'; -import Name from '../../../../../../../../components/app/name/name'; +import { shortenString } from '../../../../../../../../helpers/utils/util'; +import { useI18nContext } from '../../../../../../../../hooks/useI18nContext'; +import { useGetTokenStandardAndDetails } from '../../../../../../hooks/useGetTokenStandardAndDetails'; +import useTrackERC20WithoutDecimalInformation from '../../../../../../hooks/useTrackERC20WithoutDecimalInformation'; import { TokenDetailsERC20 } from '../../../../../../utils/token'; +import { IndividualFiatDisplay } from '../../../../../simulation-details/fiat-display'; +import { + formatAmount, + formatAmountMaxPrecision, +} from '../../../../../simulation-details/formatAmount'; +import { TOKEN_VALUE_UNLIMITED_THRESHOLD } from '../../../shared/constants'; import { getAmountColors } from '../../../utils'; type PermitSimulationValueDisplayParams = { @@ -56,6 +56,9 @@ type PermitSimulationValueDisplayParams = { /** True if value is being debited to wallet */ debit?: boolean; + + /** Whether a large amount can be substituted by "Unlimited" */ + canDisplayValueAsUnlimited?: boolean; }; const PermitSimulationValueDisplay: React.FC< @@ -68,7 +71,10 @@ const PermitSimulationValueDisplay: React.FC< value, credit, debit, + canDisplayValueAsUnlimited, }) => { + const t = useI18nContext(); + const exchangeRate = useTokenExchangeRate(tokenContract); const tokenDetails = useGetTokenStandardAndDetails(tokenContract); @@ -88,18 +94,26 @@ const PermitSimulationValueDisplay: React.FC< return undefined; }, [exchangeRate, tokenDecimals, value]); - const { tokenValue, tokenValueMaxPrecision } = useMemo(() => { - if (!value || tokenId) { - return { tokenValue: null, tokenValueMaxPrecision: null }; - } + const { tokenValue, tokenValueMaxPrecision, shouldShowUnlimitedValue } = + useMemo(() => { + if (!value || tokenId) { + return { + tokenValue: null, + tokenValueMaxPrecision: null, + shouldShowUnlimitedValue: false, + }; + } - const tokenAmount = calcTokenAmount(value, tokenDecimals); + const tokenAmount = calcTokenAmount(value, tokenDecimals); - return { - tokenValue: formatAmount('en-US', tokenAmount), - tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', tokenAmount), - }; - }, [tokenDecimals, value]); + return { + tokenValue: formatAmount('en-US', tokenAmount), + tokenValueMaxPrecision: formatAmountMaxPrecision('en-US', tokenAmount), + shouldShowUnlimitedValue: + canDisplayValueAsUnlimited && + Number(value) > TOKEN_VALUE_UNLIMITED_THRESHOLD, + }; + }, [tokenDecimals, value]); /** Temporary error capturing as we are building out Permit Simulations */ if (!tokenContract) { @@ -138,13 +152,15 @@ const PermitSimulationValueDisplay: React.FC< > {credit && '+ '} {debit && '- '} - {tokenValue !== null && - shortenString(tokenValue || '', { - truncatedCharLimit: 15, - truncatedStartChars: 15, - truncatedEndChars: 0, - skipCharacterInEnd: true, - })} + {shouldShowUnlimitedValue + ? t('unlimited') + : tokenValue !== null && + shortenString(tokenValue || '', { + truncatedCharLimit: 15, + truncatedStartChars: 15, + truncatedEndChars: 0, + skipCharacterInEnd: true, + })} {tokenId && `#${tokenId}`} From 22490c38d46f993633d35f5f1f56224894a929ba Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 19 Dec 2024 15:19:21 +0000 Subject: [PATCH 13/19] fix: Wrong icon for ETH on L2s being displayed on transfer confirmations (#29353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** We were inadvertently referencing the network icon map instead of the native token map. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29353?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29351 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../__snapshots__/native-transfer.test.tsx.snap | 2 +- .../native-send-heading/native-send-heading.tsx | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap index 4fe5c3f41bbd..406fd66ae962 100644 --- a/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/native-transfer/__snapshots__/native-transfer.test.tsx.snap @@ -8,7 +8,7 @@ exports[`NativeTransferInfo renders correctly 1`] = `
- G + E

{ const multichainNetwork = useSelector(getMultichainNetwork); const ticker = multichainNetwork?.network?.ticker; + const networkConfigurationsByChainId = useSelector( + getNetworkConfigurationsByChainId, + ); + + const network = networkConfigurationsByChainId?.[transactionMeta.chainId]; + const { nativeCurrency } = network; + const locale = useSelector(getIntlLocale); const roundedTransferValue = formatAmount(locale, nativeAssetTransferValue); @@ -76,12 +84,11 @@ const NativeSendHeading = () => { const NetworkImage = ( From 09d413695487a47ee20e07ee13186acaf2c81453 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 19 Dec 2024 16:18:48 +0000 Subject: [PATCH 14/19] fix: Add origin pill to wallet_addEthereumChain confirmation (#29317) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Adds Origin Pill component and references it on the confirmation template. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29317?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/26656 ## **Manual testing steps** 1. Go to the test dApp 2. Add a custom chain ## **Screenshots/Recordings** ### **Before** ### **After** Screenshot 2024-12-18 at 12 08 31 Screenshot 2024-12-18 at 11 58 38 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../safe-component-list.js | 130 +++++++++--------- .../ui/origin-pill/origin-pill.test.tsx | 27 ++++ ui/components/ui/origin-pill/origin-pill.tsx | 57 ++++++++ .../add-ethereum-chain.test.js.snap | 20 +++ .../templates/add-ethereum-chain.js | 8 ++ 5 files changed, 178 insertions(+), 64 deletions(-) create mode 100644 ui/components/ui/origin-pill/origin-pill.test.tsx create mode 100644 ui/components/ui/origin-pill/origin-pill.tsx diff --git a/ui/components/app/metamask-template-renderer/safe-component-list.js b/ui/components/app/metamask-template-renderer/safe-component-list.js index 03f8625733b9..962b630562d1 100644 --- a/ui/components/app/metamask-template-renderer/safe-component-list.js +++ b/ui/components/app/metamask-template-renderer/safe-component-list.js @@ -1,83 +1,111 @@ -import Button from '../../ui/button'; -import Chip from '../../ui/chip'; -import DefinitionList from '../../ui/definition-list'; -import TruncatedDefinitionList from '../../ui/truncated-definition-list'; -import Popover from '../../ui/popover'; -import Typography from '../../ui/typography'; -import Box from '../../ui/box'; -import MetaMaskTranslation from '../metamask-translation'; -import NetworkDisplay from '../network-display'; -import TextArea from '../../ui/textarea/textarea'; -import TextField from '../../ui/text-field'; import ConfirmationNetworkSwitch from '../../../pages/confirmations/confirmation/components/confirmation-network-switch'; -import UrlIcon from '../../ui/url-icon'; -import Tooltip from '../../ui/tooltip/tooltip'; +import { SmartTransactionStatusPage } from '../../../pages/smart-transactions/smart-transaction-status-page'; import { AvatarIcon, + BannerAlert, FormTextField, Text, - BannerAlert, } from '../../component-library'; -import ActionableMessage from '../../ui/actionable-message/actionable-message'; import { AccountListItem } from '../../multichain'; +import ActionableMessage from '../../ui/actionable-message/actionable-message'; +import Box from '../../ui/box'; +import Button from '../../ui/button'; +import Chip from '../../ui/chip'; +import DefinitionList from '../../ui/definition-list'; +import Preloader from '../../ui/icon/preloader'; +import OriginPill from '../../ui/origin-pill/origin-pill'; +import Popover from '../../ui/popover'; +import Spinner from '../../ui/spinner'; +import TextField from '../../ui/text-field'; +import TextArea from '../../ui/textarea/textarea'; +import Tooltip from '../../ui/tooltip/tooltip'; +import TruncatedDefinitionList from '../../ui/truncated-definition-list'; +import Typography from '../../ui/typography'; +import UrlIcon from '../../ui/url-icon'; import { ConfirmInfoRow, ConfirmInfoRowAddress, ConfirmInfoRowValueDouble, } from '../confirm/info/row'; -import { SnapDelineator } from '../snaps/snap-delineator'; +import MetaMaskTranslation from '../metamask-translation'; +import NetworkDisplay from '../network-display'; import { Copyable } from '../snaps/copyable'; -import Spinner from '../../ui/spinner'; -import Preloader from '../../ui/icon/preloader'; -import { SnapUIMarkdown } from '../snaps/snap-ui-markdown'; -import { SnapUILink } from '../snaps/snap-ui-link'; -import { SmartTransactionStatusPage } from '../../../pages/smart-transactions/smart-transaction-status-page'; +import { SnapDelineator } from '../snaps/snap-delineator'; +import { SnapUIAddress } from '../snaps/snap-ui-address'; +import { SnapUIAvatar } from '../snaps/snap-ui-avatar'; +import { SnapUIButton } from '../snaps/snap-ui-button'; +import { SnapUICard } from '../snaps/snap-ui-card'; +import { SnapUICheckbox } from '../snaps/snap-ui-checkbox'; +import { SnapUIDropdown } from '../snaps/snap-ui-dropdown'; +import { SnapUIFileInput } from '../snaps/snap-ui-file-input'; +import { SnapUIFooterButton } from '../snaps/snap-ui-footer-button'; +import { SnapUIForm } from '../snaps/snap-ui-form'; import { SnapUIIcon } from '../snaps/snap-ui-icon'; import { SnapUIImage } from '../snaps/snap-ui-image'; -import { SnapUIFileInput } from '../snaps/snap-ui-file-input'; import { SnapUIInput } from '../snaps/snap-ui-input'; -import { SnapUIForm } from '../snaps/snap-ui-form'; -import { SnapUIButton } from '../snaps/snap-ui-button'; -import { SnapUIDropdown } from '../snaps/snap-ui-dropdown'; +import { SnapUILink } from '../snaps/snap-ui-link'; +import { SnapUIMarkdown } from '../snaps/snap-ui-markdown'; import { SnapUIRadioGroup } from '../snaps/snap-ui-radio-group'; -import { SnapUICheckbox } from '../snaps/snap-ui-checkbox'; -import { SnapUITooltip } from '../snaps/snap-ui-tooltip'; -import { SnapUICard } from '../snaps/snap-ui-card'; -import { SnapUIAddress } from '../snaps/snap-ui-address'; -import { SnapUIAvatar } from '../snaps/snap-ui-avatar'; import { SnapUISelector } from '../snaps/snap-ui-selector'; -import { SnapUIFooterButton } from '../snaps/snap-ui-footer-button'; +import { SnapUITooltip } from '../snaps/snap-ui-tooltip'; ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) -import { SnapAccountSuccessMessage } from '../../../pages/confirmations/components/snap-account-success-message'; import { SnapAccountErrorMessage } from '../../../pages/confirmations/components/snap-account-error-message'; +import { SnapAccountSuccessMessage } from '../../../pages/confirmations/components/snap-account-success-message'; import { CreateSnapAccount } from '../../../pages/create-snap-account'; -import { CreateNamedSnapAccount } from '../../multichain/create-named-snap-account'; import { RemoveSnapAccount, SnapAccountCard, } from '../../../pages/remove-snap-account'; import { SnapAccountRedirect } from '../../../pages/snap-account-redirect'; +import { CreateNamedSnapAccount } from '../../multichain/create-named-snap-account'; import SnapAuthorshipHeader from '../snaps/snap-authorship-header'; ///: END:ONLY_INCLUDE_IF export const safeComponentList = { a: 'a', - ActionableMessage, AccountListItem, + ActionableMessage, AvatarIcon, b: 'b', + BannerAlert, Box, Button, Chip, ConfirmationNetworkSwitch, + ConfirmInfoRow, + ConfirmInfoRowAddress, + ConfirmInfoRowValueDouble, + Copyable, DefinitionList, div: 'div', + FormTextField, i: 'i', MetaMaskTranslation, NetworkDisplay, + OriginPill, p: 'p', Popover, + Preloader, + SnapDelineator, + SnapUIAddress, + SnapUIAvatar, + SnapUIButton, + SnapUICard, + SnapUICheckbox, + SnapUIDropdown, + SnapUIFileInput, + SnapUIForm, + SnapUIFooterButton, + SnapUIIcon, + SnapUIImage, + SnapUIInput, + SnapUILink, + SnapUIMarkdown, + SnapUIRadioGroup, + SnapUISelector, + SnapUITooltip, span: 'span', + Spinner, Text, TextArea, TextField, @@ -86,40 +114,14 @@ export const safeComponentList = { Typography, SmartTransactionStatusPage, UrlIcon, - Copyable, - SnapDelineator, - SnapUIMarkdown, - SnapUILink, - SnapUIIcon, - SnapUIImage, - BannerAlert, - Spinner, - Preloader, - ConfirmInfoRow, - ConfirmInfoRowAddress, - ConfirmInfoRowValueDouble, - SnapUIFileInput, - SnapUIInput, - SnapUIButton, - SnapUIForm, - SnapUIDropdown, - SnapUIRadioGroup, - SnapUICheckbox, - SnapUITooltip, - SnapUICard, - SnapUISelector, - SnapUIAddress, - SnapUIAvatar, - SnapUIFooterButton, - FormTextField, ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) + CreateNamedSnapAccount, CreateSnapAccount, RemoveSnapAccount, - CreateNamedSnapAccount, - SnapAccountSuccessMessage, + SnapAccountCard, SnapAccountErrorMessage, - SnapAuthorshipHeader, SnapAccountRedirect, - SnapAccountCard, + SnapAccountSuccessMessage, + SnapAuthorshipHeader, ///: END:ONLY_INCLUDE_IF }; diff --git a/ui/components/ui/origin-pill/origin-pill.test.tsx b/ui/components/ui/origin-pill/origin-pill.test.tsx new file mode 100644 index 000000000000..13034a6a83e1 --- /dev/null +++ b/ui/components/ui/origin-pill/origin-pill.test.tsx @@ -0,0 +1,27 @@ +import { screen } from '@testing-library/dom'; +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import mockState from '../../../../test/data/mock-state.json'; +import { renderWithProvider } from '../../../../test/lib/render-helpers'; +import OriginPill from './origin-pill'; + +describe('', () => { + it('renders correct elements', () => { + const defaultProps = { + origin: 'Test Origin', + dataTestId: 'test-data-test-id', + }; + const store = configureMockStore()(mockState); + + renderWithProvider(, store); + + expect(screen.getByTestId(defaultProps.dataTestId)).toBeDefined(); + expect( + screen.getByTestId(`${defaultProps.dataTestId}-avatar-favicon`), + ).toBeDefined(); + expect(screen.getByTestId(`${defaultProps.dataTestId}-text`)).toBeDefined(); + expect( + screen.getByTestId(`${defaultProps.dataTestId}-text`), + ).toHaveTextContent(defaultProps.origin); + }); +}); diff --git a/ui/components/ui/origin-pill/origin-pill.tsx b/ui/components/ui/origin-pill/origin-pill.tsx new file mode 100644 index 000000000000..6a0863f43a71 --- /dev/null +++ b/ui/components/ui/origin-pill/origin-pill.tsx @@ -0,0 +1,57 @@ +import React from 'react'; +import { useSelector } from 'react-redux'; +import { + AlignItems, + BorderColor, + BorderRadius, + BorderStyle, + Display, + JustifyContent, + TextColor, + TextVariant, +} from '../../../helpers/constants/design-system'; +import { getSubjectMetadata } from '../../../selectors'; +import { AvatarFavicon, Box, Text } from '../../component-library'; + +type OriginPillProps = { + origin: string; + dataTestId: string; +}; + +export default function OriginPill({ origin, dataTestId }: OriginPillProps) { + const subjectMetadata = useSelector(getSubjectMetadata); + + const { iconUrl: siteImage = '' } = subjectMetadata[origin] || {}; + + return ( + + + + {origin} + + + ); +} diff --git a/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap b/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap index 27baa7f5cb79..86d284d0b985 100644 --- a/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap +++ b/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap @@ -19,6 +19,26 @@ exports[`add-ethereum-chain confirmation should match snapshot 1`] = ` > This site is requesting to update your default network URL. You can edit defaults and network information any time.

+
+
+ +
+
+ https://test-dapp.metamask.io +
+
diff --git a/ui/pages/confirmations/confirmation/templates/add-ethereum-chain.js b/ui/pages/confirmations/confirmation/templates/add-ethereum-chain.js index 1df546b06003..0c375bb1e667 100644 --- a/ui/pages/confirmations/confirmation/templates/add-ethereum-chain.js +++ b/ui/pages/confirmations/confirmation/templates/add-ethereum-chain.js @@ -268,6 +268,14 @@ function getValues(pendingApproval, t, actions, history, data) { }, }, }, + { + element: 'OriginPill', + key: 'origin-pill', + props: { + origin: pendingApproval.origin, + dataTestId: 'signature-origin-pill', + }, + }, { element: 'TruncatedDefinitionList', key: 'network-details', From b030ba95b6b9dd125ce27e8318c270b9902c7645 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Thu, 19 Dec 2024 17:33:06 +0100 Subject: [PATCH 15/19] fix: Prevent unwanted `updateEditableParams` calls on send flow (#29048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR aims to prevent unwanted `updateEditableParams` calls in send flow when `useMaxValue` is settled. After investigating [Sentry error mentioned in the task](https://metamask.sentry.io/issues/5973118037/events/1ee3e017ad454f09b3666089fba3c2bd/?project=273505&referrer=previous-event) noticed `updateEditableParams` is throwing when `useMaxValue` is set to `true`. This issue is happening when we submit/cancel confirmation, component gets an update, `updateMaxValue - updateEditableParams` gets called but transaction is not `unapproved` state. So adding a condition to updating transaction value only when transaction is `unapproved` status, this will guarantee to prevent unwanted calls. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29048?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/27742?reload=1 ## **Manual testing steps** N/A ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [X] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- test/lib/render-helpers.js | 9 ++- .../confirm-transaction-base.component.js | 3 +- .../confirm-transaction-base.test.js | 68 ++++++++++++++++++- 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/test/lib/render-helpers.js b/test/lib/render-helpers.js index 574415f2f3c6..d2161fe1cf2e 100644 --- a/test/lib/render-helpers.js +++ b/test/lib/render-helpers.js @@ -69,10 +69,15 @@ const createProviderWrapper = (store, pathname = '/') => { }; }; -export function renderWithProvider(component, store, pathname = '/') { +export function renderWithProvider( + component, + store, + pathname = '/', + renderer = render, +) { const { history, Wrapper } = createProviderWrapper(store, pathname); return { - ...render(component, { wrapper: Wrapper }), + ...renderer(component, { wrapper: Wrapper }), history, }; } diff --git a/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.component.js b/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.component.js index bcc3279752e2..bf90043c9778 100644 --- a/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.component.js @@ -282,7 +282,8 @@ export default class ConfirmTransactionBase extends Component { if ( hexMaximumTransactionFee !== prevHexMaximumTransactionFee && - useMaxValue + useMaxValue && + transactionStatus === TransactionStatus.unapproved ) { this.updateValueToMax(); } diff --git a/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js b/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js index 56be8fd5aee3..7dbcc2c3b0ab 100644 --- a/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js +++ b/ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js @@ -2,7 +2,6 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { fireEvent } from '@testing-library/react'; - import { EthAccountType } from '@metamask/keyring-api'; import { TransactionStatus, @@ -242,7 +241,7 @@ const mockedStoreWithConfirmTxParams = ( const sendToRecipientSelector = '.sender-to-recipient__party--recipient .sender-to-recipient__name'; -const render = async ({ props, state } = {}) => { +const render = async ({ props, state, renderer } = {}) => { const store = configureMockStore(middleware)({ ...baseStore, ...state, @@ -260,6 +259,8 @@ const render = async ({ props, state } = {}) => { (result = renderWithProvider( , store, + undefined, + renderer, )), ); @@ -967,4 +968,67 @@ describe('Confirm Transaction Base', () => { expect(confirmButton).toBeDisabled(); }); }); + + describe('if useMaxValue is settled', () => { + const baseStoreState = { + ...baseStore, + metamask: { + ...baseStore.metamask, + ...mockNetworkState({ chainId: CHAIN_IDS.GOERLI, ticker: undefined }), + }, + }; + + const updateTransactionValue = jest.fn(); + + const maxValueSettledProps = { + useMaxValue: true, + hexMaximumTransactionFee: '0x111', + updateTransactionValue, + }; + + beforeEach(() => { + updateTransactionValue.mockClear(); + }); + + it('should update transaction value when new hexMaximumTransactionFee is provided', async () => { + const { rerender } = await render({ + state: baseStoreState, + props: maxValueSettledProps, + }); + + const newHexMaximumTransactionFee = '0x222'; + + render({ + renderer: rerender, + state: baseStoreState, + props: { + ...maxValueSettledProps, + hexMaximumTransactionFee: newHexMaximumTransactionFee, + }, + }); + + expect(updateTransactionValue).toHaveBeenCalledTimes(1); + }); + + it('should not update transaction value if transactionStatus is not unapproved', async () => { + const { rerender } = await render({ + state: baseStoreState, + props: maxValueSettledProps, + }); + + const newHexMaximumTransactionFee = '0x222'; + + render({ + renderer: rerender, + state: { ...baseStoreState }, + props: { + ...maxValueSettledProps, + hexMaximumTransactionFee: newHexMaximumTransactionFee, + transactionStatus: TransactionStatus.submitted, + }, + }); + + expect(updateTransactionValue).toHaveBeenCalledTimes(0); + }); + }); }); From b114f880693f324dab60bd5d8424a013953796bc Mon Sep 17 00:00:00 2001 From: micaelae <100321200+micaelae@users.noreply.github.com> Date: Thu, 19 Dec 2024 09:33:14 -0800 Subject: [PATCH 16/19] refactor: shared bridge types (#29254) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** There is a lint rule that flags lines in which controllers and ui components import types/variables/methods from restricted directories. This change moves the imported elements to the `shared` directory to satisfy the linter. Also uses `import type` to remove type definitions from runtime bundles when possible [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29254?quickstart=1) ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMS-1829 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A - no functional changes ## **Pre-merge author checklist** - [X] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../controllers/bridge-status/utils.ts | 4 +- .../bridge/bridge-controller.test.ts | 13 +- .../controllers/bridge/bridge-controller.ts | 32 ++- app/scripts/controllers/bridge/constants.ts | 14 +- app/scripts/controllers/bridge/types.ts | 58 +---- app/scripts/metamask-controller.js | 8 +- shared/constants/bridge.ts | 2 + .../modules/bridge-utils}/bridge.util.test.ts | 6 +- .../modules/bridge-utils}/bridge.util.ts | 33 ++- shared/modules/bridge-utils/quote.ts | 36 ++++ .../modules/bridge-utils}/validators.ts | 7 +- shared/types/bridge-status.ts | 9 +- shared/types/bridge.ts | 198 ++++++++++++++++++ test/e2e/tests/bridge/bridge-test-utils.ts | 2 +- test/e2e/tests/bridge/constants.ts | 2 +- ui/ducks/bridge/actions.ts | 12 +- ui/ducks/bridge/bridge.test.ts | 4 +- ui/ducks/bridge/bridge.ts | 10 +- ui/ducks/bridge/selectors.test.ts | 6 +- ui/ducks/bridge/selectors.ts | 28 +-- ui/ducks/bridge/utils.ts | 6 +- ui/hooks/bridge/useBridgeTokens.ts | 2 +- .../bridge/useCrossChainSwapsEventTracker.ts | 2 +- ui/hooks/bridge/useLatestBalance.test.ts | 2 +- ui/pages/bridge/hooks/useAddToken.ts | 4 +- ui/pages/bridge/hooks/useHandleApprovalTx.ts | 13 +- ui/pages/bridge/hooks/useHandleBridgeTx.ts | 2 +- ui/pages/bridge/hooks/useHandleTx.ts | 2 +- .../hooks/useSubmitBridgeTransaction.ts | 5 +- .../bridge/prepare/bridge-cta-button.test.tsx | 4 +- .../bridge/prepare/bridge-input-group.tsx | 2 +- .../prepare/prepare-bridge-page.stories.tsx | 2 +- .../bridge/prepare/prepare-bridge-page.tsx | 6 +- .../bridge/quotes/bridge-quote-card.test.tsx | 4 +- .../quotes/bridge-quotes-modal.stories.tsx | 2 +- .../quotes/bridge-quotes-modal.test.tsx | 4 +- .../bridge/quotes/bridge-quotes-modal.tsx | 6 +- ui/pages/bridge/types.ts | 159 -------------- ui/pages/bridge/utils/quote.ts | 41 +--- ui/selectors/selectors.js | 4 +- 40 files changed, 355 insertions(+), 401 deletions(-) rename {ui/pages/bridge => shared/modules/bridge-utils}/bridge.util.test.ts (98%) rename {ui/pages/bridge => shared/modules/bridge-utils}/bridge.util.ts (87%) create mode 100644 shared/modules/bridge-utils/quote.ts rename {ui/pages/bridge/utils => shared/modules/bridge-utils}/validators.ts (96%) delete mode 100644 ui/pages/bridge/types.ts diff --git a/app/scripts/controllers/bridge-status/utils.ts b/app/scripts/controllers/bridge-status/utils.ts index d8dbac9e1590..5447b483e34f 100644 --- a/app/scripts/controllers/bridge-status/utils.ts +++ b/app/scripts/controllers/bridge-status/utils.ts @@ -8,9 +8,7 @@ import { StatusRequestWithSrcTxHash, StatusRequestDto, } from '../../../../shared/types/bridge-status'; -// TODO fix this -// eslint-disable-next-line import/no-restricted-paths -import { Quote } from '../../../../ui/pages/bridge/types'; +import type { Quote } from '../../../../shared/types/bridge'; import { validateResponse, validators } from './validators'; const CLIENT_ID_HEADER = { 'X-Client-Id': BRIDGE_CLIENT_ID }; diff --git a/app/scripts/controllers/bridge/bridge-controller.test.ts b/app/scripts/controllers/bridge/bridge-controller.test.ts index 3b0d095fa0c3..e35d3c41e67c 100644 --- a/app/scripts/controllers/bridge/bridge-controller.test.ts +++ b/app/scripts/controllers/bridge/bridge-controller.test.ts @@ -5,20 +5,19 @@ import { BRIDGE_API_BASE_URL } from '../../../../shared/constants/bridge'; import { CHAIN_IDS } from '../../../../shared/constants/network'; import { SWAPS_API_V2_BASE_URL } from '../../../../shared/constants/swaps'; import { flushPromises } from '../../../../test/lib/timer-helpers'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import * as bridgeUtil from '../../../../ui/pages/bridge/bridge.util'; +import * as bridgeUtil from '../../../../shared/modules/bridge-utils/bridge.util'; import * as balanceUtils from '../../../../shared/modules/bridge-utils/balance'; import mockBridgeQuotesErc20Native from '../../../../test/data/bridge/mock-quotes-erc20-native.json'; import mockBridgeQuotesNativeErc20 from '../../../../test/data/bridge/mock-quotes-native-erc20.json'; import mockBridgeQuotesNativeErc20Eth from '../../../../test/data/bridge/mock-quotes-native-erc20-eth.json'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { QuoteResponse } from '../../../../ui/pages/bridge/types'; +import { + type QuoteResponse, + RequestStatus, +} from '../../../../shared/types/bridge'; import { decimalToHex } from '../../../../shared/modules/conversion.utils'; import BridgeController from './bridge-controller'; import { BridgeControllerMessenger } from './types'; -import { DEFAULT_BRIDGE_CONTROLLER_STATE, RequestStatus } from './constants'; +import { DEFAULT_BRIDGE_CONTROLLER_STATE } from './constants'; const EMPTY_INIT_STATE = { bridgeState: DEFAULT_BRIDGE_CONTROLLER_STATE, diff --git a/app/scripts/controllers/bridge/bridge-controller.ts b/app/scripts/controllers/bridge/bridge-controller.ts index 4770c342587c..005e2d334e9f 100644 --- a/app/scripts/controllers/bridge/bridge-controller.ts +++ b/app/scripts/controllers/bridge/bridge-controller.ts @@ -12,9 +12,7 @@ import { fetchBridgeFeatureFlags, fetchBridgeQuotes, fetchBridgeTokens, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../../ui/pages/bridge/bridge.util'; +} from '../../../../shared/modules/bridge-utils/bridge.util'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths import { fetchTopAssetsList } from '../../../../ui/pages/swaps/swaps.util'; @@ -23,30 +21,24 @@ import { sumHexes, } from '../../../../shared/modules/conversion.utils'; import { - L1GasFees, - QuoteRequest, - QuoteResponse, - TxData, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../../ui/pages/bridge/types'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { isValidQuoteRequest } from '../../../../ui/pages/bridge/utils/quote'; + type L1GasFees, + type QuoteRequest, + type QuoteResponse, + type TxData, + type BridgeControllerState, + BridgeFeatureFlagsKey, + RequestStatus, +} from '../../../../shared/types/bridge'; +import { isValidQuoteRequest } from '../../../../shared/modules/bridge-utils/quote'; import { hasSufficientBalance } from '../../../../shared/modules/bridge-utils/balance'; import { CHAIN_IDS } from '../../../../shared/constants/network'; +import { REFRESH_INTERVAL_MS } from '../../../../shared/constants/bridge'; import { BRIDGE_CONTROLLER_NAME, DEFAULT_BRIDGE_CONTROLLER_STATE, - REFRESH_INTERVAL_MS, - RequestStatus, METABRIDGE_CHAIN_TO_ADDRESS_MAP, } from './constants'; -import { - BridgeControllerState, - BridgeControllerMessenger, - BridgeFeatureFlagsKey, -} from './types'; +import type { BridgeControllerMessenger } from './types'; const metadata: StateMetadata<{ bridgeState: BridgeControllerState }> = { bridgeState: { diff --git a/app/scripts/controllers/bridge/constants.ts b/app/scripts/controllers/bridge/constants.ts index 4903a9ee2858..65e2840cfb73 100644 --- a/app/scripts/controllers/bridge/constants.ts +++ b/app/scripts/controllers/bridge/constants.ts @@ -2,21 +2,15 @@ import { zeroAddress } from 'ethereumjs-util'; import { Hex } from '@metamask/utils'; import { BRIDGE_DEFAULT_SLIPPAGE, + DEFAULT_MAX_REFRESH_COUNT, METABRIDGE_ETHEREUM_ADDRESS, + REFRESH_INTERVAL_MS, } from '../../../../shared/constants/bridge'; import { CHAIN_IDS } from '../../../../shared/constants/network'; -import { BridgeControllerState, BridgeFeatureFlagsKey } from './types'; +import { BridgeFeatureFlagsKey } from '../../../../shared/types/bridge'; +import type { BridgeControllerState } from '../../../../shared/types/bridge'; export const BRIDGE_CONTROLLER_NAME = 'BridgeController'; -export const REFRESH_INTERVAL_MS = 30 * 1000; -const DEFAULT_MAX_REFRESH_COUNT = 5; - -export enum RequestStatus { - LOADING, - FETCHED, - ERROR, -} - export const DEFAULT_BRIDGE_CONTROLLER_STATE: BridgeControllerState = { bridgeFeatureFlags: { [BridgeFeatureFlagsKey.EXTENSION_CONFIG]: { diff --git a/app/scripts/controllers/bridge/types.ts b/app/scripts/controllers/bridge/types.ts index 7cdfa43cabd0..811b8dfb119b 100644 --- a/app/scripts/controllers/bridge/types.ts +++ b/app/scripts/controllers/bridge/types.ts @@ -2,64 +2,18 @@ import { ControllerStateChangeEvent, RestrictedControllerMessenger, } from '@metamask/base-controller'; -import { Hex } from '@metamask/utils'; import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; import { NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerGetSelectedNetworkClientAction, } from '@metamask/network-controller'; -import { SwapsTokenObject } from '../../../../shared/constants/swaps'; -import { - L1GasFees, - QuoteRequest, - QuoteResponse, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../../ui/pages/bridge/types'; -import { ChainConfiguration } from '../../../../shared/types/bridge'; +import type { + BridgeBackgroundAction, + BridgeControllerState, + BridgeUserAction, +} from '../../../../shared/types/bridge'; import BridgeController from './bridge-controller'; -import { BRIDGE_CONTROLLER_NAME, RequestStatus } from './constants'; - -export enum BridgeFeatureFlagsKey { - EXTENSION_CONFIG = 'extensionConfig', -} - -export type BridgeFeatureFlags = { - [BridgeFeatureFlagsKey.EXTENSION_CONFIG]: { - refreshRate: number; - maxRefreshCount: number; - support: boolean; - chains: Record; - }; -}; - -export type BridgeControllerState = { - bridgeFeatureFlags: BridgeFeatureFlags; - srcTokens: Record; - srcTopAssets: { address: string }[]; - srcTokensLoadingStatus?: RequestStatus; - destTokensLoadingStatus?: RequestStatus; - destTokens: Record; - destTopAssets: { address: string }[]; - quoteRequest: Partial; - quotes: (QuoteResponse & L1GasFees)[]; - quotesInitialLoadTime?: number; - quotesLastFetched?: number; - quotesLoadingStatus?: RequestStatus; - quoteFetchError?: string; - quotesRefreshCount: number; -}; - -export enum BridgeUserAction { - SELECT_SRC_NETWORK = 'selectSrcNetwork', - SELECT_DEST_NETWORK = 'selectDestNetwork', - UPDATE_QUOTE_PARAMS = 'updateBridgeQuoteRequestParams', -} -export enum BridgeBackgroundAction { - SET_FEATURE_FLAGS = 'setBridgeFeatureFlags', - RESET_STATE = 'resetState', - GET_BRIDGE_ERC20_ALLOWANCE = 'getBridgeERC20Allowance', -} +import { BRIDGE_CONTROLLER_NAME } from './constants'; type BridgeControllerAction = { type: `${typeof BRIDGE_CONTROLLER_NAME}:${FunctionName}`; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 8ab78f2c7e92..933522c449f6 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -250,6 +250,10 @@ import { isSnapId } from '../../ui/helpers/utils/snaps'; import { BridgeStatusAction } from '../../shared/types/bridge-status'; import { ENVIRONMENT } from '../../development/build/constants'; import fetchWithCache from '../../shared/lib/fetch-with-cache'; +import { + BridgeUserAction, + BridgeBackgroundAction, +} from '../../shared/types/bridge'; import { BalancesController as MultichainBalancesController } from './lib/accounts/BalancesController'; import { ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) @@ -365,10 +369,6 @@ import { updateSecurityAlertResponse } from './lib/ppom/ppom-util'; import createEvmMethodsToNonEvmAccountReqFilterMiddleware from './lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware'; import { isEthAddress } from './lib/multichain/address'; import { decodeTransactionData } from './lib/transaction/decode/util'; -import { - BridgeUserAction, - BridgeBackgroundAction, -} from './controllers/bridge/types'; import BridgeController from './controllers/bridge/bridge-controller'; import { BRIDGE_CONTROLLER_NAME } from './controllers/bridge/constants'; import { diff --git a/shared/constants/bridge.ts b/shared/constants/bridge.ts index ef7cb7f8a785..57ba22da8f89 100644 --- a/shared/constants/bridge.ts +++ b/shared/constants/bridge.ts @@ -47,3 +47,5 @@ export const NETWORK_TO_SHORT_NETWORK_NAME_MAP: Record< [CHAIN_IDS.BASE]: 'Base', }; export const BRIDGE_MM_FEE_RATE = 0.875; +export const REFRESH_INTERVAL_MS = 30 * 1000; +export const DEFAULT_MAX_REFRESH_COUNT = 5; diff --git a/ui/pages/bridge/bridge.util.test.ts b/shared/modules/bridge-utils/bridge.util.test.ts similarity index 98% rename from ui/pages/bridge/bridge.util.test.ts rename to shared/modules/bridge-utils/bridge.util.test.ts index f302cd44090c..555de4fc2516 100644 --- a/ui/pages/bridge/bridge.util.test.ts +++ b/shared/modules/bridge-utils/bridge.util.test.ts @@ -1,8 +1,8 @@ -import fetchWithCache from '../../../shared/lib/fetch-with-cache'; -import { CHAIN_IDS } from '../../../shared/constants/network'; +import { zeroAddress } from 'ethereumjs-util'; +import fetchWithCache from '../../lib/fetch-with-cache'; +import { CHAIN_IDS } from '../../constants/network'; import mockBridgeQuotesErc20Erc20 from '../../../test/data/bridge/mock-quotes-erc20-erc20.json'; import mockBridgeQuotesNativeErc20 from '../../../test/data/bridge/mock-quotes-native-erc20.json'; -import { zeroAddress } from '../../__mocks__/ethereumjs-util'; import { fetchBridgeFeatureFlags, fetchBridgeQuotes, diff --git a/ui/pages/bridge/bridge.util.ts b/shared/modules/bridge-utils/bridge.util.ts similarity index 87% rename from ui/pages/bridge/bridge.util.ts rename to shared/modules/bridge-utils/bridge.util.ts index 534ea3418219..b7da42ba9cc6 100644 --- a/ui/pages/bridge/bridge.util.ts +++ b/shared/modules/bridge-utils/bridge.util.ts @@ -1,36 +1,25 @@ import { Contract } from '@ethersproject/contracts'; import { Hex, add0x } from '@metamask/utils'; import { abiERC20 } from '@metamask/metamask-eth-abis'; -import { - BridgeFeatureFlagsKey, - BridgeFeatureFlags, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../app/scripts/controllers/bridge/types'; import { BRIDGE_API_BASE_URL, BRIDGE_CLIENT_ID, ETH_USDT_ADDRESS, METABRIDGE_ETHEREUM_ADDRESS, -} from '../../../shared/constants/bridge'; -import { MINUTE } from '../../../shared/constants/time'; -import fetchWithCache from '../../../shared/lib/fetch-with-cache'; -import { - decimalToHex, - hexToDecimal, -} from '../../../shared/modules/conversion.utils'; + REFRESH_INTERVAL_MS, +} from '../../constants/bridge'; +import { MINUTE } from '../../constants/time'; +import fetchWithCache from '../../lib/fetch-with-cache'; +import { decimalToHex, hexToDecimal } from '../conversion.utils'; import { SWAPS_CHAINID_DEFAULT_TOKEN_MAP, SwapsTokenObject, -} from '../../../shared/constants/swaps'; +} from '../../constants/swaps'; import { isSwapsDefaultTokenAddress, isSwapsDefaultTokenSymbol, -} from '../../../shared/modules/swaps.utils'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { REFRESH_INTERVAL_MS } from '../../../app/scripts/controllers/bridge/constants'; -import { CHAIN_IDS } from '../../../shared/constants/network'; +} from '../swaps.utils'; +import { CHAIN_IDS } from '../../constants/network'; import { BridgeAsset, BridgeFlag, @@ -41,7 +30,9 @@ import { QuoteRequest, QuoteResponse, TxData, -} from './types'; + BridgeFeatureFlagsKey, + BridgeFeatureFlags, +} from '../../types/bridge'; import { FEATURE_FLAG_VALIDATORS, QUOTE_VALIDATORS, @@ -50,7 +41,7 @@ import { validateResponse, QUOTE_RESPONSE_VALIDATORS, FEE_DATA_VALIDATORS, -} from './utils/validators'; +} from './validators'; const CLIENT_ID_HEADER = { 'X-Client-Id': BRIDGE_CLIENT_ID }; const CACHE_REFRESH_TEN_MINUTES = 10 * MINUTE; diff --git a/shared/modules/bridge-utils/quote.ts b/shared/modules/bridge-utils/quote.ts new file mode 100644 index 000000000000..3fe4406fa333 --- /dev/null +++ b/shared/modules/bridge-utils/quote.ts @@ -0,0 +1,36 @@ +import type { QuoteRequest } from '../../types/bridge'; + +export const isValidQuoteRequest = ( + partialRequest: Partial, + requireAmount = true, +): partialRequest is QuoteRequest => { + const STRING_FIELDS = ['srcTokenAddress', 'destTokenAddress']; + if (requireAmount) { + STRING_FIELDS.push('srcTokenAmount'); + } + const NUMBER_FIELDS = ['srcChainId', 'destChainId', 'slippage']; + + return ( + STRING_FIELDS.every( + (field) => + field in partialRequest && + typeof partialRequest[field as keyof typeof partialRequest] === + 'string' && + partialRequest[field as keyof typeof partialRequest] !== undefined && + partialRequest[field as keyof typeof partialRequest] !== '' && + partialRequest[field as keyof typeof partialRequest] !== null, + ) && + NUMBER_FIELDS.every( + (field) => + field in partialRequest && + typeof partialRequest[field as keyof typeof partialRequest] === + 'number' && + partialRequest[field as keyof typeof partialRequest] !== undefined && + !isNaN(Number(partialRequest[field as keyof typeof partialRequest])) && + partialRequest[field as keyof typeof partialRequest] !== null, + ) && + (requireAmount + ? Boolean((partialRequest.srcTokenAmount ?? '').match(/^[1-9]\d*$/u)) + : true) + ); +}; diff --git a/ui/pages/bridge/utils/validators.ts b/shared/modules/bridge-utils/validators.ts similarity index 96% rename from ui/pages/bridge/utils/validators.ts rename to shared/modules/bridge-utils/validators.ts index 08fc3519ef52..edfbabdafaa3 100644 --- a/ui/pages/bridge/utils/validators.ts +++ b/shared/modules/bridge-utils/validators.ts @@ -1,10 +1,7 @@ import { isStrictHexString } from '@metamask/utils'; import { isValidHexAddress as isValidHexAddress_ } from '@metamask/controller-utils'; -import { - truthyDigitString, - validateData, -} from '../../../../shared/lib/swaps-utils'; -import { BridgeFlag, FeatureFlagResponse } from '../types'; +import { truthyDigitString, validateData } from '../../lib/swaps-utils'; +import { BridgeFlag, FeatureFlagResponse } from '../../types/bridge'; type Validator = { property: keyof ExpectedResponse | string; diff --git a/shared/types/bridge-status.ts b/shared/types/bridge-status.ts index fc9357ef968a..3e81f6e74f1d 100644 --- a/shared/types/bridge-status.ts +++ b/shared/types/bridge-status.ts @@ -1,12 +1,5 @@ import { TransactionMeta } from '@metamask/transaction-controller'; -// TODO fix this -import { - ChainId, - Quote, - QuoteMetadata, - QuoteResponse, - // eslint-disable-next-line import/no-restricted-paths -} from '../../ui/pages/bridge/types'; +import type { ChainId, Quote, QuoteMetadata, QuoteResponse } from './bridge'; // All fields need to be types not interfaces, same with their children fields // o/w you get a type error diff --git a/shared/types/bridge.ts b/shared/types/bridge.ts index 6cfab93f47ed..763c2670abad 100644 --- a/shared/types/bridge.ts +++ b/shared/types/bridge.ts @@ -1,4 +1,202 @@ +import type { Hex } from '@metamask/utils'; +import type { BigNumber } from 'bignumber.js'; +import type { AssetType } from '../constants/transaction'; +import type { SwapsTokenObject } from '../constants/swaps'; + export type ChainConfiguration = { isActiveSrc: boolean; isActiveDest: boolean; }; + +export type L1GasFees = { + l1GasFeesInHexWei?: string; // l1 fees for approval and trade in hex wei, appended by controller +}; +// Values derived from the quote response +// valueInCurrency values are calculated based on the user's selected currency + +export type QuoteMetadata = { + gasFee: { amount: BigNumber; valueInCurrency: BigNumber | null }; + totalNetworkFee: { amount: BigNumber; valueInCurrency: BigNumber | null }; // estimatedGasFees + relayerFees + totalMaxNetworkFee: { amount: BigNumber; valueInCurrency: BigNumber | null }; // maxGasFees + relayerFees + toTokenAmount: { amount: BigNumber; valueInCurrency: BigNumber | null }; + adjustedReturn: { valueInCurrency: BigNumber | null }; // destTokenAmount - totalNetworkFee + sentAmount: { amount: BigNumber; valueInCurrency: BigNumber | null }; // srcTokenAmount + metabridgeFee + swapRate: BigNumber; // destTokenAmount / sentAmount + cost: { valueInCurrency: BigNumber | null }; // sentAmount - adjustedReturn +}; +// Sort order set by the user + +export enum SortOrder { + COST_ASC = 'cost_ascending', + ETA_ASC = 'time_descending', +} + +export type BridgeToken = { + type: AssetType.native | AssetType.token; + address: string; + symbol: string; + image: string; + decimals: number; + chainId: Hex; + balance: string; // raw balance + string: string | undefined; // normalized balance as a stringified number + tokenFiatAmount?: number | null; +} | null; +// Types copied from Metabridge API + +export enum BridgeFlag { + EXTENSION_CONFIG = 'extension-config', +} +type DecimalChainId = string; +export type GasMultiplierByChainId = Record; + +export type FeatureFlagResponse = { + [BridgeFlag.EXTENSION_CONFIG]: { + refreshRate: number; + maxRefreshCount: number; + support: boolean; + chains: Record; + }; +}; + +export type BridgeAsset = { + chainId: ChainId; + address: string; + symbol: string; + name: string; + decimals: number; + icon?: string; +}; + +export type QuoteRequest = { + walletAddress: string; + destWalletAddress?: string; + srcChainId: ChainId; + destChainId: ChainId; + srcTokenAddress: string; + destTokenAddress: string; + srcTokenAmount: string; // This is the amount sent + slippage: number; + aggIds?: string[]; + bridgeIds?: string[]; + insufficientBal?: boolean; + resetApproval?: boolean; + refuel?: boolean; +}; +type Protocol = { + name: string; + displayName?: string; + icon?: string; +}; +enum ActionTypes { + BRIDGE = 'bridge', + SWAP = 'swap', + REFUEL = 'refuel', +} +type Step = { + action: ActionTypes; + srcChainId: ChainId; + destChainId?: ChainId; + srcAsset: BridgeAsset; + destAsset: BridgeAsset; + srcAmount: string; + destAmount: string; + protocol: Protocol; +}; +type RefuelData = Step; + +export type Quote = { + requestId: string; + srcChainId: ChainId; + srcAsset: BridgeAsset; + // Some tokens have a fee of 0, so sometimes it's equal to amount sent + srcTokenAmount: string; // Atomic amount, the amount sent - fees + destChainId: ChainId; + destAsset: BridgeAsset; + destTokenAmount: string; // Atomic amount, the amount received + feeData: Record & + Partial>; + bridgeId: string; + bridges: string[]; + steps: Step[]; + refuel?: RefuelData; +}; + +export type QuoteResponse = { + quote: Quote; + approval: TxData | null; + trade: TxData; + estimatedProcessingTimeInSeconds: number; +}; + +export enum ChainId { + ETH = 1, + OPTIMISM = 10, + BSC = 56, + POLYGON = 137, + ZKSYNC = 324, + BASE = 8453, + ARBITRUM = 42161, + AVALANCHE = 43114, + LINEA = 59144, +} + +export enum FeeType { + METABRIDGE = 'metabridge', + REFUEL = 'refuel', +} +export type FeeData = { + amount: string; + asset: BridgeAsset; +}; +export type TxData = { + chainId: ChainId; + to: string; + from: string; + value: string; + data: string; + gasLimit: number | null; +}; +export enum BridgeFeatureFlagsKey { + EXTENSION_CONFIG = 'extensionConfig', +} + +export type BridgeFeatureFlags = { + [BridgeFeatureFlagsKey.EXTENSION_CONFIG]: { + refreshRate: number; + maxRefreshCount: number; + support: boolean; + chains: Record; + }; +}; +export enum RequestStatus { + LOADING, + FETCHED, + ERROR, +} +export enum BridgeUserAction { + SELECT_SRC_NETWORK = 'selectSrcNetwork', + SELECT_DEST_NETWORK = 'selectDestNetwork', + UPDATE_QUOTE_PARAMS = 'updateBridgeQuoteRequestParams', +} +export enum BridgeBackgroundAction { + SET_FEATURE_FLAGS = 'setBridgeFeatureFlags', + RESET_STATE = 'resetState', + GET_BRIDGE_ERC20_ALLOWANCE = 'getBridgeERC20Allowance', +} +export type BridgeControllerState = { + bridgeFeatureFlags: BridgeFeatureFlags; + srcTokens: Record; + srcTopAssets: { address: string }[]; + srcTokensLoadingStatus?: RequestStatus; + destTokensLoadingStatus?: RequestStatus; + destTokens: Record; + destTopAssets: { address: string }[]; + quoteRequest: Partial; + quotes: (QuoteResponse & L1GasFees)[]; + quotesInitialLoadTime?: number; + quotesLastFetched?: number; + quotesLoadingStatus?: RequestStatus; + quoteFetchError?: string; + quotesRefreshCount: number; +}; diff --git a/test/e2e/tests/bridge/bridge-test-utils.ts b/test/e2e/tests/bridge/bridge-test-utils.ts index cf57deb59f96..842ff31fff39 100644 --- a/test/e2e/tests/bridge/bridge-test-utils.ts +++ b/test/e2e/tests/bridge/bridge-test-utils.ts @@ -12,7 +12,7 @@ import { SMART_CONTRACTS } from '../../seeder/smart-contracts'; import { CHAIN_IDS } from '../../../../shared/constants/network'; import { Driver } from '../../webdriver/driver'; import { isManifestV3 } from '../../../../shared/modules/mv3.utils'; -import { FeatureFlagResponse } from '../../../../ui/pages/bridge/types'; +import type { FeatureFlagResponse } from '../../../../shared/types/bridge'; import { DEFAULT_FEATURE_FLAGS_RESPONSE, ETH_CONVERSION_RATE_USD, diff --git a/test/e2e/tests/bridge/constants.ts b/test/e2e/tests/bridge/constants.ts index d5b6da9afc61..844cec673509 100644 --- a/test/e2e/tests/bridge/constants.ts +++ b/test/e2e/tests/bridge/constants.ts @@ -1,4 +1,4 @@ -import { FeatureFlagResponse } from '../../../../ui/pages/bridge/types'; +import type { FeatureFlagResponse } from '../../../../shared/types/bridge'; export const DEFAULT_FEATURE_FLAGS_RESPONSE: FeatureFlagResponse = { 'extension-config': { diff --git a/ui/ducks/bridge/actions.ts b/ui/ducks/bridge/actions.ts index 5597503206da..e2ea02bad39d 100644 --- a/ui/ducks/bridge/actions.ts +++ b/ui/ducks/bridge/actions.ts @@ -1,16 +1,12 @@ -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { Hex } from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; import { BridgeBackgroundAction, BridgeUserAction, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../app/scripts/controllers/bridge/types'; + QuoteRequest, +} from '../../../shared/types/bridge'; import { forceUpdateMetamaskState } from '../../store/actions'; import { submitRequestToBackground } from '../../store/background-connection'; -import { QuoteRequest } from '../../pages/bridge/types'; -import { MetaMaskReduxDispatch } from '../../store/store'; +import type { MetaMaskReduxDispatch } from '../../store/store'; import { bridgeSlice, setDestTokenExchangeRates, diff --git a/ui/ducks/bridge/bridge.test.ts b/ui/ducks/bridge/bridge.test.ts index d317d1b53bb8..30e2f0fe880a 100644 --- a/ui/ducks/bridge/bridge.test.ts +++ b/ui/ducks/bridge/bridge.test.ts @@ -7,9 +7,7 @@ import { setBackgroundConnection } from '../../store/background-connection'; import { BridgeBackgroundAction, BridgeUserAction, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../app/scripts/controllers/bridge/types'; +} from '../../../shared/types/bridge'; import * as util from '../../helpers/utils/util'; import { BRIDGE_DEFAULT_SLIPPAGE } from '../../../shared/constants/bridge'; import bridgeReducer from './bridge'; diff --git a/ui/ducks/bridge/bridge.ts b/ui/ducks/bridge/bridge.ts index 82bbba964868..14786b51d4dd 100644 --- a/ui/ducks/bridge/bridge.ts +++ b/ui/ducks/bridge/bridge.ts @@ -1,12 +1,11 @@ import { createAsyncThunk, createSlice } from '@reduxjs/toolkit'; import { Hex } from '@metamask/utils'; -import { swapsSlice } from '../swaps/swaps'; import { - BridgeToken, - QuoteMetadata, - QuoteResponse, + type BridgeToken, + type QuoteMetadata, + type QuoteResponse, SortOrder, -} from '../../pages/bridge/types'; +} from '../../../shared/types/bridge'; import { BRIDGE_DEFAULT_SLIPPAGE } from '../../../shared/constants/bridge'; import { getTokenExchangeRate } from './utils'; @@ -57,7 +56,6 @@ const bridgeSlice = createSlice({ name: 'bridge', initialState: { ...initialState }, reducers: { - ...swapsSlice.reducer, setToChainId: (state, action) => { state.toChainId = action.payload; }, diff --git a/ui/ducks/bridge/selectors.test.ts b/ui/ducks/bridge/selectors.test.ts index 0e948ee18784..bf0cefedeb4d 100644 --- a/ui/ducks/bridge/selectors.test.ts +++ b/ui/ducks/bridge/selectors.test.ts @@ -11,10 +11,10 @@ import { mockNetworkState } from '../../../test/stub/networks'; import mockErc20Erc20Quotes from '../../../test/data/bridge/mock-quotes-erc20-erc20.json'; import mockBridgeQuotesNativeErc20 from '../../../test/data/bridge/mock-quotes-native-erc20.json'; import { - QuoteMetadata, - QuoteResponse, + type QuoteMetadata, + type QuoteResponse, SortOrder, -} from '../../pages/bridge/types'; +} from '../../../shared/types/bridge'; import { getAllBridgeableNetworks, getBridgeQuotes, diff --git a/ui/ducks/bridge/selectors.ts b/ui/ducks/bridge/selectors.ts index 9241af57db6d..f86745e69ae3 100644 --- a/ui/ducks/bridge/selectors.ts +++ b/ui/ducks/bridge/selectors.ts @@ -1,11 +1,11 @@ -import { +import type { AddNetworkFields, NetworkConfiguration, NetworkState, } from '@metamask/network-controller'; import { orderBy, uniqBy } from 'lodash'; import { createSelector } from 'reselect'; -import { GasFeeEstimates } from '@metamask/gas-fee-controller'; +import type { GasFeeEstimates } from '@metamask/gas-fee-controller'; import { BigNumber } from 'bignumber.js'; import { calcTokenAmount } from '@metamask/notification-services-controller/push-services'; import { @@ -20,12 +20,7 @@ import { BRIDGE_PREFERRED_GAS_ESTIMATE, BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE, } from '../../../shared/constants/bridge'; -import { - BridgeControllerState, - BridgeFeatureFlagsKey, - // TODO: Remove restricted import - // eslint-disable-next-line import/no-restricted-paths -} from '../../../app/scripts/controllers/bridge/types'; +import type { BridgeControllerState } from '../../../shared/types/bridge'; import { createDeepEqualSelector } from '../../../shared/modules/selectors/util'; import { SWAPS_CHAINID_DEFAULT_TOKEN_MAP } from '../../../shared/constants/swaps'; import { @@ -33,16 +28,15 @@ import { getNetworkConfigurationsByChainId, } from '../../../shared/modules/selectors/networks'; import { getConversionRate, getGasFeeEstimates } from '../metamask/metamask'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { RequestStatus } from '../../../app/scripts/controllers/bridge/constants'; import { - L1GasFees, - BridgeToken, - QuoteMetadata, - QuoteResponse, + type L1GasFees, + type BridgeToken, + type QuoteMetadata, + type QuoteResponse, SortOrder, -} from '../../pages/bridge/types'; + BridgeFeatureFlagsKey, + RequestStatus, +} from '../../../shared/types/bridge'; import { calcAdjustedReturn, calcCost, @@ -64,7 +58,7 @@ import { exchangeRateFromMarketData, tokenPriceInNativeAsset, } from './utils'; -import { BridgeState } from './bridge'; +import type { BridgeState } from './bridge'; type BridgeAppState = { metamask: { bridgeState: BridgeControllerState } & NetworkState & { diff --git a/ui/ducks/bridge/utils.ts b/ui/ducks/bridge/utils.ts index a563b5af2c73..050829d3ca65 100644 --- a/ui/ducks/bridge/utils.ts +++ b/ui/ducks/bridge/utils.ts @@ -1,14 +1,14 @@ -import { Hex } from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; import { getAddress } from 'ethers/lib/utils'; -import { ContractMarketData } from '@metamask/assets-controllers'; +import type { ContractMarketData } from '@metamask/assets-controllers'; import { AddNetworkFields, NetworkConfiguration, } from '@metamask/network-controller'; import { decGWEIToHexWEI } from '../../../shared/modules/conversion.utils'; import { Numeric } from '../../../shared/modules/Numeric'; -import { TxData } from '../../pages/bridge/types'; +import type { TxData } from '../../../shared/types/bridge'; import { getTransaction1559GasFeeEstimates } from '../../pages/swaps/swaps.util'; import { fetchTokenExchangeRates as fetchTokenExchangeRatesUtil } from '../../helpers/utils/util'; diff --git a/ui/hooks/bridge/useBridgeTokens.ts b/ui/hooks/bridge/useBridgeTokens.ts index acddb7ec2fb0..354b90bc4a09 100644 --- a/ui/hooks/bridge/useBridgeTokens.ts +++ b/ui/hooks/bridge/useBridgeTokens.ts @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react'; import { useSelector } from 'react-redux'; import { getAllBridgeableNetworks } from '../../ducks/bridge/selectors'; -import { fetchBridgeTokens } from '../../pages/bridge/bridge.util'; +import { fetchBridgeTokens } from '../../../shared/modules/bridge-utils/bridge.util'; // This hook is used to fetch the bridge tokens for all bridgeable networks export const useBridgeTokens = () => { diff --git a/ui/hooks/bridge/useCrossChainSwapsEventTracker.ts b/ui/hooks/bridge/useCrossChainSwapsEventTracker.ts index ad4b3698fe84..bd3a14b4ce4b 100644 --- a/ui/hooks/bridge/useCrossChainSwapsEventTracker.ts +++ b/ui/hooks/bridge/useCrossChainSwapsEventTracker.ts @@ -5,7 +5,7 @@ import { MetaMetricsEventName, MetaMetricsSwapsEventSource, } from '../../../shared/constants/metametrics'; -import { SortOrder } from '../../pages/bridge/types'; +import { SortOrder } from '../../../shared/types/bridge'; import { RequestParams, RequestMetadata, diff --git a/ui/hooks/bridge/useLatestBalance.test.ts b/ui/hooks/bridge/useLatestBalance.test.ts index 25f0d0936791..f255a3a10b9b 100644 --- a/ui/hooks/bridge/useLatestBalance.test.ts +++ b/ui/hooks/bridge/useLatestBalance.test.ts @@ -1,8 +1,8 @@ import { BigNumber } from 'bignumber.js'; +import { zeroAddress } from 'ethereumjs-util'; import { renderHookWithProvider } from '../../../test/lib/render-helpers'; import { CHAIN_IDS } from '../../../shared/constants/network'; import { createBridgeMockStore } from '../../../test/jest/mock-store'; -import { zeroAddress } from '../../__mocks__/ethereumjs-util'; import { createTestProviderTools } from '../../../test/stub/provider'; import * as tokenutil from '../../../shared/lib/token-util'; import useLatestBalance from './useLatestBalance'; diff --git a/ui/pages/bridge/hooks/useAddToken.ts b/ui/pages/bridge/hooks/useAddToken.ts index 1f2dc117513b..f1a148ce7732 100644 --- a/ui/pages/bridge/hooks/useAddToken.ts +++ b/ui/pages/bridge/hooks/useAddToken.ts @@ -1,6 +1,6 @@ import { useDispatch, useSelector } from 'react-redux'; -import { NetworkConfiguration } from '@metamask/network-controller'; -import { QuoteResponse } from '../types'; +import type { NetworkConfiguration } from '@metamask/network-controller'; +import type { QuoteResponse } from '../../../../shared/types/bridge'; import { FEATURED_RPCS } from '../../../../shared/constants/network'; import { addToken, addNetwork } from '../../../store/actions'; import { diff --git a/ui/pages/bridge/hooks/useHandleApprovalTx.ts b/ui/pages/bridge/hooks/useHandleApprovalTx.ts index 23f4b19cf2b8..7e469bc78ead 100644 --- a/ui/pages/bridge/hooks/useHandleApprovalTx.ts +++ b/ui/pages/bridge/hooks/useHandleApprovalTx.ts @@ -1,8 +1,15 @@ import { TransactionType } from '@metamask/transaction-controller'; -import { Hex } from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; -import { TxData, QuoteResponse, FeeType } from '../types'; -import { isEthUsdt, getEthUsdtResetData } from '../bridge.util'; +import { + type TxData, + type QuoteResponse, + FeeType, +} from '../../../../shared/types/bridge'; +import { + isEthUsdt, + getEthUsdtResetData, +} from '../../../../shared/modules/bridge-utils/bridge.util'; import { ETH_USDT_ADDRESS } from '../../../../shared/constants/bridge'; import { getBridgeERC20Allowance } from '../../../ducks/bridge/actions'; import { decimalToPrefixedHex } from '../../../../shared/modules/conversion.utils'; diff --git a/ui/pages/bridge/hooks/useHandleBridgeTx.ts b/ui/pages/bridge/hooks/useHandleBridgeTx.ts index feb7400acc71..5d7e1a1b527c 100644 --- a/ui/pages/bridge/hooks/useHandleBridgeTx.ts +++ b/ui/pages/bridge/hooks/useHandleBridgeTx.ts @@ -1,7 +1,7 @@ import { BigNumber } from 'bignumber.js'; import { TransactionType } from '@metamask/transaction-controller'; import { Numeric } from '../../../../shared/modules/Numeric'; -import { FeeType, QuoteResponse } from '../types'; +import { FeeType, type QuoteResponse } from '../../../../shared/types/bridge'; import useHandleTx from './useHandleTx'; export default function useHandleBridgeTx() { diff --git a/ui/pages/bridge/hooks/useHandleTx.ts b/ui/pages/bridge/hooks/useHandleTx.ts index c3a3ede01002..59b6c7ee9c89 100644 --- a/ui/pages/bridge/hooks/useHandleTx.ts +++ b/ui/pages/bridge/hooks/useHandleTx.ts @@ -15,7 +15,7 @@ import { } from '../../../ducks/bridge/utils'; import { getGasFeeEstimates } from '../../../ducks/metamask/metamask'; import { checkNetworkAndAccountSupports1559 } from '../../../selectors'; -import { ChainId } from '../types'; +import type { ChainId } from '../../../../shared/types/bridge'; import { decimalToPrefixedHex } from '../../../../shared/modules/conversion.utils'; import { getIsSmartTransaction } from '../../../../shared/modules/selectors'; diff --git a/ui/pages/bridge/hooks/useSubmitBridgeTransaction.ts b/ui/pages/bridge/hooks/useSubmitBridgeTransaction.ts index 6f8ec559a6a5..e34c4e9400c6 100644 --- a/ui/pages/bridge/hooks/useSubmitBridgeTransaction.ts +++ b/ui/pages/bridge/hooks/useSubmitBridgeTransaction.ts @@ -3,7 +3,10 @@ import { zeroAddress } from 'ethereumjs-util'; import { useHistory } from 'react-router-dom'; import { TransactionMeta } from '@metamask/transaction-controller'; import { createProjectLogger, Hex } from '@metamask/utils'; -import { QuoteMetadata, QuoteResponse } from '../types'; +import type { + QuoteMetadata, + QuoteResponse, +} from '../../../../shared/types/bridge'; import { AWAITING_SIGNATURES_ROUTE, CROSS_CHAIN_SWAP_ROUTE, diff --git a/ui/pages/bridge/prepare/bridge-cta-button.test.tsx b/ui/pages/bridge/prepare/bridge-cta-button.test.tsx index 5dc368043d3a..92d6591a2808 100644 --- a/ui/pages/bridge/prepare/bridge-cta-button.test.tsx +++ b/ui/pages/bridge/prepare/bridge-cta-button.test.tsx @@ -4,9 +4,7 @@ import configureStore from '../../../store/store'; import { createBridgeMockStore } from '../../../../test/jest/mock-store'; import { CHAIN_IDS } from '../../../../shared/constants/network'; import mockBridgeQuotesNativeErc20 from '../../../../test/data/bridge/mock-quotes-native-erc20.json'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { RequestStatus } from '../../../../app/scripts/controllers/bridge/constants'; +import { RequestStatus } from '../../../../shared/types/bridge'; import { BridgeCTAButton } from './bridge-cta-button'; describe('BridgeCTAButton', () => { diff --git a/ui/pages/bridge/prepare/bridge-input-group.tsx b/ui/pages/bridge/prepare/bridge-input-group.tsx index 1734624b7dcc..f2ec234263e7 100644 --- a/ui/pages/bridge/prepare/bridge-input-group.tsx +++ b/ui/pages/bridge/prepare/bridge-input-group.tsx @@ -34,7 +34,7 @@ import { getValidationErrors, } from '../../../ducks/bridge/selectors'; import { shortenString } from '../../../helpers/utils/util'; -import { BridgeToken } from '../types'; +import type { BridgeToken } from '../../../../shared/types/bridge'; import { useCopyToClipboard } from '../../../hooks/useCopyToClipboard'; import { MINUTE } from '../../../../shared/constants/time'; import { BridgeAssetPickerButton } from './components/bridge-asset-picker-button'; diff --git a/ui/pages/bridge/prepare/prepare-bridge-page.stories.tsx b/ui/pages/bridge/prepare/prepare-bridge-page.stories.tsx index 3e955d3e34ef..4a57e9517c67 100644 --- a/ui/pages/bridge/prepare/prepare-bridge-page.stories.tsx +++ b/ui/pages/bridge/prepare/prepare-bridge-page.stories.tsx @@ -3,7 +3,6 @@ import { Provider } from 'react-redux'; import configureStore from '../../../store/store'; import { createBridgeMockStore } from '../../../../test/jest/mock-store'; import { CHAIN_IDS } from '../../../../shared/constants/network'; -import { RequestStatus } from '../../../../app/scripts/controllers/bridge/constants'; import CrossChainSwap from '../index'; import { MemoryRouter } from 'react-router-dom'; import { @@ -11,6 +10,7 @@ import { PREPARE_SWAP_ROUTE, } from '../../../helpers/constants/routes'; import mockBridgeQuotesErc20Erc20 from '../../../../test/data/bridge/mock-quotes-erc20-erc20.json'; +import { RequestStatus } from '../../../../shared/types/bridge'; const storybook = { title: 'Pages/Bridge/CrossChainSwapPage', diff --git a/ui/pages/bridge/prepare/prepare-bridge-page.tsx b/ui/pages/bridge/prepare/prepare-bridge-page.tsx index fc076b73629b..e1f220fc69fa 100644 --- a/ui/pages/bridge/prepare/prepare-bridge-page.tsx +++ b/ui/pages/bridge/prepare/prepare-bridge-page.tsx @@ -64,14 +64,14 @@ import { SWAPS_CHAINID_DEFAULT_TOKEN_MAP } from '../../../../shared/constants/sw import { useTokensWithFiltering } from '../../../hooks/bridge/useTokensWithFiltering'; import { setActiveNetwork } from '../../../store/actions'; import { hexToDecimal } from '../../../../shared/modules/conversion.utils'; -import { QuoteRequest } from '../types'; +import type { QuoteRequest } from '../../../../shared/types/bridge'; import { calcTokenValue } from '../../../../shared/lib/swaps-utils'; import { BridgeQuoteCard } from '../quotes/bridge-quote-card'; import { formatTokenAmount, isQuoteExpired as isQuoteExpiredUtil, - isValidQuoteRequest, } from '../utils/quote'; +import { isValidQuoteRequest } from '../../../../shared/modules/bridge-utils/quote'; import { getProviderConfig } from '../../../../shared/modules/selectors/networks'; import { CrossChainSwapsEventProperties, @@ -340,8 +340,6 @@ const PrepareBridgePage = () => { input: 'token_source', value: token.address, }); - dispatch(setFromToken(token)); - dispatch(setFromTokenInputValue(null)); }} networkProps={{ network: fromChain, diff --git a/ui/pages/bridge/quotes/bridge-quote-card.test.tsx b/ui/pages/bridge/quotes/bridge-quote-card.test.tsx index 59db6fa4cc63..25d095705324 100644 --- a/ui/pages/bridge/quotes/bridge-quote-card.test.tsx +++ b/ui/pages/bridge/quotes/bridge-quote-card.test.tsx @@ -5,9 +5,7 @@ import { createBridgeMockStore } from '../../../../test/jest/mock-store'; import { CHAIN_IDS } from '../../../../shared/constants/network'; import mockBridgeQuotesErc20Erc20 from '../../../../test/data/bridge/mock-quotes-erc20-erc20.json'; import mockBridgeQuotesNativeErc20 from '../../../../test/data/bridge/mock-quotes-native-erc20.json'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { RequestStatus } from '../../../../app/scripts/controllers/bridge/constants'; +import { RequestStatus } from '../../../../shared/types/bridge'; import { BridgeQuoteCard } from './bridge-quote-card'; describe('BridgeQuoteCard', () => { diff --git a/ui/pages/bridge/quotes/bridge-quotes-modal.stories.tsx b/ui/pages/bridge/quotes/bridge-quotes-modal.stories.tsx index bbdf9b47fa47..fe999108bec8 100644 --- a/ui/pages/bridge/quotes/bridge-quotes-modal.stories.tsx +++ b/ui/pages/bridge/quotes/bridge-quotes-modal.stories.tsx @@ -4,7 +4,7 @@ import configureStore from '../../../store/store'; import { BridgeQuotesModal } from './bridge-quotes-modal'; import { createBridgeMockStore } from '../../../../test/jest/mock-store'; import mockBridgeQuotesErc20Erc20 from '../../../../test/data/bridge/mock-quotes-erc20-erc20.json'; -import { SortOrder } from '../types'; +import { SortOrder } from '../../../../shared/types/bridge'; const storybook = { title: 'Pages/Bridge/BridgeQuotesModal', diff --git a/ui/pages/bridge/quotes/bridge-quotes-modal.test.tsx b/ui/pages/bridge/quotes/bridge-quotes-modal.test.tsx index ac5e384413d8..2d6f3dd992ea 100644 --- a/ui/pages/bridge/quotes/bridge-quotes-modal.test.tsx +++ b/ui/pages/bridge/quotes/bridge-quotes-modal.test.tsx @@ -1,7 +1,5 @@ import React from 'react'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { RequestStatus } from '../../../../app/scripts/controllers/bridge/constants'; +import { RequestStatus } from '../../../../shared/types/bridge'; import mockBridgeQuotesErc20Erc20 from '../../../../test/data/bridge/mock-quotes-erc20-erc20.json'; import { createBridgeMockStore } from '../../../../test/jest/mock-store'; import { renderWithProvider } from '../../../../test/lib/render-helpers'; diff --git a/ui/pages/bridge/quotes/bridge-quotes-modal.tsx b/ui/pages/bridge/quotes/bridge-quotes-modal.tsx index aaabfe8f5ddb..bc846f6d081f 100644 --- a/ui/pages/bridge/quotes/bridge-quotes-modal.tsx +++ b/ui/pages/bridge/quotes/bridge-quotes-modal.tsx @@ -26,7 +26,11 @@ import { import { useI18nContext } from '../../../hooks/useI18nContext'; import { getLocale } from '../../../selectors'; import { setSelectedQuote, setSortOrder } from '../../../ducks/bridge/actions'; -import { QuoteMetadata, QuoteResponse, SortOrder } from '../types'; +import { + type QuoteMetadata, + type QuoteResponse, + SortOrder, +} from '../../../../shared/types/bridge'; import { getBridgeQuotes, getBridgeSortOrder, diff --git a/ui/pages/bridge/types.ts b/ui/pages/bridge/types.ts deleted file mode 100644 index d0eb45fa71a5..000000000000 --- a/ui/pages/bridge/types.ts +++ /dev/null @@ -1,159 +0,0 @@ -import { BigNumber } from 'bignumber.js'; -import { Hex } from '@metamask/utils'; -import { ChainConfiguration } from '../../../shared/types/bridge'; -import type { AssetType } from '../../../shared/constants/transaction'; - -export type L1GasFees = { - l1GasFeesInHexWei?: string; // l1 fees for approval and trade in hex wei, appended by controller -}; - -// Values derived from the quote response -// valueInCurrency values are calculated based on the user's selected currency -export type QuoteMetadata = { - gasFee: { amount: BigNumber; valueInCurrency: BigNumber | null }; - totalNetworkFee: { amount: BigNumber; valueInCurrency: BigNumber | null }; // estimatedGasFees + relayerFees - totalMaxNetworkFee: { amount: BigNumber; valueInCurrency: BigNumber | null }; // maxGasFees + relayerFees - toTokenAmount: { amount: BigNumber; valueInCurrency: BigNumber | null }; - adjustedReturn: { valueInCurrency: BigNumber | null }; // destTokenAmount - totalNetworkFee - sentAmount: { amount: BigNumber; valueInCurrency: BigNumber | null }; // srcTokenAmount + metabridgeFee - swapRate: BigNumber; // destTokenAmount / sentAmount - cost: { valueInCurrency: BigNumber | null }; // sentAmount - adjustedReturn -}; - -// Sort order set by the user -export enum SortOrder { - COST_ASC = 'cost_ascending', - ETA_ASC = 'time_descending', -} - -export type BridgeToken = { - type: AssetType.native | AssetType.token; - address: string; - symbol: string; - image: string; - decimals: number; - chainId: Hex; - balance: string; // raw balance - string: string | undefined; // normalized balance as a stringified number - tokenFiatAmount?: number | null; -} | null; - -// Types copied from Metabridge API -export enum BridgeFlag { - EXTENSION_CONFIG = 'extension-config', -} - -type DecimalChainId = string; -export type GasMultiplierByChainId = Record; - -export type FeatureFlagResponse = { - [BridgeFlag.EXTENSION_CONFIG]: { - refreshRate: number; - maxRefreshCount: number; - support: boolean; - chains: Record; - }; -}; - -export type BridgeAsset = { - chainId: ChainId; - address: string; - symbol: string; - name: string; - decimals: number; - icon?: string; -}; - -export type QuoteRequest = { - walletAddress: string; - destWalletAddress?: string; - srcChainId: ChainId; - destChainId: ChainId; - srcTokenAddress: string; - destTokenAddress: string; - srcTokenAmount: string; // This is the amount sent - slippage: number; - aggIds?: string[]; - bridgeIds?: string[]; - insufficientBal?: boolean; - resetApproval?: boolean; - refuel?: boolean; -}; - -type Protocol = { - name: string; - displayName?: string; - icon?: string; -}; - -enum ActionTypes { - BRIDGE = 'bridge', - SWAP = 'swap', - REFUEL = 'refuel', -} - -type Step = { - action: ActionTypes; - srcChainId: ChainId; - destChainId?: ChainId; - srcAsset: BridgeAsset; - destAsset: BridgeAsset; - srcAmount: string; - destAmount: string; - protocol: Protocol; -}; - -type RefuelData = Step; - -export type Quote = { - requestId: string; - srcChainId: ChainId; - srcAsset: BridgeAsset; - // Some tokens have a fee of 0, so sometimes it's equal to amount sent - srcTokenAmount: string; // Atomic amount, the amount sent - fees - destChainId: ChainId; - destAsset: BridgeAsset; - destTokenAmount: string; // Atomic amount, the amount received - feeData: Record & - Partial>; - bridgeId: string; - bridges: string[]; - steps: Step[]; - refuel?: RefuelData; -}; - -export type QuoteResponse = { - quote: Quote; - approval: TxData | null; - trade: TxData; - estimatedProcessingTimeInSeconds: number; -}; - -export enum ChainId { - ETH = 1, - OPTIMISM = 10, - BSC = 56, - POLYGON = 137, - ZKSYNC = 324, - BASE = 8453, - ARBITRUM = 42161, - AVALANCHE = 43114, - LINEA = 59144, -} - -export enum FeeType { - METABRIDGE = 'metabridge', - REFUEL = 'refuel', -} -export type FeeData = { - amount: string; - asset: BridgeAsset; -}; -export type TxData = { - chainId: ChainId; - to: string; - from: string; - value: string; - data: string; - gasLimit: number | null; -}; diff --git a/ui/pages/bridge/utils/quote.ts b/ui/pages/bridge/utils/quote.ts index 25555a216c7f..ee78e055698d 100644 --- a/ui/pages/bridge/utils/quote.ts +++ b/ui/pages/bridge/utils/quote.ts @@ -1,7 +1,11 @@ import { zeroAddress } from 'ethereumjs-util'; import { BigNumber } from 'bignumber.js'; import { calcTokenAmount } from '../../../../shared/lib/transactions-controller-utils'; -import { QuoteResponse, QuoteRequest, Quote, L1GasFees } from '../types'; +import type { + QuoteResponse, + Quote, + L1GasFees, +} from '../../../../shared/types/bridge'; import { hexToDecimal, sumDecimals, @@ -26,41 +30,6 @@ export const isQuoteExpired = ( export const isNativeAddress = (address?: string | null) => address === zeroAddress() || address === '' || !address; -export const isValidQuoteRequest = ( - partialRequest: Partial, - requireAmount = true, -): partialRequest is QuoteRequest => { - const STRING_FIELDS = ['srcTokenAddress', 'destTokenAddress']; - if (requireAmount) { - STRING_FIELDS.push('srcTokenAmount'); - } - const NUMBER_FIELDS = ['srcChainId', 'destChainId', 'slippage']; - - return ( - STRING_FIELDS.every( - (field) => - field in partialRequest && - typeof partialRequest[field as keyof typeof partialRequest] === - 'string' && - partialRequest[field as keyof typeof partialRequest] !== undefined && - partialRequest[field as keyof typeof partialRequest] !== '' && - partialRequest[field as keyof typeof partialRequest] !== null, - ) && - NUMBER_FIELDS.every( - (field) => - field in partialRequest && - typeof partialRequest[field as keyof typeof partialRequest] === - 'number' && - partialRequest[field as keyof typeof partialRequest] !== undefined && - !isNaN(Number(partialRequest[field as keyof typeof partialRequest])) && - partialRequest[field as keyof typeof partialRequest] !== null, - ) && - (requireAmount - ? Boolean((partialRequest.srcTokenAmount ?? '').match(/^[1-9]\d*$/u)) - : true) - ); -}; - export const calcToAmount = ( { destTokenAmount, destAsset }: Quote, exchangeRate: number | null, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 383f9c0dd683..61c15000baba 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -106,9 +106,7 @@ import { BackgroundColor } from '../helpers/constants/design-system'; import { NOTIFICATION_DROP_LEDGER_FIREFOX } from '../../shared/notifications'; import { ENVIRONMENT_TYPE_POPUP } from '../../shared/constants/app'; import { MULTICHAIN_NETWORK_TO_ASSET_TYPES } from '../../shared/constants/multichain/assets'; -// TODO: Remove restricted import -// eslint-disable-next-line import/no-restricted-paths -import { BridgeFeatureFlagsKey } from '../../app/scripts/controllers/bridge/types'; +import { BridgeFeatureFlagsKey } from '../../shared/types/bridge'; import { hasTransactionData } from '../../shared/modules/transaction.utils'; import { toChecksumHexAddress } from '../../shared/modules/hexstring-utils'; import { createDeepEqualSelector } from '../../shared/modules/selectors/util'; From cd39d7b33cf5d0bf44b57f284e9875eb618679af Mon Sep 17 00:00:00 2001 From: micaelae <100321200+micaelae@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:52:16 -0800 Subject: [PATCH 17/19] chore: low return warning alert for bridging (#29171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Changes the Low Return tooltip into an alert Banner, and highlights network fee in the quote card [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29171?quickstart=1) ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMS-1814 ## **Manual testing steps** 1. Request quotes with a low return 2. Verify that new treatment is shown ## **Screenshots/Recordings** ### **Before** ![Screenshot 2024-12-17 at 10 47 21 AM](https://github.com/user-attachments/assets/c30a4682-7174-4041-83df-58726127f3c5) ![Screenshot 2024-12-17 at 10 47 46 AM](https://github.com/user-attachments/assets/f4576048-7f41-4677-b205-710421ecd9c6) ### **After** ![Screenshot 2024-12-17 at 10 32 28 AM](https://github.com/user-attachments/assets/c8db2d88-fce9-491e-9949-271b3d9faf96) ## **Pre-merge author checklist** - [X] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- app/_locales/en/messages.json | 4 +- shared/constants/bridge.ts | 2 +- ui/ducks/bridge/selectors.test.ts | 13 +- .../bridge/prepare/bridge-input-group.tsx | 25 +--- ui/pages/bridge/prepare/index.scss | 8 ++ .../prepare/prepare-bridge-page.stories.tsx | 7 +- .../bridge/prepare/prepare-bridge-page.tsx | 42 ++++++- .../bridge-quote-card.test.tsx.snap | 36 ++---- .../bridge-quotes-modal.test.tsx.snap | 2 +- ui/pages/bridge/quotes/bridge-quote-card.tsx | 112 ++++++++++-------- .../bridge/quotes/bridge-quotes-modal.tsx | 2 +- 11 files changed, 137 insertions(+), 116 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 7ddf609b48f0..d07953e6bde4 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2971,10 +2971,10 @@ "message": "Low" }, "lowEstimatedReturnTooltipMessage": { - "message": "Either your rate or your fees are less favorable than usual. It looks like you'll get back less than $1% of the amount you’re bridging." + "message": "You’ll pay more than $1% of your starting amount in fees. Check your receiving amount and network fees." }, "lowEstimatedReturnTooltipTitle": { - "message": "Low estimated return" + "message": "High cost" }, "lowGasSettingToolTipMessage": { "message": "Use $1 to wait for a cheaper price. Time estimates are much less accurate as prices are somewhat unpredictable.", diff --git a/shared/constants/bridge.ts b/shared/constants/bridge.ts index 57ba22da8f89..fc5ecb157016 100644 --- a/shared/constants/bridge.ts +++ b/shared/constants/bridge.ts @@ -27,7 +27,7 @@ export const ETH_USDT_ADDRESS = '0xdac17f958d2ee523a2206206994597c13d831ec7'; export const METABRIDGE_ETHEREUM_ADDRESS = '0x0439e60F02a8900a951603950d8D4527f400C3f1'; export const BRIDGE_QUOTE_MAX_ETA_SECONDS = 60 * 60; // 1 hour -export const BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE = 0.8; // if a quote returns in x times less return than the best quote, ignore it +export const BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE = 0.5; // if a quote returns in x times less return than the best quote, ignore it export const BRIDGE_PREFERRED_GAS_ESTIMATE = 'high'; export const BRIDGE_DEFAULT_SLIPPAGE = 0.5; diff --git a/ui/ducks/bridge/selectors.test.ts b/ui/ducks/bridge/selectors.test.ts index bf0cefedeb4d..f44b5d8dacea 100644 --- a/ui/ducks/bridge/selectors.test.ts +++ b/ui/ducks/bridge/selectors.test.ts @@ -1212,7 +1212,7 @@ describe('Bridge selectors', () => { ).toStrictEqual(false); }); - it('should return isEstimatedReturnLow=true return value is 20% less than sent funds', () => { + it('should return isEstimatedReturnLow=true return value is 50% less than sent funds', () => { const state = createBridgeMockStore({ featureFlagOverrides: { extensionConfig: { @@ -1228,7 +1228,7 @@ describe('Bridge selectors', () => { toToken: { address: zeroAddress(), symbol: 'TEST' }, fromTokenInputValue: '1', fromTokenExchangeRate: 2524.25, - toTokenExchangeRate: 0.798781, + toTokenExchangeRate: 0.61, }, bridgeStateOverrides: { quotes: mockBridgeQuotesNativeErc20, @@ -1264,11 +1264,11 @@ describe('Bridge selectors', () => { expect( getBridgeQuotes(state as never).activeQuote?.adjustedReturn .valueInCurrency, - ).toStrictEqual(new BigNumber('16.99676538473491988')); + ).toStrictEqual(new BigNumber('12.38316502627291988')); expect(result.isEstimatedReturnLow).toStrictEqual(true); }); - it('should return isEstimatedReturnLow=false when return value is more than 80% of sent funds', () => { + it('should return isEstimatedReturnLow=false when return value is more than 50% of sent funds', () => { const state = createBridgeMockStore({ featureFlagOverrides: { extensionConfig: { @@ -1283,7 +1283,8 @@ describe('Bridge selectors', () => { fromToken: { address: zeroAddress(), symbol: 'ETH' }, toToken: { address: zeroAddress(), symbol: 'TEST' }, fromTokenExchangeRate: 2524.25, - toTokenExchangeRate: 0.998781, + toTokenExchangeRate: 0.63, + fromTokenInputValue: 1, }, bridgeStateOverrides: { quotes: mockBridgeQuotesNativeErc20, @@ -1320,7 +1321,7 @@ describe('Bridge selectors', () => { expect( getBridgeQuotes(state as never).activeQuote?.adjustedReturn .valueInCurrency, - ).toStrictEqual(new BigNumber('21.88454578473491988')); + ).toStrictEqual(new BigNumber('12.87194306627291988')); expect(result.isEstimatedReturnLow).toStrictEqual(false); }); diff --git a/ui/pages/bridge/prepare/bridge-input-group.tsx b/ui/pages/bridge/prepare/bridge-input-group.tsx index f2ec234263e7..ac545fb834dd 100644 --- a/ui/pages/bridge/prepare/bridge-input-group.tsx +++ b/ui/pages/bridge/prepare/bridge-input-group.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useRef } from 'react'; import { useSelector } from 'react-redux'; import { BigNumber } from 'bignumber.js'; import { getAddress } from 'ethers/lib/utils'; @@ -7,7 +7,6 @@ import { TextField, TextFieldType, ButtonLink, - PopoverPosition, Button, ButtonSize, } from '../../../components/component-library'; @@ -17,7 +16,7 @@ import { useI18nContext } from '../../../hooks/useI18nContext'; import { getLocale } from '../../../selectors'; import { getCurrentCurrency } from '../../../ducks/metamask/metamask'; import { formatCurrencyAmount, formatTokenAmount } from '../utils/quote'; -import { Column, Row, Tooltip } from '../layout'; +import { Column, Row } from '../layout'; import { Display, FontWeight, @@ -27,7 +26,6 @@ import { TextColor, } from '../../../helpers/constants/design-system'; import { AssetType } from '../../../../shared/constants/transaction'; -import { BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE } from '../../../../shared/constants/bridge'; import useLatestBalance from '../../../hooks/bridge/useLatestBalance'; import { getBridgeQuotes, @@ -87,8 +85,6 @@ export const BridgeInputGroup = ({ const inputRef = useRef(null); - const [isLowReturnTooltipOpen, setIsLowReturnTooltipOpen] = useState(true); - useEffect(() => { if (inputRef.current) { inputRef.current.value = amountFieldProps?.value?.toString() ?? ''; @@ -189,23 +185,6 @@ export const BridgeInputGroup = ({ - {isAmountReadOnly && - isEstimatedReturnLow && - isLowReturnTooltipOpen && ( - setIsLowReturnTooltipOpen(false)} - triggerElement={} - flip={false} - offset={[0, 80]} - > - {t('lowEstimatedReturnTooltipMessage', [ - BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE * 100, - ])} - - )} ( ); const mockFeatureFlags = { - srcNetworkAllowlist: [CHAIN_IDS.MAINNET, CHAIN_IDS.LINEA_MAINNET], - destNetworkAllowlist: [CHAIN_IDS.MAINNET, CHAIN_IDS.LINEA_MAINNET], extensionSupport: true, extensionConfig: { refreshRate: 30000, maxRefreshCount: 5, + support: true, + chains: { + '0x1': { isActiveSrc: true, isActiveDest: true }, + '0xa': { isActiveSrc: true, isActiveDest: true }, + }, }, }; const mockBridgeSlice = { diff --git a/ui/pages/bridge/prepare/prepare-bridge-page.tsx b/ui/pages/bridge/prepare/prepare-bridge-page.tsx index e1f220fc69fa..ced60d0c6039 100644 --- a/ui/pages/bridge/prepare/prepare-bridge-page.tsx +++ b/ui/pages/bridge/prepare/prepare-bridge-page.tsx @@ -91,6 +91,7 @@ import { useBridgeTokens } from '../../../hooks/bridge/useBridgeTokens'; import { getCurrentKeyring, getLocale } from '../../../selectors'; import { isHardwareKeyring } from '../../../helpers/utils/hardware'; import { SECOND } from '../../../../shared/constants/time'; +import { BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE } from '../../../../shared/constants/bridge'; import { BridgeInputGroup } from './bridge-input-group'; import { BridgeCTAButton } from './bridge-cta-button'; @@ -151,10 +152,12 @@ const PrepareBridgePage = () => { const ticker = useSelector(getNativeCurrency); const { + isEstimatedReturnLow, isNoQuotesAvailable, isInsufficientGasForQuote, isInsufficientBalance, } = useSelector(getValidationErrors); + const { quotesRefreshCount } = useSelector(getBridgeQuotes); const { openBuyCryptoInPdapp } = useRamps(); const { balanceAmount: nativeAssetBalance } = useLatestBalance( @@ -190,6 +193,10 @@ const PrepareBridgePage = () => { const [rotateSwitchTokens, setRotateSwitchTokens] = useState(false); + // Resets the banner visibility when the estimated return is low + const [isLowReturnBannerOpen, setIsLowReturnBannerOpen] = useState(true); + useEffect(() => setIsLowReturnBannerOpen(true), [quotesRefreshCount]); + // Background updates are debounced when the switch button is clicked // To prevent putting the frontend in an unexpected state, prevent the user // from switching tokens within the debounce period @@ -211,16 +218,27 @@ const PrepareBridgePage = () => { dispatch(resetBridgeState()); }, []); - const scrollRef = useRef(null); - + // Scroll to bottom of the page when banners are shown + const insufficientBalanceBannerRef = useRef(null); + const isEstimatedReturnLowRef = useRef(null); useEffect(() => { if (isInsufficientGasForQuote(nativeAssetBalance)) { - scrollRef.current?.scrollIntoView({ + insufficientBalanceBannerRef.current?.scrollIntoView({ + behavior: 'smooth', + block: 'start', + }); + } + if (isEstimatedReturnLow) { + isEstimatedReturnLowRef.current?.scrollIntoView({ behavior: 'smooth', block: 'start', }); } - }, [isInsufficientGasForQuote(nativeAssetBalance)]); + }, [ + isEstimatedReturnLow, + isInsufficientGasForQuote(nativeAssetBalance), + isLowReturnBannerOpen, + ]); const quoteParams = useMemo( () => ({ @@ -603,12 +621,26 @@ const PrepareBridgePage = () => { textAlign={TextAlign.Left} /> )} + {isEstimatedReturnLow && isLowReturnBannerOpen && ( + setIsLowReturnBannerOpen(false)} + /> + )} {!isLoading && activeQuote && !isInsufficientBalance(srcTokenBalance) && isInsufficientGasForQuote(nativeAssetBalance) && (

Network fees

@@ -106,19 +107,10 @@ exports[`BridgeQuoteCard should render the recommended quote 1`] = ` class="mm-box mm-container mm-container--max-width-undefined mm-box--display-flex mm-box--gap-1 mm-box--flex-direction-row mm-box--flex-wrap-nowrap mm-box--justify-content-space-between mm-box--align-items-center" >

- $2.52 -

-

- - -

-

- $2.52 + $2.52 - $2.52

Network fees

@@ -270,19 +263,10 @@ exports[`BridgeQuoteCard should render the recommended quote while loading new q class="mm-box mm-container mm-container--max-width-undefined mm-box--display-flex mm-box--gap-1 mm-box--flex-direction-row mm-box--flex-wrap-nowrap mm-box--justify-content-space-between mm-box--align-items-center" >

- $2.52 -

-

- - -

-

- $2.52 + $2.52 - $2.52