From 38751958bb5f420eacdca82b1e9c1c0134d83eb1 Mon Sep 17 00:00:00 2001 From: Agustin Pane Date: Tue, 17 Nov 2020 09:34:01 -0300 Subject: [PATCH] (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 * Renames safeOwnersAddressesSelector to safeOwnersAddressesListSelector * Update src/components/forms/validator.ts Improves uniqueAddress Co-authored-by: Mikhail Mikheev * Fix tests * Fix uniqueAddress * Fix types extensions Co-authored-by: Mikhail Mikheev Co-authored-by: Daniel Sanchez Co-authored-by: Fernando --- src/components/forms/validator.test.ts | 14 +++++++--- src/components/forms/validator.ts | 27 ++++++++++--------- .../addressBook/store/selectors/index.ts | 4 +++ .../utils/__tests__/addressBookUtils.test.ts | 19 ------------- src/logic/addressBook/utils/index.ts | 3 --- src/logic/safe/store/selectors/index.ts | 11 ++++++++ src/routes/open/components/Layout.tsx | 4 +-- .../SafeOwnersConfirmationsForm/index.tsx | 12 ++------- .../SafeOwnersConfirmationsForm/validators.ts | 4 +-- src/routes/open/utils/safeDataExtractor.ts | 2 +- .../CreateEditEntryModal/index.tsx | 6 ++--- .../components/Balances/SendModal/index.tsx | 2 +- .../CollectibleSelectField/index.tsx | 2 +- .../TokenSelectField/index.tsx | 2 +- .../screens/SendCollectible/index.tsx | 2 +- .../ManageOwners/AddOwnerModal/index.tsx | 4 +-- .../AddOwnerModal/screens/OwnerForm/index.tsx | 25 +++++++++-------- .../AddOwnerModal/screens/OwnerForm/style.ts | 3 ++- .../ManageOwners/RemoveOwnerModal/index.tsx | 2 +- .../ManageOwners/ReplaceOwnerModal/index.tsx | 2 +- .../screens/OwnerForm/index.tsx | 6 ++--- 21 files changed, 76 insertions(+), 80 deletions(-) diff --git a/src/components/forms/validator.test.ts b/src/components/forms/validator.test.ts index 0eff1983..3f5d45ef 100644 --- a/src/components/forms/validator.test.ts +++ b/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) }) }) diff --git a/src/components/forms/validator.ts b/src/components/forms/validator.ts index e6f2d9d5..1f01b4d0 100644 --- a/src/components/forms/validator.ts +++ b/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 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): 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): 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( diff --git a/src/logic/addressBook/store/selectors/index.ts b/src/logic/addressBook/store/selectors/index.ts index 7460909f..128b9aa7 100644 --- a/src/logic/addressBook/store/selectors/index.ts +++ b/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, diff --git a/src/logic/addressBook/utils/__tests__/addressBookUtils.test.ts b/src/logic/addressBook/utils/__tests__/addressBookUtils.test.ts index 4541c2f7..8ff40278 100644 --- a/src/logic/addressBook/utils/__tests__/addressBookUtils.test.ts +++ b/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') diff --git a/src/logic/addressBook/utils/index.ts b/src/logic/addressBook/utils/index.ts index 11656a8b..f4e835de 100644 --- a/src/logic/addressBook/utils/index.ts +++ b/src/logic/addressBook/utils/index.ts @@ -56,9 +56,6 @@ export const saveAddressBook = async (addressBook: AddressBookState): Promise - addressBook.map((entry) => entry.address) - type GetNameFromAddressBookOptions = { filterOnlyValidName: boolean } diff --git a/src/logic/safe/store/selectors/index.ts b/src/logic/safe/store/selectors/index.ts index df9d6de6..63449c96 100644 --- a/src/logic/safe/store/selectors/index.ts +++ b/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 => { + if (!owners) { + return List([]) + } + + return owners?.map(({ address }) => address) + }, +) + export const getActiveTokensAddressesForAllSafes = createSelector(safesListSelector, (safes) => { const addresses = Set().withMutations((set) => { safes.forEach((safe) => { diff --git a/src/routes/open/components/Layout.tsx b/src/routes/open/components/Layout.tsx index 0aef861f..6ad2a29f 100644 --- a/src/routes/open/components/Layout.tsx +++ b/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" > - + diff --git a/src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx b/src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx index 6951f19a..cee758fd 100644 --- a/src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx +++ b/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 ( <> - + ) } - -export default SafeOwnersPage diff --git a/src/routes/open/components/SafeOwnersConfirmationsForm/validators.ts b/src/routes/open/components/SafeOwnersConfirmationsForm/validators.ts index f37cb4b3..b528a0f5 100644 --- a/src/routes/open/components/SafeOwnersConfirmationsForm/validators.ts +++ b/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() diff --git a/src/routes/open/utils/safeDataExtractor.ts b/src/routes/open/utils/safeDataExtractor.ts index f7a85b04..08baa207 100644 --- a/src/routes/open/utils/safeDataExtractor.ts +++ b/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) diff --git a/src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx b/src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx index c8c5073a..d77d11e0 100644 --- a/src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx +++ b/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 = { diff --git a/src/routes/safe/components/Balances/SendModal/index.tsx b/src/routes/safe/components/Balances/SendModal/index.tsx index a10b046f..71e6823e 100644 --- a/src/routes/safe/components/Balances/SendModal/index.tsx +++ b/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')) diff --git a/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/CollectibleSelectField/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/CollectibleSelectField/index.tsx index 9bb111e9..ddb24d1f 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/CollectibleSelectField/index.tsx +++ b/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) diff --git a/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/TokenSelectField/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/TokenSelectField/index.tsx index d276caca..dab0e19c 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/TokenSelectField/index.tsx +++ b/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) diff --git a/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/index.tsx index 671a9b70..e56bf7e4 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/SendCollectible/index.tsx +++ b/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' diff --git a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx b/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx index 88a170fa..a4e73801 100644 --- a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx +++ b/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: { diff --git a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/OwnerForm/index.tsx b/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/OwnerForm/index.tsx index 85d304e3..73083d62 100644 --- a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/OwnerForm/index.tsx +++ b/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 }) => { { { -