diff --git a/app/scripts/migrations/048.js b/app/scripts/migrations/048.js index c6e4c5fdd..070b12673 100644 --- a/app/scripts/migrations/048.js +++ b/app/scripts/migrations/048.js @@ -10,6 +10,7 @@ const version = 48 * 3. Add localhost network to frequentRpcListDetail. * 4. Delete CachedBalancesController.cachedBalances * 5. Convert transactions metamaskNetworkId to decimal if they are hex + * 6. Convert address book keys from decimal to hex */ export default { version, @@ -87,5 +88,78 @@ function transformState (state = {}) { }) } + // 6. Convert address book keys from decimal to hex + const addressBook = state.AddressBookController?.addressBook || {} + Object.keys(addressBook).forEach((networkKey) => { + if ((/^\d+$/ui).test(networkKey)) { + const chainId = `0x${networkKey.toString(16)}` + updateChainIds(addressBook[networkKey], chainId) + + if (addressBook[chainId]) { + mergeAddressBookKeys(addressBook, networkKey, chainId) + } else { + addressBook[chainId] = addressBook[networkKey] + } + delete addressBook[networkKey] + } + }) + return state } + +/** + * Merges the two given keys for the given address book in place. + * + * @returns {void} + */ +function mergeAddressBookKeys (addressBook, networkKey, chainIdKey) { + const networkKeyEntries = addressBook[networkKey] || {} + // For the new entries, start by copying the existing entries for the chainId + const newEntries = { ...addressBook[chainIdKey] } + + // For each address of the old/networkId key entries + Object.keys(networkKeyEntries).forEach((address) => { + if (newEntries[address] && typeof newEntries[address] === 'object') { + const mergedEntry = {} + + // Collect all keys from both entries and merge the corresponding chainId + // entry with the networkId entry + new Set([ + ...Object.keys(newEntries[address]), + ...Object.keys(networkKeyEntries[address] || {}), + ]).forEach((key) => { + // Use non-empty value for the current key, if any + mergedEntry[key] = ( + newEntries[address][key] || + networkKeyEntries[address]?.[key] || + '' + ) + }) + + newEntries[address] = mergedEntry + } else if ( + networkKeyEntries[address] && + typeof networkKeyEntries[address] === 'object' + ) { + // If there is no corresponding chainId entry, just use the networkId entry + // directly + newEntries[address] = networkKeyEntries[address] + } + }) + + addressBook[chainIdKey] = newEntries +} + +/** + * Updates the chainId key values to the given chainId in place for all values + * of the given networkEntries object. + * + * @returns {void} + */ +function updateChainIds (networkEntries, chainId) { + Object.values(networkEntries).forEach((entry) => { + if (entry && typeof entry === 'object') { + entry.chainId = chainId + } + }) +} diff --git a/test/data/mock-state.json b/test/data/mock-state.json index e851484dd..412892d9a 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -12,6 +12,9 @@ "showIncomingTransactions": true }, "network": "4", + "provider": { + "chainId": "0x4" + }, "identities": { "0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc": { "address": "0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc", @@ -111,10 +114,10 @@ "nativeCurrency": "ETH", "conversionRate": 556.12, "addressBook": { - "4": { + "0x4": { "0xc42edfcc21ed14dda456aa0756c153f7985d8813": { "address": "0xc42edfcc21ed14dda456aa0756c153f7985d8813", - "chainId": "4", + "chainId": "0x4", "isEns": false, "memo": "", "name": "Address Book Account 1" diff --git a/test/unit/migrations/048-test.js b/test/unit/migrations/048-test.js index 98ef6e007..acc80e4b1 100644 --- a/test/unit/migrations/048-test.js +++ b/test/unit/migrations/048-test.js @@ -348,4 +348,162 @@ describe('migration #48', function () { foo: 'bar', }) }) + + it('should migrate the address book', async function () { + const oldStorage = { + meta: {}, + data: { + AddressBookController: { + addressBook: { + '1': { + 'address1': { + chainId: '1', + foo: 'bar', + }, + }, + '0x2': { + 'address2': { + chainId: '0x2', + foo: 'bar', + }, + }, + }, + bar: { + baz: 'buzz', + }, + }, + foo: 'bar', + }, + } + + const newStorage = await migration48.migrate(oldStorage) + assert.deepEqual(newStorage.data, { + ...expectedPreferencesState, + AddressBookController: { + addressBook: { + '0x1': { + 'address1': { + chainId: '0x1', + foo: 'bar', + }, + }, + '0x2': { + 'address2': { + chainId: '0x2', + foo: 'bar', + }, + }, + }, + bar: { + baz: 'buzz', + }, + }, + foo: 'bar', + }) + }) + + it('should migrate the address book and merge entries', async function () { + const oldStorage = { + meta: {}, + data: { + AddressBookController: { + addressBook: { + '2': { + 'address1': { + chainId: '2', + key2: 'kaplar', + key3: 'value3', + key4: null, + foo: 'bar', + }, + 'address2': { + chainId: '2', + foo: 'bar', + }, + }, + '0x2': { + 'address1': { + chainId: '0x2', + key1: 'value1', + key2: 'value2', + foo: 'bar', + }, + 'address3': { + chainId: '0x2', + foo: 'bar', + }, + }, + }, + bar: { + baz: 'buzz', + }, + }, + foo: 'bar', + }, + } + + const newStorage = await migration48.migrate(oldStorage) + assert.deepEqual(newStorage.data, { + ...expectedPreferencesState, + AddressBookController: { + addressBook: { + '0x2': { + 'address1': { + chainId: '0x2', + key1: 'value1', + key2: 'value2', + key3: 'value3', + key4: '', + foo: 'bar', + }, + 'address2': { + chainId: '0x2', + foo: 'bar', + }, + 'address3': { + chainId: '0x2', + foo: 'bar', + }, + }, + }, + bar: { + baz: 'buzz', + }, + }, + foo: 'bar', + }) + }) + + it('should not modify address book if all entries are valid or un-parseable', async function () { + const oldStorage = { + meta: {}, + data: { + AddressBookController: { + addressBook: { + '0x1': { foo: { bar: 'baz' } }, + 'kaplar': { foo: { bar: 'baz' } }, + }, + bar: { + baz: 'buzz', + }, + }, + foo: 'bar', + }, + } + + const newStorage = await migration48.migrate(oldStorage) + assert.deepEqual(newStorage.data, { + ...expectedPreferencesState, + AddressBookController: { + addressBook: { + '0x1': { foo: { bar: 'baz' } }, + 'kaplar': { foo: { bar: 'baz' } }, + }, + bar: { + baz: 'buzz', + }, + }, + foo: 'bar', + }) + }) }) diff --git a/test/unit/ui/app/actions.spec.js b/test/unit/ui/app/actions.spec.js index d52fb5240..ff8d74155 100644 --- a/test/unit/ui/app/actions.spec.js +++ b/test/unit/ui/app/actions.spec.js @@ -14,7 +14,7 @@ import firstTimeState from '../../localhostState' const { provider } = createTestProviderTools({ scaffold: {} }) const middleware = [thunk] -const defaultState = { metamask: {} } +const defaultState = { metamask: { provider: { chainId: '0x1' } } } const mockStore = (state = defaultState) => configureStore(middleware)(state) const extensionMock = { runtime: { diff --git a/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.container.js b/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.container.js index b372fae5f..08e793ec8 100644 --- a/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.container.js +++ b/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.container.js @@ -22,7 +22,7 @@ const mapStateToProps = (state, ownProps) => { const contact = getAddressBookEntry(state, address) || state.metamask.identities[address] const { memo, name } = contact || {} - const chainId = state.metamask.network + const { chainId } = state.metamask.provider const showingMyAccounts = Boolean(pathname.match(CONTACT_MY_ACCOUNTS_EDIT_ROUTE)) diff --git a/ui/app/selectors/selectors.js b/ui/app/selectors/selectors.js index 40bce4dc6..f79431a44 100644 --- a/ui/app/selectors/selectors.js +++ b/ui/app/selectors/selectors.js @@ -156,11 +156,11 @@ export function getAssetImages (state) { } export function getAddressBook (state) { - const { network } = state.metamask - if (!state.metamask.addressBook[network]) { + const { chainId } = state.metamask.provider + if (!state.metamask.addressBook[chainId]) { return [] } - return Object.values(state.metamask.addressBook[network]) + return Object.values(state.metamask.addressBook[chainId]) } export function getAddressBookEntry (state, address) { diff --git a/ui/app/selectors/tests/selectors.test.js b/ui/app/selectors/tests/selectors.test.js index 12a9ba3fd..ef9c6826f 100644 --- a/ui/app/selectors/tests/selectors.test.js +++ b/ui/app/selectors/tests/selectors.test.js @@ -49,7 +49,7 @@ describe('Selectors', function () { [ { 'address': '0xc42edfcc21ed14dda456aa0756c153f7985d8813', - 'chainId': '4', + 'chainId': '0x4', 'isEns': false, 'memo': '', 'name': 'Address Book Account 1', diff --git a/ui/app/selectors/tests/send-selectors-test-data.js b/ui/app/selectors/tests/send-selectors-test-data.js index 4a7debcda..98cf12ed1 100644 --- a/ui/app/selectors/tests/send-selectors-test-data.js +++ b/ui/app/selectors/tests/send-selectors-test-data.js @@ -30,6 +30,10 @@ const state = { 'nativeCurrency': 'ETH', 'frequentRpcList': [], 'network': '3', + 'provider': { + 'type': 'testnet', + 'chainId': '0x3', + }, 'accounts': { '0xfdea65c8e26263f6d9a1b5de9555d2931a33b825': { 'code': '0x', @@ -57,11 +61,11 @@ const state = { }, }, 'addressBook': { - '3': { + '0x3': { '0x06195827297c7a80a443b6894d3bdb8824b43896': { 'address': '0x06195827297c7a80a443b6894d3bdb8824b43896', 'name': 'Address Book Account 1', - 'chainId': '3', + 'chainId': '0x3', }, }, }, @@ -150,9 +154,6 @@ const state = { }, ], 'selectedAddress': '0xd85a4b6a394794842887b8284293d69163007bbb', - 'provider': { - 'type': 'testnet', - }, 'send': { 'gasLimit': '0xFFFF', 'gasPrice': '0xaa', diff --git a/ui/app/selectors/tests/send.test.js b/ui/app/selectors/tests/send.test.js index 529e03958..d168cdd87 100644 --- a/ui/app/selectors/tests/send.test.js +++ b/ui/app/selectors/tests/send.test.js @@ -382,7 +382,7 @@ describe('send selectors', function () { { address: '0x06195827297c7a80a443b6894d3bdb8824b43896', name: 'Address Book Account 1', - chainId: '3', + chainId: '0x3', }, ], ) diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 2c948366f..c65ad0368 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -1605,7 +1605,7 @@ export function addToAddressBook (recipient, nickname = '', memo = '') { log.debug(`background.addToAddressBook`) return async (dispatch, getState) => { - const chainId = getState().metamask.network + const { chainId } = getState().metamask.provider let set try {