diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index a0240346af64..77958f69da2d 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -10,6 +10,7 @@ on: - opened - reopened - synchronize + merge_group: jobs: test-unit: diff --git a/.github/workflows/update-lavamoat-policies.yml b/.github/workflows/update-lavamoat-policies.yml index 1baef7fb4460..c8f9c190e533 100644 --- a/.github/workflows/update-lavamoat-policies.yml +++ b/.github/workflows/update-lavamoat-policies.yml @@ -201,7 +201,7 @@ jobs: run: | if [[ $HAS_CHANGES == 'true' ]] then - gh pr comment "${PR_NUMBER}" --body 'Policies updated' + echo -e 'Policies updated. \nšŸ‘€ Please review the diff for suspicious new powers. \n\nšŸ§  Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff' | gh pr comment "${PR_NUMBER}" --body-file - else gh pr comment "${PR_NUMBER}" --body 'No policy changes' fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 199b83700dde..9dd79055e0c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [12.3.1] +### Fixed +- Fix duplicate network validation ([#27463](https://github.com/MetaMask/metamask-extension/pull/27463)) +- Fix notification metrics ([#27435](https://github.com/MetaMask/metamask-extension/pull/27435)) +- Fix transaction metrics ([#27457](https://github.com/MetaMask/metamask-extension/pull/27457)) + ## [12.3.0] ### Added - Added the ability to name accounts during the snap account creation flow ([#25191](https://github.com/MetaMask/metamask-extension/pull/25191)) @@ -5106,7 +5112,8 @@ Update styles and spacing on the critical error page ([#20350](https://github.c - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v12.3.0...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v12.3.1...HEAD +[12.3.1]: https://github.com/MetaMask/metamask-extension/compare/v12.3.0...v12.3.1 [12.3.0]: https://github.com/MetaMask/metamask-extension/compare/v12.2.4...v12.3.0 [12.2.4]: https://github.com/MetaMask/metamask-extension/compare/v12.2.3...v12.2.4 [12.2.3]: https://github.com/MetaMask/metamask-extension/compare/v12.2.2...v12.2.3 diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index b42895983048..42a5a108fdf1 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1186,9 +1186,17 @@ "message": "Connected with" }, "connectedWithAccount": { + "message": "$1 accounts connected", + "description": "$1 represents account length" + }, + "connectedWithAccountName": { "message": "Connected with $1", "description": "$1 represents account name" }, + "connectedWithNetworks": { + "message": "$1 networks connected", + "description": "$1 represents network length" + }, "connecting": { "message": "Connecting" }, @@ -2113,6 +2121,9 @@ "message": "This gas fee has been suggested by $1. Overriding this may cause a problem with your transaction. Please reach out to $1 if you have questions.", "description": "$1 represents the Dapp's origin" }, + "gasFee": { + "message": "Gas fee" + }, "gasIsETH": { "message": "Gas is $1 " }, @@ -2427,6 +2438,9 @@ "inYourSettings": { "message": "in your Settings" }, + "included": { + "message": "included" + }, "infuraBlockedNotification": { "message": "MetaMask is unable to connect to the blockchain host. Review possible reasons $1.", "description": "$1 is a clickable link with with text defined by the 'here' key" @@ -4544,6 +4558,9 @@ "revealTheSeedPhrase": { "message": "Reveal seed phrase" }, + "review": { + "message": "Review" + }, "reviewAlert": { "message": "Review alert" }, @@ -5657,12 +5674,22 @@ "message": "Gas fees are paid to crypto miners who process transactions on the $1 network. MetaMask does not profit from gas fees.", "description": "$1 is the selected network, e.g. Ethereum or BSC" }, + "swapGasIncludedTooltipExplanation": { + "message": "This quote incorporates gas fees by adjusting the token amount sent or received. You may receive ETH in a separate transaction on your activity list." + }, + "swapGasIncludedTooltipExplanationLinkText": { + "message": "Learn more about gas fees" + }, "swapHighSlippage": { "message": "High slippage" }, "swapHighSlippageWarning": { "message": "Slippage amount is very high." }, + "swapIncludesGasAndMetaMaskFee": { + "message": "Includes gas and a $1% MetaMask fee", + "description": "Provides information about the fee that metamask takes for swaps. $1 is a decimal number." + }, "swapIncludesMMFee": { "message": "Includes a $1% MetaMask fee.", "description": "Provides information about the fee that metamask takes for swaps. $1 is a decimal number." diff --git a/app/scripts/controllers/mmi-controller.ts b/app/scripts/controllers/mmi-controller.ts index cbb08308ec59..0c43684d7f58 100644 --- a/app/scripts/controllers/mmi-controller.ts +++ b/app/scripts/controllers/mmi-controller.ts @@ -458,7 +458,7 @@ export default class MMIController extends EventEmitter { const allAccounts = await this.keyringController.getAccounts(); const accountsToTrack = [ - ...new Set( + ...new Set( oldAccounts.concat(allAccounts.map((a: string) => a.toLowerCase())), ), ]; diff --git a/app/scripts/controllers/swaps/swaps.test.ts b/app/scripts/controllers/swaps/swaps.test.ts index 3fa4f1ff9409..4ed1b545f170 100644 --- a/app/scripts/controllers/swaps/swaps.test.ts +++ b/app/scripts/controllers/swaps/swaps.test.ts @@ -26,6 +26,7 @@ const MOCK_FETCH_PARAMS: FetchTradesInfoParams = { fromAddress: '0x7F18BB4Dd92CF2404C54CBa1A9BE4A1153bdb078', exchangeList: 'zeroExV1', balanceError: false, + enableGasIncludedQuotes: false, }; const TEST_AGG_ID_1 = 'TEST_AGG_1'; @@ -1164,6 +1165,7 @@ describe('SwapsController', function () { fromAddress: '', exchangeList: 'zeroExV1', balanceError: false, + enableGasIncludedQuotes: false, metaData: {} as FetchTradesInfoParamsMetadata, }; const swapsFeatureIsLive = false; diff --git a/app/scripts/controllers/swaps/swaps.types.ts b/app/scripts/controllers/swaps/swaps.types.ts index 44e4d4939742..ca059723277a 100644 --- a/app/scripts/controllers/swaps/swaps.types.ts +++ b/app/scripts/controllers/swaps/swaps.types.ts @@ -308,6 +308,7 @@ export type FetchTradesInfoParams = { fromAddress: string; exchangeList: string; balanceError: boolean; + enableGasIncludedQuotes: boolean; }; export type FetchTradesInfoParamsMetadata = { diff --git a/app/scripts/lib/account-tracker.test.js b/app/scripts/lib/account-tracker.test.js deleted file mode 100644 index 4bd73a472811..000000000000 --- a/app/scripts/lib/account-tracker.test.js +++ /dev/null @@ -1,729 +0,0 @@ -import EventEmitter from 'events'; -import { ControllerMessenger } from '@metamask/base-controller'; - -import { flushPromises } from '../../../test/lib/timer-helpers'; -import { createTestProviderTools } from '../../../test/stub/provider'; -import AccountTracker from './account-tracker'; - -const noop = () => true; -const currentNetworkId = '5'; -const currentChainId = '0x5'; -const VALID_ADDRESS = '0x0000000000000000000000000000000000000000'; -const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001'; - -const SELECTED_ADDRESS = '0x123'; - -const INITIAL_BALANCE_1 = '0x1'; -const INITIAL_BALANCE_2 = '0x2'; -const UPDATE_BALANCE = '0xabc'; -const UPDATE_BALANCE_HOOK = '0xabcd'; - -const GAS_LIMIT = '0x111111'; -const GAS_LIMIT_HOOK = '0x222222'; - -// The below three values were generated by running MetaMask in the browser -// The response to eth_call, which is called via `ethContract.balances` -// in `_updateAccountsViaBalanceChecker` of account-tracker.js, needs to be properly -// formatted or else ethers will throw an error. -const ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN = - '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000038d7ea4c6800600000000000000000000000000000000000000000000000000000000000186a0'; -const EXPECTED_CONTRACT_BALANCE_1 = '0x038d7ea4c68006'; -const EXPECTED_CONTRACT_BALANCE_2 = '0x0186a0'; - -const mockAccounts = { - [VALID_ADDRESS]: { address: VALID_ADDRESS, balance: INITIAL_BALANCE_1 }, - [VALID_ADDRESS_TWO]: { - address: VALID_ADDRESS_TWO, - balance: INITIAL_BALANCE_2, - }, -}; - -function buildMockBlockTracker({ shouldStubListeners = true } = {}) { - const blockTrackerStub = new EventEmitter(); - blockTrackerStub.getCurrentBlock = noop; - blockTrackerStub.getLatestBlock = noop; - if (shouldStubListeners) { - jest.spyOn(blockTrackerStub, 'addListener').mockImplementation(); - jest.spyOn(blockTrackerStub, 'removeListener').mockImplementation(); - } - return blockTrackerStub; -} - -function buildAccountTracker({ - completedOnboarding = false, - useMultiAccountBalanceChecker = false, - ...accountTrackerOptions -} = {}) { - const { provider } = createTestProviderTools({ - scaffold: { - eth_getBalance: UPDATE_BALANCE, - eth_call: ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN, - eth_getBlockByNumber: { gasLimit: GAS_LIMIT }, - }, - networkId: currentNetworkId, - chainId: currentNetworkId, - }); - const blockTrackerStub = buildMockBlockTracker(); - - const providerFromHook = createTestProviderTools({ - scaffold: { - eth_getBalance: UPDATE_BALANCE_HOOK, - eth_call: ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN, - eth_getBlockByNumber: { gasLimit: GAS_LIMIT_HOOK }, - }, - networkId: '0x1', - chainId: '0x1', - }).provider; - - const blockTrackerFromHookStub = buildMockBlockTracker(); - - const getNetworkClientByIdStub = jest.fn().mockReturnValue({ - configuration: { - chainId: '0x1', - }, - blockTracker: blockTrackerFromHookStub, - provider: providerFromHook, - }); - - const controllerMessenger = new ControllerMessenger(); - controllerMessenger.registerActionHandler( - 'AccountsController:getSelectedAccount', - () => ({ - id: 'accountId', - address: SELECTED_ADDRESS, - }), - ); - - const accountTracker = new AccountTracker({ - provider, - blockTracker: blockTrackerStub, - getNetworkClientById: getNetworkClientByIdStub, - getNetworkIdentifier: jest.fn(), - preferencesController: { - store: { - getState: () => ({ - useMultiAccountBalanceChecker, - }), - subscribe: noop, - }, - }, - onboardingController: { - state: { - completedOnboarding, - }, - }, - controllerMessenger, - onAccountRemoved: noop, - getCurrentChainId: () => currentChainId, - ...accountTrackerOptions, - }); - - return { accountTracker, blockTrackerFromHookStub, blockTrackerStub }; -} - -describe('Account Tracker', () => { - afterEach(() => { - jest.resetAllMocks(); - }); - - describe('start', () => { - it('restarts the subscription to the block tracker and update accounts', async () => { - const { accountTracker, blockTrackerStub } = buildAccountTracker(); - const updateAccountsSpy = jest - .spyOn(accountTracker, 'updateAccounts') - .mockResolvedValue(); - - accountTracker.start(); - - expect(blockTrackerStub.removeListener).toHaveBeenNthCalledWith( - 1, - 'latest', - expect.any(Function), - ); - expect(blockTrackerStub.addListener).toHaveBeenNthCalledWith( - 1, - 'latest', - expect.any(Function), - ); - expect(updateAccountsSpy).toHaveBeenNthCalledWith(1); // called first time with no args - - accountTracker.start(); - - expect(blockTrackerStub.removeListener).toHaveBeenNthCalledWith( - 2, - 'latest', - expect.any(Function), - ); - expect(blockTrackerStub.addListener).toHaveBeenNthCalledWith( - 2, - 'latest', - expect.any(Function), - ); - expect(updateAccountsSpy).toHaveBeenNthCalledWith(2); // called second time with no args - - accountTracker.stop(); - }); - }); - - describe('stop', () => { - it('ends the subscription to the block tracker', async () => { - const { accountTracker, blockTrackerStub } = buildAccountTracker(); - - accountTracker.stop(); - - expect(blockTrackerStub.removeListener).toHaveBeenNthCalledWith( - 1, - 'latest', - expect.any(Function), - ); - }); - }); - - describe('startPollingByNetworkClientId', () => { - it('should subscribe to the block tracker and update accounts if not already using the networkClientId', async () => { - const { accountTracker, blockTrackerFromHookStub } = - buildAccountTracker(); - - const updateAccountsSpy = jest - .spyOn(accountTracker, 'updateAccounts') - .mockResolvedValue(); - - accountTracker.startPollingByNetworkClientId('mainnet'); - - expect(blockTrackerFromHookStub.addListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - expect(updateAccountsSpy).toHaveBeenCalledWith('mainnet'); - - accountTracker.startPollingByNetworkClientId('mainnet'); - - expect(blockTrackerFromHookStub.addListener).toHaveBeenCalledTimes(1); - expect(updateAccountsSpy).toHaveBeenCalledTimes(1); - - accountTracker.stopAllPolling(); - }); - - it('should subscribe to the block tracker and update accounts for each networkClientId', async () => { - const blockTrackerFromHookStub1 = buildMockBlockTracker(); - const blockTrackerFromHookStub2 = buildMockBlockTracker(); - const blockTrackerFromHookStub3 = buildMockBlockTracker(); - const getNetworkClientByIdStub = jest - .fn() - .mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - configuration: { - chainId: '0x1', - }, - blockTracker: blockTrackerFromHookStub1, - }; - case 'goerli': - return { - configuration: { - chainId: '0x5', - }, - blockTracker: blockTrackerFromHookStub2, - }; - case 'networkClientId1': - return { - configuration: { - chainId: '0xa', - }, - blockTracker: blockTrackerFromHookStub3, - }; - default: - throw new Error('unexpected networkClientId'); - } - }); - const { accountTracker } = buildAccountTracker({ - getNetworkClientById: getNetworkClientByIdStub, - }); - - const updateAccountsSpy = jest - .spyOn(accountTracker, 'updateAccounts') - .mockResolvedValue(); - - accountTracker.startPollingByNetworkClientId('mainnet'); - - expect(blockTrackerFromHookStub1.addListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - expect(updateAccountsSpy).toHaveBeenCalledWith('mainnet'); - - accountTracker.startPollingByNetworkClientId('goerli'); - - expect(blockTrackerFromHookStub2.addListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - expect(updateAccountsSpy).toHaveBeenCalledWith('goerli'); - - accountTracker.startPollingByNetworkClientId('networkClientId1'); - - expect(blockTrackerFromHookStub3.addListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - expect(updateAccountsSpy).toHaveBeenCalledWith('networkClientId1'); - - accountTracker.stopAllPolling(); - }); - }); - - describe('stopPollingByPollingToken', () => { - it('should unsubscribe from the block tracker when called with a valid polling that was the only active pollingToken for a given networkClient', async () => { - const { accountTracker, blockTrackerFromHookStub } = - buildAccountTracker(); - - jest.spyOn(accountTracker, 'updateAccounts').mockResolvedValue(); - - const pollingToken = - accountTracker.startPollingByNetworkClientId('mainnet'); - - accountTracker.stopPollingByPollingToken(pollingToken); - - expect(blockTrackerFromHookStub.removeListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - }); - - it('should not unsubscribe from the block tracker if called with one of multiple active polling tokens for a given networkClient', async () => { - const { accountTracker, blockTrackerFromHookStub } = - buildAccountTracker(); - - jest.spyOn(accountTracker, 'updateAccounts').mockResolvedValue(); - - const pollingToken1 = - accountTracker.startPollingByNetworkClientId('mainnet'); - accountTracker.startPollingByNetworkClientId('mainnet'); - - accountTracker.stopPollingByPollingToken(pollingToken1); - - expect(blockTrackerFromHookStub.removeListener).not.toHaveBeenCalled(); - - accountTracker.stopAllPolling(); - }); - - it('should error if no pollingToken is passed', () => { - const { accountTracker } = buildAccountTracker(); - - expect(() => { - accountTracker.stopPollingByPollingToken(undefined); - }).toThrow('pollingToken required'); - }); - - it('should error if no matching pollingToken is found', () => { - const { accountTracker } = buildAccountTracker(); - - expect(() => { - accountTracker.stopPollingByPollingToken('potato'); - }).toThrow('pollingToken not found'); - }); - }); - - describe('stopAll', () => { - it('should end all subscriptions', async () => { - const blockTrackerFromHookStub1 = buildMockBlockTracker(); - const blockTrackerFromHookStub2 = buildMockBlockTracker(); - const getNetworkClientByIdStub = jest - .fn() - .mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - configuration: { - chainId: '0x1', - }, - blockTracker: blockTrackerFromHookStub1, - }; - case 'goerli': - return { - configuration: { - chainId: '0x5', - }, - blockTracker: blockTrackerFromHookStub2, - }; - default: - throw new Error('unexpected networkClientId'); - } - }); - const { accountTracker, blockTrackerStub } = buildAccountTracker({ - getNetworkClientById: getNetworkClientByIdStub, - }); - - jest.spyOn(accountTracker, 'updateAccounts').mockResolvedValue(); - - accountTracker.startPollingByNetworkClientId('mainnet'); - - accountTracker.startPollingByNetworkClientId('goerli'); - - accountTracker.stopAllPolling(); - - expect(blockTrackerStub.removeListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - expect(blockTrackerFromHookStub1.removeListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - expect(blockTrackerFromHookStub2.removeListener).toHaveBeenCalledWith( - 'latest', - expect.any(Function), - ); - }); - }); - - describe('blockTracker "latest" events', () => { - it('updates currentBlockGasLimit, currentBlockGasLimitByChainId, and accounts when polling is initiated via `start`', async () => { - const blockTrackerStub = buildMockBlockTracker({ - shouldStubListeners: false, - }); - const { accountTracker } = buildAccountTracker({ - blockTracker: blockTrackerStub, - }); - - const updateAccountsSpy = jest - .spyOn(accountTracker, 'updateAccounts') - .mockResolvedValue(); - - accountTracker.start(); - blockTrackerStub.emit('latest', 'blockNumber'); - - await flushPromises(); - - expect(updateAccountsSpy).toHaveBeenCalledWith(null); - - const newState = accountTracker.store.getState(); - - expect(newState).toStrictEqual({ - accounts: {}, - accountsByChainId: {}, - currentBlockGasLimit: GAS_LIMIT, - currentBlockGasLimitByChainId: { - [currentChainId]: GAS_LIMIT, - }, - }); - - accountTracker.stop(); - }); - - it('updates only the currentBlockGasLimitByChainId and accounts when polling is initiated via `startPollingByNetworkClientId`', async () => { - const blockTrackerFromHookStub = buildMockBlockTracker({ - shouldStubListeners: false, - }); - const providerFromHook = createTestProviderTools({ - scaffold: { - eth_getBalance: UPDATE_BALANCE_HOOK, - eth_call: ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN, - eth_getBlockByNumber: { gasLimit: GAS_LIMIT_HOOK }, - }, - networkId: '0x1', - chainId: '0x1', - }).provider; - const getNetworkClientByIdStub = jest.fn().mockReturnValue({ - configuration: { - chainId: '0x1', - }, - blockTracker: blockTrackerFromHookStub, - provider: providerFromHook, - }); - const { accountTracker } = buildAccountTracker({ - getNetworkClientById: getNetworkClientByIdStub, - }); - - const updateAccountsSpy = jest - .spyOn(accountTracker, 'updateAccounts') - .mockResolvedValue(); - - accountTracker.startPollingByNetworkClientId('mainnet'); - - blockTrackerFromHookStub.emit('latest', 'blockNumber'); - - await flushPromises(); - - expect(updateAccountsSpy).toHaveBeenCalledWith('mainnet'); - - const newState = accountTracker.store.getState(); - - expect(newState).toStrictEqual({ - accounts: {}, - accountsByChainId: {}, - currentBlockGasLimit: '', - currentBlockGasLimitByChainId: { - '0x1': GAS_LIMIT_HOOK, - }, - }); - - accountTracker.stopAllPolling(); - }); - }); - - describe('updateAccountsAllActiveNetworks', () => { - it('updates accounts for the globally selected network and all currently polling networks', async () => { - const { accountTracker } = buildAccountTracker(); - - const updateAccountsSpy = jest - .spyOn(accountTracker, 'updateAccounts') - .mockResolvedValue(); - await accountTracker.startPollingByNetworkClientId('networkClientId1'); - await accountTracker.startPollingByNetworkClientId('networkClientId2'); - await accountTracker.startPollingByNetworkClientId('networkClientId3'); - - expect(updateAccountsSpy).toHaveBeenCalledTimes(3); - - await accountTracker.updateAccountsAllActiveNetworks(); - - expect(updateAccountsSpy).toHaveBeenCalledTimes(7); - expect(updateAccountsSpy).toHaveBeenNthCalledWith(4); // called with no args - expect(updateAccountsSpy).toHaveBeenNthCalledWith(5, 'networkClientId1'); - expect(updateAccountsSpy).toHaveBeenNthCalledWith(6, 'networkClientId2'); - expect(updateAccountsSpy).toHaveBeenNthCalledWith(7, 'networkClientId3'); - }); - }); - - describe('updateAccounts', () => { - it('does not update accounts if completedOnBoarding is false', async () => { - const { accountTracker } = buildAccountTracker({ - completedOnboarding: false, - }); - - await accountTracker.updateAccounts(); - - const state = accountTracker.store.getState(); - expect(state).toStrictEqual({ - accounts: {}, - currentBlockGasLimit: '', - accountsByChainId: {}, - currentBlockGasLimitByChainId: {}, - }); - }); - - describe('chain does not have single call balance address', () => { - const getCurrentChainIdStub = () => '0x999'; // chain without single call balance address - const mockAccountsWithSelectedAddress = { - ...mockAccounts, - [SELECTED_ADDRESS]: { - address: SELECTED_ADDRESS, - balance: '0x0', - }, - }; - const mockInitialState = { - accounts: mockAccountsWithSelectedAddress, - accountsByChainId: { - '0x999': mockAccountsWithSelectedAddress, - }, - }; - - describe('when useMultiAccountBalanceChecker is true', () => { - it('updates all accounts directly', async () => { - const { accountTracker } = buildAccountTracker({ - completedOnboarding: true, - useMultiAccountBalanceChecker: true, - getCurrentChainId: getCurrentChainIdStub, - }); - accountTracker.store.updateState(mockInitialState); - - await accountTracker.updateAccounts(); - - const accounts = { - [VALID_ADDRESS]: { - address: VALID_ADDRESS, - balance: UPDATE_BALANCE, - }, - [VALID_ADDRESS_TWO]: { - address: VALID_ADDRESS_TWO, - balance: UPDATE_BALANCE, - }, - [SELECTED_ADDRESS]: { - address: SELECTED_ADDRESS, - balance: UPDATE_BALANCE, - }, - }; - - const newState = accountTracker.store.getState(); - expect(newState).toStrictEqual({ - accounts, - accountsByChainId: { - '0x999': accounts, - }, - currentBlockGasLimit: '', - currentBlockGasLimitByChainId: {}, - }); - }); - }); - - describe('when useMultiAccountBalanceChecker is false', () => { - it('updates only the selectedAddress directly, setting other balances to null', async () => { - const { accountTracker } = buildAccountTracker({ - completedOnboarding: true, - useMultiAccountBalanceChecker: false, - getCurrentChainId: getCurrentChainIdStub, - }); - accountTracker.store.updateState(mockInitialState); - - await accountTracker.updateAccounts(); - - const accounts = { - [VALID_ADDRESS]: { address: VALID_ADDRESS, balance: null }, - [VALID_ADDRESS_TWO]: { address: VALID_ADDRESS_TWO, balance: null }, - [SELECTED_ADDRESS]: { - address: SELECTED_ADDRESS, - balance: UPDATE_BALANCE, - }, - }; - - const newState = accountTracker.store.getState(); - expect(newState).toStrictEqual({ - accounts, - accountsByChainId: { - '0x999': accounts, - }, - currentBlockGasLimit: '', - currentBlockGasLimitByChainId: {}, - }); - }); - }); - }); - - describe('chain does have single call balance address and network is not localhost', () => { - const getNetworkIdentifierStub = jest - .fn() - .mockReturnValue('http://not-localhost:8545'); - const controllerMessenger = new ControllerMessenger(); - controllerMessenger.registerActionHandler( - 'AccountsController:getSelectedAccount', - () => ({ - id: 'accountId', - address: VALID_ADDRESS, - }), - ); - const getCurrentChainIdStub = () => '0x1'; // chain with single call balance address - const mockInitialState = { - accounts: { ...mockAccounts }, - accountsByChainId: { - '0x1': { ...mockAccounts }, - }, - }; - - describe('when useMultiAccountBalanceChecker is true', () => { - it('updates all accounts via balance checker', async () => { - const { accountTracker } = buildAccountTracker({ - completedOnboarding: true, - useMultiAccountBalanceChecker: true, - controllerMessenger, - getNetworkIdentifier: getNetworkIdentifierStub, - getCurrentChainId: getCurrentChainIdStub, - }); - - accountTracker.store.updateState(mockInitialState); - - await accountTracker.updateAccounts('mainnet'); - - const accounts = { - [VALID_ADDRESS]: { - address: VALID_ADDRESS, - balance: EXPECTED_CONTRACT_BALANCE_1, - }, - [VALID_ADDRESS_TWO]: { - address: VALID_ADDRESS_TWO, - balance: EXPECTED_CONTRACT_BALANCE_2, - }, - }; - - const newState = accountTracker.store.getState(); - expect(newState).toStrictEqual({ - accounts, - accountsByChainId: { - '0x1': accounts, - }, - currentBlockGasLimit: '', - currentBlockGasLimitByChainId: {}, - }); - }); - }); - }); - }); - - describe('onAccountRemoved', () => { - it('should remove an account from state', () => { - let accountRemovedListener; - const { accountTracker } = buildAccountTracker({ - onAccountRemoved: (callback) => { - accountRemovedListener = callback; - }, - }); - accountTracker.store.updateState({ - accounts: { ...mockAccounts }, - accountsByChainId: { - [currentChainId]: { - ...mockAccounts, - }, - '0x1': { - ...mockAccounts, - }, - '0x2': { - ...mockAccounts, - }, - }, - }); - - accountRemovedListener(VALID_ADDRESS); - - const newState = accountTracker.store.getState(); - - const accounts = { - [VALID_ADDRESS_TWO]: mockAccounts[VALID_ADDRESS_TWO], - }; - - expect(newState).toStrictEqual({ - accounts, - accountsByChainId: { - [currentChainId]: accounts, - '0x1': accounts, - '0x2': accounts, - }, - currentBlockGasLimit: '', - currentBlockGasLimitByChainId: {}, - }); - }); - }); - - describe('clearAccounts', () => { - it('should reset state', () => { - const { accountTracker } = buildAccountTracker(); - - accountTracker.store.updateState({ - accounts: { ...mockAccounts }, - accountsByChainId: { - [currentChainId]: { - ...mockAccounts, - }, - '0x1': { - ...mockAccounts, - }, - '0x2': { - ...mockAccounts, - }, - }, - }); - - accountTracker.clearAccounts(); - - const newState = accountTracker.store.getState(); - - expect(newState).toStrictEqual({ - accounts: {}, - accountsByChainId: { - [currentChainId]: {}, - }, - currentBlockGasLimit: '', - currentBlockGasLimitByChainId: {}, - }); - }); - }); -}); diff --git a/app/scripts/lib/account-tracker.test.ts b/app/scripts/lib/account-tracker.test.ts new file mode 100644 index 000000000000..7cc0dcba14c7 --- /dev/null +++ b/app/scripts/lib/account-tracker.test.ts @@ -0,0 +1,805 @@ +import EventEmitter from 'events'; +import { ControllerMessenger } from '@metamask/base-controller'; +import { InternalAccount } from '@metamask/keyring-api'; +import { Hex } from '@metamask/utils'; +import { BlockTracker, Provider } from '@metamask/network-controller'; + +import { flushPromises } from '../../../test/lib/timer-helpers'; +import PreferencesController from '../controllers/preferences-controller'; +import OnboardingController from '../controllers/onboarding'; +import { createTestProviderTools } from '../../../test/stub/provider'; +import AccountTracker, { + AccountTrackerOptions, + AllowedActions, + AllowedEvents, + getDefaultAccountTrackerState, +} from './account-tracker'; + +const noop = () => true; +const currentNetworkId = '5'; +const currentChainId = '0x5'; +const VALID_ADDRESS = '0x0000000000000000000000000000000000000000'; +const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001'; + +const SELECTED_ADDRESS = '0x123'; + +const INITIAL_BALANCE_1 = '0x1'; +const INITIAL_BALANCE_2 = '0x2'; +const UPDATE_BALANCE = '0xabc'; +const UPDATE_BALANCE_HOOK = '0xabcd'; + +const GAS_LIMIT = '0x111111'; +const GAS_LIMIT_HOOK = '0x222222'; + +// The below three values were generated by running MetaMask in the browser +// The response to eth_call, which is called via `ethContract.balances` +// in `_updateAccountsViaBalanceChecker` of account-tracker.js, needs to be properly +// formatted or else ethers will throw an error. +const ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN = + '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000038d7ea4c6800600000000000000000000000000000000000000000000000000000000000186a0'; +const EXPECTED_CONTRACT_BALANCE_1 = '0x038d7ea4c68006'; +const EXPECTED_CONTRACT_BALANCE_2 = '0x0186a0'; + +const mockAccounts = { + [VALID_ADDRESS]: { address: VALID_ADDRESS, balance: INITIAL_BALANCE_1 }, + [VALID_ADDRESS_TWO]: { + address: VALID_ADDRESS_TWO, + balance: INITIAL_BALANCE_2, + }, +}; + +class MockBlockTracker extends EventEmitter { + getCurrentBlock = noop; + + getLatestBlock = noop; +} + +function buildMockBlockTracker({ shouldStubListeners = true } = {}) { + const blockTrackerStub = new MockBlockTracker(); + if (shouldStubListeners) { + jest.spyOn(blockTrackerStub, 'addListener').mockImplementation(); + jest.spyOn(blockTrackerStub, 'removeListener').mockImplementation(); + } + return blockTrackerStub; +} + +type WithControllerOptions = { + completedOnboarding?: boolean; + useMultiAccountBalanceChecker?: boolean; + getNetworkClientById?: jest.Mock; + getSelectedAccount?: jest.Mock; +} & Partial; + +type WithControllerCallback = ({ + controller, + blockTrackerFromHookStub, + blockTrackerStub, + triggerOnAccountRemoved, +}: { + controller: AccountTracker; + blockTrackerFromHookStub: MockBlockTracker; + blockTrackerStub: MockBlockTracker; + triggerOnAccountRemoved: (address: string) => void; +}) => ReturnValue; + +type WithControllerArgs = + | [WithControllerCallback] + | [WithControllerOptions, WithControllerCallback]; + +function withController( + ...args: WithControllerArgs +): ReturnValue { + const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]]; + const { + completedOnboarding = false, + useMultiAccountBalanceChecker = false, + getNetworkClientById, + getSelectedAccount, + ...accountTrackerOptions + } = rest; + const { provider } = createTestProviderTools({ + scaffold: { + eth_getBalance: UPDATE_BALANCE, + eth_call: ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN, + eth_getBlockByNumber: { gasLimit: GAS_LIMIT }, + }, + networkId: currentNetworkId, + chainId: currentNetworkId, + }); + const blockTrackerStub = buildMockBlockTracker(); + + const controllerMessenger = new ControllerMessenger< + AllowedActions, + AllowedEvents + >(); + const getSelectedAccountStub = () => + ({ + id: 'accountId', + address: SELECTED_ADDRESS, + } as InternalAccount); + controllerMessenger.registerActionHandler( + 'AccountsController:getSelectedAccount', + getSelectedAccount || getSelectedAccountStub, + ); + + const { provider: providerFromHook } = createTestProviderTools({ + scaffold: { + eth_getBalance: UPDATE_BALANCE_HOOK, + eth_call: ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN, + eth_getBlockByNumber: { gasLimit: GAS_LIMIT_HOOK }, + }, + networkId: '0x1', + chainId: '0x1', + }); + + const blockTrackerFromHookStub = buildMockBlockTracker(); + + const getNetworkClientByIdStub = jest.fn().mockReturnValue({ + configuration: { + chainId: '0x1', + }, + blockTracker: blockTrackerFromHookStub, + provider: providerFromHook, + }); + + controllerMessenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + getNetworkClientById || getNetworkClientByIdStub, + ); + + const controller = new AccountTracker({ + initState: getDefaultAccountTrackerState(), + provider: provider as Provider, + blockTracker: blockTrackerStub as unknown as BlockTracker, + getNetworkIdentifier: jest.fn(), + preferencesController: { + store: { + getState: () => ({ + useMultiAccountBalanceChecker, + }), + }, + } as PreferencesController, + onboardingController: { + state: { + completedOnboarding, + }, + } as OnboardingController, + controllerMessenger, + getCurrentChainId: () => currentChainId, + ...accountTrackerOptions, + }); + + return fn({ + controller, + blockTrackerFromHookStub, + blockTrackerStub, + triggerOnAccountRemoved: (address: string) => { + controllerMessenger.publish('KeyringController:accountRemoved', address); + }, + }); +} + +describe('Account Tracker', () => { + describe('start', () => { + it('restarts the subscription to the block tracker and update accounts', async () => { + withController(({ controller, blockTrackerStub }) => { + const updateAccountsSpy = jest + .spyOn(controller, 'updateAccounts') + .mockResolvedValue(); + + controller.start(); + + expect(blockTrackerStub.removeListener).toHaveBeenNthCalledWith( + 1, + 'latest', + expect.any(Function), + ); + expect(blockTrackerStub.addListener).toHaveBeenNthCalledWith( + 1, + 'latest', + expect.any(Function), + ); + expect(updateAccountsSpy).toHaveBeenNthCalledWith(1); // called first time with no args + + controller.start(); + + expect(blockTrackerStub.removeListener).toHaveBeenNthCalledWith( + 2, + 'latest', + expect.any(Function), + ); + expect(blockTrackerStub.addListener).toHaveBeenNthCalledWith( + 2, + 'latest', + expect.any(Function), + ); + expect(updateAccountsSpy).toHaveBeenNthCalledWith(2); // called second time with no args + + controller.stop(); + }); + }); + }); + + describe('stop', () => { + it('ends the subscription to the block tracker', async () => { + withController(({ controller, blockTrackerStub }) => { + controller.stop(); + + expect(blockTrackerStub.removeListener).toHaveBeenNthCalledWith( + 1, + 'latest', + expect.any(Function), + ); + }); + }); + }); + + describe('startPollingByNetworkClientId', () => { + it('should subscribe to the block tracker and update accounts if not already using the networkClientId', async () => { + withController(({ controller, blockTrackerFromHookStub }) => { + const updateAccountsSpy = jest + .spyOn(controller, 'updateAccounts') + .mockResolvedValue(); + + controller.startPollingByNetworkClientId('mainnet'); + + expect(blockTrackerFromHookStub.addListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + expect(updateAccountsSpy).toHaveBeenCalledWith('mainnet'); + + controller.startPollingByNetworkClientId('mainnet'); + + expect(blockTrackerFromHookStub.addListener).toHaveBeenCalledTimes(1); + expect(updateAccountsSpy).toHaveBeenCalledTimes(1); + + controller.stopAllPolling(); + }); + }); + + it('should subscribe to the block tracker and update accounts for each networkClientId', async () => { + const blockTrackerFromHookStub1 = buildMockBlockTracker(); + const blockTrackerFromHookStub2 = buildMockBlockTracker(); + const blockTrackerFromHookStub3 = buildMockBlockTracker(); + withController( + { + getNetworkClientById: jest + .fn() + .mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + configuration: { + chainId: '0x1', + }, + blockTracker: blockTrackerFromHookStub1, + }; + case 'goerli': + return { + configuration: { + chainId: '0x5', + }, + blockTracker: blockTrackerFromHookStub2, + }; + case 'networkClientId1': + return { + configuration: { + chainId: '0xa', + }, + blockTracker: blockTrackerFromHookStub3, + }; + default: + throw new Error('unexpected networkClientId'); + } + }), + }, + ({ controller }) => { + const updateAccountsSpy = jest + .spyOn(controller, 'updateAccounts') + .mockResolvedValue(); + + controller.startPollingByNetworkClientId('mainnet'); + + expect(blockTrackerFromHookStub1.addListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + expect(updateAccountsSpy).toHaveBeenCalledWith('mainnet'); + + controller.startPollingByNetworkClientId('goerli'); + + expect(blockTrackerFromHookStub2.addListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + expect(updateAccountsSpy).toHaveBeenCalledWith('goerli'); + + controller.startPollingByNetworkClientId('networkClientId1'); + + expect(blockTrackerFromHookStub3.addListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + expect(updateAccountsSpy).toHaveBeenCalledWith('networkClientId1'); + + controller.stopAllPolling(); + }, + ); + }); + }); + + describe('stopPollingByPollingToken', () => { + it('should unsubscribe from the block tracker when called with a valid polling that was the only active pollingToken for a given networkClient', async () => { + withController(({ controller, blockTrackerFromHookStub }) => { + jest.spyOn(controller, 'updateAccounts').mockResolvedValue(); + + const pollingToken = + controller.startPollingByNetworkClientId('mainnet'); + + controller.stopPollingByPollingToken(pollingToken); + + expect(blockTrackerFromHookStub.removeListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + }); + }); + + it('should not unsubscribe from the block tracker if called with one of multiple active polling tokens for a given networkClient', async () => { + withController(({ controller, blockTrackerFromHookStub }) => { + jest.spyOn(controller, 'updateAccounts').mockResolvedValue(); + + const pollingToken1 = + controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('mainnet'); + + controller.stopPollingByPollingToken(pollingToken1); + + expect(blockTrackerFromHookStub.removeListener).not.toHaveBeenCalled(); + + controller.stopAllPolling(); + }); + }); + + it('should error if no pollingToken is passed', () => { + withController(({ controller }) => { + expect(() => { + controller.stopPollingByPollingToken(undefined); + }).toThrow('pollingToken required'); + }); + }); + + it('should error if no matching pollingToken is found', () => { + withController(({ controller }) => { + expect(() => { + controller.stopPollingByPollingToken('potato'); + }).toThrow('pollingToken not found'); + }); + }); + }); + + describe('stopAll', () => { + it('should end all subscriptions', async () => { + const blockTrackerFromHookStub1 = buildMockBlockTracker(); + const blockTrackerFromHookStub2 = buildMockBlockTracker(); + const getNetworkClientByIdStub = jest + .fn() + .mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + configuration: { + chainId: '0x1', + }, + blockTracker: blockTrackerFromHookStub1, + }; + case 'goerli': + return { + configuration: { + chainId: '0x5', + }, + blockTracker: blockTrackerFromHookStub2, + }; + default: + throw new Error('unexpected networkClientId'); + } + }); + withController( + { + getNetworkClientById: getNetworkClientByIdStub, + }, + ({ controller, blockTrackerStub }) => { + jest.spyOn(controller, 'updateAccounts').mockResolvedValue(); + + controller.startPollingByNetworkClientId('mainnet'); + + controller.startPollingByNetworkClientId('goerli'); + + controller.stopAllPolling(); + + expect(blockTrackerStub.removeListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + expect(blockTrackerFromHookStub1.removeListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + expect(blockTrackerFromHookStub2.removeListener).toHaveBeenCalledWith( + 'latest', + expect.any(Function), + ); + }, + ); + }); + }); + + describe('blockTracker "latest" events', () => { + it('updates currentBlockGasLimit, currentBlockGasLimitByChainId, and accounts when polling is initiated via `start`', async () => { + const blockTrackerStub = buildMockBlockTracker({ + shouldStubListeners: false, + }); + withController( + { + blockTracker: blockTrackerStub as unknown as BlockTracker, + }, + async ({ controller }) => { + const updateAccountsSpy = jest + .spyOn(controller, 'updateAccounts') + .mockResolvedValue(); + + controller.start(); + blockTrackerStub.emit('latest', 'blockNumber'); + + await flushPromises(); + + expect(updateAccountsSpy).toHaveBeenCalledWith(undefined); + + const newState = controller.store.getState(); + + expect(newState).toStrictEqual({ + accounts: {}, + accountsByChainId: {}, + currentBlockGasLimit: GAS_LIMIT, + currentBlockGasLimitByChainId: { + [currentChainId]: GAS_LIMIT, + }, + }); + + controller.stop(); + }, + ); + }); + + it('updates only the currentBlockGasLimitByChainId and accounts when polling is initiated via `startPollingByNetworkClientId`', async () => { + const blockTrackerFromHookStub = buildMockBlockTracker({ + shouldStubListeners: false, + }); + const providerFromHook = createTestProviderTools({ + scaffold: { + eth_getBalance: UPDATE_BALANCE_HOOK, + eth_call: ETHERS_CONTRACT_BALANCES_ETH_CALL_RETURN, + eth_getBlockByNumber: { gasLimit: GAS_LIMIT_HOOK }, + }, + networkId: '0x1', + chainId: '0x1', + }).provider; + const getNetworkClientByIdStub = jest.fn().mockReturnValue({ + configuration: { + chainId: '0x1', + }, + blockTracker: blockTrackerFromHookStub, + provider: providerFromHook, + }); + withController( + { + getNetworkClientById: getNetworkClientByIdStub, + }, + async ({ controller }) => { + const updateAccountsSpy = jest + .spyOn(controller, 'updateAccounts') + .mockResolvedValue(); + + controller.startPollingByNetworkClientId('mainnet'); + + blockTrackerFromHookStub.emit('latest', 'blockNumber'); + + await flushPromises(); + + expect(updateAccountsSpy).toHaveBeenCalledWith('mainnet'); + + const newState = controller.store.getState(); + + expect(newState).toStrictEqual({ + accounts: {}, + accountsByChainId: {}, + currentBlockGasLimit: '', + currentBlockGasLimitByChainId: { + '0x1': GAS_LIMIT_HOOK, + }, + }); + + controller.stopAllPolling(); + }, + ); + }); + }); + + describe('updateAccountsAllActiveNetworks', () => { + it('updates accounts for the globally selected network and all currently polling networks', async () => { + withController(async ({ controller }) => { + const updateAccountsSpy = jest + .spyOn(controller, 'updateAccounts') + .mockResolvedValue(); + await controller.startPollingByNetworkClientId('networkClientId1'); + await controller.startPollingByNetworkClientId('networkClientId2'); + await controller.startPollingByNetworkClientId('networkClientId3'); + + expect(updateAccountsSpy).toHaveBeenCalledTimes(3); + + await controller.updateAccountsAllActiveNetworks(); + + expect(updateAccountsSpy).toHaveBeenCalledTimes(7); + expect(updateAccountsSpy).toHaveBeenNthCalledWith(4); // called with no args + expect(updateAccountsSpy).toHaveBeenNthCalledWith( + 5, + 'networkClientId1', + ); + expect(updateAccountsSpy).toHaveBeenNthCalledWith( + 6, + 'networkClientId2', + ); + expect(updateAccountsSpy).toHaveBeenNthCalledWith( + 7, + 'networkClientId3', + ); + }); + }); + }); + + describe('updateAccounts', () => { + it('does not update accounts if completedOnBoarding is false', async () => { + withController( + { + completedOnboarding: false, + }, + async ({ controller }) => { + await controller.updateAccounts(); + + const state = controller.store.getState(); + expect(state).toStrictEqual({ + accounts: {}, + currentBlockGasLimit: '', + accountsByChainId: {}, + currentBlockGasLimitByChainId: {}, + }); + }, + ); + }); + + describe('chain does not have single call balance address', () => { + const getCurrentChainIdStub: () => Hex = () => '0x999'; // chain without single call balance address + const mockAccountsWithSelectedAddress = { + ...mockAccounts, + [SELECTED_ADDRESS]: { + address: SELECTED_ADDRESS, + balance: '0x0', + }, + }; + const mockInitialState = { + accounts: mockAccountsWithSelectedAddress, + accountsByChainId: { + '0x999': mockAccountsWithSelectedAddress, + }, + }; + + describe('when useMultiAccountBalanceChecker is true', () => { + it('updates all accounts directly', async () => { + withController( + { + completedOnboarding: true, + useMultiAccountBalanceChecker: true, + getCurrentChainId: getCurrentChainIdStub, + }, + async ({ controller }) => { + controller.store.updateState(mockInitialState); + + await controller.updateAccounts(); + + const accounts = { + [VALID_ADDRESS]: { + address: VALID_ADDRESS, + balance: UPDATE_BALANCE, + }, + [VALID_ADDRESS_TWO]: { + address: VALID_ADDRESS_TWO, + balance: UPDATE_BALANCE, + }, + [SELECTED_ADDRESS]: { + address: SELECTED_ADDRESS, + balance: UPDATE_BALANCE, + }, + }; + + const newState = controller.store.getState(); + expect(newState).toStrictEqual({ + accounts, + accountsByChainId: { + '0x999': accounts, + }, + currentBlockGasLimit: '', + currentBlockGasLimitByChainId: {}, + }); + }, + ); + }); + }); + + describe('when useMultiAccountBalanceChecker is false', () => { + it('updates only the selectedAddress directly, setting other balances to null', async () => { + withController( + { + completedOnboarding: true, + useMultiAccountBalanceChecker: false, + getCurrentChainId: getCurrentChainIdStub, + }, + async ({ controller }) => { + controller.store.updateState(mockInitialState); + + await controller.updateAccounts(); + + const accounts = { + [VALID_ADDRESS]: { address: VALID_ADDRESS, balance: null }, + [VALID_ADDRESS_TWO]: { + address: VALID_ADDRESS_TWO, + balance: null, + }, + [SELECTED_ADDRESS]: { + address: SELECTED_ADDRESS, + balance: UPDATE_BALANCE, + }, + }; + + const newState = controller.store.getState(); + expect(newState).toStrictEqual({ + accounts, + accountsByChainId: { + '0x999': accounts, + }, + currentBlockGasLimit: '', + currentBlockGasLimitByChainId: {}, + }); + }, + ); + }); + }); + }); + + describe('chain does have single call balance address and network is not localhost', () => { + describe('when useMultiAccountBalanceChecker is true', () => { + it('updates all accounts via balance checker', async () => { + withController( + { + completedOnboarding: true, + useMultiAccountBalanceChecker: true, + getNetworkIdentifier: jest + .fn() + .mockReturnValue('http://not-localhost:8545'), + getCurrentChainId: () => '0x1', // chain with single call balance address + getSelectedAccount: jest.fn().mockReturnValue({ + id: 'accountId', + address: VALID_ADDRESS, + } as InternalAccount), + }, + async ({ controller }) => { + controller.store.updateState({ + accounts: { ...mockAccounts }, + accountsByChainId: { + '0x1': { ...mockAccounts }, + }, + }); + + await controller.updateAccounts('mainnet'); + + const accounts = { + [VALID_ADDRESS]: { + address: VALID_ADDRESS, + balance: EXPECTED_CONTRACT_BALANCE_1, + }, + [VALID_ADDRESS_TWO]: { + address: VALID_ADDRESS_TWO, + balance: EXPECTED_CONTRACT_BALANCE_2, + }, + }; + + const newState = controller.store.getState(); + expect(newState).toStrictEqual({ + accounts, + accountsByChainId: { + '0x1': accounts, + }, + currentBlockGasLimit: '', + currentBlockGasLimitByChainId: {}, + }); + }, + ); + }); + }); + }); + }); + + describe('onAccountRemoved', () => { + it('should remove an account from state', () => { + withController(({ controller, triggerOnAccountRemoved }) => { + controller.store.updateState({ + accounts: { ...mockAccounts }, + accountsByChainId: { + [currentChainId]: { + ...mockAccounts, + }, + '0x1': { + ...mockAccounts, + }, + '0x2': { + ...mockAccounts, + }, + }, + }); + + triggerOnAccountRemoved(VALID_ADDRESS); + + const newState = controller.store.getState(); + + const accounts = { + [VALID_ADDRESS_TWO]: mockAccounts[VALID_ADDRESS_TWO], + }; + + expect(newState).toStrictEqual({ + accounts, + accountsByChainId: { + [currentChainId]: accounts, + '0x1': accounts, + '0x2': accounts, + }, + currentBlockGasLimit: '', + currentBlockGasLimitByChainId: {}, + }); + }); + }); + }); + + describe('clearAccounts', () => { + it('should reset state', () => { + withController(({ controller }) => { + controller.store.updateState({ + accounts: { ...mockAccounts }, + accountsByChainId: { + [currentChainId]: { + ...mockAccounts, + }, + '0x1': { + ...mockAccounts, + }, + '0x2': { + ...mockAccounts, + }, + }, + }); + + controller.clearAccounts(); + + const newState = controller.store.getState(); + + expect(newState).toStrictEqual({ + accounts: {}, + accountsByChainId: { + [currentChainId]: {}, + }, + currentBlockGasLimit: '', + currentBlockGasLimitByChainId: {}, + }); + }); + }); + }); +}); diff --git a/app/scripts/lib/account-tracker.js b/app/scripts/lib/account-tracker.ts similarity index 60% rename from app/scripts/lib/account-tracker.js rename to app/scripts/lib/account-tracker.ts index 6dbd13f1c2df..8ca119ccf83f 100644 --- a/app/scripts/lib/account-tracker.js +++ b/app/scripts/lib/account-tracker.ts @@ -17,52 +17,127 @@ import { Web3Provider } from '@ethersproject/providers'; import { Contract } from '@ethersproject/contracts'; import SINGLE_CALL_BALANCES_ABI from 'single-call-balance-checker-abi'; import { cloneDeep } from 'lodash'; +import { + BlockTracker, + NetworkClientConfiguration, + NetworkClientId, + NetworkControllerGetNetworkClientByIdAction, + Provider, +} from '@metamask/network-controller'; +import { hasProperty, Hex } from '@metamask/utils'; +import { ControllerMessenger } from '@metamask/base-controller'; +import { + AccountsControllerGetSelectedAccountAction, + AccountsControllerSelectedEvmAccountChangeEvent, +} from '@metamask/accounts-controller'; +import { KeyringControllerAccountRemovedEvent } from '@metamask/keyring-controller'; +import { InternalAccount } from '@metamask/keyring-api'; + +import OnboardingController, { + OnboardingControllerStateChangeEvent, +} from '../controllers/onboarding'; +import PreferencesController from '../controllers/preferences-controller'; import { LOCALHOST_RPC_URL } from '../../../shared/constants/network'; - import { SINGLE_CALL_BALANCES_ADDRESSES } from '../constants/contracts'; import { previousValueComparator } from './util'; +type Account = { + address: string; + balance: string | null; +}; + +export type AccountTrackerState = { + accounts: Record>; + currentBlockGasLimit: string; + accountsByChainId: Record; + currentBlockGasLimitByChainId: Record; +}; + +export const getDefaultAccountTrackerState = (): AccountTrackerState => ({ + accounts: {}, + currentBlockGasLimit: '', + accountsByChainId: {}, + currentBlockGasLimitByChainId: {}, +}); + +export type AllowedActions = + | AccountsControllerGetSelectedAccountAction + | NetworkControllerGetNetworkClientByIdAction; + +export type AllowedEvents = + | AccountsControllerSelectedEvmAccountChangeEvent + | KeyringControllerAccountRemovedEvent + | OnboardingControllerStateChangeEvent; + +export type AccountTrackerOptions = { + initState: Partial; + provider: Provider; + blockTracker: BlockTracker; + getCurrentChainId: () => Hex; + getNetworkIdentifier: (config?: NetworkClientConfiguration) => string; + preferencesController: PreferencesController; + onboardingController: OnboardingController; + controllerMessenger: ControllerMessenger; +}; + /** * This module is responsible for tracking any number of accounts and caching their current balances & transaction * counts. * * It also tracks transaction hashes, and checks their inclusion status on each new block. * - * @typedef {object} AccountTracker - * @property {object} store The stored object containing all accounts to track, as well as the current block's gas limit. - * @property {object} store.accounts The accounts currently stored in this AccountTracker - * @property {object} store.accountsByChainId The accounts currently stored in this AccountTracker keyed by chain id - * @property {string} store.currentBlockGasLimit A hex string indicating the gas limit of the current block - * @property {string} store.currentBlockGasLimitByChainId A hex string indicating the gas limit of the current block keyed by chain id + * AccountTracker + * + * @property store The stored object containing all accounts to track, as well as the current block's gas limit. + * @property store.accounts The accounts currently stored in this AccountTracker + * @property store.accountsByChainId The accounts currently stored in this AccountTracker keyed by chain id + * @property store.currentBlockGasLimit A hex string indicating the gas limit of the current block + * @property store.currentBlockGasLimitByChainId A hex string indicating the gas limit of the current block keyed by chain id */ export default class AccountTracker { /** - * @param {object} opts - Options for initializing the controller - * @param {object} opts.provider - An EIP-1193 provider instance that uses the current global network - * @param {object} opts.blockTracker - A block tracker, which emits events for each new block - * @param {Function} opts.getCurrentChainId - A function that returns the `chainId` for the current global network - * @param {Function} opts.getNetworkClientById - Gets the network client with the given id from the NetworkController. - * @param {Function} opts.getNetworkIdentifier - A function that returns the current network or passed nework configuration - * @param {Function} opts.onAccountRemoved - Allows subscribing to keyring controller accountRemoved event + * Observable store containing controller data. */ - #pollingTokenSets = new Map(); + store: ObservableStore; - #listeners = {}; + resetState: () => void; - #provider = null; + #pollingTokenSets = new Map>(); - #blockTracker = null; + #listeners: Record Promise> = + {}; - #currentBlockNumberByChainId = {}; + #provider: Provider; - constructor(opts = {}) { - const initState = { - accounts: {}, - currentBlockGasLimit: '', - accountsByChainId: {}, - currentBlockGasLimitByChainId: {}, - }; - this.store = new ObservableStore({ ...initState, ...opts.initState }); + #blockTracker: BlockTracker; + + #currentBlockNumberByChainId: Record = {}; + + #getCurrentChainId: AccountTrackerOptions['getCurrentChainId']; + + #getNetworkIdentifier: AccountTrackerOptions['getNetworkIdentifier']; + + #preferencesController: AccountTrackerOptions['preferencesController']; + + #onboardingController: AccountTrackerOptions['onboardingController']; + + #controllerMessenger: AccountTrackerOptions['controllerMessenger']; + + #selectedAccount: InternalAccount; + + /** + * @param opts - Options for initializing the controller + * @param opts.provider - An EIP-1193 provider instance that uses the current global network + * @param opts.blockTracker - A block tracker, which emits events for each new block + * @param opts.getCurrentChainId - A function that returns the `chainId` for the current global network + * @param opts.getNetworkIdentifier - A function that returns the current network or passed nework configuration + */ + constructor(opts: AccountTrackerOptions) { + const initState = getDefaultAccountTrackerState(); + this.store = new ObservableStore({ + ...initState, + ...opts.initState, + }); this.resetState = () => { this.store.updateState(initState); @@ -71,17 +146,19 @@ export default class AccountTracker { this.#provider = opts.provider; this.#blockTracker = opts.blockTracker; - this.getCurrentChainId = opts.getCurrentChainId; - this.getNetworkClientById = opts.getNetworkClientById; - this.getNetworkIdentifier = opts.getNetworkIdentifier; - this.preferencesController = opts.preferencesController; - this.onboardingController = opts.onboardingController; - this.controllerMessenger = opts.controllerMessenger; + this.#getCurrentChainId = opts.getCurrentChainId; + this.#getNetworkIdentifier = opts.getNetworkIdentifier; + this.#preferencesController = opts.preferencesController; + this.#onboardingController = opts.onboardingController; + this.#controllerMessenger = opts.controllerMessenger; // subscribe to account removal - opts.onAccountRemoved((address) => this.removeAccounts([address])); + this.#controllerMessenger.subscribe( + 'KeyringController:accountRemoved', + (address) => this.removeAccounts([address]), + ); - this.controllerMessenger.subscribe( + this.#controllerMessenger.subscribe( 'OnboardingController:stateChange', previousValueComparator((prevState, currState) => { const { completedOnboarding: prevCompletedOnboarding } = prevState; @@ -89,24 +166,25 @@ export default class AccountTracker { if (!prevCompletedOnboarding && currCompletedOnboarding) { this.updateAccountsAllActiveNetworks(); } - }, this.onboardingController.state), + return true; + }, this.#onboardingController.state), ); - this.selectedAccount = this.controllerMessenger.call( + this.#selectedAccount = this.#controllerMessenger.call( 'AccountsController:getSelectedAccount', ); - this.controllerMessenger.subscribe( + this.#controllerMessenger.subscribe( 'AccountsController:selectedEvmAccountChange', (newAccount) => { const { useMultiAccountBalanceChecker } = - this.preferencesController.store.getState(); + this.#preferencesController.store.getState(); if ( - this.selectedAccount.id !== newAccount.id && + this.#selectedAccount.id !== newAccount.id && !useMultiAccountBalanceChecker ) { - this.selectedAccount = newAccount; + this.#selectedAccount = newAccount; this.updateAccountsAllActiveNetworks(); } }, @@ -116,13 +194,14 @@ export default class AccountTracker { /** * Starts polling with global selected network */ - start() { + start(): void { // blockTracker.currentBlock may be null this.#currentBlockNumberByChainId = { - [this.getCurrentChainId()]: this.#blockTracker.getCurrentBlock(), + [this.#getCurrentChainId()]: this.#blockTracker.getCurrentBlock(), }; this.#blockTracker.once('latest', (blockNumber) => { - this.#currentBlockNumberByChainId[this.getCurrentChainId()] = blockNumber; + this.#currentBlockNumberByChainId[this.#getCurrentChainId()] = + blockNumber; }); // remove first to avoid double add @@ -136,7 +215,7 @@ export default class AccountTracker { /** * Stops polling with global selected network */ - stop() { + stop(): void { // remove listener this.#blockTracker.removeListener('latest', this.#updateForBlock); } @@ -148,22 +227,31 @@ export default class AccountTracker { * @param networkClientId - Optional networkClientId to fetch a network client with * @returns network client config */ - #getCorrectNetworkClient(networkClientId) { + #getCorrectNetworkClient(networkClientId?: NetworkClientId): { + chainId: Hex; + provider: Provider; + blockTracker: BlockTracker; + identifier: string; + } { if (networkClientId) { - const networkClient = this.getNetworkClientById(networkClientId); + const { configuration, provider, blockTracker } = + this.#controllerMessenger.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); return { - chainId: networkClient.configuration.chainId, - provider: networkClient.provider, - blockTracker: networkClient.blockTracker, - identifier: this.getNetworkIdentifier(networkClient.configuration), + chainId: configuration.chainId, + provider, + blockTracker, + identifier: this.#getNetworkIdentifier(configuration), }; } return { - chainId: this.getCurrentChainId(), + chainId: this.#getCurrentChainId(), provider: this.#provider, blockTracker: this.#blockTracker, - identifier: this.getNetworkIdentifier(), + identifier: this.#getNetworkIdentifier(), }; } @@ -173,14 +261,14 @@ export default class AccountTracker { * @param networkClientId - The networkClientId to start polling for * @returns pollingToken */ - startPollingByNetworkClientId(networkClientId) { + startPollingByNetworkClientId(networkClientId: NetworkClientId): string { const pollToken = random(); const pollingTokenSet = this.#pollingTokenSets.get(networkClientId); if (pollingTokenSet) { pollingTokenSet.add(pollToken); } else { - const set = new Set(); + const set = new Set(); set.add(pollToken); this.#pollingTokenSets.set(networkClientId, set); this.#subscribeWithNetworkClientId(networkClientId); @@ -191,7 +279,7 @@ export default class AccountTracker { /** * Stops polling for all networkClientIds */ - stopAllPolling() { + stopAllPolling(): void { this.stop(); this.#pollingTokenSets.forEach((tokenSet, _networkClientId) => { tokenSet.forEach((token) => { @@ -205,7 +293,7 @@ export default class AccountTracker { * * @param pollingToken - The polling token to stop polling for */ - stopPollingByPollingToken(pollingToken) { + stopPollingByPollingToken(pollingToken: string | undefined): void { if (!pollingToken) { throw new Error('pollingToken required'); } @@ -228,9 +316,9 @@ export default class AccountTracker { /** * Subscribes from the block tracker for the given networkClientId if not currently subscribed * - * @param {string} networkClientId - network client ID to fetch a block tracker with + * @param networkClientId - network client ID to fetch a block tracker with */ - #subscribeWithNetworkClientId(networkClientId) { + #subscribeWithNetworkClientId(networkClientId: NetworkClientId): void { if (this.#listeners[networkClientId]) { return; } @@ -249,9 +337,9 @@ export default class AccountTracker { /** * Unsubscribes from the block tracker for the given networkClientId if currently subscribed * - * @param {string} networkClientId - The network client ID to fetch a block tracker with + * @param networkClientId - The network client ID to fetch a block tracker with */ - #unsubscribeWithNetworkClientId(networkClientId) { + #unsubscribeWithNetworkClientId(networkClientId: NetworkClientId): void { if (!this.#listeners[networkClientId]) { return; } @@ -265,16 +353,15 @@ export default class AccountTracker { * Returns the accounts object for the chain ID, or initializes it from the globally selected * if it doesn't already exist. * - * @private - * @param {string} chainId - The chain ID + * @param chainId - The chain ID */ - #getAccountsForChainId(chainId) { + #getAccountsForChainId(chainId: Hex): AccountTrackerState['accounts'] { const { accounts, accountsByChainId } = this.store.getState(); if (accountsByChainId[chainId]) { return cloneDeep(accountsByChainId[chainId]); } - const newAccounts = {}; + const newAccounts: AccountTrackerState['accounts'] = {}; Object.keys(accounts).forEach((address) => { newAccounts[address] = {}; }); @@ -288,21 +375,21 @@ export default class AccountTracker { * Once this AccountTracker's accounts are up to date with those referenced by the passed addresses, each * of these accounts are given an updated balance via EthQuery. * - * @param {Array} addresses - The array of hex addresses for accounts with which this AccountTracker's accounts should be + * @param addresses - The array of hex addresses for accounts with which this AccountTracker's accounts should be * in sync */ - syncWithAddresses(addresses) { + syncWithAddresses(addresses: string[]): void { const { accounts } = this.store.getState(); const locals = Object.keys(accounts); - const accountsToAdd = []; + const accountsToAdd: string[] = []; addresses.forEach((upstream) => { if (!locals.includes(upstream)) { accountsToAdd.push(upstream); } }); - const accountsToRemove = []; + const accountsToRemove: string[] = []; locals.forEach((local) => { if (!addresses.includes(local)) { accountsToRemove.push(local); @@ -317,9 +404,9 @@ export default class AccountTracker { * Adds new addresses to track the balances of * given a balance as long this.#currentBlockNumberByChainId is defined for the chainId. * - * @param {Array} addresses - An array of hex addresses of new accounts to track + * @param addresses - An array of hex addresses of new accounts to track */ - addAccounts(addresses) { + addAccounts(addresses: string[]): void { const { accounts: _accounts, accountsByChainId: _accountsByChainId } = this.store.getState(); const accounts = cloneDeep(_accounts); @@ -338,7 +425,7 @@ export default class AccountTracker { this.store.updateState({ accounts, accountsByChainId }); // fetch balances for the accounts if there is block number ready - if (this.#currentBlockNumberByChainId[this.getCurrentChainId()]) { + if (this.#currentBlockNumberByChainId[this.#getCurrentChainId()]) { this.updateAccounts(); } this.#pollingTokenSets.forEach((_tokenSet, networkClientId) => { @@ -352,9 +439,9 @@ export default class AccountTracker { /** * Removes accounts from being tracked * - * @param {Array} addresses - An array of hex addresses to stop tracking. + * @param addresses - An array of hex addresses to stop tracking. */ - removeAccounts(addresses) { + removeAccounts(addresses: string[]): void { const { accounts: _accounts, accountsByChainId: _accountsByChainId } = this.store.getState(); const accounts = cloneDeep(_accounts); @@ -376,11 +463,11 @@ export default class AccountTracker { /** * Removes all addresses and associated balances */ - clearAccounts() { + clearAccounts(): void { this.store.updateState({ accounts: {}, accountsByChainId: { - [this.getCurrentChainId()]: {}, + [this.#getCurrentChainId()]: {}, }, }); } @@ -390,11 +477,11 @@ export default class AccountTracker { * each local account's balance via EthQuery * * @private - * @param {number} blockNumber - the block number to update to. + * @param blockNumber - the block number to update to. * @fires 'block' The updated state, if all account updates are successful */ - #updateForBlock = async (blockNumber) => { - await this.#updateForBlockByNetworkClientId(null, blockNumber); + #updateForBlock = async (blockNumber: string): Promise => { + await this.#updateForBlockByNetworkClientId(undefined, blockNumber); }; /** @@ -402,11 +489,14 @@ export default class AccountTracker { * via EthQuery * * @private - * @param {string} networkClientId - optional network client ID to use instead of the globally selected network. - * @param {number} blockNumber - the block number to update to. + * @param networkClientId - optional network client ID to use instead of the globally selected network. + * @param blockNumber - the block number to update to. * @fires 'block' The updated state, if all account updates are successful */ - async #updateForBlockByNetworkClientId(networkClientId, blockNumber) { + async #updateForBlockByNetworkClientId( + networkClientId: NetworkClientId | undefined, + blockNumber: string, + ): Promise { const { chainId, provider } = this.#getCorrectNetworkClient(networkClientId); this.#currentBlockNumberByChainId[chainId] = blockNumber; @@ -422,7 +512,7 @@ export default class AccountTracker { const currentBlockGasLimit = currentBlock.gasLimit; const { currentBlockGasLimitByChainId } = this.store.getState(); this.store.updateState({ - ...(chainId === this.getCurrentChainId() && { + ...(chainId === this.#getCurrentChainId() && { currentBlockGasLimit, }), currentBlockGasLimitByChainId: { @@ -442,9 +532,8 @@ export default class AccountTracker { * Updates accounts for the globally selected network * and all networks that are currently being polled. * - * @returns {Promise} after all account balances updated */ - async updateAccountsAllActiveNetworks() { + async updateAccountsAllActiveNetworks(): Promise { await this.updateAccounts(); await Promise.all( Array.from(this.#pollingTokenSets).map(([networkClientId]) => { @@ -457,11 +546,10 @@ export default class AccountTracker { * balanceChecker is deployed on main eth (test)nets and requires a single call * for all other networks, calls this.#updateAccount for each account in this.store * - * @param {string} networkClientId - optional network client ID to use instead of the globally selected network. - * @returns {Promise} after all account balances updated + * @param networkClientId - optional network client ID to use instead of the globally selected network. */ - async updateAccounts(networkClientId) { - const { completedOnboarding } = this.onboardingController.state; + async updateAccounts(networkClientId?: NetworkClientId): Promise { + const { completedOnboarding } = this.#onboardingController.state; if (!completedOnboarding) { return; } @@ -469,7 +557,7 @@ export default class AccountTracker { const { chainId, provider, identifier } = this.#getCorrectNetworkClient(networkClientId); const { useMultiAccountBalanceChecker } = - this.preferencesController.store.getState(); + this.#preferencesController.store.getState(); let addresses = []; if (useMultiAccountBalanceChecker) { @@ -477,7 +565,7 @@ export default class AccountTracker { addresses = Object.keys(accounts); } else { - const selectedAddress = this.controllerMessenger.call( + const selectedAddress = this.#controllerMessenger.call( 'AccountsController:getSelectedAccount', ).address; @@ -485,7 +573,10 @@ export default class AccountTracker { } const rpcUrl = 'http://127.0.0.1:8545'; - const singleCallBalancesAddress = SINGLE_CALL_BALANCES_ADDRESSES[chainId]; + const singleCallBalancesAddress = + SINGLE_CALL_BALANCES_ADDRESSES[ + chainId as keyof typeof SINGLE_CALL_BALANCES_ADDRESSES + ]; if ( identifier === LOCALHOST_RPC_URL || identifier === rpcUrl || @@ -510,15 +601,18 @@ export default class AccountTracker { * Updates the current balance of an account. * * @private - * @param {string} address - A hex address of a the account to be updated - * @param {object} provider - The provider instance to fetch the balance with - * @param {string} chainId - The chain ID to update in state - * @returns {Promise} after the account balance is updated + * @param address - A hex address of a the account to be updated + * @param provider - The provider instance to fetch the balance with + * @param chainId - The chain ID to update in state */ - async #updateAccount(address, provider, chainId) { + async #updateAccount( + address: string, + provider: Provider, + chainId: Hex, + ): Promise { const { useMultiAccountBalanceChecker } = - this.preferencesController.store.getState(); + this.#preferencesController.store.getState(); let balance = '0x0'; @@ -526,7 +620,16 @@ export default class AccountTracker { try { balance = await pify(new EthQuery(provider)).getBalance(address); } catch (error) { - if (error.data?.request?.method !== 'eth_getBalance') { + if ( + error && + typeof error === 'object' && + hasProperty(error, 'data') && + error.data && + hasProperty(error.data, 'request') && + error.data.request && + hasProperty(error.data.request, 'method') && + error.data.request.method !== 'eth_getBalance' + ) { throw error; } } @@ -556,7 +659,7 @@ export default class AccountTracker { const { accountsByChainId } = this.store.getState(); this.store.updateState({ - ...(chainId === this.getCurrentChainId() && { + ...(chainId === this.#getCurrentChainId() && { accounts: newAccounts, }), accountsByChainId: { @@ -570,18 +673,17 @@ export default class AccountTracker { * Updates current address balances from balanceChecker deployed contract instance * * @private - * @param {Array} addresses - A hex addresses of a the accounts to be updated - * @param {string} deployedContractAddress - The contract address to fetch balances with - * @param {object} provider - The provider instance to fetch the balance with - * @param {string} chainId - The chain ID to update in state - * @returns {Promise} after the account balance is updated + * @param addresses - A hex addresses of a the accounts to be updated + * @param deployedContractAddress - The contract address to fetch balances with + * @param provider - The provider instance to fetch the balance with + * @param chainId - The chain ID to update in state */ async #updateAccountsViaBalanceChecker( - addresses, - deployedContractAddress, - provider, - chainId, - ) { + addresses: string[], + deployedContractAddress: string, + provider: Provider, + chainId: Hex, + ): Promise { const ethContract = await new Contract( deployedContractAddress, SINGLE_CALL_BALANCES_ABI, @@ -593,7 +695,7 @@ export default class AccountTracker { const balances = await ethContract.balances(addresses, ethBalance); const accounts = this.#getAccountsForChainId(chainId); - const newAccounts = {}; + const newAccounts: AccountTrackerState['accounts'] = {}; Object.keys(accounts).forEach((address) => { if (!addresses.includes(address)) { newAccounts[address] = { address, balance: null }; @@ -606,7 +708,7 @@ export default class AccountTracker { const { accountsByChainId } = this.store.getState(); this.store.updateState({ - ...(chainId === this.getCurrentChainId() && { + ...(chainId === this.#getCurrentChainId() && { accounts: newAccounts, }), accountsByChainId: { diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index c424354bc984..14e3bc0934d8 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -134,7 +134,7 @@ function getTracesSampleRate(sentryTarget) { return 1.0; } - return 0.01; + return 0.02; } /** diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a838469f32fa..2e9abd901e23 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1672,17 +1672,14 @@ export default class MetamaskController extends EventEmitter { onboardingController: this.onboardingController, controllerMessenger: this.controllerMessenger.getRestricted({ name: 'AccountTracker', + allowedActions: ['AccountsController:getSelectedAccount'], allowedEvents: [ 'AccountsController:selectedEvmAccountChange', 'OnboardingController:stateChange', + 'KeyringController:accountRemoved', ], - allowedActions: ['AccountsController:getSelectedAccount'], }), initState: { accounts: {} }, - onAccountRemoved: this.controllerMessenger.subscribe.bind( - this.controllerMessenger, - 'KeyringController:accountRemoved', - ), }); // start and stop polling for balances based on activeControllerConnections diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 95835b028ee9..70552b0d32a7 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -2039,12 +2039,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 95835b028ee9..70552b0d32a7 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -2039,12 +2039,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 95835b028ee9..70552b0d32a7 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -2039,12 +2039,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 8c60013092de..2116f0a1a7c8 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -2131,12 +2131,10 @@ "URL": true, "URLSearchParams": true, "addEventListener": true, - "clearInterval": true, "console.error": true, "dispatchEvent": true, "fetch": true, "removeEventListener": true, - "setInterval": true, "setTimeout": true }, "packages": { diff --git a/package.json b/package.json index 86b0718ff463..238f1c6c0c95 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "12.3.0", + "version": "12.3.1", "private": true, "repository": { "type": "git", @@ -344,7 +344,7 @@ "@metamask/post-message-stream": "^8.0.0", "@metamask/ppom-validator": "0.34.0", "@metamask/preinstalled-example-snap": "^0.1.0", - "@metamask/profile-sync-controller": "^0.9.3", + "@metamask/profile-sync-controller": "^0.9.4", "@metamask/providers": "^14.0.2", "@metamask/queued-request-controller": "^2.0.0", "@metamask/rate-limit-controller": "^6.0.0", diff --git a/shared/lib/swaps-utils.js b/shared/lib/swaps-utils.js index d80d70902810..c51a3ac1198e 100644 --- a/shared/lib/swaps-utils.js +++ b/shared/lib/swaps-utils.js @@ -265,6 +265,7 @@ export async function fetchTradesInfo( value, fromAddress, exchangeList, + enableGasIncludedQuotes, }, { chainId }, ) { @@ -275,6 +276,7 @@ export async function fetchTradesInfo( slippage, timeout: SECOND * 10, walletAddress: fromAddress, + enableGasIncludedQuotes, }; if (exchangeList) { diff --git a/shared/lib/swaps-utils.test.js b/shared/lib/swaps-utils.test.js index 06080a8f55e7..891c1c5fb961 100644 --- a/shared/lib/swaps-utils.test.js +++ b/shared/lib/swaps-utils.test.js @@ -87,6 +87,7 @@ describe('Swaps Utils', () => { sourceDecimals: TOKENS[0].decimals, sourceTokenInfo: { ...TOKENS[0] }, destinationTokenInfo: { ...TOKENS[1] }, + enableGasIncludedQuotes: false, }, { chainId: CHAIN_IDS.MAINNET }, ); diff --git a/test/data/confirmations/contract-interaction.ts b/test/data/confirmations/contract-interaction.ts index 0556789ccdb2..507a27a48dc3 100644 --- a/test/data/confirmations/contract-interaction.ts +++ b/test/data/confirmations/contract-interaction.ts @@ -161,43 +161,3 @@ export const genUnapprovedContractInteractionConfirmation = ({ userFeeLevel: 'medium', verifiedOnBlockchain: false, } as SignatureRequestType); - -export const genUnapprovedApproveConfirmation = ({ - address = CONTRACT_INTERACTION_SENDER_ADDRESS, - chainId = CHAIN_ID, -}: { - address?: Hex; - chainId?: string; -} = {}) => ({ - ...genUnapprovedContractInteractionConfirmation({ chainId }), - txParams: { - from: address, - data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', - gas: '0x16a92', - to: '0x076146c765189d51be3160a2140cf80bfc73ad68', - value: '0x0', - maxFeePerGas: '0x5b06b0c0d', - maxPriorityFeePerGas: '0x59682f00', - }, - type: TransactionType.tokenMethodApprove, -}); - -export const genUnapprovedSetApprovalForAllConfirmation = ({ - address = CONTRACT_INTERACTION_SENDER_ADDRESS, - chainId = CHAIN_ID, -}: { - address?: Hex; - chainId?: string; -} = {}) => ({ - ...genUnapprovedContractInteractionConfirmation({ chainId }), - txParams: { - from: address, - data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', - gas: '0x16a92', - to: '0x076146c765189d51be3160a2140cf80bfc73ad68', - value: '0x0', - maxFeePerGas: '0x5b06b0c0d', - maxPriorityFeePerGas: '0x59682f00', - }, - type: TransactionType.tokenMethodSetApprovalForAll, -}); diff --git a/test/data/confirmations/helper.ts b/test/data/confirmations/helper.ts index 9eb8bb234768..6669c043d0ea 100644 --- a/test/data/confirmations/helper.ts +++ b/test/data/confirmations/helper.ts @@ -1,18 +1,17 @@ import { ApprovalType } from '@metamask/controller-utils'; import { merge } from 'lodash'; +import { CHAIN_IDS } from '../../../shared/constants/network'; import { Confirmation, SignatureRequestType, } from '../../../ui/pages/confirmations/types/confirm'; import mockState from '../mock-state.json'; -import { CHAIN_IDS } from '../../../shared/constants/network'; -import { - genUnapprovedApproveConfirmation, - genUnapprovedContractInteractionConfirmation, - genUnapprovedSetApprovalForAllConfirmation, -} from './contract-interaction'; +import { genUnapprovedContractInteractionConfirmation } from './contract-interaction'; import { unapprovedPersonalSignMsg } from './personal_sign'; +import { genUnapprovedSetApprovalForAllConfirmation } from './set-approval-for-all'; +import { genUnapprovedApproveConfirmation } from './token-approve'; +import { genUnapprovedTokenTransferConfirmation } from './token-transfer'; import { unapprovedTypedSignMsgV4 } from './typed_sign'; type RootState = { metamask: Record } & Record< @@ -183,3 +182,16 @@ export const getMockSetApprovalForAllConfirmState = () => { genUnapprovedSetApprovalForAllConfirmation({ chainId: '0x5' }), ); }; + +export const getMockTokenTransferConfirmState = ({ + isWalletInitiatedConfirmation = false, +}: { + isWalletInitiatedConfirmation?: boolean; +}) => { + return getMockConfirmStateForTransaction( + genUnapprovedTokenTransferConfirmation({ + chainId: '0x5', + isWalletInitiatedConfirmation, + }), + ); +}; diff --git a/test/data/confirmations/set-approval-for-all.ts b/test/data/confirmations/set-approval-for-all.ts new file mode 100644 index 000000000000..ca997f6212af --- /dev/null +++ b/test/data/confirmations/set-approval-for-all.ts @@ -0,0 +1,27 @@ +import { TransactionType } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import { + CHAIN_ID, + CONTRACT_INTERACTION_SENDER_ADDRESS, + genUnapprovedContractInteractionConfirmation, +} from './contract-interaction'; + +export const genUnapprovedSetApprovalForAllConfirmation = ({ + address = CONTRACT_INTERACTION_SENDER_ADDRESS, + chainId = CHAIN_ID, +}: { + address?: Hex; + chainId?: string; +} = {}) => ({ + ...genUnapprovedContractInteractionConfirmation({ chainId }), + txParams: { + from: address, + data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', + gas: '0x16a92', + to: '0x076146c765189d51be3160a2140cf80bfc73ad68', + value: '0x0', + maxFeePerGas: '0x5b06b0c0d', + maxPriorityFeePerGas: '0x59682f00', + }, + type: TransactionType.tokenMethodSetApprovalForAll, +}); diff --git a/test/data/confirmations/token-approve.ts b/test/data/confirmations/token-approve.ts new file mode 100644 index 000000000000..c77d59101a99 --- /dev/null +++ b/test/data/confirmations/token-approve.ts @@ -0,0 +1,27 @@ +import { TransactionType } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import { + CHAIN_ID, + CONTRACT_INTERACTION_SENDER_ADDRESS, + genUnapprovedContractInteractionConfirmation, +} from './contract-interaction'; + +export const genUnapprovedApproveConfirmation = ({ + address = CONTRACT_INTERACTION_SENDER_ADDRESS, + chainId = CHAIN_ID, +}: { + address?: Hex; + chainId?: string; +} = {}) => ({ + ...genUnapprovedContractInteractionConfirmation({ chainId }), + txParams: { + from: address, + data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', + gas: '0x16a92', + to: '0x076146c765189d51be3160a2140cf80bfc73ad68', + value: '0x0', + maxFeePerGas: '0x5b06b0c0d', + maxPriorityFeePerGas: '0x59682f00', + }, + type: TransactionType.tokenMethodApprove, +}); diff --git a/test/data/confirmations/token-transfer.ts b/test/data/confirmations/token-transfer.ts new file mode 100644 index 000000000000..22d0cb2d00b4 --- /dev/null +++ b/test/data/confirmations/token-transfer.ts @@ -0,0 +1,32 @@ +import { TransactionType } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import { + CHAIN_ID, + CONTRACT_INTERACTION_SENDER_ADDRESS, + genUnapprovedContractInteractionConfirmation, +} from './contract-interaction'; + +export const genUnapprovedTokenTransferConfirmation = ({ + address = CONTRACT_INTERACTION_SENDER_ADDRESS, + chainId = CHAIN_ID, + isWalletInitiatedConfirmation = false, +}: { + address?: Hex; + chainId?: string; + isWalletInitiatedConfirmation?: boolean; +} = {}) => ({ + ...genUnapprovedContractInteractionConfirmation({ chainId }), + txParams: { + from: address, + data: '0x095ea7b30000000000000000000000002e0d7e8c45221fca00d74a3609a0f7097035d09b0000000000000000000000000000000000000000000000000000000000000001', + gas: '0x16a92', + to: '0x076146c765189d51be3160a2140cf80bfc73ad68', + value: '0x0', + maxFeePerGas: '0x5b06b0c0d', + maxPriorityFeePerGas: '0x59682f00', + }, + type: TransactionType.tokenMethodTransfer, + origin: isWalletInitiatedConfirmation + ? 'metamask' + : 'https://metamask.github.io', +}); diff --git a/test/e2e/tests/confirmations/navigation.spec.ts b/test/e2e/tests/confirmations/navigation.spec.ts index 8d195656dc44..d6befdff0a26 100644 --- a/test/e2e/tests/confirmations/navigation.spec.ts +++ b/test/e2e/tests/confirmations/navigation.spec.ts @@ -1,12 +1,12 @@ import { strict as assert } from 'assert'; import { TransactionEnvelopeType } from '@metamask/transaction-controller'; import { Suite } from 'mocha'; +import { By } from 'selenium-webdriver'; import { DAPP_HOST_ADDRESS, - WINDOW_TITLES, openDapp, - regularDelayMs, unlockWallet, + WINDOW_TITLES, } from '../../helpers'; import { Driver } from '../../webdriver/driver'; import { withRedesignConfirmationFixtures } from './helpers'; @@ -98,7 +98,6 @@ describe('Navigation Signature - Different signature types', function (this: Sui await unlockWallet(driver); await openDapp(driver); await queueSignatures(driver); - await driver.delay(regularDelayMs); await driver.clickElement('[data-testid="confirm-nav__reject-all"]'); @@ -166,11 +165,13 @@ async function queueSignatures(driver: Driver) { await driver.waitUntilXWindowHandles(3); await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.findElement({ text: 'Reject all' }); + await driver.waitForSelector(By.xpath("//div[normalize-space(.)='1 of 2']")); await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); await driver.clickElement('#signTypedDataV4'); await driver.waitUntilXWindowHandles(3); await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); + await driver.waitForSelector(By.xpath("//div[normalize-space(.)='1 of 3']")); } async function queueSignaturesAndTransactions(driver: Driver) { diff --git a/test/jest/mock-store.js b/test/jest/mock-store.js index 625b6dcf6c83..736b9c4eb325 100644 --- a/test/jest/mock-store.js +++ b/test/jest/mock-store.js @@ -210,7 +210,7 @@ export const createSwapsMockStore = () => { }, ], useCurrencyRateCheck: true, - currentCurrency: 'ETH', + currentCurrency: 'usd', currencyRates: { ETH: { conversionRate: 1, @@ -469,6 +469,23 @@ export const createSwapsMockStore = () => { decimals: 18, }, fee: 1, + isGasIncludedTrade: false, + approvalTxFees: { + feeEstimate: 42000000000000, + fees: [ + { maxFeePerGas: 2310003200, maxPriorityFeePerGas: 513154852 }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, + tradeTxFees: { + feeEstimate: 42000000000000, + fees: [ + { maxFeePerGas: 2310003200, maxPriorityFeePerGas: 513154852 }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, }, TEST_AGG_2: { trade: { @@ -503,6 +520,36 @@ export const createSwapsMockStore = () => { decimals: 18, }, fee: 1, + isGasIncludedTrade: false, + approvalTxFees: { + feeEstimate: 42000000000000, + fees: [ + { maxFeePerGas: 2310003200, maxPriorityFeePerGas: 513154852 }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, + tradeTxFees: { + feeEstimate: 42000000000000, + fees: [ + { + maxFeePerGas: 2310003200, + maxPriorityFeePerGas: 513154852, + tokenFees: [ + { + token: { + address: '0x6b175474e89094c44da98b954eedeac495271d0f', + symbol: 'DAI', + decimals: 18, + }, + balanceNeededToken: '0x426dc933c2e5a', + }, + ], + }, + ], + gasLimit: 21000, + gasUsed: 21000, + }, }, }, fetchParams: { diff --git a/types/single-call-balance-checker-abi.d.ts b/types/single-call-balance-checker-abi.d.ts new file mode 100644 index 000000000000..ae42a6e98775 --- /dev/null +++ b/types/single-call-balance-checker-abi.d.ts @@ -0,0 +1,6 @@ +declare module 'single-call-balance-checker-abi' { + import { ContractInterface } from '@ethersproject/contracts'; + + const SINGLE_CALL_BALANCES_ABI: ContractInterface; + export default SINGLE_CALL_BALANCES_ABI; +} diff --git a/ui/components/app/permission-page-container/permission-page-container.component.js b/ui/components/app/permission-page-container/permission-page-container.component.js index da7719f6d4dc..f5f69da8f947 100644 --- a/ui/components/app/permission-page-container/permission-page-container.component.js +++ b/ui/components/app/permission-page-container/permission-page-container.component.js @@ -147,7 +147,7 @@ export default class PermissionPageContainer extends Component { ); const permittedChainsPermission = - _request.permissions[PermissionNames.permittedChains]; + _request.permissions?.[PermissionNames.permittedChains]; const approvedChainIds = permittedChainsPermission?.caveats.find( (caveat) => caveat.type === CaveatTypes.restrictNetworkSwitching, )?.value; @@ -155,8 +155,8 @@ export default class PermissionPageContainer extends Component { const request = { ..._request, permissions: { ..._request.permissions }, - ...(_request.permissions.eth_accounts && { approvedAccounts }), - ...(_request.permissions[PermissionNames.permittedChains] && { + ...(_request.permissions?.eth_accounts && { approvedAccounts }), + ...(_request.permissions?.[PermissionNames.permittedChains] && { approvedChainIds, }), }; diff --git a/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx b/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx index cedcc375c17e..08fef2f9a6b7 100644 --- a/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx +++ b/ui/components/app/snaps/snap-ui-button/snap-ui-button.tsx @@ -23,7 +23,7 @@ export const SnapUIButton: FunctionComponent< > = ({ name, children, - type, + type = ButtonType.Button, variant = 'primary', disabled = false, className = '', diff --git a/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx b/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx index c9000c13839b..f2cb85cc4ef0 100644 --- a/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx +++ b/ui/components/app/snaps/snap-ui-dropdown/snap-ui-dropdown.tsx @@ -34,7 +34,7 @@ export const SnapUIDropdown: FunctionComponent = ({ const [value, setValue] = useState(initialValue ?? ''); useEffect(() => { - if (initialValue) { + if (initialValue !== undefined && initialValue !== null) { setValue(initialValue); } }, [initialValue]); diff --git a/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx b/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx index b6f68c646ec5..51c1c9a54a7a 100644 --- a/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx +++ b/ui/components/app/snaps/snap-ui-input/snap-ui-input.tsx @@ -2,6 +2,7 @@ import React, { ChangeEvent, FunctionComponent, useEffect, + useRef, useState, } from 'react'; import { useSnapInterfaceContext } from '../../../../contexts/snaps'; @@ -15,26 +16,44 @@ export type SnapUIInputProps = { export const SnapUIInput: FunctionComponent< SnapUIInputProps & FormTextFieldProps<'div'> > = ({ name, form, ...props }) => { - const { handleInputChange, getValue } = useSnapInterfaceContext(); + const { handleInputChange, getValue, focusedInput, setCurrentFocusedInput } = + useSnapInterfaceContext(); + + const inputRef = useRef(null); const initialValue = getValue(name, form) as string; const [value, setValue] = useState(initialValue ?? ''); useEffect(() => { - if (initialValue) { + if (initialValue !== undefined && initialValue !== null) { setValue(initialValue); } }, [initialValue]); + /* + * Focus input if the last focused input was this input + * This avoids loosing the focus when the UI is re-rendered + */ + useEffect(() => { + if (inputRef.current && name === focusedInput) { + (inputRef.current.children[0] as HTMLInputElement).focus(); + } + }, [inputRef]); + const handleChange = (event: ChangeEvent) => { setValue(event.target.value); handleInputChange(name, event.target.value ?? null, form); }; + const handleFocus = () => setCurrentFocusedInput(name); + const handleBlur = () => setCurrentFocusedInput(null); + return ( = ({ const [isModalOpen, setIsModalOpen] = useState(false); useEffect(() => { - if (initialValue) { + if (initialValue !== undefined && initialValue !== null) { setSelectedOption(initialValue); } }, [initialValue]); diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap b/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap index 5dc31c8e210a..ae198ab79882 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/__snapshots__/site-cell-connection-list-item.test.js.snap @@ -3,7 +3,7 @@ exports[`SiteCellConnectionListItem renders correctly with required props 1`] = `

