From 709b64c128de74dbc31166c28c47c6232ce5ae01 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Thu, 12 Jan 2023 13:05:48 -0600 Subject: [PATCH] Remove dead addressbook migration code (#17127) * remove dead address book migration code * cleanup * addToFrequentRpcList -> upsertToFrequentRpcList * remove migrateAddressBookState function --- app/scripts/controllers/preferences.js | 89 +-------------------- app/scripts/controllers/preferences.test.js | 41 +--------- app/scripts/metamask-controller.js | 48 ++--------- 3 files changed, 13 insertions(+), 165 deletions(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index cd85599f5..555af2c41 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -1,12 +1,7 @@ -import { strict as assert } from 'assert'; import { ObservableStore } from '@metamask/obs-store'; import { normalize as normalizeAddress } from 'eth-sig-util'; import { ethers } from 'ethers'; -import log from 'loglevel'; -import { - IPFS_DEFAULT_GATEWAY_URL, - BUILT_IN_NETWORKS, -} from '../../../shared/constants/network'; +import { IPFS_DEFAULT_GATEWAY_URL } from '../../../shared/constants/network'; import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils'; import { LEDGER_TRANSPORT_TYPES } from '../../../shared/constants/hardware-wallets'; import { THEME_TYPE } from '../../../ui/pages/settings/settings-tab/settings-tab.constant'; @@ -81,7 +76,6 @@ export default class PreferencesController { this.store = new ObservableStore(initState); this.store.setMaxListeners(12); this.openPopup = opts.openPopup; - this.migrateAddressBookState = opts.migrateAddressBookState; this.tokenListController = opts.tokenListController; this._subscribeToInfuraAvailability(); @@ -395,82 +389,6 @@ export default class PreferencesController { return Promise.resolve(label); } - /** - * updates custom RPC details - * - * @param {object} newRpcDetails - Options bag. - * @param {string} newRpcDetails.rpcUrl - The RPC url to add to frequentRpcList. - * @param {string} newRpcDetails.chainId - The chainId of the selected network. - * @param {string} [newRpcDetails.ticker] - Optional ticker symbol of the selected network. - * @param {string} [newRpcDetails.nickname] - Optional nickname of the selected network. - * @param {object} [newRpcDetails.rpcPrefs] - Optional RPC preferences, such as the block explorer URL - */ - async updateRpc(newRpcDetails) { - const rpcList = this.getFrequentRpcListDetail(); - const index = rpcList.findIndex((element) => { - return element.rpcUrl === newRpcDetails.rpcUrl; - }); - if (index > -1) { - const rpcDetail = rpcList[index]; - const updatedRpc = { ...rpcDetail, ...newRpcDetails }; - if (rpcDetail.chainId !== updatedRpc.chainId) { - // When the chainId is changed, associated address book entries should - // also be migrated. The address book entries are keyed by the `network` state, - // which for custom networks is the chainId with a fallback to the networkId - // if the chainId is not set. - - let addressBookKey = rpcDetail.chainId; - if (!addressBookKey) { - // We need to find the networkId to determine what these addresses were keyed by - try { - addressBookKey = await this.ethersProvider.send('net_version'); - assert(typeof addressBookKey === 'string'); - } catch (error) { - log.debug(error); - log.warn( - `Failed to get networkId from ${rpcDetail.rpcUrl}; skipping address book migration`, - ); - } - } - - // There is an edge case where two separate RPC endpoints are keyed by the same - // value. In this case, the contact book entries are duplicated so that they remain - // on both networks, since we don't know which network each contact is intended for. - - let duplicate = false; - const builtInProviderNetworkIds = Object.values(BUILT_IN_NETWORKS).map( - (ids) => ids.networkId, - ); - const otherRpcEntries = rpcList.filter( - (entry) => entry.rpcUrl !== newRpcDetails.rpcUrl, - ); - if ( - builtInProviderNetworkIds.includes(addressBookKey) || - otherRpcEntries.some((entry) => entry.chainId === addressBookKey) - ) { - duplicate = true; - } - - this.migrateAddressBookState( - addressBookKey, - updatedRpc.chainId, - duplicate, - ); - } - rpcList[index] = updatedRpc; - this.store.updateState({ frequentRpcListDetail: rpcList }); - } else { - const { - rpcUrl, - chainId, - ticker, - nickname, - rpcPrefs = {}, - } = newRpcDetails; - this.addToFrequentRpcList(rpcUrl, chainId, ticker, nickname, rpcPrefs); - } - } - /** * Adds custom RPC url to state. * @@ -480,7 +398,7 @@ export default class PreferencesController { * @param {string} [nickname] - Nickname of the selected network. * @param {object} [rpcPrefs] - Optional RPC preferences, such as the block explorer URL */ - addToFrequentRpcList( + upsertToFrequentRpcList( rpcUrl, chainId, ticker = 'ETH', @@ -493,7 +411,8 @@ export default class PreferencesController { return element.rpcUrl === rpcUrl; }); if (index !== -1) { - rpcList.splice(index, 1); + rpcList.splice(index, 1, { rpcUrl, chainId, ticker, nickname, rpcPrefs }); + return; } if (!isPrefixedFormattedHexString(chainId)) { diff --git a/app/scripts/controllers/preferences.test.js b/app/scripts/controllers/preferences.test.js index 97bea8e56..079bac240 100644 --- a/app/scripts/controllers/preferences.test.js +++ b/app/scripts/controllers/preferences.test.js @@ -12,7 +12,6 @@ describe('preferences controller', function () { let currentChainId; let provider; let tokenListController; - const migrateAddressBookState = sinon.stub(); beforeEach(function () { const sandbox = sinon.createSandbox(); @@ -44,7 +43,6 @@ describe('preferences controller', function () { preferencesController = new PreferencesController({ initLangCode: 'en_US', - migrateAddressBookState, network, provider, tokenListController, @@ -180,40 +178,9 @@ describe('preferences controller', function () { }); }); - describe('#updateRpc', function () { - it('should update the rpcDetails properly', async function () { - preferencesController.store.updateState({ - frequentRpcListDetail: [{}, { rpcUrl: 'test', chainId: '0x1' }, {}], - }); - await preferencesController.updateRpc({ rpcUrl: 'test', chainId: '0x1' }); - await preferencesController.updateRpc({ - rpcUrl: 'test/1', - chainId: '0x1', - }); - await preferencesController.updateRpc({ - rpcUrl: 'test/2', - chainId: '0x1', - }); - await preferencesController.updateRpc({ - rpcUrl: 'test/3', - chainId: '0x1', - }); - const list = preferencesController.getFrequentRpcListDetail(); - assert.deepEqual(list[1], { rpcUrl: 'test', chainId: '0x1' }); - }); - - it('should migrate address book entries if chainId changes', async function () { - preferencesController.store.updateState({ - frequentRpcListDetail: [{}, { rpcUrl: 'test', chainId: '1' }, {}], - }); - await preferencesController.updateRpc({ rpcUrl: 'test', chainId: '0x1' }); - assert(migrateAddressBookState.calledWith('1', '0x1')); - }); - }); - describe('adding and removing from frequentRpcListDetail', function () { it('should add custom RPC url to state', function () { - preferencesController.addToFrequentRpcList('rpc_url', '0x1'); + preferencesController.upsertToFrequentRpcList('rpc_url', '0x1'); assert.deepEqual( preferencesController.store.getState().frequentRpcListDetail, [ @@ -226,7 +193,7 @@ describe('preferences controller', function () { }, ], ); - preferencesController.addToFrequentRpcList('rpc_url', '0x1'); + preferencesController.upsertToFrequentRpcList('rpc_url', '0x1'); assert.deepEqual( preferencesController.store.getState().frequentRpcListDetail, [ @@ -243,12 +210,12 @@ describe('preferences controller', function () { it('should throw if chainId is invalid', function () { assert.throws(() => { - preferencesController.addToFrequentRpcList('rpc_url', '1'); + preferencesController.upsertToFrequentRpcList('rpc_url', '1'); }, 'should throw on invalid chainId'); }); it('should remove custom RPC url from state', function () { - preferencesController.addToFrequentRpcList('rpc_url', '0x1'); + preferencesController.upsertToFrequentRpcList('rpc_url', '0x1'); assert.deepEqual( preferencesController.store.getState().frequentRpcListDetail, [ diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 3f8dc7ef9..5ab6bad7e 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -285,7 +285,6 @@ export default class MetamaskController extends EventEmitter { network: this.networkController, tokenListController: this.tokenListController, provider: this.provider, - migrateAddressBookState: this.migrateAddressBookState.bind(this), }); this.tokensController = new TokensController({ @@ -2136,7 +2135,7 @@ export default class MetamaskController extends EventEmitter { async addCustomNetwork(customRpc, actionId) { const { chainId, chainName, rpcUrl, ticker, blockExplorerUrl } = customRpc; - this.preferencesController.addToFrequentRpcList( + this.preferencesController.upsertToFrequentRpcList( rpcUrl, chainId, ticker, @@ -3833,7 +3832,7 @@ export default class MetamaskController extends EventEmitter { chainName, rpcUrl, } = {}) => { - await this.preferencesController.addToFrequentRpcList( + await this.preferencesController.upsertToFrequentRpcList( rpcUrl, chainId, ticker, @@ -4210,43 +4209,6 @@ export default class MetamaskController extends EventEmitter { return nonceLock.nextNonce; } - /** - * Migrate address book state from old to new chainId. - * - * Address book state is keyed by the `networkStore` state from the network controller. This value is set to the - * `networkId` for our built-in Infura networks, but it's set to the `chainId` for custom networks. - * When this `chainId` value is changed for custom RPC endpoints, we need to migrate any contacts stored under the - * old key to the new key. - * - * The `duplicate` parameter is used to specify that the contacts under the old key should not be removed. This is - * useful in the case where two RPC endpoints shared the same set of contacts, and we're not sure which one each - * contact belongs under. Duplicating the contacts under both keys is the only way to ensure they are not lost. - * - * @param {string} oldChainId - The old chainId - * @param {string} newChainId - The new chainId - * @param {boolean} [duplicate] - Whether to duplicate the addresses on both chainIds (default: false) - */ - async migrateAddressBookState(oldChainId, newChainId, duplicate = false) { - const { addressBook } = this.addressBookController.state; - - if (!addressBook[oldChainId]) { - return; - } - - for (const address of Object.keys(addressBook[oldChainId])) { - const entry = addressBook[oldChainId][address]; - this.addressBookController.set( - address, - entry.name, - newChainId, - entry.memo, - ); - if (!duplicate) { - this.addressBookController.delete(oldChainId, address); - } - } - } - //============================================================================= // CONFIG //============================================================================= @@ -4278,13 +4240,13 @@ export default class MetamaskController extends EventEmitter { nickname, rpcPrefs, ); - await this.preferencesController.updateRpc({ + await this.preferencesController.upsertToFrequentRpcList( rpcUrl, chainId, ticker, nickname, rpcPrefs, - }); + ); return rpcUrl; } @@ -4327,7 +4289,7 @@ export default class MetamaskController extends EventEmitter { nickname, rpcPrefs, ); - await this.preferencesController.addToFrequentRpcList( + await this.preferencesController.upsertToFrequentRpcList( rpcUrl, chainId, ticker,