fix: checksum lowercase addresses + validate checksums (#1127)

* fix: checksum lowercase addresses + validate

* fix: cached value

* fix: add + fix tests

* fix: add uppercase test

src/components/common/AddressInput/index.test.tsx

@@ -8,9 +8,9 @@ import useNameResolver from '@/components/common/AddressInput/useNameResolver'
 // mock useCurrentChain
 jest.mock('@/hooks/useChains', () => ({
   useCurrentChain: jest.fn(() => ({
-    shortName: 'rin',
-    chainId: '4',
-    chainName: 'Rinkeby',
+    shortName: 'gor',
+    chainId: '5',
+    chainName: 'Goerli',
     features: ['DOMAIN_LOOKUP'],
   })),
 }))
@@ -99,7 +99,7 @@ describe('AddressInput tests', () => {
     )
 
     act(() => {
-      fireEvent.change(input, { target: { value: 'rin:0x123' } })
+      fireEvent.change(input, { target: { value: 'gor:0x123' } })
       jest.advanceTimersByTime(1000)
     })
 
@@ -110,14 +110,14 @@ describe('AddressInput tests', () => {
     const { input, utils } = setup('', (val) => `${val} is wrong`)
 
     act(() => {
-      fireEvent.change(input, { target: { value: `rin:${TEST_ADDRESS_A}` } })
+      fireEvent.change(input, { target: { value: `gor:${TEST_ADDRESS_A}` } })
       jest.advanceTimersByTime(1000)
     })
 
     await waitFor(() => expect(utils.getByLabelText(`${TEST_ADDRESS_A} is wrong`, { exact: false })).toBeDefined())
 
     act(() => {
-      fireEvent.change(input, { target: { value: `rin:${TEST_ADDRESS_B}` } })
+      fireEvent.change(input, { target: { value: `gor:${TEST_ADDRESS_B}` } })
       jest.advanceTimersByTime(1000)
     })
 
@@ -127,11 +127,12 @@ describe('AddressInput tests', () => {
   it('should resolve ENS names', async () => {
     const { input } = setup('')
 
-    await act(async () => {
+    act(() => {
       fireEvent.change(input, { target: { value: 'zero.eth' } })
-      await waitFor(() => expect(input.value).toBe('rin:0x0000000000000000000000000000000000000000'))
     })
 
+    await waitFor(() => expect(input.value).toBe('0x0000000000000000000000000000000000000000'))
+
     expect(useNameResolver).toHaveBeenCalledWith('zero.eth')
   })
 
@@ -149,9 +150,9 @@ describe('AddressInput tests', () => {
 
   it('should not resolve ENS names if this feature is disabled', async () => {
     ;(useCurrentChain as jest.Mock).mockImplementation(() => ({
-      shortName: 'rin',
-      chainId: '4',
-      chainName: 'Rinkeby',
+      shortName: 'gor',
+      chainId: '5',
+      chainName: 'Goerli',
       features: [],
     }))
 
@@ -172,17 +173,24 @@ describe('AddressInput tests', () => {
 
     await waitFor(() => expect(input.value).toBe(TEST_ADDRESS_A))
 
-    expect(input.previousElementSibling?.textContent).toBe('rin:')
+    expect(input.previousElementSibling?.textContent).toBe('gor:')
   })
 
   it('should not show adornment when the value contains correct prefix', async () => {
-    const { input } = setup(`rin:${TEST_ADDRESS_B}`)
+    ;(useCurrentChain as jest.Mock).mockImplementation(() => ({
+      shortName: 'gor',
+      chainId: '5',
+      chainName: 'Goerli',
+      features: [],
+    }))
+
+    const { input } = setup(`gor:${TEST_ADDRESS_A}`)
 
     act(() => {
-      fireEvent.change(input, { target: { value: 'rin:${TEST_ADDRESS_B}' } })
+      fireEvent.change(input, { target: { value: `gor:${TEST_ADDRESS_B}` } })
     })
 
-    expect(input.previousElementSibling).toBe(null)
+    await waitFor(() => expect(input.previousElementSibling).toBe(null))
   })
 
   it('should keep a bare address in the form state', async () => {
@@ -212,7 +220,7 @@ describe('AddressInput tests', () => {
     const input = utils.getByLabelText('Recipient', { exact: false }) as HTMLInputElement
 
     act(() => {
-      fireEvent.change(input, { target: { value: `rin:${TEST_ADDRESS_A}` } })
+      fireEvent.change(input, { target: { value: `gor:${TEST_ADDRESS_A}` } })
     })
 
     expect(methods.getValues().recipient).toBe(TEST_ADDRESS_A)

src/components/common/AddressInput/index.tsx

@@ -89,10 +89,9 @@ const AddressInput = ({ name, validate, required = true, deps, ...props }: Addre
             required,
 
             setValueAs: (value: string): string => {
-              const { address, prefix } = parsePrefixedAddress(value)
               rawValueRef.current = value
-              // Return a bare address if the prefx is the correct shortName
-              return prefix === currentShortName ? address : value
+              // This also checksums the address
+              return parsePrefixedAddress(value).address
             },
 
             validate: () => {
@@ -105,6 +104,9 @@ const AddressInput = ({ name, validate, required = true, deps, ...props }: Addre
             // Workaround for a bug in react-hook-form that it restores a cached error state on blur
             onBlur: () => setTimeout(() => trigger(name), 100),
           })}
+          // Workaround for a bug in react-hook-form when `register().value` is cached after `setValueAs`
+          // Only seems to occur on the `/load` route
+          value={watchedValue}
         />
       </Grid>
 

src/services/ls-migration/addressBook.ts

@@ -1,5 +1,5 @@
 import { type AddressBookState } from '@/store/addressBookSlice'
-import { utils } from 'ethers'
+import { isChecksummedAddress } from '@/utils/addresses'
 import type { LOCAL_STORAGE_DATA } from './common'
 import { parseLsValue } from './common'
 
@@ -13,7 +13,7 @@ export const migrateAddressBook = (lsData: LOCAL_STORAGE_DATA): AddressBookState
     console.log('Migrating address book')
 
     const newAb = legacyAb.reduce<AddressBookState>((acc, { address, name, chainId }) => {
-      if (!name || !address || !utils.isAddress(address)) {
+      if (!name || !address || !isChecksummedAddress(address)) {
         return acc
       }
       acc[chainId] = acc[chainId] || {}

src/services/ls-migration/tests.test.ts

@@ -57,6 +57,7 @@ describe('Local storage migration', () => {
           { address: '0x1F2504De05f5167650bE5B28c472601Be434b60A', name: 'Alice', chainId: '1' },
           { address: 'sdfgsdfg', name: 'Bob', chainId: '1' },
           { address: '0x9913B9180C20C6b0F21B6480c84422F6ebc4B808', name: 'Charlie', chainId: '5' },
+          { address: '0x62da87ff2e2216f1858603a3db9313e178da3112 ', name: 'Not checksummed', chainId: '5' },
           { address: '', name: 'Dave', chainId: '5' },
           { address: undefined, name: 'John', chainId: '5' },
         ]),

src/utils/__tests__/addresses.test.ts

@@ -0,0 +1,102 @@
+import { checksumAddress, isChecksummedAddress, parsePrefixedAddress, sameAddress } from '../addresses'
+
+describe('Addresses', () => {
+  describe('checksumAddress', () => {
+    it('should checksum lowercase addresses', () => {
+      const value = checksumAddress('0x62da87ff2e2216f1858603a3db9313e178da3112')
+      expect(value).toBe('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+    })
+
+    it('should return checksummed addresses as is', () => {
+      const value = checksumAddress('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+      expect(value).toBe('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+    })
+
+    it('should return mixed case addresses as is', () => {
+      const value = checksumAddress('0x62Da87ff2E2216F1858603A3Db9313E178da3112')
+      expect(value).toBe('0x62Da87ff2E2216F1858603A3Db9313E178da3112')
+    })
+
+    it('should return uppercase addresses as is', () => {
+      const value = checksumAddress('0X62DA87FF2E2216F1858603A3DB9313E178DA3112')
+      expect(value).toBe('0X62DA87FF2E2216F1858603A3DB9313E178DA3112')
+    })
+
+    it('should return non-addresses as is', () => {
+      const value = checksumAddress('sdfgsdfg')
+      expect(value).toBe('sdfgsdfg')
+    })
+  })
+
+  describe('isChecksummedAddress', () => {
+    it('should return true for checksummed addresses', () => {
+      const value = isChecksummedAddress('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+      expect(value).toBe(true)
+    })
+
+    it('should return false for lowercase addresses', () => {
+      const value = isChecksummedAddress('0x62da87ff2e2216f1858603a3db9313e178da3112')
+      expect(value).toBe(false)
+    })
+
+    it('should return false for mixed case addresses', () => {
+      const value = isChecksummedAddress('0x62Da87ff2E2216F1858603A3Db9313E178da3112')
+      expect(value).toBe(false)
+    })
+
+    it('should return false for uppercase addresses', () => {
+      const value = isChecksummedAddress('0X62DA87FF2E2216F1858603A3DB9313E178DA3112')
+      expect(value).toBe(false)
+    })
+
+    it('should return false for non-/invalid addresses', () => {
+      const value = isChecksummedAddress('sdfgsdfg')
+      expect(value).toBe(false)
+    })
+  })
+
+  describe('sameAddress', () => {
+    it('returns false if the first or second address is undefined', () => {
+      const address = '0x62Da87FF2E2216F1858603A3Db9313E178da3112'
+      expect(sameAddress(undefined, address)).toBe(false)
+      expect(sameAddress(address, undefined)).toBe(false)
+    })
+
+    it('returns false if the addresses are different', () => {
+      const address1 = '0x62Da87FF2E2216F1858603A3Db9313E178da3112'
+      const address2 = '0x62Da87FF2E2216F1858603A3Db9313E178da3113'
+      expect(sameAddress(address1, address2)).toBe(false)
+    })
+
+    it('returns true if the addresses are the same', () => {
+      const address = '0x62Da87FF2E2216F1858603A3Db9313E178da3112'
+      expect(sameAddress(address, address)).toBe(true)
+    })
+  })
+
+  describe('parsePrefixedAddress', () => {
+    it('should parse a prefixed address', () => {
+      const { prefix, address } = parsePrefixedAddress('prefix:0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+      expect(prefix).toBe('prefix')
+      expect(address).toBe('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+    })
+
+    it('should parse a non-prefixed address', () => {
+      const { prefix, address } = parsePrefixedAddress('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+      expect(prefix).toBeUndefined()
+      expect(address).toBe('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+    })
+
+    it('should checksum addresses', () => {
+      const { prefix, address } = parsePrefixedAddress('0x62da87ff2e2216f1858603a3db9313e178da3112')
+      expect(prefix).toBeUndefined()
+      expect(address).toBe('0x62Da87FF2E2216F1858603A3Db9313E178da3112')
+    })
+
+    it('should parse a non-addresses', () => {
+      const { prefix, address } = parsePrefixedAddress('sdfgsdfg')
+      expect(prefix).toBeUndefined()
+      expect(address).toBe('sdfgsdfg')
+    })
+  })
+})

src/utils/addresses.ts

@@ -1,6 +1,22 @@
 import { getAddress } from 'ethers/lib/utils'
 import { isAddress } from '@ethersproject/address'
 
+export const checksumAddress = (address: string): string => {
+  return isAddress(address) ? getAddress(address) : address
+}
+
+export const isChecksummedAddress = (address: string): boolean => {
+  if (!isAddress(address)) {
+    return false
+  }
+
+  try {
+    return getAddress(address) === address
+  } catch {
+    return false
+  }
+}
+
 export const sameAddress = (firstAddress: string | undefined, secondAddress: string | undefined): boolean => {
   if (!firstAddress || !secondAddress) {
     return false
@@ -14,6 +30,11 @@ export type PrefixedAddress = {
   address: string
 }
 
+/**
+ * Parses a string that may/may not contain an address and returns the `prefix` and checksummed `address`
+ * @param value (prefixed) address
+ * @returns `prefix` and checksummed `address`
+ */
 export const parsePrefixedAddress = (value: string): PrefixedAddress => {
   let [prefix, address] = value.split(':')
 
@@ -24,7 +45,7 @@ export const parsePrefixedAddress = (value: string): PrefixedAddress => {
 
   return {
     prefix: prefix || undefined,
-    address: isAddress(address) ? getAddress(address) : value,
+    address: checksumAddress(address),
   }
 }
 

src/utils/validation.ts

@@ -1,6 +1,5 @@
 import chains from '@/config/chains'
-import { isAddress } from '@ethersproject/address'
-import { parsePrefixedAddress, sameAddress } from './addresses'
+import { parsePrefixedAddress, sameAddress, isChecksummedAddress } from './addresses'
 import { safeFormatUnits, safeParseUnits } from './formatters'
 
 export const validateAddress = (address: string) => {
@@ -10,7 +9,7 @@ export const validateAddress = (address: string) => {
     return 'Invalid address format'
   }
 
-  if (!isAddress(address)) {
+  if (!isChecksummedAddress(address)) {
     return 'Invalid address checksum'
   }
 }