diff --git a/src/services/ls-migration/addedSafes.ts b/src/services/ls-migration/addedSafes.ts index d718a4bbf2..20bd458978 100644 --- a/src/services/ls-migration/addedSafes.ts +++ b/src/services/ls-migration/addedSafes.ts @@ -1,6 +1,9 @@ import { type AddedSafesState, type AddedSafesOnChain } from '@/store/addedSafesSlice' import type { LOCAL_STORAGE_DATA } from './common' import { parseLsValue } from './common' +import { isChecksummedAddress } from '@/utils/addresses' +import { isObject } from 'lodash' +import type { AddressEx } from '@safe-global/safe-gateway-typescript-sdk' const IMMORTAL_PREFIX = '_immortal|v2_' @@ -23,11 +26,33 @@ type OldAddedSafes = Record< address: string chainId: string ethBalance: string - owners: string[] + owners: Array threshold: number } > +export const migrateAddedSafesOwners = ( + owners: OldAddedSafes[string]['owners'], +): AddedSafesState[string][string]['owners'] | undefined => { + const migratedOwners = owners + .map((value) => { + if (typeof value === 'string' && isChecksummedAddress(value)) { + return { value } + } + + if (isObject(value) && typeof value.address === 'string' && isChecksummedAddress(value.address)) { + const owner: AddressEx = { + value: value.address, + ...(typeof value.name === 'string' && { name: value.name }), + } + return owner + } + }) + .filter((owner): owner is AddressEx => !!owner) + + return migratedOwners.length > 0 ? migratedOwners : undefined +} + export const migrateAddedSafes = (lsData: LOCAL_STORAGE_DATA): AddedSafesState | void => { const newAddedSafes: AddedSafesState = {} @@ -39,11 +64,16 @@ export const migrateAddedSafes = (lsData: LOCAL_STORAGE_DATA): AddedSafesState | console.log('Migrating added safes on chain', chainId) const safesPerChain = Object.values(legacyAddedSafes).reduce((acc, oldItem) => { - acc[oldItem.address] = { - ethBalance: oldItem.ethBalance, - owners: oldItem.owners.map((value) => ({ value })), - threshold: oldItem.threshold, + const migratedOwners = migrateAddedSafesOwners(oldItem.owners) + + if (migratedOwners) { + acc[oldItem.address] = { + ethBalance: oldItem.ethBalance, + owners: migratedOwners, + threshold: oldItem.threshold, + } } + return acc }, {}) diff --git a/src/services/ls-migration/index.ts b/src/services/ls-migration/index.ts index 0e68c27954..f360900eaa 100644 --- a/src/services/ls-migration/index.ts +++ b/src/services/ls-migration/index.ts @@ -15,7 +15,10 @@ const useStorageMigration = (): void => { const [isMigrationFinished = false, setIsMigrationFinished] = useLocalStorage(MIGRATION_KEY) useEffect(() => { - if (isMigrationFinished) return + if (isMigrationFinished) { + dispatch(addedSafesSlice.actions.migrateLegacyOwners()) + return + } const unmount = createMigrationBus((lsData: LOCAL_STORAGE_DATA) => { const abData = migrateAddressBook(lsData) diff --git a/src/services/ls-migration/tests.test.ts b/src/services/ls-migration/tests.test.ts index 7b9ac47313..310a9523c8 100644 --- a/src/services/ls-migration/tests.test.ts +++ b/src/services/ls-migration/tests.test.ts @@ -1,6 +1,6 @@ import { waitFor } from '@testing-library/react' import { migrateAddressBook } from './addressBook' -import { migrateAddedSafes } from './addedSafes' +import { migrateAddedSafes, migrateAddedSafesOwners } from './addedSafes' import { createIframe, sendReadyMessage, receiveMessage } from './iframe' describe('Local storage migration', () => { @@ -97,6 +97,96 @@ describe('Local storage migration', () => { }) }) + describe('migratedAddedSafesOwners', () => { + it('should migrate the owners of the added Safes', () => { + const oldOwners = [ + { + address: '0x1F2504De05f5167650bE5B28c472601Be434b60A', + }, + { + address: '0x501E66bF7a8F742FA40509588eE751e93fA354Df', + name: 'Alice', + }, + { + address: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', + name: 'Bob', + }, + '0xdef', + ] + + const newData = migrateAddedSafesOwners(oldOwners) + + expect(newData).toEqual([ + { + value: '0x1F2504De05f5167650bE5B28c472601Be434b60A', + }, + { + value: '0x501E66bF7a8F742FA40509588eE751e93fA354Df', + name: 'Alice', + }, + { + value: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', + name: 'Bob', + }, + ]) + }) + + it('should return undefined if there are no owners', () => { + const newData = migrateAddedSafesOwners([]) + + expect(newData).toEqual(undefined) + }) + + it('should format invalid owners', () => { + const oldOwners = [ + { + address: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', + name: 'Bob', + }, + { + address: '0x501E66bF7a8F742FA40509588eE751e93fA354Df', + name: 123, + }, + { + address: 123, + name: 'Alice', + }, + '0xdef', + { invalid: 'Object' }, + null, + true, + ] as Parameters[0] + + const newData = migrateAddedSafesOwners(oldOwners) + + expect(newData).toEqual([ + { + value: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', + name: 'Bob', + }, + { + value: '0x501E66bF7a8F742FA40509588eE751e93fA354Df', + }, + ]) + }) + + it('should undefined if all owners are invalid', () => { + const oldOwners = [ + { + address: 123, + name: 'Alice', + }, + { invalid: 'Object' } as unknown as Parameters[0][number], + null, + true, + ] as Parameters[0] + + const newData = migrateAddedSafesOwners(oldOwners) + + expect(newData).toEqual(undefined) + }) + }) + describe('migrateAddedSafes', () => { const oldStorage = { '_immortal|v2_MAINNET__SAFES': JSON.stringify({ @@ -111,7 +201,14 @@ describe('Local storage migration', () => { address: '0x501E66bF7a8F742FA40509588eE751e93fA354Df', chainId: '1', ethBalance: '20.3', - owners: ['0x501E66bF7a8F742FA40509588eE751e93fA354Df', '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808'], + owners: [ + '0x501E66bF7a8F742FA40509588eE751e93fA354Df', + { address: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', name: 'Charlie' }, + { invalid: 'Object' }, + null, + true, + 123, + ], threshold: 2, }, }), @@ -127,7 +224,11 @@ describe('Local storage migration', () => { address: '0x979774d85274A5F63C85786aC4Fa54B9A4f391c2', chainId: '1313161554', ethBalance: '0.00001', - owners: ['0x979774d85274A5F63C85786aC4Fa54B9A4f391c2', '0xdef', '0x1F2504De05f5167650bE5B28c472601Be434b60A'], + owners: [ + '0x979774d85274A5F63C85786aC4Fa54B9A4f391c2', + '0xdef', + { address: '0x1F2504De05f5167650bE5B28c472601Be434b60A' }, + ], threshold: 2, }, }), @@ -147,7 +248,7 @@ describe('Local storage migration', () => { ethBalance: '20.3', owners: [ { value: '0x501E66bF7a8F742FA40509588eE751e93fA354Df' }, - { value: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808' }, + { value: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', name: 'Charlie' }, ], threshold: 2, }, @@ -162,7 +263,6 @@ describe('Local storage migration', () => { ethBalance: '0.00001', owners: [ { value: '0x979774d85274A5F63C85786aC4Fa54B9A4f391c2' }, - { value: '0xdef' }, { value: '0x1F2504De05f5167650bE5B28c472601Be434b60A' }, ], threshold: 2, diff --git a/src/store/__tests__/addedSafesSlice.test.ts b/src/store/__tests__/addedSafesSlice.test.ts index 2d964ee52f..54cc9241d0 100644 --- a/src/store/__tests__/addedSafesSlice.test.ts +++ b/src/store/__tests__/addedSafesSlice.test.ts @@ -1,126 +1,362 @@ import type { SafeBalanceResponse, SafeInfo, TokenType } from '@safe-global/safe-gateway-typescript-sdk' +import { hexZeroPad } from 'ethers/lib/utils' import type { AddedSafesState } from '../addedSafesSlice' import { addOrUpdateSafe, removeSafe, addedSafesSlice, updateAddedSafeBalance } from '../addedSafesSlice' describe('addedSafesSlice', () => { - it('should add a Safe to the store', () => { - const safe0 = { chainId: '1', address: { value: '0x0' }, threshold: 1, owners: [{ value: '0x123' }] } as SafeInfo - const state = addedSafesSlice.reducer(undefined, addOrUpdateSafe({ safe: safe0 })) - expect(state).toEqual({ - '1': { ['0x0']: { owners: [{ value: '0x123' }], threshold: 1 } }, + describe('addOrUpdateSafe', () => { + it('should add a Safe to the store', () => { + const safe0 = { chainId: '1', address: { value: '0x0' }, threshold: 1, owners: [{ value: '0x123' }] } as SafeInfo + const state = addedSafesSlice.reducer(undefined, addOrUpdateSafe({ safe: safe0 })) + expect(state).toEqual({ + '1': { ['0x0']: { owners: [{ value: '0x123' }], threshold: 1 } }, + }) + + const safe1 = { chainId: '4', address: { value: '0x1' }, threshold: 1, owners: [{ value: '0x456' }] } as SafeInfo + const stateB = addedSafesSlice.reducer(state, addOrUpdateSafe({ safe: safe1 })) + expect(stateB).toEqual({ + '1': { ['0x0']: { owners: [{ value: '0x123' }], threshold: 1 } }, + '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }] } }, + }) + + const safe2 = { chainId: '1', address: { value: '0x2' }, threshold: 1, owners: [{ value: '0x789' }] } as SafeInfo + const stateC = addedSafesSlice.reducer(stateB, addOrUpdateSafe({ safe: safe2 })) + expect(stateC).toEqual({ + '1': { + ['0x0']: { owners: [{ value: '0x123' }], threshold: 1 }, + ['0x2']: { owners: [{ value: '0x789' }], threshold: 1 }, + }, + '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }] } }, + }) }) + }) + + describe('updateAddedSafeBalance', () => { + it('should add the Safe balance to the store', () => { + const balances: SafeBalanceResponse = { + fiatTotal: '', + items: [ + { + tokenInfo: { + type: 'NATIVE_TOKEN' as TokenType, + address: '', + decimals: 18, + symbol: '', + name: '', + logoUri: '', + }, + balance: '8000000000000000000', + fiatBalance: '', + fiatConversion: '', + }, + { + tokenInfo: { + type: 'ERC20' as TokenType, + address: '', + decimals: 18, + symbol: '', + name: '', + logoUri: '', + }, + balance: '9000000000000000000', + fiatBalance: '', + fiatConversion: '', + }, + ], + } + const state: AddedSafesState = { + '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }] } }, + } - const safe1 = { chainId: '4', address: { value: '0x1' }, threshold: 1, owners: [{ value: '0x456' }] } as SafeInfo - const stateB = addedSafesSlice.reducer(state, addOrUpdateSafe({ safe: safe1 })) - expect(stateB).toEqual({ - '1': { ['0x0']: { owners: [{ value: '0x123' }], threshold: 1 } }, - '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }] } }, + const result = addedSafesSlice.reducer(state, updateAddedSafeBalance({ chainId: '4', address: '0x1', balances })) + expect(result).toEqual({ + '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }], ethBalance: '8' } }, + }) }) - const safe2 = { chainId: '1', address: { value: '0x2' }, threshold: 1, owners: [{ value: '0x789' }] } as SafeInfo - const stateC = addedSafesSlice.reducer(stateB, addOrUpdateSafe({ safe: safe2 })) - expect(stateC).toEqual({ - '1': { - ['0x0']: { owners: [{ value: '0x123' }], threshold: 1 }, - ['0x2']: { owners: [{ value: '0x789' }], threshold: 1 }, - }, - '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }] } }, + it("shouldn't add the balance if the Safe isn't added", () => { + const balances: SafeBalanceResponse = { + fiatTotal: '', + items: [ + { + tokenInfo: { + type: 'NATIVE_TOKEN' as TokenType, + address: '', + decimals: 18, + symbol: '', + name: '', + logoUri: '', + }, + balance: '123', + fiatBalance: '', + fiatConversion: '', + }, + { + tokenInfo: { + type: 'ERC20' as TokenType, + address: '', + decimals: 18, + symbol: '', + name: '', + logoUri: '', + }, + balance: '456', + fiatBalance: '', + fiatConversion: '', + }, + ], + } + const state: AddedSafesState = {} + + const result = addedSafesSlice.reducer(state, updateAddedSafeBalance({ chainId: '4', address: '0x1', balances })) + expect(result).toStrictEqual({}) }) }) - it('should add the Safe balance to the store', () => { - const balances: SafeBalanceResponse = { - fiatTotal: '', - items: [ + describe('removeSafe', () => { + it('should remove a Safe from the store', () => { + const state = addedSafesSlice.reducer( + { '1': { ['0x0']: {} as SafeInfo, ['0x1']: {} as SafeInfo }, '4': { ['0x0']: {} as SafeInfo } }, + removeSafe({ chainId: '1', address: '0x1' }), + ) + expect(state).toEqual({ '1': { ['0x0']: {} as SafeInfo }, '4': { ['0x0']: {} as SafeInfo } }) + }) + + it('should remove the chain from the store', () => { + const state = addedSafesSlice.reducer( + { '1': { ['0x0']: {} as SafeInfo }, '4': { ['0x0']: {} as SafeInfo } }, + removeSafe({ chainId: '1', address: '0x0' }), + ) + expect(state).toEqual({ '4': { ['0x0']: {} as SafeInfo } }) + }) + }) + + describe('migrateLegacyOwners', () => { + const ADDRESS_1 = hexZeroPad('0x1', 20) + const ADDRESS_2 = hexZeroPad('0x2', 20) + + it('should fix legacy owners', () => { + const state = addedSafesSlice.reducer( { - tokenInfo: { - type: 'NATIVE_TOKEN' as TokenType, - address: '', - decimals: 18, - symbol: '', - name: '', - logoUri: '', + '1': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + name: true, + } as unknown as SafeInfo['owners'][number], + ], + } as SafeInfo, + ['0x1']: { + owners: [ + { + value: { address: ADDRESS_1 }, + }, + { value: ADDRESS_2 }, + ], + } as unknown as SafeInfo, + }, + '4': { + ['0x0']: { + owners: [ + { + value: { address: ADDRESS_1, name: 'Test' }, + }, + ], + } as unknown as SafeInfo, }, - balance: '8000000000000000000', - fiatBalance: '', - fiatConversion: '', }, + addedSafesSlice.actions.migrateLegacyOwners(), + ) + expect(state).toEqual({ + '1': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + }, + ], + } as SafeInfo, + ['0x1']: { + owners: [ + { + value: ADDRESS_1, + }, + { value: ADDRESS_2 }, + ], + } as SafeInfo, + }, + '4': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + name: 'Test', + }, + ], + } as SafeInfo, + }, + }) + }) + + it('should remove corrupt owners', () => { + const state = addedSafesSlice.reducer( { - tokenInfo: { - type: 'ERC20' as TokenType, - address: '', - decimals: 18, - symbol: '', - name: '', - logoUri: '', + '1': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + }, + ], + } as SafeInfo, + ['0x1']: { + owners: [ + { + value: { address: true }, + }, + { value: ADDRESS_2 }, + ], + } as unknown as SafeInfo, + }, + '4': { + ['0x0']: { + owners: [ + { + value: { address: ADDRESS_1, name: 'Test' }, + }, + { + value: { address: null, name: 'Test' } as unknown as SafeInfo['owners'][number], + }, + ], + } as unknown as SafeInfo, }, - balance: '9000000000000000000', - fiatBalance: '', - fiatConversion: '', }, - ], - } - const state: AddedSafesState = { - '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }] } }, - } - - const result = addedSafesSlice.reducer(state, updateAddedSafeBalance({ chainId: '4', address: '0x1', balances })) - expect(result).toEqual({ - '4': { ['0x1']: { threshold: 1, owners: [{ value: '0x456' }], ethBalance: '8' } }, + addedSafesSlice.actions.migrateLegacyOwners(), + ) + expect(state).toEqual({ + '1': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + }, + ], + } as SafeInfo, + ['0x1']: { + owners: [{ value: ADDRESS_2 }], + } as SafeInfo, + }, + '4': { + ['0x0']: { + owners: [ + { + name: 'Test', + value: ADDRESS_1, + }, + ], + } as SafeInfo, + }, + }) }) - }) - it("shouldn't add the balance if the Safe isn't added", () => { - const balances: SafeBalanceResponse = { - fiatTotal: '', - items: [ + it('should remove added Safe if all owners are corrupt', () => { + const state = addedSafesSlice.reducer( { - tokenInfo: { - type: 'NATIVE_TOKEN' as TokenType, - address: '', - decimals: 18, - symbol: '', - name: '', - logoUri: '', + '1': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + }, + ], + } as SafeInfo, + ['0x1']: { + owners: [ + { + value: { address: 123 }, + }, + { value: null }, + { value: '0x123' }, + ], + } as unknown as SafeInfo, }, - balance: '123', - fiatBalance: '', - fiatConversion: '', + '4': { + ['0x0']: { + owners: [ + { + value: { + address: ADDRESS_1, + name: 'Test', + }, + }, + ], + } as unknown as SafeInfo, + }, + }, + addedSafesSlice.actions.migrateLegacyOwners(), + ) + expect(state).toEqual({ + '1': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + }, + ], + } as SafeInfo, }, + '4': { + ['0x0']: { + owners: [ + { + value: ADDRESS_1, + name: 'Test', + }, + ], + } as SafeInfo, + }, + }) + }) + + it('should remove the chain if all Safes are removed due to all corrupt owners', () => { + const state = addedSafesSlice.reducer( { - tokenInfo: { - type: 'ERC20' as TokenType, - address: '', - decimals: 18, - symbol: '', - name: '', - logoUri: '', + '1': { + ['0x0']: { + owners: [ + { + value: 1234, + } as unknown as SafeInfo['owners'][number], + ], + } as SafeInfo, + ['0x1']: { + owners: [ + { + value: { address: 123 }, + }, + { value: null }, + { value: '0x123' }, + ], + } as unknown as SafeInfo, + }, + '4': { + ['0x0']: { + owners: [ + { + value: { + address: true, + name: 'Test', + }, + }, + ], + } as unknown as SafeInfo, }, - balance: '456', - fiatBalance: '', - fiatConversion: '', }, - ], - } - const state: AddedSafesState = {} + addedSafesSlice.actions.migrateLegacyOwners(), + ) - const result = addedSafesSlice.reducer(state, updateAddedSafeBalance({ chainId: '4', address: '0x1', balances })) - expect(result).toStrictEqual({}) - }) - - it('should remove a Safe from the store', () => { - const state = addedSafesSlice.reducer( - { '1': { ['0x0']: {} as SafeInfo, ['0x1']: {} as SafeInfo }, '4': { ['0x0']: {} as SafeInfo } }, - removeSafe({ chainId: '1', address: '0x1' }), - ) - expect(state).toEqual({ '1': { ['0x0']: {} as SafeInfo }, '4': { ['0x0']: {} as SafeInfo } }) - }) - - it('should remove the chain from the store', () => { - const state = addedSafesSlice.reducer( - { '1': { ['0x0']: {} as SafeInfo }, '4': { ['0x0']: {} as SafeInfo } }, - removeSafe({ chainId: '1', address: '0x0' }), - ) - expect(state).toEqual({ '4': { ['0x0']: {} as SafeInfo } }) + expect(state).toEqual({}) + }) }) }) diff --git a/src/store/addedSafesSlice.ts b/src/store/addedSafesSlice.ts index 7fb2d2141c..77727c75d7 100644 --- a/src/store/addedSafesSlice.ts +++ b/src/store/addedSafesSlice.ts @@ -7,6 +7,7 @@ import { selectSafeInfo, safeInfoSlice } from '@/store/safeInfoSlice' import { balancesSlice } from './balancesSlice' import { safeFormatUnits } from '@/utils/formatters' import type { Loadable } from './common' +import { migrateAddedSafesOwners } from '@/services/ls-migration/addedSafes' export type AddedSafesOnChain = { [safeAddress: string]: { @@ -36,6 +37,26 @@ export const addedSafesSlice = createSlice({ // Otherwise, migrate return action.payload }, + migrateLegacyOwners: (state) => { + for (const [chainId, addedSafesOnChain] of Object.entries(state)) { + for (const [safeAddress, safe] of Object.entries(addedSafesOnChain)) { + // Previously migrated corrupt owners + if (safe.owners.some(({ value }) => value !== 'string')) { + const migratedOwners = migrateAddedSafesOwners(safe.owners.map(({ value }) => value)) + + if (migratedOwners) { + state[chainId][safeAddress].owners = migratedOwners + } else { + delete state[chainId][safeAddress] + } + } + } + + if (Object.keys(state[chainId]).length === 0) { + delete state[chainId] + } + } + }, setAddedSafes: (_, action: PayloadAction) => { return action.payload },