diff --git a/app/components/UI/CaipAccountSelectorList/CaipAccountSelectorList.tsx b/app/components/UI/CaipAccountSelectorList/CaipAccountSelectorList.tsx index cb72dd7605ab..3f697d888bb5 100644 --- a/app/components/UI/CaipAccountSelectorList/CaipAccountSelectorList.tsx +++ b/app/components/UI/CaipAccountSelectorList/CaipAccountSelectorList.tsx @@ -32,7 +32,7 @@ import { Account, Assets } from '../../hooks/useAccounts'; import Engine from '../../../core/Engine'; import { removeAccountsFromPermissions, - sortAccountsByLastSelected, + sortMultichainAccountsByLastSelected, } from '../../../core/Permissions'; import Routes from '../../../constants/navigation/Routes'; @@ -44,8 +44,7 @@ import { WalletViewSelectorsIDs } from '../../../../e2e/selectors/wallet/WalletV import { RootState } from '../../../reducers'; import { ACCOUNT_SELECTOR_LIST_TESTID } from './CaipAccountSelectorList.constants'; import { toHex } from '@metamask/controller-utils'; -import { CaipAccountId, Hex } from '@metamask/utils'; -import { parseAccountId } from '@walletconnect/utils'; +import { CaipAccountId, parseCaipAccountId } from '@metamask/utils'; const CaipAccountSelectorList = ({ onSelectAccount, @@ -155,21 +154,15 @@ const CaipAccountSelectorList = ({ const nextCaipAccountIds = selectedAddresses.filter( (selectedAddress) => selectedAddress !== caipAccountId, ); - const nextAddresses = nextCaipAccountIds.map( - (nextCaipAccountId) => { - const { address: nextAddress } = - parseAccountId(nextCaipAccountId); - return nextAddress as Hex; - }, - ); + const [nextCaipAccountId] = + sortMultichainAccountsByLastSelected(nextCaipAccountIds); - const nextAddressesSorted = - sortAccountsByLastSelected(nextAddresses); + const nextAddress = nextCaipAccountId ? parseCaipAccountId(nextCaipAccountId).address : ''; const selectedAccountAddress = accounts.find( (acc) => acc.isSelected, )?.address; nextActiveAddress = - nextAddressesSorted[0] || selectedAccountAddress || ''; + nextAddress || selectedAccountAddress || ''; } // Switching accounts on the PreferencesController must happen before account is removed from the KeyringController, otherwise UI will break. diff --git a/app/components/Views/AccountConnect/AccountConnect.tsx b/app/components/Views/AccountConnect/AccountConnect.tsx index a80e79df7eac..02b2e81d1dbb 100644 --- a/app/components/Views/AccountConnect/AccountConnect.tsx +++ b/app/components/Views/AccountConnect/AccountConnect.tsx @@ -470,7 +470,7 @@ const AccountConnect = (props: AccountConnectProps) => { ), }, }; - + const connectedAccountLength = selectedAddresses.length; const activeAddress = selectedAddresses[0]; diff --git a/app/components/Views/AccountPermissions/AccountPermissions.test.tsx b/app/components/Views/AccountPermissions/AccountPermissions.test.tsx index 91afb4a3249d..fc7e53254856 100644 --- a/app/components/Views/AccountPermissions/AccountPermissions.test.tsx +++ b/app/components/Views/AccountPermissions/AccountPermissions.test.tsx @@ -135,6 +135,7 @@ jest.mock('../../../core/Engine', () => ({ }, AccountsController: { listAccounts: jest.fn(() => MOCK_USE_ACCOUNTS_RETURN), + listMultichainAccounts: jest.fn(() => MOCK_INTERNAL_ACCOUNTS), getAccountByAddress: jest.fn((address: string) => MOCK_INTERNAL_ACCOUNTS.find((account) => account.address === address), ), @@ -240,13 +241,6 @@ const mockInitialState = ( }, }); -const evmNetworkConfigurationsByChainId = - mockInitialState().engine?.backgroundState?.NetworkController - ?.networkConfigurationsByChainId; -const nonEvmNetworkConfigurationsByChainId = - mockInitialState().engine?.backgroundState?.MultichainNetworkController - ?.multichainNetworkConfigurationsByChainId; - describe('AccountPermissions', () => { beforeEach(() => { mockUpdatePermittedChains.mockReset(); @@ -426,8 +420,6 @@ describe('AccountPermissions', () => { expect(mockAddPermittedAccounts).toHaveBeenCalledWith( 'test', ['eip155:0:0xd018538C87232FF95acbCe4870629b75640a78E7'], - evmNetworkConfigurationsByChainId, - nonEvmNetworkConfigurationsByChainId, ); }); @@ -457,8 +449,6 @@ describe('AccountPermissions', () => { expect(mockAddPermittedAccounts).toHaveBeenCalledWith( 'test', ['eip155:0:0xd018538C87232FF95acbCe4870629b75640a78E7'], - evmNetworkConfigurationsByChainId, - nonEvmNetworkConfigurationsByChainId, ); expect(mockRemovePermittedAccounts).not.toHaveBeenCalled(); }); @@ -527,8 +517,6 @@ describe('AccountPermissions', () => { expect(mockAddPermittedAccounts).toHaveBeenCalledWith( 'test', ['eip155:0:0xd018538C87232FF95acbCe4870629b75640a78E7'], - evmNetworkConfigurationsByChainId, - nonEvmNetworkConfigurationsByChainId, ); expect(mockRemovePermittedAccounts).toHaveBeenCalledWith('test', [ '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272', diff --git a/app/components/Views/AccountPermissions/AccountPermissions.tsx b/app/components/Views/AccountPermissions/AccountPermissions.tsx index 027b469c1e01..4de837238d68 100755 --- a/app/components/Views/AccountPermissions/AccountPermissions.tsx +++ b/app/components/Views/AccountPermissions/AccountPermissions.tsx @@ -23,6 +23,7 @@ import { getPermittedCaipAccountIdsByHostname, removePermittedAccounts, getPermittedCaipChainIdsByHostname, + sortMultichainAccountsByLastSelected, } from '../../../core/Permissions'; import AccountConnectMultiSelector from '../AccountConnect/AccountConnectMultiSelector'; import NetworkConnectMultiSelector from '../NetworkConnect/NetworkConnectMultiSelector'; @@ -87,7 +88,6 @@ import { parseChainId } from '@walletconnect/utils'; import { NetworkConfiguration } from '@metamask/network-controller'; import { NetworkAvatarProps } from '../AccountConnect/AccountConnect.types'; import styleSheet from './AccountPermissions.styles'; -import { selectNonEvmNetworkConfigurationsByChainId } from '../../../selectors/multichainNetworkController'; const AccountPermissions = (props: AccountPermissionsProps) => { const { navigate } = useNavigation(); @@ -109,8 +109,6 @@ const AccountPermissions = (props: AccountPermissionsProps) => { const accountsLength = useSelector(selectAccountsLength); const currentEvmChainId = useSelector(selectEvmChainId); - const evmNetworkConfigurationsByChainId = useSelector(selectEvmNetworkConfigurationsByChainId); - const nonEvmNetworkConfigurationsByChainId = useSelector(selectNonEvmNetworkConfigurationsByChainId); const networkInfo = useNetworkInfo(hostname); const nonTestnetNetworks = useSelector( (state: RootState) => @@ -138,7 +136,8 @@ const AccountPermissions = (props: AccountPermissionsProps) => { permittedAccountsList, hostname, ); - const permittedCaipAccountIds = uniq( + const permittedCaipAccountIds = useMemo(() => { + const unsortedPermittedAccounts = uniq( nonRemappedPermittedAccounts.map((caipAccountId) => { const { address, @@ -149,8 +148,10 @@ const AccountPermissions = (props: AccountPermissionsProps) => { return `eip155:0:${address}` as CaipAccountId; } return caipAccountId; - }), - ); + })); + + return sortMultichainAccountsByLastSelected(unsortedPermittedAccounts); + }, [nonRemappedPermittedAccounts]); const permittedCaipChainIds = getPermittedCaipChainIdsByHostname( permittedAccountsList, @@ -424,7 +425,7 @@ const AccountPermissions = (props: AccountPermissionsProps) => { ); if (accountsToAdd.length > 0) { - addPermittedAccounts(hostname, accountsToAdd, evmNetworkConfigurationsByChainId, nonEvmNetworkConfigurationsByChainId); + addPermittedAccounts(hostname, accountsToAdd); newPermittedAccounts = [...newPermittedAccounts, ...accountsToAdd]; } @@ -484,8 +485,6 @@ const AccountPermissions = (props: AccountPermissionsProps) => { } }, [ - evmNetworkConfigurationsByChainId, - nonEvmNetworkConfigurationsByChainId, permittedCaipAccountIds, setIsLoading, hostname, @@ -623,7 +622,8 @@ const AccountPermissions = (props: AccountPermissionsProps) => { onDismissSheet={hideSheet} accounts={accountsFilteredByPermissions.permitted} ensByAccountAddress={ensByAccountAddress} - selectedAddresses={permittedCaipAccountIds} + // This is only okay because permittedCaipAccountIds is sorted by lastSelected already + selectedAddresses={permittedCaipAccountIds.length > 0 ? [permittedCaipAccountIds[0]] : []} favicon={faviconSource} hostname={hostname} urlWithProtocol={urlWithProtocol} diff --git a/app/components/Views/AccountPermissions/AccountPermissionsConnected/AccountPermissionsConnected.tsx b/app/components/Views/AccountPermissions/AccountPermissionsConnected/AccountPermissionsConnected.tsx index 9c7f4b01054c..8ac1b4f735c0 100644 --- a/app/components/Views/AccountPermissions/AccountPermissionsConnected/AccountPermissionsConnected.tsx +++ b/app/components/Views/AccountPermissions/AccountPermissionsConnected/AccountPermissionsConnected.tsx @@ -5,7 +5,6 @@ import { View } from 'react-native'; // External dependencies. import SheetActions from '../../../../component-library/components-temp/SheetActions'; import { strings } from '../../../../../locales/i18n'; -import EvmAccountSelectorList from '../../../../components/UI/EvmAccountSelectorList'; import { AccountPermissionsScreens } from '../AccountPermissions.types'; import { ToastContext, @@ -30,6 +29,8 @@ import Button, { ButtonWidthTypes, } from '../../../../component-library/components/Buttons/Button'; import Engine from '../../../../core/Engine'; +import CaipAccountSelectorList from '../../../UI/CaipAccountSelectorList'; +import { CaipAccountId, parseCaipAccountId } from '@metamask/utils'; const AccountPermissionsConnected = ({ ensByAccountAddress, @@ -42,7 +43,6 @@ const AccountPermissionsConnected = ({ favicon, accountAvatarType, }: AccountPermissionsConnectedProps) => { - const activeAddress = selectedAddresses[0]; const { toastRef } = useContext(ToastContext); const onConnectMoreAccounts = useCallback(() => { @@ -50,13 +50,12 @@ const AccountPermissionsConnected = ({ }, [onSetPermissionsScreen]); const switchActiveAccount = useCallback( - (address: string) => { - if (address !== activeAddress) { - Engine.setSelectedAddress(address); - } + (caipAccountId: CaipAccountId) => { + const {address} = parseCaipAccountId(caipAccountId); + Engine.setSelectedAddress(address); onDismissSheet(); const activeAccountName = getAccountNameWithENS({ - accountAddress: address, + caipAccountId, accounts, ensByAccountAddress, }); @@ -75,7 +74,6 @@ const AccountPermissionsConnected = ({ }); }, [ - activeAddress, onDismissSheet, accounts, ensByAccountAddress, @@ -120,7 +118,7 @@ const AccountPermissionsConnected = ({ {strings('accounts.connected_accounts_title')} - { isLoading?: boolean; - selectedAddresses: string[]; + selectedAddresses: CaipAccountId[]; onSetPermissionsScreen: (screen: AccountPermissionsScreens) => void; onDismissSheet: () => void; hostname: string; diff --git a/app/components/Views/AccountPermissions/__snapshots__/AccountPermissions.test.tsx.snap b/app/components/Views/AccountPermissions/__snapshots__/AccountPermissions.test.tsx.snap index 7ae7f2068536..32c43d5c9007 100644 --- a/app/components/Views/AccountPermissions/__snapshots__/AccountPermissions.test.tsx.snap +++ b/app/components/Views/AccountPermissions/__snapshots__/AccountPermissions.test.tsx.snap @@ -242,7 +242,7 @@ exports[`AccountPermissions renders correctly 1`] = ` style={ { "alignItems": "center", - "backgroundColor": "#ffffff", + "backgroundColor": "#4459ff1a", "flexDirection": "row", } } @@ -569,6 +569,34 @@ exports[`AccountPermissions renders correctly 1`] = ` + + + { const checkIfActiveAccountChanged = (hostnameForToastCheck) => { - const permittedAccounts = getPermittedAccounts(hostnameForToastCheck); - const activeAccountAddress = permittedAccounts?.[0]; + const permittedEvmAccounts = getPermittedAccounts(hostnameForToastCheck); + const activeAccountAddress = permittedEvmAccounts?.[0]; if (activeAccountAddress) { const accountName = getAccountNameWithENS({ - accountAddress: activeAccountAddress, + caipAccountId: `eip155:0:${activeAccountAddress}`, accounts, ensByAccountAddress, }); @@ -495,4 +495,4 @@ Browser.propTypes = { export { default as createBrowserNavDetails } from './Browser.types'; -export default connect(mapStateToProps, mapDispatchToProps)(Browser); \ No newline at end of file +export default connect(mapStateToProps, mapDispatchToProps)(Browser); diff --git a/app/components/Views/Browser/index.test.tsx b/app/components/Views/Browser/index.test.tsx index c478d17e666d..45ee83c85044 100644 --- a/app/components/Views/Browser/index.test.tsx +++ b/app/components/Views/Browser/index.test.tsx @@ -275,12 +275,13 @@ describe('Browser', () => { // Mock accounts and ENS data const mockAccounts = [ - { - address: testAccountAddress, + { + address: testAccountAddress, name: mockAccountName, type: KeyringTypes.simple, yOffset: 0, - isSelected: true + isSelected: true, + caipAccountId: `eip155:0:${testAccountAddress}` } ]; const mockEnsByAccountAddress = { @@ -365,8 +366,8 @@ describe('Browser', () => { const mockAccountName2 = 'Account 2'; const mockAccounts = [ - { address: testAccountAddress1, name: mockAccountName1, type: KeyringTypes.simple, yOffset: 0, isSelected: true }, - { address: testAccountAddress2, name: mockAccountName2, type: KeyringTypes.simple, yOffset: 0, isSelected: false }, + { address: testAccountAddress1, name: mockAccountName1, type: KeyringTypes.simple, yOffset: 0, isSelected: true, caipAccountId: `eip155:0:${testAccountAddress1}` }, + { address: testAccountAddress2, name: mockAccountName2, type: KeyringTypes.simple, yOffset: 0, isSelected: false, caipAccountId: `eip155:0:${testAccountAddress2}` }, ]; const mockEnsByAccountAddress = { [testAccountAddress1]: 'account1.eth', diff --git a/app/components/Views/BrowserTab/BrowserTab.tsx b/app/components/Views/BrowserTab/BrowserTab.tsx index 2561d3e96843..1a1bf905b977 100644 --- a/app/components/Views/BrowserTab/BrowserTab.tsx +++ b/app/components/Views/BrowserTab/BrowserTab.tsx @@ -55,6 +55,7 @@ import { getCaip25Caveat, getPermittedCaipAccountIdsByHostname, getPermittedEvmAddressesByHostname, + sortMultichainAccountsByLastSelected, } from '../../../core/Permissions'; import Routes from '../../../constants/navigation/Routes'; import { @@ -187,7 +188,7 @@ export const BrowserTab: React.FC = React.memo(({ const backgroundBridgeRef = useRef<{ url: string; hostname: string; - sendNotification: (payload: unknown) => void; + sendNotificationEip1193: (payload: unknown) => void; onDisconnect: () => void; onMessage: (message: Record) => void; }>(); @@ -211,10 +212,11 @@ export const BrowserTab: React.FC = React.memo(({ permissionsControllerState, hostname, ); - const permittedAccountAddresses = permittedAccountIds.map((accountId) => { - const { address } = parseCaipAccountId(accountId) + const sortedPermittedAccountIds = sortMultichainAccountsByLastSelected(permittedAccountIds); + const permittedAccountAddresses = sortedPermittedAccountIds.map((accountId) => { + const { address } = parseCaipAccountId(accountId); return address; - }) + }); return permittedAccountAddresses; }, isEqual); @@ -248,7 +250,7 @@ export const BrowserTab: React.FC = React.memo(({ }, []); const notifyAllConnections = useCallback((payload: unknown) => { - backgroundBridgeRef.current?.sendNotification(payload); + backgroundBridgeRef.current?.sendNotificationEip1193(payload); }, []); /** diff --git a/app/components/Views/BrowserTab/index.test.tsx b/app/components/Views/BrowserTab/index.test.tsx index ee341d53211b..6722cafae071 100644 --- a/app/components/Views/BrowserTab/index.test.tsx +++ b/app/components/Views/BrowserTab/index.test.tsx @@ -41,6 +41,9 @@ jest.mock('../../../core/Engine', () => ({ maybeUpdateState: jest.fn(), test: () => ({ result: true, name: 'test' }), }, + AccountsController: { + listMultichainAccounts: () => [] + } }, })); diff --git a/app/core/BackgroundBridge/BackgroundBridge.js b/app/core/BackgroundBridge/BackgroundBridge.js index 0948779cbbf2..02771fe44157 100644 --- a/app/core/BackgroundBridge/BackgroundBridge.js +++ b/app/core/BackgroundBridge/BackgroundBridge.js @@ -37,6 +37,7 @@ import { walletGetSession, walletInvokeMethod, walletRevokeSession, + MultichainApiNotifications, } from '@metamask/multichain-api-middleware'; import { createEngineStream } from '@metamask/json-rpc-middleware-stream'; @@ -48,7 +49,7 @@ const pump = require('pump'); const EventEmitter = require('events').EventEmitter; const { NOTIFICATION_NAMES } = AppConstants; import DevLogger from '../SDKConnect/utils/DevLogger'; -import { getCaip25Caveat, getPermittedAccounts } from '../Permissions'; +import { getCaip25Caveat, getPermittedAccounts, sortMultichainAccountsByLastSelected } from '../Permissions'; import { NetworkStatus } from '@metamask/network-controller'; import { NETWORK_ID_LOADING } from '../redux/slices/inpageProvider'; import createUnsupportedMethodMiddleware from '../RPCMethods/createUnsupportedMethodMiddleware'; @@ -60,7 +61,9 @@ import { createEip1193MethodMiddleware } from '../RPCMethods/createEip1193Method import { Caip25CaveatType, Caip25EndowmentPermissionName, + getPermittedAccountsForScopes, getSessionScopes, + KnownSessionProperties, } from '@metamask/chain-agnostic-permission'; import { makeMethodMiddlewareMaker, @@ -73,6 +76,9 @@ import { } from '../../util/permissions'; import { createMultichainMethodMiddleware } from '../RPCMethods/createMultichainMethodMiddleware'; import { getAuthorizedScopes } from '../../selectors/permissions'; +import { SolAccountType, SolScope } from '@metamask/keyring-api'; +import { uniq } from 'lodash'; +import { parseCaipAccountId } from '@metamask/utils'; const legacyNetworkId = () => { const { networksMetadata, selectedNetworkClientId } = @@ -128,6 +134,8 @@ export class BackgroundBridge extends EventEmitter { this.multichainSubscriptionManager = null; this.multichainMiddlewareManager = null; + this.lastSelectedSolanaAccountAddress = null; + const networkClientId = Engine.controllerMessenger.call( 'SelectedNetworkController:getNetworkClientIdForDomain', this.hostname, @@ -243,21 +251,12 @@ export class BackgroundBridge extends EventEmitter { if (this.isRemoteConn) { // Not sending the lock event in case of a remote connection as this is handled correctly already by the SDK - // In case we want to send, use new structure - /*const memState = this.getState(); - const selectedAddress = memState.selectedAddress; + // In case we want to send, use new structure - this.sendNotification({ - method: NOTIFICATION_NAMES.unlockStateChanged, - params: { - isUnlocked: true, - accounts: [selectedAddress], - }, - });*/ return; } - this.sendNotification({ + this.sendNotificationEip1193({ method: NOTIFICATION_NAMES.unlockStateChanged, params: true, }); @@ -269,17 +268,12 @@ export class BackgroundBridge extends EventEmitter { if (this.isRemoteConn) { // Not sending the lock event in case of a remote connection as this is handled correctly already by the SDK - // In case we want to send, use new structure - /*this.sendNotification({ - method: NOTIFICATION_NAMES.unlockStateChanged, - params: { - isUnlocked: false, - }, - });*/ + // In case we want to send, use new structure + return; } - this.sendNotification({ + this.sendNotificationEip1193({ method: NOTIFICATION_NAMES.unlockStateChanged, params: false, }); @@ -322,7 +316,7 @@ export class BackgroundBridge extends EventEmitter { async notifyChainChanged(params) { DevLogger.log(`notifyChainChanged: `, params); - this.sendNotification({ + this.sendNotificationEip1193({ method: NOTIFICATION_NAMES.chainChanged, params: params ?? (await this.getProviderNetworkState(this.origin)), }); @@ -370,7 +364,7 @@ export class BackgroundBridge extends EventEmitter { `notifySelectedAddressChanged url: ${this.url} hostname: ${this.hostname}: ${selectedAddress}`, approvedAccounts, ); - this.sendNotification({ + this.sendNotificationEip1193({ method: NOTIFICATION_NAMES.accountsChanged, params: approvedAccounts, }); @@ -432,21 +426,30 @@ export class BackgroundBridge extends EventEmitter { }; onDisconnect = () => { + const { controllerMessenger, context: {AccountsController, PermissionController} } = Engine; this.disconnected = true; - Engine.controllerMessenger.tryUnsubscribe( + controllerMessenger.tryUnsubscribe( AppConstants.NETWORK_STATE_CHANGE_EVENT, this.sendStateUpdate, ); - Engine.controllerMessenger.tryUnsubscribe( + controllerMessenger.tryUnsubscribe( 'PreferencesController:stateChange', this.sendStateUpdate, ); if (AppConstants.MULTICHAIN_API && !this.isMMSDK && !this.isWalletConnect) { - Engine.controllerMessenger.unsubscribe( - `${Engine.context.PermissionController.name}:stateChange`, + controllerMessenger.unsubscribe( + `${PermissionController.name}:stateChange`, this.handleCaipSessionScopeChanges, ); + controllerMessenger.unsubscribe( + `${PermissionController.name}:stateChange`, + this.handleSolanaAccountChangedFromScopeChanges, + ); + controllerMessenger.unsubscribe( + `${AccountsController.name}:selectedAccountChange`, + this.handleSolanaAccountChangedFromSelectedAccountChanges + ); } this.port.emit('disconnect', { name: this.port.name, data: null }); @@ -482,6 +485,14 @@ export class BackgroundBridge extends EventEmitter { engine: this.multichainEngine, }); + // This is not delayed like it is in Extension because Mobile does not have to + // support externally_connectable but instead only the faked window.postMessage + // transport. Unlike externally_connectable's chrome.runtime.connect() API, the + // window.postMessage API allows the inpage provider to setup listeners for + // messages before attempting to establish the connection meaning that it will + // have listeners ready for this solana accountChanged event below. + this.notifySolanaAccountChangedForCurrentAccount(); + pump(outStream, providerStream, outStream, (err) => { // handle any middleware cleanup this.multichainEngine.destroy(); @@ -820,14 +831,37 @@ export class BackgroundBridge extends EventEmitter { * This handles CAIP-25 authorization changes every time relevant permission state changes, for any reason. */ setupCaipEventSubscriptions() { - const { controllerMessenger, context } = Engine; + const { controllerMessenger, context: {AccountsController, PermissionController} } = Engine; + + // this throws if there is no solana account... perhaps we should handle this better at the controller level + try { + this.lastSelectedSolanaAccountAddress = + AccountsController.getSelectedMultichainAccount( + SolScope.Mainnet, + )?.address; + } catch { + // noop + } // wallet_sessionChanged and eth_subscription setup/teardown controllerMessenger.subscribe( - `${context.PermissionController.name}:stateChange`, + `${PermissionController.name}:stateChange`, this.handleCaipSessionScopeChanges, getAuthorizedScopes(this.origin), ); + + // wallet_notify for solana accountChanged when permission changes + controllerMessenger.subscribe( + `${PermissionController.name}:stateChange`, + this.handleSolanaAccountChangedFromScopeChanges, + getAuthorizedScopes(this.origin), + ); + + // wallet_notify for solana accountChanged when selected account changes + controllerMessenger.subscribe( + `${AccountsController.name}:selectedAccountChange`, + this.handleSolanaAccountChangedFromSelectedAccountChanges + ); } /** @@ -893,11 +927,122 @@ export class BackgroundBridge extends EventEmitter { this.notifyCaipAuthorizationChange(changedAuthorization); }; - sendNotification(payload) { - DevLogger.log(`BackgroundBridge::sendNotification: `, payload); + handleSolanaAccountChangedFromScopeChanges = (currentValue, previousValue) => { + const previousSolanaAccountChangedNotificationsEnabled = Boolean( + previousValue?.sessionProperties?.[ + KnownSessionProperties.SolanaAccountChangedNotifications + ], + ); + const currentSolanaAccountChangedNotificationsEnabled = Boolean( + currentValue?.sessionProperties?.[ + KnownSessionProperties.SolanaAccountChangedNotifications + ], + ); + + if ( + !previousSolanaAccountChangedNotificationsEnabled && + !currentSolanaAccountChangedNotificationsEnabled + ) { + return; + } + + const previousSolanaCaipAccountIds = previousValue + ? getPermittedAccountsForScopes(previousValue, [ + SolScope.Mainnet, + SolScope.Devnet, + SolScope.Testnet, + ]) + : []; + + + const [previousSelectedSolanaAccountId] = + sortMultichainAccountsByLastSelected( + previousSolanaCaipAccountIds, + ); + const previousSelectedSolanaAccountAddress = previousSelectedSolanaAccountId ? parseCaipAccountId(previousSelectedSolanaAccountId).address : ''; + + const currentSolanaCaipAccountIds = currentValue + ? getPermittedAccountsForScopes(currentValue, [ + SolScope.Mainnet, + SolScope.Devnet, + SolScope.Testnet, + ]) + : []; + const [currentSelectedSolanaAccountId] = + sortMultichainAccountsByLastSelected( + currentSolanaCaipAccountIds, + ); + const currentSelectedSolanaAccountAddress = currentSelectedSolanaAccountId ? parseCaipAccountId(currentSelectedSolanaAccountId).address : ''; + + if ( + previousSelectedSolanaAccountAddress !== + currentSelectedSolanaAccountAddress + ) { + this._notifySolanaAccountChange( + currentSelectedSolanaAccountAddress + ? [currentSelectedSolanaAccountAddress] + : [], + ); + } + }; + + handleSolanaAccountChangedFromSelectedAccountChanges = (account) => { + if ( + account.type === SolAccountType.DataAccount && + account.address !== this.lastSelectedSolanaAccountAddress + ) { + this.lastSelectedSolanaAccountAddress = account.address; + + + let caip25Caveat; + try { + caip25Caveat = Engine.context.PermissionController.getCaveat( + this.origin, + Caip25EndowmentPermissionName, + Caip25CaveatType, + ); + } catch { + // noop + } + if (!caip25Caveat) { + return; + } + + const shouldNotifySolanaAccountChanged = caip25Caveat.value.sessionProperties?.[KnownSessionProperties.SolanaAccountChangedNotifications]; + if (!shouldNotifySolanaAccountChanged) { + return; + } + + const solanaAccounts = getPermittedAccountsForScopes( + caip25Caveat.value, + [ + SolScope.Mainnet, + SolScope.Devnet, + SolScope.Testnet, + ], + ); + + const parsedSolanaAddresses = solanaAccounts.map((caipAccountId) => { + const { address } = parseCaipAccountId(caipAccountId); + return address; + }); + + if (parsedSolanaAddresses.includes(account.address)) { + this._notifySolanaAccountChange([account.address]); + } + } + }; + + sendNotificationEip1193(payload) { + DevLogger.log(`BackgroundBridge::sendNotificationEip1193: `, payload); this.engine && this.engine.emit('notification', payload); } + sendNotificationMultichain(payload) { + DevLogger.log(`BackgroundBridge::sendNotificationMultichain: `, payload); + this.multichainEngine && this.multichainEngine.emit('notification', payload); + } + /** * The metamask-state of the various controllers, made available to the UI * @@ -976,6 +1121,71 @@ export class BackgroundBridge extends EventEmitter { }); } } + + + /** + * For origins with a solana scope permitted, sends a wallet_notify -> metamask_accountChanged + * event to fire for the solana scope with the currently selected solana account if any are + * permitted or empty array otherwise. + * + * @param {string} origin - The origin to notify with the current solana account + */ + notifySolanaAccountChangedForCurrentAccount() { + let caip25Caveat; + try { + caip25Caveat = Engine.context.PermissionController.getCaveat( + this.origin, + Caip25EndowmentPermissionName, + Caip25CaveatType, + ); + } catch { + // noop + } + if (!caip25Caveat) { + return; + } + const solanaAccountsChangedNotifications = + caip25Caveat.value.sessionProperties[ + KnownSessionProperties.SolanaAccountChangedNotifications + ]; + + const sessionScopes = getSessionScopes(caip25Caveat.value, { + getNonEvmSupportedMethods: this.getNonEvmSupportedMethods.bind(this), + }); + + const solanaScope = + sessionScopes[SolScope.Mainnet] || + sessionScopes[SolScope.Devnet] || + sessionScopes[SolScope.Testnet]; + + if (solanaAccountsChangedNotifications && solanaScope) { + const { accounts } = solanaScope; + + const [accountIdToEmit] = sortMultichainAccountsByLastSelected( + accounts, + ); + + if (accountIdToEmit) { + const accountAddressToEmit = parseCaipAccountId(accountIdToEmit).address; + this._notifySolanaAccountChange([accountAddressToEmit]); + } + } + } + + _notifySolanaAccountChange(value) { + this.sendNotificationMultichain( + { + method: MultichainApiNotifications.walletNotify, + params: { + scope: SolScope.Mainnet, + notification: { + method: NOTIFICATION_NAMES.accountsChanged, + params: value, + }, + }, + }, + ); + } } export default BackgroundBridge; diff --git a/app/core/BackgroundBridge/BackgroundBridge.test.js b/app/core/BackgroundBridge/BackgroundBridge.test.js index 46e602c3d571..c14dd2eabeb0 100644 --- a/app/core/BackgroundBridge/BackgroundBridge.test.js +++ b/app/core/BackgroundBridge/BackgroundBridge.test.js @@ -6,6 +6,9 @@ import createEthAccountsMethodMiddleware from '../RPCMethods/createEthAccountsMe import { getPermittedAccounts } from '../Permissions'; import { getCaip25PermissionFromLegacyPermissions } from '../../util/permissions'; import AppConstants from '../../core/AppConstants'; +import { PermissionController } from '@metamask/permission-controller'; +import { Caip25CaveatType } from '@metamask/chain-agnostic-permission'; +import { EthAccountType, SolAccountType, SolScope } from '@metamask/keyring-api'; jest.mock('../../util/permissions', () => ({ getCaip25PermissionFromLegacyPermissions: jest.fn(), })); @@ -69,6 +72,20 @@ function setupBackgroundBridge(url, isMMSDK = false) { AccountsController.getSelectedAccount.mockReturnValue({ address: mockAddress, }); + AccountsController.listMultichainAccounts.mockReturnValue([ + { + address: '123', + metadata: { + lastSelected: 1 + } + }, + { + address: '456', + metadata: { + lastSelected: 2 + } + } + ]); // Setup permission controller mocks PermissionController.getPermissions.mockReturnValue({ @@ -114,7 +131,7 @@ function setupBackgroundBridge(url, isMMSDK = false) { ...defaultBridgeParams, }); - // Setup the engine property to support sendNotification + // Setup the engine property to support sendNotificationEip1193 bridge.engine = { emit: jest.fn() }; @@ -123,10 +140,11 @@ function setupBackgroundBridge(url, isMMSDK = false) { } describe('BackgroundBridge', () => { + const { KeyringController, PermissionController } = Engine.context; + beforeEach(() => jest.clearAllMocks()); - describe('constructor', () => { - const { KeyringController, PermissionController } = Engine.context; + describe('constructor', () => { it('creates Eip1193MethodMiddleware with expected hooks', async () => { const url = 'https:www.mock.io'; const origin = new URL(url).hostname; @@ -229,7 +247,7 @@ describe('BackgroundBridge', () => { // Verify the spy was called with the correct URL expect(getProviderSpy).toHaveBeenCalledWith(new URL(url).hostname); expect(mmGetProviderSpy).toHaveBeenCalledWith(mmBridge.channelId); - }) + }); it('notifies of chain changes when network state is updated', async () => { // Create the new network state with a different chain @@ -244,8 +262,8 @@ describe('BackgroundBridge', () => { }; const url = 'https:www.mock.io'; const bridge = setupBackgroundBridge(url); - const sendNotificationSpy = jest.spyOn(bridge, 'sendNotification'); - const getProviderSpy = jest.spyOn(bridge, 'getProviderNetworkState') + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationEip1193'); + const getProviderSpy = jest.spyOn(bridge, 'getProviderNetworkState'); expect(bridge.lastChainIdSent).toBe(oldMockNetworkState.chainId); expect(bridge.networkVersionSent).toBe(oldMockNetworkState.networkVersion); @@ -275,4 +293,436 @@ describe('BackgroundBridge', () => { }); }); }); + + describe('notifySolanaAccountChangedForCurrentAccount', () => { + it('emits nothing if there is no CAIP-25 permission' , () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + + bridge.notifySolanaAccountChangedForCurrentAccount(); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + + }); + + it('emits nothing if there are no permitted solana scopes and `solana_accountChanged_notifications` session property is set' , () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + 'eip155:1': { + accounts: [] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + } + }); + + bridge.notifySolanaAccountChangedForCurrentAccount(); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits nothing if there are permitted solana accounts, but the `solana_accountChanged_notifications` session property is not set' , () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:someaddress` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: {} + } + }); + + bridge.notifySolanaAccountChangedForCurrentAccount(); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits nothing if there are permitted solana scopes but no accounts and the `solana_accountChanged_notifications` session property is set', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + } + }); + + bridge.notifySolanaAccountChangedForCurrentAccount(); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits a solana accountChanged event when there are permitted solana accounts and the `solana_accountChanged_notifications` session property is set', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:someaddress` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + } + }); + + bridge.notifySolanaAccountChangedForCurrentAccount(); + + expect(sendNotificationSpy).toHaveBeenCalledWith({ + method: 'wallet_notify', + params: { + notification: { + method: 'metamask_accountsChanged', + params: [ + 'someaddress', + ], + }, + scope: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + }, + }); + }); + }); + + describe('handleSolanaAccountChangedFromScopeChanges', () => { + it('emits nothing if the current and previous permissions both did not have `solana_accountChanged_notifications` session property set', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + + const currentValue = { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:456` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: {} + }; + + const previousValue = { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:123` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: {} + }; + + bridge.handleSolanaAccountChangedFromScopeChanges(currentValue, previousValue); + + expect(sendNotificationSpy).not.toHaveBeenCalledWith(); + }); + + it('emits nothing if currently and previously selected solana accounts did not change', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + + const currentValue = { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:123` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + }; + + const previousValue = { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:123` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + }; + + bridge.handleSolanaAccountChangedFromScopeChanges(currentValue, previousValue); + + expect(sendNotificationSpy).not.toHaveBeenCalledWith(); + }); + + it('emits the currently selected solana account if the currently selected solana accounts did change', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + + const currentValue = { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:456` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + }; + + const previousValue = { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:123` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + }; + + bridge.handleSolanaAccountChangedFromScopeChanges(currentValue, previousValue); + + expect(sendNotificationSpy).toHaveBeenCalledWith({ + method: 'wallet_notify', + params: { + notification: { + method: 'metamask_accountsChanged', + params: [ + '456', + ], + }, + scope: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + }, + }); + }); + }); + + describe('handleSolanaAccountChangedFromSelectedAccountChanges', () => { + it('emits nothing if the selected account is not a solana account', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:someaddress` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + } + }); + + bridge.handleSolanaAccountChangedFromSelectedAccountChanges({ + type: EthAccountType.Eoa, + address: 'someaddress' + }); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits nothing if the selected account did not change from the last seen solana account', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:someaddress` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + } + }); + bridge.lastSelectedSolanaAccountAddress = 'someaddress'; + + bridge.handleSolanaAccountChangedFromSelectedAccountChanges({ + type: SolAccountType.DataAccount, + address: 'someaddress' + }); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits nothing if there is no CAIP-25 permission' , () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue(); + + bridge.handleSolanaAccountChangedFromSelectedAccountChanges({ + type: SolAccountType.DataAccount, + address: 'someaddress' + }); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits nothing if the `solana_accountChanged_notifications` session property is not set' , () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:someaddress` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: {} + } + }); + + bridge.handleSolanaAccountChangedFromSelectedAccountChanges({ + type: SolAccountType.DataAccount, + address: 'someaddress' + }); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits nothing if the selected account does not match a permitted solana account' , () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:someaddress` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + } + }); + + bridge.handleSolanaAccountChangedFromSelectedAccountChanges({ + type: SolAccountType.DataAccount, + address: 'differentaddress' + }); + + expect(sendNotificationSpy).not.toHaveBeenCalled(); + }); + + it('emits a solana accountChanged event for the selected account if it does match a permitted solana account', () => { + const url = 'https:www.mock.io'; + const bridge = setupBackgroundBridge(url); + const sendNotificationSpy = jest.spyOn(bridge, 'sendNotificationMultichain'); + PermissionController.getCaveat.mockReturnValue({ + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: { + [SolScope.Mainnet]: { + accounts: [ + `${SolScope.Mainnet}:someaddress` + ] + } + }, + isMultichainOrigin: true, + sessionProperties: { + solana_accountChanged_notifications: true + } + } + }); + + bridge.handleSolanaAccountChangedFromSelectedAccountChanges({ + type: SolAccountType.DataAccount, + address: 'someaddress' + }); + + expect(sendNotificationSpy).toHaveBeenCalledWith({ + method: 'wallet_notify', + params: { + notification: { + method: 'metamask_accountsChanged', + params: [ + 'someaddress', + ], + }, + scope: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + }, + }); + }); + }); }); diff --git a/app/core/Permissions/index.test.ts b/app/core/Permissions/index.test.ts index b1f2455cfbea..cfb7516bf499 100644 --- a/app/core/Permissions/index.test.ts +++ b/app/core/Permissions/index.test.ts @@ -26,7 +26,8 @@ import { removePermittedAccounts, removeAccountsFromPermissions, updatePermittedChains, - sortAccountsByLastSelected, + sortEvmAccountsByLastSelected, + sortMultichainAccountsByLastSelected, getPermittedAccounts, removePermittedChain, } from '.'; @@ -52,6 +53,8 @@ const mockGetCaveat = Engine.context.PermissionController .getCaveat as jest.Mock; const mockListAccounts = Engine.context.AccountsController .listAccounts as jest.Mock; +const mockListMultichainAccounts = Engine.context.AccountsController + .listMultichainAccounts as jest.Mock; const mockGetAccountByAddress = Engine.context.AccountsController .getAccountByAddress as jest.Mock; const mockIsUnlocked = Engine.context.KeyringController.isUnlocked as jest.Mock; @@ -152,8 +155,8 @@ describe('Permission Utility Functions', () => { }, }; - // Mock sortAccountsByLastSelected to return empty array - jest.spyOn(permissions, 'sortAccountsByLastSelected').mockReturnValue([]); + // Mock sortEvmAccountsByLastSelected to return empty array + jest.spyOn(permissions, 'sortEvmAccountsByLastSelected').mockReturnValue([]); const result = getPermittedEvmAddressesByHostname( mockState, @@ -250,8 +253,8 @@ describe('Permission Utility Functions', () => { }, }; - // Mock sortAccountsByLastSelected to return empty array - jest.spyOn(permissions, 'sortAccountsByLastSelected').mockReturnValue([]); + // Mock sortEvmAccountsByLastSelected to return empty array + jest.spyOn(permissions, 'sortEvmAccountsByLastSelected').mockReturnValue([]); const result = getPermittedCaipAccountIdsByHostname( mockState, @@ -467,7 +470,7 @@ describe('Permission Utility Functions', () => { // The updated accounts would be here in the real implementation }); - addPermittedAccounts('https://example.com', newAccounts, {}, {}); + addPermittedAccounts('https://example.com', newAccounts); expect(setNonSCACaipAccountIdsInCaip25CaveatValue).toHaveBeenCalledWith( mockCaveat.value, @@ -487,7 +490,7 @@ describe('Permission Utility Functions', () => { mockGetCaveat.mockReturnValue(undefined); expect(() => - addPermittedAccounts('https://example.com', ['eip155:0:0x1'], {}, {}), + addPermittedAccounts('https://example.com', ['eip155:0:0x1']), ).toThrow( 'Cannot add account permissions for origin "https://example.com": no permission currently exists for this origin.', ); @@ -805,7 +808,7 @@ describe('Permission Utility Functions', () => { }); }); - describe('sortAccountsByLastSelected', () => { + describe('sortEvmAccountsByLastSelected', () => { it('should sort accounts by lastSelected timestamp', () => { const accounts: Hex[] = ['0x1', '0x2', '0x3']; const internalAccounts = [ @@ -825,7 +828,7 @@ describe('Permission Utility Functions', () => { mockListAccounts.mockReturnValue(internalAccounts); - const result = sortAccountsByLastSelected(accounts); + const result = sortEvmAccountsByLastSelected(accounts); expect(result).toEqual(['0x2', '0x3', '0x1']); }); @@ -848,7 +851,7 @@ describe('Permission Utility Functions', () => { mockListAccounts.mockReturnValue(internalAccounts); - const result = sortAccountsByLastSelected(accounts); + const result = sortEvmAccountsByLastSelected(accounts); expect(result).toEqual(['0x3', '0x1', '0x2']); }); @@ -871,7 +874,7 @@ describe('Permission Utility Functions', () => { mockListAccounts.mockReturnValue(internalAccounts); - const result = sortAccountsByLastSelected(accounts); + const result = sortEvmAccountsByLastSelected(accounts); // We don't assert the exact order for accounts with the same lastSelected value expect(result).toContain('0x1'); expect(result).toContain('0x2'); @@ -898,7 +901,7 @@ describe('Permission Utility Functions', () => { Engine.context.KeyringController.getAccountKeyringType as jest.Mock ).mockResolvedValue('Simple Key Pair'); - expect(() => sortAccountsByLastSelected(accounts)).toThrow( + expect(() => sortEvmAccountsByLastSelected(accounts)).toThrow( 'Missing identity for address: "0x2".', ); expect(captureException).toHaveBeenCalled(); @@ -923,11 +926,148 @@ describe('Permission Utility Functions', () => { mockListAccounts.mockReturnValue(internalAccounts); - const result = sortAccountsByLastSelected(accounts); + const result = sortEvmAccountsByLastSelected(accounts); expect(result).toEqual(['0x2', '0x3', '0x1']); }); }); + describe('sortMultichainAccountsByLastSelected', () => { + it('should sort accounts by lastSelected timestamp', () => { + const accounts: CaipAccountId[] = ['eip155:0:0x1', 'eip155:0:0x2', 'eip155:0:0x3']; + const internalAccounts = [ + { + address: '0x1', + scopes: ['eip155:0'], + metadata: { lastSelected: 100 }, + }, + { + address: '0x2', + scopes: ['eip155:0'], + metadata: { lastSelected: 300 }, + }, + { + address: '0x3', + scopes: ['eip155:0'], + metadata: { lastSelected: 200 }, + }, + ]; + + mockListMultichainAccounts.mockReturnValue(internalAccounts); + + const result = sortMultichainAccountsByLastSelected(accounts); + expect(result).toEqual(['eip155:0:0x2', 'eip155:0:0x3', 'eip155:0:0x1']); + }); + + it('should handle accounts with undefined lastSelected', () => { + const accounts: CaipAccountId[] = ['eip155:0:0x1', 'eip155:0:0x2', 'eip155:0:0x3']; + const internalAccounts = [ + { + address: '0x1', + scopes: ['eip155:0'], + metadata: { lastSelected: 100 }, + }, + { + address: '0x2', + scopes: ['eip155:0'], + metadata: { lastSelected: undefined }, + }, + { + address: '0x3', + scopes: ['eip155:0'], + metadata: { lastSelected: 200 }, + }, + ]; + + mockListMultichainAccounts.mockReturnValue(internalAccounts); + + const result = sortMultichainAccountsByLastSelected(accounts); + expect(result).toEqual(['eip155:0:0x3', 'eip155:0:0x1', 'eip155:0:0x2']); + }); + + it('should handle accounts with same lastSelected value', () => { + const accounts: CaipAccountId[] = ['eip155:0:0x1', 'eip155:0:0x2', 'eip155:0:0x3']; + const internalAccounts = [ + { + address: '0x1', + scopes: ['eip155:0'], + metadata: { lastSelected: 100 }, + }, + { + address: '0x2', + scopes: ['eip155:0'], + metadata: { lastSelected: 100 }, + }, + { + address: '0x3', + scopes: ['eip155:0'], + metadata: { lastSelected: 200 }, + }, + ]; + + mockListMultichainAccounts.mockReturnValue(internalAccounts); + + const result = sortMultichainAccountsByLastSelected(accounts); + // We don't assert the exact order for accounts with the same lastSelected value + expect(result).toContain('eip155:0:0x1'); + expect(result).toContain('eip155:0:0x2'); + expect(result).toContain('eip155:0:0x3'); + expect(result[0]).toBe('eip155:0:0x3'); // The one with highest lastSelected should be first + }); + + it('should throw error if account is missing from identities', () => { + const accounts: CaipAccountId[] = ['eip155:0:0x1', 'eip155:0:0x2', 'eip155:0:0x3']; + const internalAccounts = [ + { + address: '0x1', + scopes: ['eip155:0'], + metadata: { lastSelected: 100 }, + }, + // 0x2 is missing + { + address: '0x3', + scopes: ['eip155:0'], + metadata: { lastSelected: 200 }, + }, + ]; + + mockListMultichainAccounts.mockReturnValue(internalAccounts); + ( + Engine.context.KeyringController.getAccountKeyringType as jest.Mock + ).mockResolvedValue('Simple Key Pair'); + + expect(() => sortMultichainAccountsByLastSelected(accounts)).toThrow( + 'Missing identity for address: "eip155:0:0x2".', + ); + expect(captureException).toHaveBeenCalled(); + }); + + it('should handle case insensitive address comparison', () => { + const accounts: CaipAccountId[] = ['eip155:0:0x1', 'eip155:0:0x2', 'eip155:0:0x3']; + const internalAccounts = [ + { + address: '0X1', // Uppercase + scopes: ['eip155:0'], + metadata: { lastSelected: 100 }, + }, + { + address: '0x2', + scopes: ['eip155:0'], + metadata: { lastSelected: 300 }, + }, + { + address: '0x3', + scopes: ['eip155:0'], + metadata: { lastSelected: 200 }, + }, + ]; + + mockListMultichainAccounts.mockReturnValue(internalAccounts); + + const result = sortMultichainAccountsByLastSelected(accounts); + expect(result).toEqual(['eip155:0:0x2', 'eip155:0:0x3', 'eip155:0:0x1']); + }); + }); + describe('getPermittedAccounts', () => { it('should return selected account for internal origins', () => { const selectedAccount = { @@ -971,7 +1111,7 @@ describe('Permission Utility Functions', () => { (getEthAccounts as jest.Mock).mockReturnValue(ethAccounts); jest - .spyOn(permissions, 'sortAccountsByLastSelected') + .spyOn(permissions, 'sortEvmAccountsByLastSelected') .mockReturnValue(sortedAccounts); const result = getPermittedAccounts('https://example.com'); @@ -1020,9 +1160,9 @@ describe('Permission Utility Functions', () => { mockIsUnlocked.mockReturnValue(false); (getEthAccounts as jest.Mock).mockReturnValue(ethAccounts); - // Mock sortAccountsByLastSelected + // Mock sortEvmAccountsByLastSelected jest - .spyOn(permissions, 'sortAccountsByLastSelected') + .spyOn(permissions, 'sortEvmAccountsByLastSelected') .mockReturnValue(sortedAccounts); const result = getPermittedAccounts('https://example.com', { diff --git a/app/core/Permissions/index.ts b/app/core/Permissions/index.ts index b39c71171edc..92211de26442 100644 --- a/app/core/Permissions/index.ts +++ b/app/core/Permissions/index.ts @@ -28,8 +28,6 @@ import { } from '@metamask/permission-controller'; import { captureException } from '@sentry/react-native'; import { getNetworkConfigurationsByCaipChainId } from '../../selectors/networkController'; -import { NetworkConfiguration } from '@metamask/network-controller'; -import { MultichainNetworkConfiguration } from '@metamask/multichain-network-controller'; const INTERNAL_ORIGINS = [process.env.MM_FOX_CODE, TransactionTypes.MMM]; @@ -42,11 +40,11 @@ const Engine = ImportedEngine as any; * an error to sentry for any accounts that were expected but are missing from the wallet. * * @param [internalAccounts] - The list of evm accounts the wallet knows about. - * @param [accounts] - The list of evm accounts addresses that should exist. + * @param [accounts] - The list of accounts addresses that should exist. */ const captureKeyringTypesWithMissingIdentities = ( internalAccounts: InternalAccount[] = [], - accounts: Hex[] = [], + accounts: string[] = [], ) => { const accountsMissingIdentities = accounts.filter( (address) => @@ -73,17 +71,11 @@ const captureKeyringTypesWithMissingIdentities = ( }; /** - * Sorts a list of evm account addresses by most recently selected by using - * the lastSelected value for the matching InternalAccount object stored in state. - * - * @param accounts - The list of evm accounts addresses to sort. - * @returns The sorted evm accounts addresses. + * Sorts a list of addresses by most recently selected by using the lastSelected value for + * the matching InternalAccount object from the list of internalAccounts provided. */ -export const sortAccountsByLastSelected = (accounts: Hex[]) => { - const internalAccounts: InternalAccount[] = - Engine.context.AccountsController.listAccounts(); - - return accounts.sort((firstAddress, secondAddress) => { +const sortAddressesWithInternalAccounts = (addresses: T[], internalAccounts: InternalAccount[]): T[] => + [...addresses].sort((firstAddress, secondAddress) => { const firstAccount = internalAccounts.find( (internalAccount) => internalAccount.address.toLowerCase() === firstAddress.toLowerCase(), @@ -95,13 +87,73 @@ export const sortAccountsByLastSelected = (accounts: Hex[]) => { ); if (!firstAccount) { - captureKeyringTypesWithMissingIdentities(internalAccounts, accounts); + captureKeyringTypesWithMissingIdentities( + internalAccounts, + addresses, + ); throw new Error(`Missing identity for address: "${firstAddress}".`); } else if (!secondAccount) { - captureKeyringTypesWithMissingIdentities(internalAccounts, accounts); + captureKeyringTypesWithMissingIdentities( + internalAccounts, + addresses, + ); throw new Error(`Missing identity for address: "${secondAddress}".`); } else if ( - firstAccount.metadata.lastSelected === secondAccount.metadata.lastSelected + firstAccount.metadata.lastSelected === + secondAccount.metadata.lastSelected + ) { + return 0; + } else if (firstAccount.metadata.lastSelected === undefined) { + return 1; + } else if (secondAccount.metadata.lastSelected === undefined) { + return -1; + } + + return ( + secondAccount.metadata.lastSelected - firstAccount.metadata.lastSelected + ); + }); + +/** + * Sorts a list of evm account addresses by most recently selected by using + * the lastSelected value for the matching InternalAccount object stored in state. + */ +export const sortEvmAccountsByLastSelected = (addresses: Hex[]): Hex[] => { + const internalAccounts = Engine.context.AccountsController.listAccounts(); + return sortAddressesWithInternalAccounts(addresses, internalAccounts); +}; + +/** + * Sorts a list of caip account id by most recently selected by using the lastSelected value for + * the matching InternalAccount object from the list of internalAccounts provided. + */ +const sortCaipAccountIdsWithInternalAccounts = (caipAccountIds: CaipAccountId[], internalAccounts: InternalAccount[]): CaipAccountId[] => + [...caipAccountIds].sort((firstAccountId, secondAccountId) => { + const firstAccount = internalAccounts.find( + (internalAccount) => + isInternalAccountInPermittedAccountIds(internalAccount, [firstAccountId]) + ); + + const secondAccount = internalAccounts.find( + (internalAccount) => + isInternalAccountInPermittedAccountIds(internalAccount, [secondAccountId]) + ); + + if (!firstAccount) { + captureKeyringTypesWithMissingIdentities( + internalAccounts, + caipAccountIds, + ); + throw new Error(`Missing identity for address: "${firstAccountId}".`); + } else if (!secondAccount) { + captureKeyringTypesWithMissingIdentities( + internalAccounts, + caipAccountIds, + ); + throw new Error(`Missing identity for address: "${secondAccountId}".`); + } else if ( + firstAccount.metadata.lastSelected === + secondAccount.metadata.lastSelected ) { return 0; } else if (firstAccount.metadata.lastSelected === undefined) { @@ -114,6 +166,14 @@ export const sortAccountsByLastSelected = (accounts: Hex[]) => { secondAccount.metadata.lastSelected - firstAccount.metadata.lastSelected ); }); + +/** + * Sorts a list of multichain account ids by most recently selected by using + * the lastSelected value for the matching InternalAccount object stored in state. + */ +export const sortMultichainAccountsByLastSelected = (caipAccountIds: CaipAccountId[]) => { + const internalAccounts = Engine.context.AccountsController.listMultichainAccounts(); + return sortCaipAccountIdsWithInternalAccounts(caipAccountIds, internalAccounts); }; // TODO: Replace "any" with type @@ -158,7 +218,7 @@ function getEvmAddessesFromSubject(subject: any) { ); if (caveat) { const ethAccounts = getEthAccounts(caveat.value); - return sortAccountsByLastSelected(ethAccounts); + return sortEvmAccountsByLastSelected(ethAccounts); } return []; @@ -257,8 +317,6 @@ export const getCaip25Caveat = (origin: string) => { export const addPermittedAccounts = ( origin: string, accounts: CaipAccountId[], - evmNetworkConfigurationsByChainId: Record, - nonEvmNetworkConfigurationsByChainId: Record, ) => { const caip25Caveat = getCaip25Caveat(origin); if (!caip25Caveat) { @@ -281,6 +339,8 @@ export const addPermittedAccounts = ( let updatedPermittedChainIds = [...existingPermittedChainIds]; + const evmNetworkConfigurationsByChainId = Engine.context.NetworkController.state.networkConfigurationsByChainId; + const nonEvmNetworkConfigurationsByChainId = Engine.context.MultichainNetworkController.state.multichainNetworkConfigurationsByChainId; const networkConfigurations = getNetworkConfigurationsByCaipChainId(evmNetworkConfigurationsByChainId, nonEvmNetworkConfigurationsByChainId); const allNetworksList = Object.keys(networkConfigurations) as CaipChainId[]; @@ -535,7 +595,7 @@ export const getPermittedAccounts = ( } const ethAccounts = getEthAccounts(caveat.value); - return sortAccountsByLastSelected(ethAccounts); + return sortEvmAccountsByLastSelected(ethAccounts); }; /** diff --git a/app/core/__mocks__/MockedEngine.ts b/app/core/__mocks__/MockedEngine.ts index 735e1965af0a..d0fa6b11f5a9 100644 --- a/app/core/__mocks__/MockedEngine.ts +++ b/app/core/__mocks__/MockedEngine.ts @@ -30,6 +30,7 @@ export const mockedEngine = { context: { AccountsController: { listAccounts: jest.fn(), + listMultichainAccounts: jest.fn(), getSelectedAccount: jest.fn(), getAccountByAddress: jest.fn(), }, diff --git a/app/util/accounts/index.ts b/app/util/accounts/index.ts index bf104955449e..e8f90bdb12e5 100644 --- a/app/util/accounts/index.ts +++ b/app/util/accounts/index.ts @@ -1,33 +1,39 @@ // External dependencies. +import { isCaipAccountIdInPermittedAccountIds } from '@metamask/chain-agnostic-permission'; import { Account, EnsByAccountAddress, } from '../../components/hooks/useAccounts'; -import { safeToChecksumAddress } from '../address'; import { isDefaultAccountName } from '../ENSUtils'; +import { CaipAccountId, KnownCaipNamespace, parseCaipAccountId } from '@metamask/utils'; /** * Gets the Account nickname, ENS name, or default account name - Whichever one is available. * - * @param params.accountAddress - Address of the account. + * @param params.caipAccountId - Caip Account ID of the account. * @param params.accounts - Array of accounts returned from useAccounts hook. * @param params.ensByAccountAddress - ENS name map returned from useAccounts hook. * @returns - Account nickname, ENS name, or default account name. */ export const getAccountNameWithENS = ({ - accountAddress, + caipAccountId, accounts, ensByAccountAddress, }: { - accountAddress: string; + caipAccountId: CaipAccountId; accounts: Account[]; ensByAccountAddress: EnsByAccountAddress; }) => { const account = accounts.find( - ({ address }) => - safeToChecksumAddress(address) === safeToChecksumAddress(accountAddress), + (acc) => + isCaipAccountIdInPermittedAccountIds( + acc.caipAccountId, + [caipAccountId] + ) ); - const ensName = ensByAccountAddress[accountAddress]; + + const {address, chain: {namespace}} = parseCaipAccountId(caipAccountId); + const ensName = namespace === KnownCaipNamespace.Eip155 ? ensByAccountAddress[address] : undefined; return isDefaultAccountName(account?.name) && ensName ? ensName : account?.name || '';