Skip to content

Commit

Permalink
Feat(Multichain): simplify multichain naming options [SW-187] [SW-221…
Browse files Browse the repository at this point in the history
…] [SW-211] (#4243)

* feat: disable renaming multichain sub safes from the context menu

* fix: when removing a CF safe, also remove it from the address book

* feat: remove name input on replay safe modal

* feat: do not show chain indicator when renaming a multichain safe

* refactor: combine upsertAddressBookEntry and upsertMultichainAddressBookEntry

* refactor: simplify upsertAddressBookEntries if-else block
  • Loading branch information
jmealy committed Sep 25, 2024
1 parent ffe9139 commit 54f7c47
Show file tree
Hide file tree
Showing 14 changed files with 62 additions and 66 deletions.
15 changes: 8 additions & 7 deletions src/components/address-book/EntryDialog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import ModalDialog from '@/components/common/ModalDialog'
import NameInput from '@/components/common/NameInput'
import useChainId from '@/hooks/useChainId'
import { useAppDispatch } from '@/store'
import { upsertAddressBookEntry, upsertMultichainAddressBookEntry } from '@/store/addressBookSlice'
import madProps from '@/utils/mad-props'
import { upsertAddressBookEntries } from '@/store/addressBookSlice'

export type AddressEntry = {
name: string
Expand Down Expand Up @@ -41,11 +41,7 @@ function EntryDialog({
const { handleSubmit, formState } = methods

const submitCallback = handleSubmit((data: AddressEntry) => {
if (chainIds) {
dispatch(upsertMultichainAddressBookEntry({ ...data, chainIds }))
} else {
dispatch(upsertAddressBookEntry({ ...data, chainId: currentChainId }))
}
dispatch(upsertAddressBookEntries({ ...data, chainIds: chainIds ?? [currentChainId] }))
handleClose()
})

Expand All @@ -55,7 +51,12 @@ function EntryDialog({
}

return (
<ModalDialog open onClose={handleClose} dialogTitle={defaultValues.name ? 'Edit entry' : 'Create entry'}>
<ModalDialog
open
onClose={handleClose}
dialogTitle={defaultValues.name ? 'Edit entry' : 'Create entry'}
hideChainIndicator={chainIds && chainIds.length > 1}
>
<FormProvider {...methods}>
<form onSubmit={onSubmit}>
<DialogContent>
Expand Down
4 changes: 2 additions & 2 deletions src/components/address-book/ImportDialog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { ParseResult } from 'papaparse'
import { type ReactElement, useState, type MouseEvent, useMemo } from 'react'

import ModalDialog from '@/components/common/ModalDialog'
import { upsertAddressBookEntry } from '@/store/addressBookSlice'
import { upsertAddressBookEntries } from '@/store/addressBookSlice'
import { useAppDispatch } from '@/store'

import css from './styles.module.css'
Expand Down Expand Up @@ -60,7 +60,7 @@ const ImportDialog = ({ handleClose }: { handleClose: () => void }): ReactElemen

for (const entry of entries) {
const [address, name, chainId] = entry
dispatch(upsertAddressBookEntry({ address, name, chainId: chainId.trim() }))
dispatch(upsertAddressBookEntries({ address, name, chainIds: [chainId.trim()] }))
}

trackEvent({ ...ADDRESS_BOOK_EVENTS.IMPORT, label: entries.length })
Expand Down
8 changes: 4 additions & 4 deletions src/components/new-safe/create/logic/address-book.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { AppThunk } from '@/store'
import { addOrUpdateSafe } from '@/store/addedSafesSlice'
import { upsertAddressBookEntry } from '@/store/addressBookSlice'
import { upsertAddressBookEntries } from '@/store/addressBookSlice'
import { defaultSafeInfo } from '@/store/safeInfoSlice'
import type { NamedAddress } from '@/components/new-safe/create/types'

Expand All @@ -13,8 +13,8 @@ export const updateAddressBook = (
): AppThunk => {
return (dispatch) => {
dispatch(
upsertAddressBookEntry({
chainId,
upsertAddressBookEntries({
chainIds: [chainId],
address,
name,
}),
Expand All @@ -23,7 +23,7 @@ export const updateAddressBook = (
owners.forEach((owner) => {
const entryName = owner.name || owner.ens
if (entryName) {
dispatch(upsertAddressBookEntry({ chainId, address: owner.address, name: entryName }))
dispatch(upsertAddressBookEntries({ chainIds: [chainId], address: owner.address, name: entryName }))
}
})

Expand Down
10 changes: 5 additions & 5 deletions src/components/new-safe/load/steps/SafeReviewStep/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import { useAppDispatch } from '@/store'
import { useRouter } from 'next/router'
import { addOrUpdateSafe } from '@/store/addedSafesSlice'
import { defaultSafeInfo } from '@/store/safeInfoSlice'
import { upsertAddressBookEntry } from '@/store/addressBookSlice'
import { LOAD_SAFE_EVENTS, OPEN_SAFE_LABELS, OVERVIEW_EVENTS, trackEvent } from '@/services/analytics'
import { AppRoutes } from '@/config/routes'
import ReviewRow from '@/components/new-safe/ReviewRow'
import { upsertAddressBookEntries } from '@/store/addressBookSlice'

const SafeReviewStep = ({ data, onBack }: StepRenderProps<LoadSafeFormData>) => {
const chain = useCurrentChain()
Expand Down Expand Up @@ -44,8 +44,8 @@ const SafeReviewStep = ({ data, onBack }: StepRenderProps<LoadSafeFormData>) =>
)

dispatch(
upsertAddressBookEntry({
chainId,
upsertAddressBookEntries({
chainIds: [chainId],
address: safeAddress,
name: safeName,
}),
Expand All @@ -59,8 +59,8 @@ const SafeReviewStep = ({ data, onBack }: StepRenderProps<LoadSafeFormData>) =>
}

dispatch(
upsertAddressBookEntry({
chainId,
upsertAddressBookEntries({
chainIds: [chainId],
address,
name: entryName,
}),
Expand Down
6 changes: 3 additions & 3 deletions src/components/settings/owner/EditOwnerDialog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import NameInput from '@/components/common/NameInput'
import Track from '@/components/common/Track'
import { SETTINGS_EVENTS } from '@/services/analytics/events/settings'
import { useAppDispatch } from '@/store'
import { upsertAddressBookEntry } from '@/store/addressBookSlice'
import EditIcon from '@/public/images/common/edit.svg'
import { Box, Button, DialogActions, DialogContent, IconButton, Tooltip, SvgIcon } from '@mui/material'
import { useState } from 'react'
import { FormProvider, useForm } from 'react-hook-form'
import { upsertAddressBookEntries } from '@/store/addressBookSlice'

type EditOwnerValues = {
name: string
Expand All @@ -24,8 +24,8 @@ export const EditOwnerDialog = ({ chainId, address, name }: { chainId: string; a
const onSubmit = (data: EditOwnerValues) => {
if (data.name !== name) {
dispatch(
upsertAddressBookEntry({
chainId,
upsertAddressBookEntries({
chainIds: [chainId],
address,
name: data.name,
}),
Expand Down
16 changes: 10 additions & 6 deletions src/components/sidebar/SafeListContextMenu/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ const SafeListContextMenu = ({
address,
chainId,
addNetwork,
rename,
}: {
name: string
address: string
chainId: string
addNetwork: boolean
rename: boolean
}): ReactElement => {
const addedSafes = useAppSelector((state) => selectAddedSafes(state, chainId))
const isAdded = !!addedSafes?.[address]
Expand Down Expand Up @@ -78,12 +80,14 @@ const SafeListContextMenu = ({
<MoreVertIcon sx={({ palette }) => ({ color: palette.border.main })} />
</IconButton>
<ContextMenu anchorEl={anchorEl} open={!!anchorEl} onClose={handleCloseContextMenu}>
<MenuItem onClick={handleOpenModal(ModalType.RENAME, OVERVIEW_EVENTS.SIDEBAR_RENAME)}>
<ListItemIcon>
<SvgIcon component={EditIcon} inheritViewBox fontSize="small" color="success" />
</ListItemIcon>
<ListItemText data-testid="rename-btn">{hasName ? 'Rename' : 'Give name'}</ListItemText>
</MenuItem>
{rename && (
<MenuItem onClick={handleOpenModal(ModalType.RENAME, OVERVIEW_EVENTS.SIDEBAR_RENAME)}>
<ListItemIcon>
<SvgIcon component={EditIcon} inheritViewBox fontSize="small" color="success" />
</ListItemIcon>
<ListItemText data-testid="rename-btn">{hasName ? 'Rename' : 'Give name'}</ListItemText>
</MenuItem>
)}

{isAdded && (
<MenuItem onClick={handleOpenModal(ModalType.REMOVE, OVERVIEW_EVENTS.REMOVE_FROM_WATCHLIST)}>
Expand Down
2 changes: 2 additions & 0 deletions src/components/sidebar/SafeListRemoveDialog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Track from '@/components/common/Track'
import { OVERVIEW_EVENTS, OVERVIEW_LABELS } from '@/services/analytics'
import { AppRoutes } from '@/config/routes'
import router from 'next/router'
import { removeAddressBookEntry } from '@/store/addressBookSlice'

const SafeListRemoveDialog = ({
handleClose,
Expand All @@ -31,6 +32,7 @@ const SafeListRemoveDialog = ({

const handleConfirm = () => {
dispatch(removeSafe({ chainId, address }))
dispatch(removeAddressBookEntry({ chainId, address }))
handleClose()
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import useSafeInfo from '@/hooks/useSafeInfo'
import { trackEvent, SETTINGS_EVENTS } from '@/services/analytics'
import { createSwapOwnerTx, createAddOwnerTx } from '@/services/tx/tx-sender'
import { useAppDispatch } from '@/store'
import { upsertAddressBookEntry } from '@/store/addressBookSlice'
import { upsertAddressBookEntries } from '@/store/addressBookSlice'
import { SafeTxContext } from '../../SafeTxProvider'
import type { AddOwnerFlowProps } from '.'
import type { ReplaceOwnerFlowProps } from '../ReplaceOwner'
Expand Down Expand Up @@ -44,8 +44,8 @@ export const ReviewOwner = ({ params }: { params: AddOwnerFlowProps | ReplaceOwn
const addAddressBookEntryAndSubmit = () => {
if (typeof newOwner.name !== 'undefined') {
dispatch(
upsertAddressBookEntry({
chainId,
upsertAddressBookEntries({
chainIds: [chainId],
address: newOwner.address,
name: newOwner.name,
}),
Expand Down
2 changes: 1 addition & 1 deletion src/components/welcome/MyAccounts/AccountItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ const AccountItem = ({ onLinkClick, safeItem, safeOverview }: AccountItemProps)
</Link>
</Track>

<SafeListContextMenu name={name} address={address} chainId={chainId} addNetwork={isReplayable} />
<SafeListContextMenu name={name} address={address} chainId={chainId} addNetwork={isReplayable} rename />

<QueueActions
queued={safeOverview?.queued || 0}
Expand Down
4 changes: 3 additions & 1 deletion src/components/welcome/MyAccounts/SubAccountItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ const SubAccountItem = ({ onLinkClick, safeItem, safeOverview }: SubAccountItem)
</Link>
</Track>

<SafeListContextMenu name={name} address={address} chainId={chainId} addNetwork={false} />
{undeployedSafe && (
<SafeListContextMenu name={name} address={address} chainId={chainId} addNetwork={false} rename={false} />
)}

<QueueActions
queued={safeOverview?.queued || 0}
Expand Down
6 changes: 3 additions & 3 deletions src/features/counterfactual/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { getSafeSDKWithSigner, getUncheckedSigner, tryOffChainTxSigning } from '
import { getRelayTxStatus, TaskState } from '@/services/tx/txMonitor'
import type { AppDispatch } from '@/store'
import { addOrUpdateSafe } from '@/store/addedSafesSlice'
import { upsertAddressBookEntry } from '@/store/addressBookSlice'
import { upsertAddressBookEntries } from '@/store/addressBookSlice'
import { defaultSafeInfo } from '@/store/safeInfoSlice'
import { didRevert, type EthersError } from '@/utils/ethers-utils'
import { assertProvider, assertTx, assertWallet } from '@/utils/helpers'
Expand Down Expand Up @@ -171,7 +171,7 @@ export const createCounterfactualSafe = (
}

dispatch(addUndeployedSafe(undeployedSafe))
dispatch(upsertAddressBookEntry({ chainId: chain.chainId, address: safeAddress, name: data.name }))
dispatch(upsertAddressBookEntries({ chainIds: [chain.chainId], address: safeAddress, name: data.name }))
dispatch(
addOrUpdateSafe({
safe: {
Expand Down Expand Up @@ -217,7 +217,7 @@ export const replayCounterfactualSafeDeployment = (
}

dispatch(addUndeployedSafe(undeployedSafe))
dispatch(upsertAddressBookEntry({ chainId, address: safeAddress, name }))
dispatch(upsertAddressBookEntries({ chainIds: [chainId], address: safeAddress, name }))
dispatch(
addOrUpdateSafe({
safe: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import ModalDialog from '@/components/common/ModalDialog'
import NameInput from '@/components/common/NameInput'
import NetworkInput from '@/components/common/NetworkInput'
import ErrorMessage from '@/components/tx/ErrorMessage'
import { OVERVIEW_EVENTS, trackEvent } from '@/services/analytics'
Expand All @@ -22,7 +21,6 @@ import { useMemo, useState } from 'react'
import { useCompatibleNetworks } from '../../hooks/useCompatibleNetworks'

type CreateSafeOnNewChainForm = {
name: string
chainId: string
}

Expand Down Expand Up @@ -50,7 +48,6 @@ const ReplaySafeDialog = ({
const formMethods = useForm<CreateSafeOnNewChainForm>({
mode: 'all',
defaultValues: {
name: currentName,
chainId: chain?.chainId || '',
},
})
Expand Down Expand Up @@ -96,7 +93,13 @@ const ReplaySafeDialog = ({
trackEvent({ ...OVERVIEW_EVENTS.SUBMIT_ADD_NEW_NETWORK, label: selectedChain.chainName })

// 2. Replay Safe creation and add it to the counterfactual Safes
replayCounterfactualSafeDeployment(selectedChain.chainId, safeAddress, safeCreationData, data.name, dispatch)
replayCounterfactualSafeDeployment(
selectedChain.chainId,
safeAddress,
safeCreationData,
currentName || '',
dispatch,
)

router.push({
query: {
Expand Down Expand Up @@ -152,8 +155,6 @@ const ReplaySafeDialog = ({
<ErrorMessage level="error">This Safe cannot be replayed on any chains.</ErrorMessage>
) : (
<>
<NameInput name="name" label="Name" />

{chain ? (
<ChainIndicator chainId={chain.chainId} />
) : (
Expand Down
19 changes: 9 additions & 10 deletions src/store/__tests__/addressBookSlice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ import { faker } from '@faker-js/faker'
import {
addressBookSlice,
setAddressBook,
upsertAddressBookEntry,
removeAddressBookEntry,
selectAddressBookByChain,
upsertMultichainAddressBookEntry,
upsertAddressBookEntries,
} from '../addressBookSlice'

const initialState = {
Expand All @@ -22,8 +21,8 @@ describe('addressBookSlice', () => {
it('should insert an entry in the address book', () => {
const state = addressBookSlice.reducer(
initialState,
upsertAddressBookEntry({
chainId: '1',
upsertAddressBookEntries({
chainIds: ['1'],
address: '0x2',
name: 'Fred',
}),
Expand All @@ -37,8 +36,8 @@ describe('addressBookSlice', () => {
it('should ignore empty names in the address book', () => {
const state = addressBookSlice.reducer(
initialState,
upsertAddressBookEntry({
chainId: '1',
upsertAddressBookEntries({
chainIds: ['1'],
address: '0x2',
name: '',
}),
Expand All @@ -49,8 +48,8 @@ describe('addressBookSlice', () => {
it('should edit an entry in the address book', () => {
const state = addressBookSlice.reducer(
initialState,
upsertAddressBookEntry({
chainId: '1',
upsertAddressBookEntries({
chainIds: ['1'],
address: '0x0',
name: 'Alice in Wonderland',
}),
Expand All @@ -65,7 +64,7 @@ describe('addressBookSlice', () => {
const address = faker.finance.ethereumAddress()
const state = addressBookSlice.reducer(
initialState,
upsertMultichainAddressBookEntry({
upsertAddressBookEntries({
chainIds: ['1', '10', '100', '137'],
address,
name: 'Max',
Expand All @@ -84,7 +83,7 @@ describe('addressBookSlice', () => {
const address = faker.finance.ethereumAddress()
const state = addressBookSlice.reducer(
initialState,
upsertMultichainAddressBookEntry({
upsertAddressBookEntries({
chainIds: ['1', '10', '100', '137'],
address,
name: '',
Expand Down
17 changes: 2 additions & 15 deletions src/store/addressBookSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,7 @@ export const addressBookSlice = createSlice({
return action.payload
},

upsertAddressBookEntry: (state, action: PayloadAction<{ chainId: string; address: string; name: string }>) => {
const { chainId, address, name } = action.payload
if (name.trim() === '') {
return
}
if (!state[chainId]) state[chainId] = {}
state[chainId][address] = name
},

upsertMultichainAddressBookEntry: (
state,
action: PayloadAction<{ chainIds: string[]; address: string; name: string }>,
) => {
upsertAddressBookEntries: (state, action: PayloadAction<{ chainIds: string[]; address: string; name: string }>) => {
const { chainIds, address, name } = action.payload
if (name.trim() === '') {
return
Expand All @@ -57,8 +45,7 @@ export const addressBookSlice = createSlice({
},
})

export const { setAddressBook, upsertAddressBookEntry, upsertMultichainAddressBookEntry, removeAddressBookEntry } =
addressBookSlice.actions
export const { setAddressBook, upsertAddressBookEntries, removeAddressBookEntry } = addressBookSlice.actions

export const selectAllAddressBooks = (state: RootState): AddressBookState => {
return state[addressBookSlice.name]
Expand Down

0 comments on commit 54f7c47

Please sign in to comment.