Title

@@ -27,7 +27,7 @@ exports[`SiteCellConnectionListItem renders correctly with required props 1`] = class="mm-box mm-box--display-flex mm-box--gap-1 mm-box--flex-direction-row mm-box--align-items-center" > Unconnected Message @@ -38,6 +38,7 @@ exports[`SiteCellConnectionListItem renders correctly with required props 1`] =
diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js index 85e50b0b0fed..be2aafb7257b 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.js @@ -15,10 +15,7 @@ import { AvatarIcon, AvatarIconSize, Box, - ButtonIcon, - ButtonIconSize, ButtonLink, - IconName, Text, } from '../../../../component-library'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; @@ -31,6 +28,8 @@ export const SiteCellConnectionListItem = ({ isConnectFlow, onClick, content, + paddingTopValue, + paddingBottomValue, }) => { const t = useI18nContext(); @@ -42,9 +41,10 @@ export const SiteCellConnectionListItem = ({ alignItems={AlignItems.baseline} width={BlockSize.Full} backgroundColor={BackgroundColor.backgroundDefault} - padding={4} gap={4} className="multichain-connection-list-item" + paddingTop={paddingTopValue} + paddingBottom={paddingBottomValue} > - + {title} {isConnectFlow ? unconnectedMessage : connectedMessage} @@ -79,17 +79,9 @@ export const SiteCellConnectionListItem = ({ {content} - {isConnectFlow ? ( - onClick()}>{t('edit')} - ) : ( - onClick()} - size={ButtonIconSize.Sm} - /> - )} + onClick()} data-testid="edit"> + {t('edit')} + ); }; @@ -109,6 +101,16 @@ SiteCellConnectionListItem.propTypes = { */ connectedMessage: PropTypes.string, + /** + * Padding Top Value to keep the padding between list items same + */ + paddingTopValue: PropTypes.number, + + /** + * Padding Bottom Value to keep the padding between list items same + */ + paddingBottomValue: PropTypes.number, + /** * The message that should be displayed when there are no connected accounts */ diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js index 613f07f348f3..954d1e2d1fc2 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell-connection-list-item.test.js @@ -36,4 +36,8 @@ describe('SiteCellConnectionListItem', () => { it('returns wallet icon correctly', () => { expect(getByText('Title')).toBeDefined(); }); + + it('returns edit button correctly', () => { + expect(getByTestId('edit')).toBeDefined(); + }); }); diff --git a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx index 2ed1fce8fddd..4bc42604adf3 100644 --- a/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx +++ b/ui/components/multichain/pages/review-permissions-page/site-cell/site-cell.tsx @@ -1,10 +1,15 @@ import React, { useState } from 'react'; import { Hex } from '@metamask/utils'; -import { BorderColor } from '../../../../../helpers/constants/design-system'; +import { + BackgroundColor, + BorderColor, + BorderRadius, +} from '../../../../../helpers/constants/design-system'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; import { AvatarAccount, AvatarAccountSize, + Box, IconName, } from '../../../../component-library'; import { EditAccountsModal, EditNetworksModal } from '../../..'; @@ -55,13 +60,15 @@ export const SiteCell: React.FC = ({ selectedChainIds.includes(chainId), ); + const selectedChainIdsLength = selectedChainIds.length; + // Determine the messages for connected and not connected states const accountMessageConnectedState = selectedAccounts.length === 1 - ? t('connectedWithAccount', [ + ? t('connectedWithAccountName', [ selectedAccounts[0].label || selectedAccounts[0].metadata.name, ]) - : t('connectedWith'); + : t('connectedWithAccount', [accounts.length]); const accountMessageNotConnectedState = selectedAccounts.length === 1 ? t('requestingForAccount', [ @@ -71,36 +78,48 @@ export const SiteCell: React.FC = ({ return ( <> - setShowEditAccountsModal(true)} - content={ - // Why this difference? - selectedAccounts.length === 1 ? ( - - ) : ( - - ) - } - /> - setShowEditNetworksModal(true)} - content={} - /> - + + setShowEditAccountsModal(true)} + paddingBottomValue={2} + paddingTopValue={0} + content={ + // Why this difference? + selectedAccounts.length === 1 ? ( + + ) : ( + + ) + } + /> + setShowEditNetworksModal(true)} + paddingTopValue={2} + paddingBottomValue={0} + content={} + /> + {showEditAccountsModal && ( void; +export type SetCurrentInputFocus = (name: string | null) => void; + export type SnapInterfaceContextType = { handleEvent: HandleEvent; getValue: GetValue; handleInputChange: HandleInputChange; handleFileChange: HandleFileChange; + setCurrentFocusedInput: SetCurrentInputFocus; + focusedInput: string | null; snapId: string; }; @@ -80,6 +84,7 @@ export const SnapInterfaceContextProvider: FunctionComponent< // UI. It's kept in a ref to avoid useless re-rendering of the entire tree of // components. const internalState = useRef(initialState ?? {}); + const focusedInput = useRef(null); // Since the internal state is kept in a reference, it won't update when the // interface is updated. We have to manually update it. @@ -230,13 +235,16 @@ export const SnapInterfaceContextProvider: FunctionComponent< ? (initialState[form] as FormState)?.[name] : (initialState as FormState)?.[name]; - if (value) { + if (value !== undefined && value !== null) { return value; } return undefined; }; + const setCurrentFocusedInput: SetCurrentInputFocus = (name) => + (focusedInput.current = name); + return ( diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index 8abb63aa4a14..e399dc663806 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -435,7 +435,14 @@ export const getPendingSmartTransactions = (state) => { }; export const getSmartTransactionFees = (state) => { - return state.metamask.smartTransactionsState?.fees; + const usedQuote = getUsedQuote(state); + if (!usedQuote?.isGasIncludedTrade) { + return state.metamask.smartTransactionsState?.fees; + } + return { + approvalTxFees: usedQuote.approvalTxFees, + tradeTxFees: usedQuote.tradeTxFees, + }; }; export const getSmartTransactionEstimatedGas = (state) => { @@ -780,6 +787,8 @@ export const fetchQuotesAndSetQuoteState = ( fromAddress: selectedAccount.address, balanceError, sourceDecimals: fromTokenDecimals, + enableGasIncludedQuotes: + currentSmartTransactionsEnabled && smartTransactionsOptInStatus, }, { sourceTokenInfo, @@ -933,6 +942,7 @@ export const signAndSendSwapsSmartTransaction = ({ stx_enabled: smartTransactionsEnabled, current_stx_enabled: currentSmartTransactionsEnabled, stx_user_opt_in: smartTransactionsOptInStatus, + is_gas_included_trade: usedQuote.isGasIncludedTrade, ...additionalTrackingParams, }; trackEvent({ @@ -964,13 +974,18 @@ export const signAndSendSwapsSmartTransaction = ({ value: '0x0', }; } - const fees = await dispatch( - fetchSwapsSmartTransactionFees({ - unsignedTransaction, - approveTxParams: updatedApproveTxParams, - fallbackOnNotEnoughFunds: true, - }), - ); + let fees; + if (usedQuote.isGasIncludedTrade) { + fees = getSmartTransactionFees(state); + } else { + fees = await dispatch( + fetchSwapsSmartTransactionFees({ + unsignedTransaction, + approveTxParams: updatedApproveTxParams, + fallbackOnNotEnoughFunds: true, + }), + ); + } if (!fees) { log.error('"fetchSwapsSmartTransactionFees" failed'); dispatch(setSwapsSTXSubmitLoading(false)); diff --git a/ui/ducks/swaps/swaps.test.js b/ui/ducks/swaps/swaps.test.js index 0bba5e5a68be..83c133572c0d 100644 --- a/ui/ducks/swaps/swaps.test.js +++ b/ui/ducks/swaps/swaps.test.js @@ -652,13 +652,36 @@ describe('Ducks - Swaps', () => { }); describe('getSmartTransactionFees', () => { - it('returns unsigned transactions and estimates', () => { + it('returns estimates from the STX controller', () => { const state = createSwapsMockStore(); const smartTransactionFees = swaps.getSmartTransactionFees(state); expect(smartTransactionFees).toMatchObject( state.metamask.smartTransactionsState.fees, ); }); + + it('returns estimates from a selected quote', () => { + const state = createSwapsMockStore(); + state.metamask.swapsState.quotes.TEST_AGG_2.isGasIncludedTrade = true; + const smartTransactionFees = swaps.getSmartTransactionFees(state); + expect(smartTransactionFees).toMatchObject({ + approvalTxFees: + state.metamask.swapsState.quotes.TEST_AGG_2.approvalTxFees, + tradeTxFees: state.metamask.swapsState.quotes.TEST_AGG_2.tradeTxFees, + }); + }); + + it('returns estimates from a top quote if no quote is selected', () => { + const state = createSwapsMockStore(); + state.metamask.swapsState.selectedAggId = null; + state.metamask.swapsState.quotes.TEST_AGG_BEST.isGasIncludedTrade = true; + const smartTransactionFees = swaps.getSmartTransactionFees(state); + expect(smartTransactionFees).toMatchObject({ + approvalTxFees: + state.metamask.swapsState.quotes.TEST_AGG_BEST.approvalTxFees, + tradeTxFees: state.metamask.swapsState.quotes.TEST_AGG_BEST.tradeTxFees, + }); + }); }); describe('getSmartTransactionEstimatedGas', () => { diff --git a/ui/helpers/constants/zendesk-url.js b/ui/helpers/constants/zendesk-url.js index 1e29bd9b1fc7..3986df5b41ef 100644 --- a/ui/helpers/constants/zendesk-url.js +++ b/ui/helpers/constants/zendesk-url.js @@ -8,6 +8,8 @@ const ZENDESK_URLS = { CUSTOMIZE_NONCE: 'https://support.metamask.io/transactions-and-gas/transactions/how-to-customize-a-transaction-nonce/', GAS_FEES: 'https://support.metamask.io/transactions-and-gas/gas-fees/', + SWAPS_GAS_FEES: + 'https://support.metamask.io/token-swaps/user-guide-swaps/#gas-fees', HARDWARE_CONNECTION: 'https://support.metamask.io/privacy-and-security/hardware-wallet-hub/', IMPORT_ACCOUNTS: diff --git a/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap b/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap index 46bf53c2a7bc..1af0810d285f 100644 --- a/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/header/__snapshots__/header.test.tsx.snap @@ -113,6 +113,170 @@ exports[`Header should match snapshot with signature confirmation 1`] = `
`; +exports[`Header should match snapshot with token transfer confirmation initiated in a dApp 1`] = ` +
+
+
+
+
+
+
+ + + + + +
+
+
+
+ G +
+
+
+

+

+ Goerli +

+
+
+
+
+
+
+ +
+
+
+ +
+
+
+
+
+`; + +exports[`Header should match snapshot with token transfer confirmation initiated in the wallet 1`] = ` +
+
+ +

+ Review +

+
+ +
+
+
+`; + exports[`Header should match snapshot with transaction confirmation 1`] = `
should match snapshot 1`] = ` +
+
+ +

+ Review +

+
+ +
+
+
+`; diff --git a/ui/pages/confirmations/components/confirm/header/header.test.tsx b/ui/pages/confirmations/components/confirm/header/header.test.tsx index c6b8481c01fc..841c0196ae29 100644 --- a/ui/pages/confirmations/components/confirm/header/header.test.tsx +++ b/ui/pages/confirmations/components/confirm/header/header.test.tsx @@ -4,6 +4,7 @@ import { DefaultRootState } from 'react-redux'; import { getMockContractInteractionConfirmState, + getMockTokenTransferConfirmState, getMockTypedSignConfirmState, } from '../../../../../../test/data/confirmations/helper'; import { renderWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; @@ -28,6 +29,26 @@ describe('Header', () => { expect(container).toMatchSnapshot(); }); + it('should match snapshot with token transfer confirmation initiated in a dApp', () => { + const { container } = render( + getMockTokenTransferConfirmState({ + isWalletInitiatedConfirmation: false, + }), + ); + + expect(container).toMatchSnapshot(); + }); + + it('should match snapshot with token transfer confirmation initiated in the wallet', () => { + const { container } = render( + getMockTokenTransferConfirmState({ + isWalletInitiatedConfirmation: true, + }), + ); + + expect(container).toMatchSnapshot(); + }); + it('contains network name and account name', () => { const { getByText } = render(); expect(getByText('Test Account')).toBeInTheDocument(); diff --git a/ui/pages/confirmations/components/confirm/header/header.tsx b/ui/pages/confirmations/components/confirm/header/header.tsx index 255384c58b82..9c113effe6a5 100644 --- a/ui/pages/confirmations/components/confirm/header/header.tsx +++ b/ui/pages/confirmations/components/confirm/header/header.tsx @@ -1,3 +1,7 @@ +import { + TransactionMeta, + TransactionType, +} from '@metamask/transaction-controller'; import React from 'react'; import { AvatarNetwork, @@ -14,15 +18,29 @@ import { TextVariant, } from '../../../../../helpers/constants/design-system'; import { getAvatarNetworkColor } from '../../../../../helpers/utils/accounts'; +import { useConfirmContext } from '../../../context/confirm'; import useConfirmationNetworkInfo from '../../../hooks/useConfirmationNetworkInfo'; import useConfirmationRecipientInfo from '../../../hooks/useConfirmationRecipientInfo'; +import { Confirmation } from '../../../types/confirm'; import HeaderInfo from './header-info'; +import { WalletInitiatedHeader } from './wallet-initiated-header'; const Header = () => { const { networkImageUrl, networkDisplayName } = useConfirmationNetworkInfo(); const { senderAddress: fromAddress, senderName: fromName } = useConfirmationRecipientInfo(); + const { currentConfirmation } = useConfirmContext(); + + if (currentConfirmation?.type === TransactionType.tokenMethodTransfer) { + const isWalletInitiated = + (currentConfirmation as TransactionMeta).origin === 'metamask'; + + if (isWalletInitiated) { + return ; + } + } + return ( { + const store = configureStore(state); + return renderWithConfirmContextProvider(, store); +}; + +describe('', () => { + it.only('should match snapshot', () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/header/wallet-initiated-header.tsx b/ui/pages/confirmations/components/confirm/header/wallet-initiated-header.tsx new file mode 100644 index 000000000000..c1bca06c74b0 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/header/wallet-initiated-header.tsx @@ -0,0 +1,103 @@ +import { TransactionMeta } from '@metamask/transaction-controller'; +import React, { useCallback } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { useHistory } from 'react-router-dom'; +import { AssetType } from '../../../../../../shared/constants/transaction'; +import { + Box, + ButtonIcon, + ButtonIconSize, + IconName, + Text, +} from '../../../../../components/component-library'; +import { clearConfirmTransaction } from '../../../../../ducks/confirm-transaction/confirm-transaction.duck'; +import { editExistingTransaction } from '../../../../../ducks/send'; +import { + AlignItems, + BackgroundColor, + BorderRadius, + Display, + FlexDirection, + IconColor, + JustifyContent, + TextColor, + TextVariant, +} from '../../../../../helpers/constants/design-system'; +import { SEND_ROUTE } from '../../../../../helpers/constants/routes'; +import { useI18nContext } from '../../../../../hooks/useI18nContext'; +import { + setConfirmationAdvancedDetailsOpen, + showSendTokenPage, +} from '../../../../../store/actions'; +import { useConfirmContext } from '../../../context/confirm'; +import { selectConfirmationAdvancedDetailsOpen } from '../../../selectors/preferences'; + +export const WalletInitiatedHeader = () => { + const t = useI18nContext(); + const dispatch = useDispatch(); + const history = useHistory(); + + const { currentConfirmation } = useConfirmContext(); + + const showAdvancedDetails = useSelector( + selectConfirmationAdvancedDetailsOpen, + ); + + const setShowAdvancedDetails = (value: boolean): void => { + dispatch(setConfirmationAdvancedDetailsOpen(value)); + }; + + const handleBackButtonClick = useCallback(async () => { + const { id } = currentConfirmation; + + await dispatch(editExistingTransaction(AssetType.token, id.toString())); + dispatch(clearConfirmTransaction()); + dispatch(showSendTokenPage()); + + history.push(SEND_ROUTE); + }, [currentConfirmation, dispatch, history]); + + return ( + + + + {t('review')} + + + { + setShowAdvancedDetails(!showAdvancedDetails); + }} + /> + + + ); +}; diff --git a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts index 874e817cc20a..a6e92167e558 100644 --- a/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts +++ b/ui/pages/confirmations/components/confirm/info/approve/hooks/use-received-token.test.ts @@ -1,10 +1,8 @@ import { TransactionMeta } from '@metamask/transaction-controller'; -import { - CONTRACT_INTERACTION_SENDER_ADDRESS, - genUnapprovedApproveConfirmation, -} from '../../../../../../../../test/data/confirmations/contract-interaction'; +import { CONTRACT_INTERACTION_SENDER_ADDRESS } from '../../../../../../../../test/data/confirmations/contract-interaction'; import { getMockConfirmStateForTransaction } from '../../../../../../../../test/data/confirmations/helper'; +import { genUnapprovedApproveConfirmation } from '../../../../../../../../test/data/confirmations/token-approve'; import { renderHookWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; import { useAccountTotalFiatBalance } from '../../../../../../../hooks/useAccountTotalFiatBalance'; import { useReceivedToken } from './use-received-token'; diff --git a/ui/pages/confirmations/components/confirm/info/info.tsx b/ui/pages/confirmations/components/confirm/info/info.tsx index 5a9c4757158e..3e87f4f7908c 100644 --- a/ui/pages/confirmations/components/confirm/info/info.tsx +++ b/ui/pages/confirmations/components/confirm/info/info.tsx @@ -6,6 +6,7 @@ import ApproveInfo from './approve/approve'; import BaseTransactionInfo from './base-transaction-info/base-transaction-info'; import PersonalSignInfo from './personal-sign/personal-sign'; import SetApprovalForAllInfo from './set-approval-for-all-info/set-approval-for-all-info'; +import TokenTransferInfo from './token-transfer/token-transfer'; import TypedSignV1Info from './typed-sign-v1/typed-sign-v1'; import TypedSignInfo from './typed-sign/typed-sign'; @@ -29,6 +30,7 @@ const Info = () => { [TransactionType.tokenMethodIncreaseAllowance]: () => ApproveInfo, [TransactionType.tokenMethodSetApprovalForAll]: () => SetApprovalForAllInfo, + [TransactionType.tokenMethodTransfer]: () => TokenTransferInfo, }), [currentConfirmation], ); diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap new file mode 100644 index 000000000000..c3aa8e4e26ea --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/__snapshots__/token-transfer.test.tsx.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`TokenTransferInfo renders correctly 1`] = `
`; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx new file mode 100644 index 000000000000..384a8f161e9b --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.stories.tsx @@ -0,0 +1,26 @@ +import React from 'react'; +import { Provider } from 'react-redux'; +import { getMockTokenTransferConfirmState } from '../../../../../../../test/data/confirmations/helper'; +import configureStore from '../../../../../../store/store'; +import { ConfirmContextProvider } from '../../../../context/confirm'; +import TokenTransferInfo from './token-transfer'; + +const store = configureStore(getMockTokenTransferConfirmState({})); + +const Story = { + title: 'Components/App/Confirm/info/TokenTransferInfo', + component: TokenTransferInfo, + decorators: [ + (story: () => any) => ( + + {story()} + + ), + ], +}; + +export default Story; + +export const DefaultStory = () => ; + +DefaultStory.storyName = 'Default'; diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.test.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.test.tsx new file mode 100644 index 000000000000..186505ee7740 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.test.tsx @@ -0,0 +1,26 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import { getMockTokenTransferConfirmState } from '../../../../../../../test/data/confirmations/helper'; +import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; +import TokenTransferInfo from './token-transfer'; + +jest.mock( + '../../../../../../components/app/alert-system/contexts/alertMetricsContext', + () => ({ + useAlertMetrics: jest.fn(() => ({ + trackAlertMetrics: jest.fn(), + })), + }), +); + +describe('TokenTransferInfo', () => { + it('renders correctly', () => { + const state = getMockTokenTransferConfirmState({}); + const mockStore = configureMockStore([])(state); + const { container } = renderWithConfirmContextProvider( + , + mockStore, + ); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx new file mode 100644 index 000000000000..8da9493ebbc4 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/token-transfer.tsx @@ -0,0 +1,5 @@ +const TokenTransferInfo = () => { + return null; +}; + +export default TokenTransferInfo; diff --git a/ui/pages/confirmations/components/confirm/nav/nav.tsx b/ui/pages/confirmations/components/confirm/nav/nav.tsx index 2fd394f18ae2..6546b882b784 100644 --- a/ui/pages/confirmations/components/confirm/nav/nav.tsx +++ b/ui/pages/confirmations/components/confirm/nav/nav.tsx @@ -32,9 +32,9 @@ import { import { useI18nContext } from '../../../../../hooks/useI18nContext'; import { pendingConfirmationsSortedSelector } from '../../../../../selectors'; import { rejectPendingApproval } from '../../../../../store/actions'; +import { useConfirmContext } from '../../../context/confirm'; import { useQueuedConfirmationsEvent } from '../../../hooks/useQueuedConfirmationEvents'; import { isSignatureApprovalRequest } from '../../../utils'; -import { useConfirmContext } from '../../../context/confirm'; const Nav = () => { const history = useHistory(); diff --git a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts index 40886608870b..9bea069d0935 100644 --- a/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts +++ b/ui/pages/confirmations/components/confirm/title/hooks/useCurrentSpendingCap.test.ts @@ -1,8 +1,6 @@ import { TransactionMeta } from '@metamask/transaction-controller'; -import { - CONTRACT_INTERACTION_SENDER_ADDRESS, - genUnapprovedApproveConfirmation, -} from '../../../../../../../test/data/confirmations/contract-interaction'; +import { CONTRACT_INTERACTION_SENDER_ADDRESS } from '../../../../../../../test/data/confirmations/contract-interaction'; +import { genUnapprovedApproveConfirmation } from '../../../../../../../test/data/confirmations/token-approve'; import mockState from '../../../../../../../test/data/mock-state.json'; import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; import { useCurrentSpendingCap } from './useCurrentSpendingCap'; diff --git a/ui/pages/confirmations/hooks/useLedgerConnection.test.ts b/ui/pages/confirmations/hooks/useLedgerConnection.test.ts new file mode 100644 index 000000000000..7041b11b1aa4 --- /dev/null +++ b/ui/pages/confirmations/hooks/useLedgerConnection.test.ts @@ -0,0 +1,319 @@ +import type { KeyringObject } from '@metamask/keyring-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { Hex } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; +import { + HardwareTransportStates, + LEDGER_USB_VENDOR_ID, + LedgerTransportTypes, + WebHIDConnectedStatuses, +} from '../../../../shared/constants/hardware-wallets'; +import { KeyringType } from '../../../../shared/constants/keyring'; +import { getMockConfirmStateForTransaction } from '../../../../test/data/confirmations/helper'; +import { genUnapprovedApproveConfirmation } from '../../../../test/data/confirmations/token-approve'; +import { renderHookWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers'; +import { flushPromises } from '../../../../test/lib/timer-helpers'; +import * as appActions from '../../../ducks/app/app'; +import { attemptLedgerTransportCreation } from '../../../store/actions'; +import useLedgerConnection from './useLedgerConnection'; + +jest.mock('../../../store/actions', () => ({ + ...jest.requireActual('../../../store/actions'), + attemptLedgerTransportCreation: jest.fn(), +})); + +type RootState = { + metamask: Record; + appState: Record; +} & Record; + +const MOCK_LEDGER_ACCOUNT = '0x1234567890abcdef1234567890abcdef12345678'; + +const updateLedgerHardwareAccounts = (keyrings: KeyringObject[]) => { + const ledgerHardwareIndex = keyrings.findIndex( + (keyring) => keyring.type === KeyringType.ledger, + ); + + if (ledgerHardwareIndex === -1) { + // If 'Ledger Hardware' does not exist, create a new entry + keyrings.push({ + type: KeyringType.ledger, + accounts: [MOCK_LEDGER_ACCOUNT], + }); + } else { + // If 'Ledger Hardware' exists, update its accounts + keyrings[ledgerHardwareIndex].accounts = [MOCK_LEDGER_ACCOUNT]; + } + + return keyrings; +}; + +const generateUnapprovedConfirmationOnLedgerState = (address: Hex) => { + const transactionMeta = genUnapprovedApproveConfirmation({ + address, + chainId: '0x5', + }) as TransactionMeta; + + const clonedState = cloneDeep( + getMockConfirmStateForTransaction(transactionMeta), + ) as RootState; + + clonedState.metamask.keyrings = updateLedgerHardwareAccounts( + clonedState.metamask.keyrings as KeyringObject[], + ); + + clonedState.metamask.ledgerTransportType = LedgerTransportTypes.webhid; + + return clonedState; +}; + +describe('useLedgerConnection', () => { + const mockAttemptLedgerTransportCreation = jest.mocked( + attemptLedgerTransportCreation, + ); + + let state: RootState; + let originalNavigatorHid: HID; + + beforeEach(() => { + originalNavigatorHid = window.navigator.hid; + jest.resetAllMocks(); + Object.defineProperty(window.navigator, 'hid', { + value: { + getDevices: jest + .fn() + .mockImplementation(() => + Promise.resolve([{ vendorId: Number(LEDGER_USB_VENDOR_ID) }]), + ), + }, + configurable: true, + }); + + state = generateUnapprovedConfirmationOnLedgerState(MOCK_LEDGER_ACCOUNT); + }); + + afterAll(() => { + Object.defineProperty(window.navigator, 'hid', { + value: originalNavigatorHid, + configurable: true, + }); + }); + + describe('checks hid devices initially', () => { + it('set LedgerWebHidConnectedStatus to connected if it finds Ledger hid', async () => { + const spyOnSetLedgerWebHidConnectedStatus = jest.spyOn( + appActions, + 'setLedgerWebHidConnectedStatus', + ); + + state.appState.ledgerWebHidConnectedStatus = + WebHIDConnectedStatuses.notConnected; + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerWebHidConnectedStatus).toHaveBeenCalledWith( + WebHIDConnectedStatuses.connected, + ); + }); + + it('set LedgerWebHidConnectedStatus to notConnected if it does not find Ledger hid', async () => { + const spyOnSetLedgerWebHidConnectedStatus = jest.spyOn( + appActions, + 'setLedgerWebHidConnectedStatus', + ); + + state.appState.ledgerWebHidConnectedStatus = + WebHIDConnectedStatuses.unknown; + + (window.navigator.hid.getDevices as jest.Mock).mockImplementationOnce( + () => Promise.resolve([]), + ); + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerWebHidConnectedStatus).toHaveBeenCalledWith( + WebHIDConnectedStatuses.notConnected, + ); + }); + }); + + describe('determines transport status', () => { + it('set LedgerTransportStatus to verified if transport creation is successful', async () => { + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + mockAttemptLedgerTransportCreation.mockResolvedValue(true); + + state.appState.ledgerWebHidConnectedStatus = + WebHIDConnectedStatuses.connected; + state.appState.ledgerTransportStatus = HardwareTransportStates.none; + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerTransportStatus).toHaveBeenCalledWith( + HardwareTransportStates.verified, + ); + }); + + it('set LedgerTransportStatus to unknownFailure if transport creation fails', async () => { + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + mockAttemptLedgerTransportCreation.mockResolvedValue(false); + + state.appState.ledgerWebHidConnectedStatus = + WebHIDConnectedStatuses.connected; + state.appState.ledgerTransportStatus = HardwareTransportStates.none; + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerTransportStatus).toHaveBeenCalledWith( + HardwareTransportStates.unknownFailure, + ); + }); + + it('set LedgerTransportStatus to deviceOpenFailure if device open fails', async () => { + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + mockAttemptLedgerTransportCreation.mockRejectedValue( + new Error('Failed to open the device'), + ); + + state.appState.ledgerWebHidConnectedStatus = + WebHIDConnectedStatuses.connected; + state.appState.ledgerTransportStatus = HardwareTransportStates.none; + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerTransportStatus).toHaveBeenCalledWith( + HardwareTransportStates.deviceOpenFailure, + ); + }); + + it('set LedgerTransportStatus to verified if device is already open', async () => { + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + mockAttemptLedgerTransportCreation.mockRejectedValue( + new Error('the device is already open'), + ); + + state.appState.ledgerWebHidConnectedStatus = + WebHIDConnectedStatuses.connected; + state.appState.ledgerTransportStatus = HardwareTransportStates.none; + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerTransportStatus).toHaveBeenCalledWith( + HardwareTransportStates.verified, + ); + }); + + it('set LedgerTransportStatus to unknownFailure if an unknown error occurs', async () => { + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + mockAttemptLedgerTransportCreation.mockRejectedValue( + new Error('Unknown error'), + ); + + state.appState.ledgerWebHidConnectedStatus = + WebHIDConnectedStatuses.connected; + state.appState.ledgerTransportStatus = HardwareTransportStates.none; + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerTransportStatus).toHaveBeenCalledWith( + HardwareTransportStates.unknownFailure, + ); + }); + }); + + it('reset LedgerTransportStatus to none on unmount', () => { + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + const { unmount } = renderHookWithConfirmContextProvider( + useLedgerConnection, + state, + ); + + unmount(); + + expect(spyOnSetLedgerTransportStatus).toHaveBeenCalledWith( + HardwareTransportStates.none, + ); + }); + + describe('does nothing', () => { + it('when address is not a ledger address', async () => { + const spyOnSetLedgerWebHidConnectedStatus = jest.spyOn( + appActions, + 'setLedgerWebHidConnectedStatus', + ); + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + // Set state to have empty keyrings, simulating a non-Ledger address + state.metamask.keyrings = []; + + renderHookWithConfirmContextProvider(useLedgerConnection, state); + + await flushPromises(); + + expect(spyOnSetLedgerWebHidConnectedStatus).not.toHaveBeenCalled(); + expect(spyOnSetLedgerTransportStatus).not.toHaveBeenCalled(); + }); + + it('when from address is not defined in currentConfirmation', async () => { + const tempState = generateUnapprovedConfirmationOnLedgerState( + undefined as unknown as Hex, + ); + + const spyOnSetLedgerWebHidConnectedStatus = jest.spyOn( + appActions, + 'setLedgerWebHidConnectedStatus', + ); + const spyOnSetLedgerTransportStatus = jest.spyOn( + appActions, + 'setLedgerTransportStatus', + ); + + renderHookWithConfirmContextProvider(useLedgerConnection, tempState); + + await flushPromises(); + + expect(spyOnSetLedgerWebHidConnectedStatus).not.toHaveBeenCalled(); + expect(spyOnSetLedgerTransportStatus).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/ui/pages/confirmations/utils/confirm.ts b/ui/pages/confirmations/utils/confirm.ts index 8c2846b6b69a..41ffd2832169 100644 --- a/ui/pages/confirmations/utils/confirm.ts +++ b/ui/pages/confirmations/utils/confirm.ts @@ -26,6 +26,7 @@ export const REDESIGN_USER_TRANSACTION_TYPES = [ export const REDESIGN_DEV_TRANSACTION_TYPES = [ ...REDESIGN_USER_TRANSACTION_TYPES, + TransactionType.tokenMethodTransfer, ]; const SIGNATURE_APPROVAL_TYPES = [ diff --git a/ui/pages/permissions-connect/connect-page/__snapshots__/connect-page.test.tsx.snap b/ui/pages/permissions-connect/connect-page/__snapshots__/connect-page.test.tsx.snap index 931b4dac797f..6353df3e96cc 100644 --- a/ui/pages/permissions-connect/connect-page/__snapshots__/connect-page.test.tsx.snap +++ b/ui/pages/permissions-connect/connect-page/__snapshots__/connect-page.test.tsx.snap @@ -6,11 +6,11 @@ exports[`ConnectPage should render correctly 1`] = ` class="mm-box multichain-page mm-box--display-flex mm-box--flex-direction-row mm-box--justify-content-center mm-box--width-full mm-box--height-full mm-box--background-color-background-alternative" >
- -
-
-

- See your accounts and suggest transactions -

+
+
+

- Requesting for Test Account - + See your accounts and suggest transactions +

-
- -
-
-
- +
-

- Use your enabled networks -

+
+
+

- Requesting for - + Use your enabled networks +

Alerts"" - data-tooltipped="" - style="display: inline;" + class="mm-box mm-box--display-flex mm-box--gap-1 mm-box--flex-direction-row mm-box--align-items-center" > + + Requesting for +
Alerts"" + data-tooltipped="" + style="display: inline;" >
- G +
+ G +
-
-
- Custom Mainnet RPC logo +
+ Custom Mainnet RPC logo +
+
-
- - + + + +

+
+
- Confirm - + + +
diff --git a/ui/pages/permissions-connect/connect-page/connect-page.test.tsx b/ui/pages/permissions-connect/connect-page/connect-page.test.tsx index 9440d5031334..d7c50c6aa501 100644 --- a/ui/pages/permissions-connect/connect-page/connect-page.test.tsx +++ b/ui/pages/permissions-connect/connect-page/connect-page.test.tsx @@ -69,7 +69,7 @@ describe('ConnectPage', () => { it('should render confirm and cancel button', () => { const { getByText } = render(); - const confirmButton = getByText('Confirm'); + const confirmButton = getByText('Connect'); const cancelButton = getByText('Cancel'); expect(confirmButton).toBeDefined(); expect(cancelButton).toBeDefined(); diff --git a/ui/pages/permissions-connect/connect-page/connect-page.tsx b/ui/pages/permissions-connect/connect-page/connect-page.tsx index f332ba6cc07e..a30047fbd38a 100644 --- a/ui/pages/permissions-connect/connect-page/connect-page.tsx +++ b/ui/pages/permissions-connect/connect-page/connect-page.tsx @@ -24,13 +24,16 @@ import { } from '../../../components/multichain/pages/page'; import { SiteCell } from '../../../components/multichain/pages/review-permissions-page'; import { + BackgroundColor, BlockSize, Display, + FlexDirection, TextVariant, } from '../../../helpers/constants/design-system'; import { MergedInternalAccount } from '../../../selectors/selectors.types'; import { mergeAccounts } from '../../../components/multichain/account-list-menu/account-list-menu'; import { TEST_CHAINS } from '../../../../shared/constants/network'; +import PermissionsConnectFooter from '../../../components/app/permissions-connect-footer'; export type ConnectPageRequest = { id: string; @@ -97,14 +100,15 @@ export const ConnectPage: React.FC = ({ return ( -
+
{t('connectWithMetaMask')} {t('connectionDescription')}:
- + = ({ />
- - - + + + + + +
diff --git a/ui/pages/permissions-connect/index.scss b/ui/pages/permissions-connect/index.scss index 513809505d50..954ec7a1121c 100644 --- a/ui/pages/permissions-connect/index.scss +++ b/ui/pages/permissions-connect/index.scss @@ -44,4 +44,8 @@ justify-self: flex-end; font-weight: bold; } + + .connect-page { + background-color: var(--color-background-alternative); // main-container adds the width but overrides the boxProps. So, we need extra class to apply css + } } diff --git a/ui/pages/permissions-connect/permissions-connect.component.js b/ui/pages/permissions-connect/permissions-connect.component.js index 7e489296bfb2..09befa7218cd 100644 --- a/ui/pages/permissions-connect/permissions-connect.component.js +++ b/ui/pages/permissions-connect/permissions-connect.component.js @@ -148,8 +148,6 @@ export default class PermissionConnect extends Component { history.replace(DEFAULT_ROUTE); return; } - history.replace(confirmPermissionPath); - // if this is an incremental permission request for permitted chains, skip the account selection if ( permissionsRequest?.diff?.permissionDiffMap?.[ @@ -340,56 +338,8 @@ export default class PermissionConnect extends Component { ( - this.selectAccounts(addresses)} - selectNewAccountViaModal={(handleAccountClick) => { - showNewAccountModal({ - onCreateNewAccount: (address) => - handleAccountClick(address), - newAccountNumber, - }); - }} - addressLastConnectedMap={addressLastConnectedMap} - cancelPermissionsRequest={(requestId) => - this.cancelPermissionsRequest(requestId) - } - permissionsRequestId={permissionsRequestId} - selectedAccountAddresses={selectedAccountAddresses} - targetSubjectMetadata={targetSubjectMetadata} - /> - )} - /> - - permissionsRequest?.diff ? ( - { - approvePermissionsRequest(...args); - this.redirect(true); - }} - rejectPermissionsRequest={(requestId) => - this.cancelPermissionsRequest(requestId) - } - selectedAccounts={accounts.filter((account) => - selectedAccountAddresses.has(account.address), - )} - targetSubjectMetadata={targetSubjectMetadata} - history={this.props.history} - connectPath={connectPath} - snapsInstallPrivacyWarningShown={ - snapsInstallPrivacyWarningShown - } - setSnapsInstallPrivacyWarningShownStatus={ - setSnapsInstallPrivacyWarningShownStatus - } - /> - ) : ( + process.env.CHAIN_PERMISSIONS ? ( this.cancelPermissionsRequest(requestId) @@ -399,9 +349,59 @@ export default class PermissionConnect extends Component { permissionsRequestId={permissionsRequestId} approveConnection={this.approveConnection} /> + ) : ( + + this.selectAccounts(addresses) + } + selectNewAccountViaModal={(handleAccountClick) => { + showNewAccountModal({ + onCreateNewAccount: (address) => + handleAccountClick(address), + newAccountNumber, + }); + }} + addressLastConnectedMap={addressLastConnectedMap} + cancelPermissionsRequest={(requestId) => + this.cancelPermissionsRequest(requestId) + } + permissionsRequestId={permissionsRequestId} + selectedAccountAddresses={selectedAccountAddresses} + targetSubjectMetadata={targetSubjectMetadata} + /> ) } /> + ( + { + approvePermissionsRequest(...args); + this.redirect(true); + }} + rejectPermissionsRequest={(requestId) => + this.cancelPermissionsRequest(requestId) + } + selectedAccounts={accounts.filter((account) => + selectedAccountAddresses.has(account.address), + )} + targetSubjectMetadata={targetSubjectMetadata} + history={this.props.history} + connectPath={connectPath} + snapsInstallPrivacyWarningShown={ + snapsInstallPrivacyWarningShown + } + setSnapsInstallPrivacyWarningShownStatus={ + setSnapsInstallPrivacyWarningShownStatus + } + /> + )} + /> 0; + const isTokenEligibleForMaxBalance = + isSmartTransaction || (!isSmartTransaction && isNonDefaultToken); const showMaxBalanceLink = fromTokenSymbol && - !isSwapsDefaultTokenSymbol(fromTokenSymbol, chainId) && - rawFromTokenBalance > 0; + isTokenEligibleForMaxBalance && + hasPositiveFromTokenBalance; return (
diff --git a/ui/pages/swaps/prepare-swap-page/review-quote.js b/ui/pages/swaps/prepare-swap-page/review-quote.js index 496ae5ee6d9e..31cf9959f231 100644 --- a/ui/pages/swaps/prepare-swap-page/review-quote.js +++ b/ui/pages/swaps/prepare-swap-page/review-quote.js @@ -23,7 +23,6 @@ import { useGasFeeInputs } from '../../confirmations/hooks/useGasFeeInputs'; import { MetaMetricsContext } from '../../../contexts/metametrics'; import { getQuotes, - getSelectedQuote, getApproveTxParams, getFetchParams, setBalanceError, @@ -36,6 +35,7 @@ import { getDestinationTokenInfo, getUsedSwapsGasPrice, getTopQuote, + getUsedQuote, signAndSendTransactions, getBackgroundSwapRouteState, swapsQuoteSelected, @@ -84,6 +84,7 @@ import { decimalToHex, decWEIToDecETH, sumHexes, + hexToDecimal, } from '../../../../shared/modules/conversion.utils'; import { getCustomTxParamsData } from '../../confirmations/confirm-approve/confirm-approve.util'; import { @@ -113,6 +114,7 @@ import { Size, FlexDirection, Severity, + FontStyle, } from '../../../helpers/constants/design-system'; import { BannerAlert, @@ -143,11 +145,41 @@ import { import ExchangeRateDisplay from '../exchange-rate-display'; import InfoTooltip from '../../../components/ui/info-tooltip'; import useRamps from '../../../hooks/ramps/useRamps/useRamps'; +import { getTokenFiatAmount } from '../../../helpers/utils/token-util'; +import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; import ViewQuotePriceDifference from './view-quote-price-difference'; import SlippageNotificationModal from './slippage-notification-modal'; let intervalId; +const ViewAllQuotesLink = React.memo(function ViewAllQuotesLink({ + trackAllAvailableQuotesOpened, + setSelectQuotePopoverShown, + t, +}) { + const handleClick = useCallback(() => { + trackAllAvailableQuotesOpened(); + setSelectQuotePopoverShown(true); + }, [trackAllAvailableQuotesOpened, setSelectQuotePopoverShown]); + + return ( + + {t('viewAllQuotes')} + + ); +}); + +ViewAllQuotesLink.propTypes = { + trackAllAvailableQuotesOpened: PropTypes.func.isRequired, + setSelectQuotePopoverShown: PropTypes.func.isRequired, + t: PropTypes.func.isRequired, +}; + export default function ReviewQuote({ setReceiveToAmount }) { const history = useHistory(); const dispatch = useDispatch(); @@ -206,9 +238,8 @@ export default function ReviewQuote({ setReceiveToAmount }) { const balanceError = useSelector(getBalanceError); const fetchParams = useSelector(getFetchParams, isEqual); const approveTxParams = useSelector(getApproveTxParams, shallowEqual); - const selectedQuote = useSelector(getSelectedQuote, isEqual); const topQuote = useSelector(getTopQuote, isEqual); - const usedQuote = selectedQuote || topQuote; + const usedQuote = useSelector(getUsedQuote, isEqual); const tradeValue = usedQuote?.trade?.value ?? '0x0'; const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual); const chainId = useSelector(getCurrentChainId); @@ -229,6 +260,7 @@ export default function ReviewQuote({ setReceiveToAmount }) { const smartTransactionFees = useSelector(getSmartTransactionFees, isEqual); const swapsNetworkConfig = useSelector(getSwapsNetworkConfig, shallowEqual); const unsignedTransaction = usedQuote.trade; + const { isGasIncludedTrade } = usedQuote; const isSmartTransaction = currentSmartTransactionsEnabled && smartTransactionsOptInStatus; @@ -880,7 +912,9 @@ export default function ReviewQuote({ setReceiveToAmount }) { ]); useEffect(() => { - if (isSmartTransaction && !insufficientTokens) { + // If it's a smart transaction, has sufficient tokens, and gas is not included in the trade, + // set up gas fee polling. + if (isSmartTransaction && !insufficientTokens && !isGasIncludedTrade) { const unsignedTx = { from: unsignedTransaction.from, to: unsignedTransaction.to, @@ -923,6 +957,7 @@ export default function ReviewQuote({ setReceiveToAmount }) { chainId, swapsNetworkConfig.stxGetTransactionsRefreshTime, insufficientTokens, + isGasIncludedTrade, ]); useEffect(() => { @@ -1045,6 +1080,40 @@ export default function ReviewQuote({ setReceiveToAmount }) { } }; + const gasTokenFiatAmount = useMemo(() => { + if (!isGasIncludedTrade) { + return undefined; + } + const tradeTxTokenFee = + smartTransactionFees?.tradeTxFees?.fees?.[0]?.tokenFees?.[0]; + if (!tradeTxTokenFee) { + return undefined; + } + const { token: { address, decimals, symbol } = {}, balanceNeededToken } = + tradeTxTokenFee; + const checksumAddress = toChecksumHexAddress(address); + const contractExchangeRate = memoizedTokenConversionRates[checksumAddress]; + const gasTokenAmountDec = calcTokenAmount( + hexToDecimal(balanceNeededToken), + decimals, + ).toString(10); + return getTokenFiatAmount( + contractExchangeRate, + conversionRate, + currentCurrency, + gasTokenAmountDec, + symbol, + true, + true, + ); + }, [ + isGasIncludedTrade, + smartTransactionFees, + memoizedTokenConversionRates, + conversionRate, + currentCurrency, + ]); + return (
@@ -1122,9 +1191,9 @@ export default function ReviewQuote({ setReceiveToAmount }) { - {t('quoteRate')} + {t('quoteRate')}* - + {isGasIncludedTrade && ( - - {t('transactionDetailGasHeading')} - - - {t('swapGasFeesExplanation', [ + + {t('gasFee')} + + +

+ {t('swapGasIncludedTooltipExplanation')} +

{ trackEvent({ - event: 'Clicked "Gas Fees: Learn More" Link', + event: + 'Clicked "GasIncluded tooltip: Learn More" Link', category: MetaMetricsEventCategory.Swaps, }); }} > - {t('swapGasFeesExplanationLinkText')} - , - ])} -

- } - /> + {t('swapGasIncludedTooltipExplanationLinkText')} + + + } + /> + + + + {gasTokenFiatAmount} + + + {t('included')} + + + )} + {!isGasIncludedTrade && ( - - {feeInEth} - - + {t('transactionDetailGasHeading')} + + + {t('swapGasFeesExplanation', [ + { + trackEvent({ + event: 'Clicked "Gas Fees: Learn More" Link', + category: MetaMetricsEventCategory.Swaps, + }); + }} + > + {t('swapGasFeesExplanationLinkText')} + , + ])} +

+ } + /> + + - {` ${feeInFiat}`} - + + {feeInEth} + + + {` ${feeInFiat}`} + + - - {(maxFeeInFiat || maxFeeInEth) && ( + )} + {!isGasIncludedTrade && (maxFeeInFiat || maxFeeInEth) && ( @@ -1248,7 +1395,7 @@ export default function ReviewQuote({ setReceiveToAmount }) { {t('swapEnableTokenForSwapping', [tokenApprovalTextComponent])} @@ -1264,32 +1411,55 @@ export default function ReviewQuote({ setReceiveToAmount }) { )} - - - {t('swapIncludesMetaMaskFeeViewAllQuotes', [ - metaMaskFee, - { - trackAllAvailableQuotesOpened(); - setSelectQuotePopoverShown(true); + {isGasIncludedTrade && ( + + + * {t('swapIncludesGasAndMetaMaskFee', [metaMaskFee])} + + + + + + )} + {!isGasIncludedTrade && ( + + + * + {t('swapIncludesMetaMaskFeeViewAllQuotes', [ + metaMaskFee, + - {t('viewAllQuotes')} - , - ])} - - + setSelectQuotePopoverShown={setSelectQuotePopoverShown} + t={t} + />, + ])} + + + )}
{ const props = createProps(); const { getByText } = renderWithProvider(, store); expect(getByText('New quotes in')).toBeInTheDocument(); - expect(getByText('Quote rate')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); expect(getByText('Includes a 1% MetaMask fee ā€“')).toBeInTheDocument(); expect(getByText('view all quotes')).toBeInTheDocument(); expect(getByText('Estimated gas fee')).toBeInTheDocument(); @@ -73,7 +74,7 @@ describe('ReviewQuote', () => { const props = createProps(); const { getByText } = renderWithProvider(, store); expect(getByText('New quotes in')).toBeInTheDocument(); - expect(getByText('Quote rate')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); expect(getByText('Includes a 1% MetaMask fee ā€“')).toBeInTheDocument(); expect(getByText('view all quotes')).toBeInTheDocument(); expect(getByText('Estimated gas fee')).toBeInTheDocument(); @@ -96,7 +97,7 @@ describe('ReviewQuote', () => { const props = createProps(); const { getByText } = renderWithProvider(, store); expect(getByText('New quotes in')).toBeInTheDocument(); - expect(getByText('Quote rate')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); expect(getByText('Includes a 1% MetaMask fee ā€“')).toBeInTheDocument(); expect(getByText('view all quotes')).toBeInTheDocument(); expect(getByText('Estimated gas fee')).toBeInTheDocument(); @@ -106,4 +107,34 @@ describe('ReviewQuote', () => { expect(getByText('Edit limit')).toBeInTheDocument(); expect(getByText('Swap')).toBeInTheDocument(); }); + + it('renders the component with gas included quotes', () => { + const state = createSwapsMockStore(); + state.metamask.swapsState.quotes.TEST_AGG_2.isGasIncludedTrade = true; + state.metamask.marketData[CHAIN_IDS.MAINNET][ + '0x6B175474E89094C44Da98b954EedeAC495271d0F' // DAI token contract address. + ] = { + price: 2, + contractPercentChange1d: 0.004, + priceChange1d: 0.00004, + }; + state.metamask.currencyRates.ETH = { + conversionDate: 1708532473.416, + conversionRate: 2918.02, + usdConversionRate: 2918.02, + }; + const store = configureMockStore(middleware)(state); + const props = createProps(); + const { getByText } = renderWithProvider(, store); + expect(getByText('New quotes in')).toBeInTheDocument(); + expect(getByText('Quote rate*')).toBeInTheDocument(); + expect( + getByText('* Includes gas and a 1% MetaMask fee'), + ).toBeInTheDocument(); + expect(getByText('view all quotes')).toBeInTheDocument(); + expect(getByText('Gas fee')).toBeInTheDocument(); + // $6.82 gas fee is calculated based on params set in the the beginning of the test. + expect(getByText('$6.82')).toBeInTheDocument(); + expect(getByText('Swap')).toBeInTheDocument(); + }); }); diff --git a/ui/pages/swaps/smart-transaction-status/index.scss b/ui/pages/swaps/smart-transaction-status/index.scss index 4229acca71c0..d19add085c65 100644 --- a/ui/pages/swaps/smart-transaction-status/index.scss +++ b/ui/pages/swaps/smart-transaction-status/index.scss @@ -36,26 +36,13 @@ width: 100%; } - &__background-animation { - position: relative; - left: -88px; - background-repeat: repeat; - background-position: 0 0; - + &__spacer-box { &--top { - width: 1634px; height: 54px; - background-size: 817px 54px; - background-image: url('/images/transaction-background-top.svg'); - animation: shift 19s linear infinite; } &--bottom { - width: 1600px; height: 62px; - background-size: 800px 62px; - background-image: url('/images/transaction-background-bottom.svg'); - animation: shift 22s linear infinite; } } diff --git a/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js b/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js index 157190687f31..b103ead2097c 100644 --- a/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js +++ b/ui/pages/swaps/smart-transaction-status/smart-transaction-status.js @@ -8,11 +8,10 @@ import { getFetchParams, prepareToLeaveSwaps, getCurrentSmartTransactions, - getSelectedQuote, - getTopQuote, getCurrentSmartTransactionsEnabled, getSwapsNetworkConfig, cancelSwapsSmartTransaction, + getUsedQuote, } from '../../../ducks/swaps/swaps'; import { isHardwareWallet, @@ -74,9 +73,7 @@ export default function SmartTransactionStatusPage() { const hardwareWalletUsed = useSelector(isHardwareWallet); const hardwareWalletType = useSelector(getHardwareWalletType); const needsTwoConfirmations = true; - const selectedQuote = useSelector(getSelectedQuote, isEqual); - const topQuote = useSelector(getTopQuote, isEqual); - const usedQuote = selectedQuote || topQuote; + const usedQuote = useSelector(getUsedQuote, isEqual); const currentSmartTransactions = useSelector( getCurrentSmartTransactions, isEqual, @@ -368,7 +365,7 @@ export default function SmartTransactionStatusPage() { {icon && ( @@ -443,7 +440,7 @@ export default function SmartTransactionStatusPage() { )} {subDescription && ( ( 'submitSignedTransactions', [ { signedTransactions, - signedCanceledTransactions, + // The "signedCanceledTransactions" parameter is still expected by the STX controller but is no longer used. + // So we are passing an empty array. The parameter may be deprecated in a future update. + signedCanceledTransactions: [], txParams: unsignedTransaction, }, ], diff --git a/yarn.lock b/yarn.lock index 22889932623c..1434d86d4e62 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6053,9 +6053,9 @@ __metadata: languageName: node linkType: hard -"@metamask/profile-sync-controller@npm:^0.9.3": - version: 0.9.3 - resolution: "@metamask/profile-sync-controller@npm:0.9.3" +"@metamask/profile-sync-controller@npm:^0.9.4": + version: 0.9.4 + resolution: "@metamask/profile-sync-controller@npm:0.9.4" dependencies: "@metamask/base-controller": "npm:^7.0.1" "@metamask/keyring-api": "npm:^8.1.3" @@ -6071,7 +6071,7 @@ __metadata: "@metamask/accounts-controller": ^18.1.1 "@metamask/keyring-controller": ^17.2.0 "@metamask/snaps-controllers": ^9.7.0 - checksum: 10/31efea63cac0b5f01024163fb6911f971aeb6f7e7a7d71fa4a43b8e31e0fc60033e99bcfec19283f9410e7258bcd0ce3bf751bed374e6b3d09ea4a9782731320 + checksum: 10/86079da552eed316f2754bd899047de1d8d9d15d390c9cdee0aef66b95bea708b5c7929a8d8d946210cc0e4c52347fee971a5cf5130149d0ca60abdc85f47774 languageName: node linkType: hard @@ -26138,7 +26138,7 @@ __metadata: "@metamask/post-message-stream": "npm:^8.0.0" "@metamask/ppom-validator": "npm:0.34.0" "@metamask/preinstalled-example-snap": "npm:^0.1.0" - "@metamask/profile-sync-controller": "npm:^0.9.3" + "@metamask/profile-sync-controller": "npm:^0.9.4" "@metamask/providers": "npm:^14.0.2" "@metamask/queued-request-controller": "npm:^2.0.0" "@metamask/rate-limit-controller": "npm:^6.0.0"