(Fix) - #1154 Add more owners on safe creation (#1583)

* Add types in getAddressValidator
Remove unused props

* Absolute import

* Adds safeOwnersAddressesSelector

* Fix uniqueAddress function

* Replaces getAddressesListFromAddressBook with addressBookAddressesListSelector

* Remove comment

* Types

* Fix tests

* Improves safeOwnersAddressesSelector

* Remove memoize from uniqueAddress

* Improves uniqueAddress validation

* Improves uniqueAddress function to check checksum values
Add tests

* Removes toArray from safeOwnersAddressesSelector
Allows uniqueAddress to use both string[] and List<string>

* Renames safeOwnersAddressesSelector to safeOwnersAddressesListSelector

* Update src/components/forms/validator.ts

Improves uniqueAddress

Co-authored-by: Mikhail Mikheev <mmvsha73@gmail.com>

* Fix tests

* Fix uniqueAddress

* Fix types extensions

Co-authored-by: Mikhail Mikheev <mmvsha73@gmail.com>
Co-authored-by: Daniel Sanchez <daniel.sanchez@gnosis.pm>
Co-authored-by: Fernando <fernando.greco@gmail.com>

src/components/forms/validator.test.ts

@@ -169,13 +169,19 @@ describe('Forms > Validators', () => {
     it('Returns undefined for an address not contained in the passed array', async () => {
       const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe']
 
-      expect(uniqueAddress(addresses)('0xe7e3272a84cf3fe180345b9f7234ba705eB5E2CA')).toBeUndefined()
+      expect(uniqueAddress(addresses)()).toBeUndefined()
     })
 
-    it('Returns an error message for an address already contained in the array', async () => {
-      const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe']
+    it('Returns an error message for an array with duplicated values', async () => {
+      const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', '0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe']
 
-      expect(uniqueAddress(addresses)(addresses[0])).toEqual(ADDRESS_REPEATED_ERROR)
+      expect(uniqueAddress(addresses)()).toEqual(ADDRESS_REPEATED_ERROR)
+    })
+
+    it('Returns an error message for an array with duplicated checksum and not checksum values', async () => {
+      const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', '0xde0b295669a9fd93d5f28d9ec85e40f4cb697bae']
+
+      expect(uniqueAddress(addresses)()).toEqual(ADDRESS_REPEATED_ERROR)
     })
   })
 

src/components/forms/validator.ts

@@ -1,13 +1,11 @@
-import { List } from 'immutable'
+import { getWeb3 } from 'src/logic/wallets/getWeb3'
 import memoize from 'lodash.memoize'
 import { isFeatureEnabled } from 'src/config'
 import { FEATURES } from 'src/config/networks/network.d'
-
-import { sameAddress } from 'src/logic/wallets/ethAddresses'
-import { getWeb3 } from 'src/logic/wallets/getWeb3'
+import { List } from 'immutable'
 
 type ValidatorReturnType = string | undefined
-type GenericValidatorType = (...args: unknown[]) => ValidatorReturnType
+export type GenericValidatorType = (...args: unknown[]) => ValidatorReturnType
 type AsyncValidator = (...args: unknown[]) => Promise<ValidatorReturnType>
 export type Validator = GenericValidatorType | AsyncValidator
 
@@ -89,13 +87,18 @@ export const minMaxLength = (minLen: number, maxLen: number) => (value: string):
 
 export const ADDRESS_REPEATED_ERROR = 'Address already introduced'
 
-export const uniqueAddress = (addresses: string[] | List<string>): GenericValidatorType =>
-  memoize(
-    (value: string): ValidatorReturnType => {
-      const addressAlreadyExists = addresses.some((address) => sameAddress(value, address))
-      return addressAlreadyExists ? ADDRESS_REPEATED_ERROR : undefined
-    },
-  )
+export const uniqueAddress = (addresses: string[] | List<string>): GenericValidatorType => (): ValidatorReturnType => {
+  // @ts-expect-error both list and array have signatures for map but TS thinks they're not compatible
+  const lowercaseAddresses = addresses.map((address) => address.toLowerCase())
+  const uniqueAddresses = new Set(lowercaseAddresses)
+  const lengthPropName = 'size' in addresses ? 'size' : 'length'
+
+  if (uniqueAddresses.size !== addresses?.[lengthPropName]) {
+    return ADDRESS_REPEATED_ERROR
+  }
+
+  return undefined
+}
 
 export const composeValidators = (...validators: Validator[]) => (value: unknown): ValidatorReturnType =>
   validators.reduce(

src/logic/addressBook/store/selectors/index.ts

@@ -8,6 +8,10 @@ import { AddressBookState } from 'src/logic/addressBook/model/addressBook'
 
 export const addressBookSelector = (state: AppReduxState): AddressBookState => state[ADDRESS_BOOK_REDUCER_ID]
 
+export const addressBookAddressesListSelector = createSelector(addressBookSelector, (addressBook): string[] => {
+  return addressBook.map((entry) => entry.address)
+})
+
 export const getNameFromAddressBookSelector = createSelector(
   addressBookSelector,
   (_, address) => address,

src/logic/addressBook/utils/__tests__/addressBookUtils.test.ts

@@ -2,7 +2,6 @@ import { List } from 'immutable'
 import {
   checkIfEntryWasDeletedFromAddressBook,
   getAddressBookFromStorage,
-  getAddressesListFromAddressBook,
   getNameFromAddressBook,
   getOwnersWithNameFromAddressBook,
   isValidAddressBookName,
@@ -28,24 +27,6 @@ const getMockOldAddressBookEntry = ({ address = '', name = '', isOwner = false }
   }
 }
 
-describe('getAddressesListFromAdbk', () => {
-  const entry1 = getMockAddressBookEntry('123456', 'test1')
-  const entry2 = getMockAddressBookEntry('78910', 'test2')
-  const entry3 = getMockAddressBookEntry('4781321', 'test3')
-
-  it('It should returns the list of addresses within the addressBook given a safeAddressBook', () => {
-    // given
-    const safeAddressBook = [entry1, entry2, entry3]
-    const expectedResult = [entry1.address, entry2.address, entry3.address]
-
-    // when
-    const result = getAddressesListFromAddressBook(safeAddressBook)
-
-    // then
-    expect(result).toStrictEqual(expectedResult)
-  })
-})
-
 describe('getNameFromSafeAddressBook', () => {
   const entry1 = getMockAddressBookEntry('123456', 'test1')
   const entry2 = getMockAddressBookEntry('78910', 'test2')

src/logic/addressBook/utils/index.ts

@@ -56,9 +56,6 @@ export const saveAddressBook = async (addressBook: AddressBookState): Promise<vo
   }
 }
 
-export const getAddressesListFromAddressBook = (addressBook: AddressBookState): string[] =>
-  addressBook.map((entry) => entry.address)
-
 type GetNameFromAddressBookOptions = {
   filterOnlyValidName: boolean
 }

src/logic/safe/store/selectors/index.ts

@@ -208,6 +208,17 @@ export const safeModulesSelector = createSelector(safeSelector, safeFieldSelecto
 
 export const safeFeaturesEnabledSelector = createSelector(safeSelector, safeFieldSelector('featuresEnabled'))
 
+export const safeOwnersAddressesListSelector = createSelector(
+  safeOwnersSelector,
+  (owners): List<string> => {
+    if (!owners) {
+      return List([])
+    }
+
+    return owners?.map(({ address }) => address)
+  },
+)
+
 export const getActiveTokensAddressesForAllSafes = createSelector(safesListSelector, (safes) => {
   const addresses = Set().withMutations((set) => {
     safes.forEach((safe) => {

src/routes/open/components/Layout.tsx

@@ -9,7 +9,7 @@ import Row from 'src/components/layout/Row'
 import { initContracts } from 'src/logic/contracts/safeContracts'
 import Review from 'src/routes/open/components/ReviewInformation'
 import SafeNameField from 'src/routes/open/components/SafeNameForm'
-import SafeOwnersFields from 'src/routes/open/components/SafeOwnersConfirmationsForm'
+import { SafeOwnersPage } from 'src/routes/open/components/SafeOwnersConfirmationsForm'
 import {
   FIELD_CONFIRMATIONS,
   FIELD_SAFE_NAME,
@@ -129,7 +129,7 @@ const Layout = (props: LayoutProps): React.ReactElement => {
             testId="create-safe-form"
           >
             <StepperPage component={SafeNameField} />
-            <StepperPage component={SafeOwnersFields} />
+            <StepperPage component={SafeOwnersPage} />
             <StepperPage network={network} userAccount={userAccount} component={Review} />
           </Stepper>
         </Block>

src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx

@@ -236,21 +236,13 @@ const SafeOwnersForm = (props): React.ReactElement => {
   )
 }
 
-const SafeOwnersPage = ({ updateInitialProps }) =>
+export const SafeOwnersPage = () =>
   function OpenSafeOwnersPage(controls, { errors, form, values }) {
     return (
       <>
         <OpenPaper controls={controls} padding={false}>
-          <SafeOwnersForm
-            errors={errors}
-            form={form}
-            otherAccounts={getAccountsFrom(values)}
-            updateInitialProps={updateInitialProps}
-            values={values}
-          />
+          <SafeOwnersForm errors={errors} form={form} otherAccounts={getAccountsFrom(values)} values={values} />
         </OpenPaper>
       </>
     )
   }
-
-export default SafeOwnersPage

src/routes/open/components/SafeOwnersConfirmationsForm/validators.ts

@@ -1,6 +1,6 @@
-import { uniqueAddress } from 'src/components/forms/validator'
+import { GenericValidatorType, uniqueAddress } from 'src/components/forms/validator'
 
-export const getAddressValidator = (addresses, position) => {
+export const getAddressValidator = (addresses: string[], position: number): GenericValidatorType => {
   // thanks Rich Harris
   // https://twitter.com/Rich_Harris/status/1125850391155965952
   const copy = addresses.slice()

src/routes/open/utils/safeDataExtractor.ts

@@ -1,7 +1,7 @@
 import { List } from 'immutable'
 
 import { makeOwner } from 'src/logic/safe/store/models/owner'
-import { SafeOwner } from '../../../logic/safe/store/models/safe'
+import { SafeOwner } from 'src/logic/safe/store/models/safe'
 
 export const getAccountsFrom = (values) => {
   const accounts = Object.keys(values)

src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx

@@ -19,8 +19,7 @@ import Col from 'src/components/layout/Col'
 import Hairline from 'src/components/layout/Hairline'
 import Paragraph from 'src/components/layout/Paragraph'
 import Row from 'src/components/layout/Row'
-import { addressBookSelector } from 'src/logic/addressBook/store/selectors'
-import { getAddressesListFromAddressBook } from 'src/logic/addressBook/utils'
+import { addressBookAddressesListSelector } from 'src/logic/addressBook/store/selectors'
 
 export const CREATE_ENTRY_INPUT_NAME_ID = 'create-entry-input-name'
 export const CREATE_ENTRY_INPUT_ADDRESS_ID = 'create-entry-input-address'
@@ -42,8 +41,7 @@ const CreateEditEntryModalComponent = ({
     }
   }
 
-  const addressBook = useSelector(addressBookSelector)
-  const addressBookAddressesList = getAddressesListFromAddressBook(addressBook)
+  const addressBookAddressesList = useSelector(addressBookAddressesListSelector)
   const entryDoesntExist = uniqueAddress(addressBookAddressesList)
 
   const formMutators = {

src/routes/safe/components/Balances/SendModal/index.tsx

@@ -9,7 +9,7 @@ import { CustomTx } from './screens/ContractInteraction/ReviewCustomTx'
 import { ContractInteractionTx } from './screens/ContractInteraction'
 import { CustomTxProps } from './screens/ContractInteraction/SendCustomTx'
 import { ReviewTxProp } from './screens/ReviewTx'
-import { NFTToken } from 'src/logic/collectibles/sources/collectibles'
+import { NFTToken } from 'src/logic/collectibles/sources/collectibles.d'
 import { SendCollectibleTxInfo } from './screens/SendCollectible'
 
 const ChooseTxType = React.lazy(() => import('./screens/ChooseTxType'))

src/routes/safe/components/Balances/SendModal/screens/SendCollectible/CollectibleSelectField/index.tsx

@@ -13,7 +13,7 @@ import Img from 'src/components/layout/Img'
 import Paragraph from 'src/components/layout/Paragraph'
 import { setImageToPlaceholder } from 'src/routes/safe/components/Balances/utils'
 import { textShortener } from 'src/utils/strings'
-import { NFTToken } from 'src/logic/collectibles/sources/collectibles'
+import { NFTToken } from 'src/logic/collectibles/sources/collectibles.d'
 
 const useSelectedCollectibleStyles = makeStyles(selectedTokenStyles)
 

src/routes/safe/components/Balances/SendModal/screens/SendCollectible/TokenSelectField/index.tsx

@@ -14,7 +14,7 @@ import Paragraph from 'src/components/layout/Paragraph'
 import { formatAmount } from 'src/logic/tokens/utils/formatAmount'
 import { setImageToPlaceholder } from 'src/routes/safe/components/Balances/utils'
 import { textShortener } from 'src/utils/strings'
-import { NFTAssets } from 'src/logic/collectibles/sources/collectibles'
+import { NFTAssets } from 'src/logic/collectibles/sources/collectibles.d'
 
 const useSelectedTokenStyles = makeStyles(selectedTokenStyles)
 

src/routes/safe/components/Balances/SendModal/screens/SendCollectible/index.tsx

@@ -21,7 +21,7 @@ import { getNameFromAddressBook } from 'src/logic/addressBook/utils'
 import { nftTokensSelector, safeActiveSelectorMap } from 'src/logic/collectibles/store/selectors'
 import SafeInfo from 'src/routes/safe/components/Balances/SendModal/SafeInfo'
 import { AddressBookInput } from 'src/routes/safe/components/Balances/SendModal/screens/AddressBookInput'
-import { NFTToken } from 'src/logic/collectibles/sources/collectibles'
+import { NFTToken } from 'src/logic/collectibles/sources/collectibles.d'
 import { getExplorerInfo } from 'src/config'
 import { sameAddress } from 'src/logic/wallets/ethAddresses'
 import { sm } from 'src/theme/variables'

src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx

@@ -2,7 +2,7 @@ import { createStyles, makeStyles } from '@material-ui/core/styles'
 import React, { useEffect, useState } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
 
-import OwnerForm from './screens/OwnerForm'
+import { OwnerForm } from './screens/OwnerForm'
 import ReviewAddOwner from './screens/Review'
 import ThresholdForm from './screens/ThresholdForm'
 
@@ -16,7 +16,7 @@ import createTransaction from 'src/logic/safe/store/actions/createTransaction'
 import { safeParamAddressFromStateSelector } from 'src/logic/safe/store/selectors'
 import { checksumAddress } from 'src/utils/checksumAddress'
 import { makeAddressBookEntry } from 'src/logic/addressBook/model/addressBook'
-import { Dispatch } from 'src/logic/safe/store/actions/types'
+import { Dispatch } from 'src/logic/safe/store/actions/types.d'
 
 const styles = createStyles({
   biggerModalWindow: {

src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/OwnerForm/index.tsx

@@ -1,5 +1,5 @@
 import IconButton from '@material-ui/core/IconButton'
-import { withStyles } from '@material-ui/core/styles'
+import { makeStyles } from '@material-ui/core/styles'
 import Close from '@material-ui/icons/Close'
 import React from 'react'
 import { useSelector } from 'react-redux'
@@ -18,7 +18,7 @@ import Col from 'src/components/layout/Col'
 import Hairline from 'src/components/layout/Hairline'
 import Paragraph from 'src/components/layout/Paragraph'
 import Row from 'src/components/layout/Row'
-import { safeOwnersSelector } from 'src/logic/safe/store/selectors'
+import { safeOwnersAddressesListSelector } from 'src/logic/safe/store/selectors'
 
 export const ADD_OWNER_NAME_INPUT_TEST_ID = 'add-owner-name-input'
 export const ADD_OWNER_ADDRESS_INPUT_TEST_ID = 'add-owner-address-testid'
@@ -30,12 +30,20 @@ const formMutators = {
   },
 }
 
-const OwnerForm = ({ classes, onClose, onSubmit }) => {
+const useStyles = makeStyles(styles)
+
+type OwnerFormProps = {
+  onClose: () => void
+  onSubmit: (values) => void
+}
+
+export const OwnerForm = ({ onClose, onSubmit }: OwnerFormProps): React.ReactElement => {
+  const classes = useStyles()
   const handleSubmit = (values) => {
     onSubmit(values)
   }
-  const owners = useSelector(safeOwnersSelector)
-  const ownerDoesntExist = uniqueAddress(owners?.map((o) => o.address) || [])
+  const owners = useSelector(safeOwnersAddressesListSelector)
+  const ownerDoesntExist = uniqueAddress(owners)
 
   return (
     <>
@@ -72,7 +80,6 @@ const OwnerForm = ({ classes, onClose, onSubmit }) => {
                 <Row margin="md">
                   <Col xs={8}>
                     <Field
-                      className={classes.addressInput}
                       component={TextField}
                       name="ownerName"
                       placeholder="Owner name*"
@@ -86,7 +93,6 @@ const OwnerForm = ({ classes, onClose, onSubmit }) => {
                 <Row margin="md">
                   <Col xs={8}>
                     <AddressInput
-                      className={classes.addressInput}
                       fieldMutator={mutators.setOwnerAddress}
                       name="ownerAddress"
                       placeholder="Owner address*"
@@ -102,11 +108,10 @@ const OwnerForm = ({ classes, onClose, onSubmit }) => {
               </Block>
               <Hairline />
               <Row align="center" className={classes.buttonRow}>
-                <Button className={classes.button} minWidth={140} onClick={onClose}>
+                <Button minWidth={140} onClick={onClose}>
                   Cancel
                 </Button>
                 <Button
-                  className={classes.button}
                   color="primary"
                   minWidth={140}
                   testId={ADD_OWNER_NEXT_BTN_TEST_ID}
@@ -123,5 +128,3 @@ const OwnerForm = ({ classes, onClose, onSubmit }) => {
     </>
   )
 }
-
-export default withStyles(styles as any)(OwnerForm)

src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/OwnerForm/style.ts

@@ -1,6 +1,7 @@
 import { lg, md, secondaryText, sm } from 'src/theme/variables'
+import { createStyles } from '@material-ui/core'
 
-export const styles = () => ({
+export const styles = createStyles({
   heading: {
     padding: `${sm} ${lg}`,
     justifyContent: 'flex-start',

src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/index.tsx

@@ -13,7 +13,7 @@ import createTransaction from 'src/logic/safe/store/actions/createTransaction'
 import removeSafeOwner from 'src/logic/safe/store/actions/removeSafeOwner'
 
 import { safeParamAddressFromStateSelector, safeThresholdSelector } from 'src/logic/safe/store/selectors'
-import { Dispatch } from 'src/logic/safe/store/actions/types'
+import { Dispatch } from 'src/logic/safe/store/actions/types.d'
 
 const styles = createStyles({
   biggerModalWindow: {

src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/index.tsx

@@ -15,7 +15,7 @@ import { safeParamAddressFromStateSelector, safeThresholdSelector } from 'src/lo
 import { checksumAddress } from 'src/utils/checksumAddress'
 import { makeAddressBookEntry } from 'src/logic/addressBook/model/addressBook'
 import { sameAddress } from 'src/logic/wallets/ethAddresses'
-import { Dispatch } from 'src/logic/safe/store/actions/types'
+import { Dispatch } from 'src/logic/safe/store/actions/types.d'
 
 const styles = createStyles({
   biggerModalWindow: {

src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/OwnerForm/index.tsx

@@ -19,7 +19,7 @@ import Hairline from 'src/components/layout/Hairline'
 import Paragraph from 'src/components/layout/Paragraph'
 import Row from 'src/components/layout/Row'
 import { ScanQRWrapper } from 'src/components/ScanQRModal/ScanQRWrapper'
-import { safeOwnersSelector } from 'src/logic/safe/store/selectors'
+import { safeOwnersAddressesListSelector } from 'src/logic/safe/store/selectors'
 
 import { styles } from './style'
 import { getExplorerInfo } from 'src/config'
@@ -39,8 +39,8 @@ const OwnerForm = ({ classes, onClose, onSubmit, ownerAddress, ownerName }) => {
   const handleSubmit = (values) => {
     onSubmit(values)
   }
-  const owners = useSelector(safeOwnersSelector)
-  const ownerDoesntExist = uniqueAddress(owners?.map((o) => o.address) || [])
+  const owners = useSelector(safeOwnersAddressesListSelector)
+  const ownerDoesntExist = uniqueAddress(owners)
 
   return (
     <>