Skip to content

Commit

Permalink
fix: migrate/sanitise legacy owner format (#1604)
Browse files Browse the repository at this point in the history
* fix: migrate/sanitise legacy owner format

* fix: test

* fix: check structure, checksum + add tests

* fix: rename action

* fix: alter production flag

* fix: revert production flag change
  • Loading branch information
iamacook authored Jan 30, 2023
1 parent c066c45 commit bb6fa15
Show file tree
Hide file tree
Showing 5 changed files with 498 additions and 108 deletions.
40 changes: 35 additions & 5 deletions src/services/ls-migration/addedSafes.ts
Original file line number Diff line number Diff line change
@@ -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_'

Expand All @@ -23,11 +26,33 @@ type OldAddedSafes = Record<
address: string
chainId: string
ethBalance: string
owners: string[]
owners: Array<string | { name?: string; address: string }>
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 = {}

Expand All @@ -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<AddedSafesOnChain>((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
}, {})

Expand Down
5 changes: 4 additions & 1 deletion src/services/ls-migration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ const useStorageMigration = (): void => {
const [isMigrationFinished = false, setIsMigrationFinished] = useLocalStorage<boolean>(MIGRATION_KEY)

useEffect(() => {
if (isMigrationFinished) return
if (isMigrationFinished) {
dispatch(addedSafesSlice.actions.migrateLegacyOwners())
return
}

const unmount = createMigrationBus((lsData: LOCAL_STORAGE_DATA) => {
const abData = migrateAddressBook(lsData)
Expand Down
110 changes: 105 additions & 5 deletions src/services/ls-migration/tests.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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<typeof migrateAddedSafesOwners>[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<typeof migrateAddedSafesOwners>[0][number],
null,
true,
] as Parameters<typeof migrateAddedSafesOwners>[0]

const newData = migrateAddedSafesOwners(oldOwners)

expect(newData).toEqual(undefined)
})
})

describe('migrateAddedSafes', () => {
const oldStorage = {
'_immortal|v2_MAINNET__SAFES': JSON.stringify({
Expand All @@ -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,
},
}),
Expand All @@ -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,
},
}),
Expand All @@ -147,7 +248,7 @@ describe('Local storage migration', () => {
ethBalance: '20.3',
owners: [
{ value: '0x501E66bF7a8F742FA40509588eE751e93fA354Df' },
{ value: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808' },
{ value: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', name: 'Charlie' },
],
threshold: 2,
},
Expand All @@ -162,7 +263,6 @@ describe('Local storage migration', () => {
ethBalance: '0.00001',
owners: [
{ value: '0x979774d85274A5F63C85786aC4Fa54B9A4f391c2' },
{ value: '0xdef' },
{ value: '0x1F2504De05f5167650bE5B28c472601Be434b60A' },
],
threshold: 2,
Expand Down
Loading

0 comments on commit bb6fa15

Please sign in to comment.