diff --git a/src/components/settings/Recovery/index.tsx b/src/components/settings/Recovery/index.tsx index 3783806a2c..ba7a6f4379 100644 --- a/src/components/settings/Recovery/index.tsx +++ b/src/components/settings/Recovery/index.tsx @@ -5,10 +5,16 @@ import type { ReactElement } from 'react' import { EnableRecoveryFlow } from '@/components/tx-flow/flows/EnableRecovery' import { TxModalContext } from '@/components/tx-flow' import { useDarkMode } from '@/hooks/useDarkMode' +import { RecoverAccountFlow } from '@/components/tx-flow/flows/RecoverAccount' +import useWallet from '@/hooks/wallets/useWallet' +import { useAppSelector } from '@/store' +import { selectRecoveryByGuardian } from '@/store/recoverySlice' export function Recovery(): ReactElement { const { setTxFlow } = useContext(TxModalContext) const isDarkMode = useDarkMode() + const wallet = useWallet() + const recovery = useAppSelector((state) => selectRecoveryByGuardian(state, wallet?.address ?? '')) return ( @@ -35,9 +41,16 @@ export function Recovery(): ReactElement { Enabling the Account recovery module will require a transactions. - + {recovery ? ( + // TODO: Move to correct location when widget is ready + + ) : ( + + )} diff --git a/src/components/tx-flow/common/NewOwnerList/index.tsx b/src/components/tx-flow/common/NewOwnerList/index.tsx new file mode 100644 index 0000000000..2164ba756a --- /dev/null +++ b/src/components/tx-flow/common/NewOwnerList/index.tsx @@ -0,0 +1,30 @@ +import { Paper, Typography, SvgIcon } from '@mui/material' +import type { AddressEx } from '@safe-global/safe-gateway-typescript-sdk' +import type { ReactElement } from 'react' + +import PlusIcon from '@/public/images/common/plus.svg' +import EthHashInfo from '@/components/common/EthHashInfo' + +import css from './styles.module.css' + +export function NewOwnerList({ newOwners }: { newOwners: Array }): ReactElement { + return ( + + + + New owner{newOwners.length > 1 ? 's' : ''} + + {newOwners.map((newOwner) => ( + + ))} + + ) +} diff --git a/src/components/tx-flow/common/NewOwnerList/styles.module.css b/src/components/tx-flow/common/NewOwnerList/styles.module.css new file mode 100644 index 0000000000..dcf6189778 --- /dev/null +++ b/src/components/tx-flow/common/NewOwnerList/styles.module.css @@ -0,0 +1,7 @@ +.container { + display: flex; + flex-direction: column; + gap: var(--space-1); + padding: var(--space-2); + background-color: var(--color-success-background); +} diff --git a/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx b/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx index d2c18301b3..f8ca7bbe70 100644 --- a/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx +++ b/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx @@ -10,7 +10,7 @@ import { upsertAddressBookEntry } from '@/store/addressBookSlice' import { SafeTxContext } from '../../SafeTxProvider' import type { AddOwnerFlowProps } from '.' import type { ReplaceOwnerFlowProps } from '../ReplaceOwner' -import PlusIcon from '@/public/images/common/plus.svg' +import { NewOwnerList } from '../../common/NewOwnerList' import MinusIcon from '@/public/images/common/minus.svg' import EthHashInfo from '@/components/common/EthHashInfo' import commonCss from '@/components/tx-flow/common/styles.module.css' @@ -68,13 +68,7 @@ export const ReviewOwner = ({ params }: { params: AddOwnerFlowProps | ReplaceOwn /> )} - palette.success.background, p: 2 }}> - - - New owner - - - + Any transaction requires the confirmation of: diff --git a/src/components/tx-flow/flows/RecoverAccount/RecoverAccountFlowReview.tsx b/src/components/tx-flow/flows/RecoverAccount/RecoverAccountFlowReview.tsx new file mode 100644 index 0000000000..a7689a09eb --- /dev/null +++ b/src/components/tx-flow/flows/RecoverAccount/RecoverAccountFlowReview.tsx @@ -0,0 +1,158 @@ +import { CardActions, Button, Typography, Divider, Box } from '@mui/material' +import { useContext, useEffect, useState } from 'react' +import type { ReactElement } from 'react' + +import useSafeInfo from '@/hooks/useSafeInfo' +import { getRecoveryProposalTransactions } from '@/services/recovery/transaction' +import DecodedTx from '@/components/tx/DecodedTx' +import ErrorMessage from '@/components/tx/ErrorMessage' +import { RedefineBalanceChanges } from '@/components/tx/security/redefine/RedefineBalanceChange' +import ConfirmationTitle, { ConfirmationTitleTypes } from '@/components/tx/SignOrExecuteForm/ConfirmationTitle' +import TxChecks from '@/components/tx/SignOrExecuteForm/TxChecks' +import { WrongChainWarning } from '@/components/tx/WrongChainWarning' +import useDecodeTx from '@/hooks/useDecodeTx' +import TxCard from '../../common/TxCard' +import { SafeTxContext } from '../../SafeTxProvider' +import CheckWallet from '@/components/common/CheckWallet' +import { createMultiSendCallOnlyTx, createTx, dispatchRecoveryProposal } from '@/services/tx/tx-sender' +import { RecoverAccountFlowFields } from '.' +import { NewOwnerList } from '../../common/NewOwnerList' +import { useAppSelector } from '@/store' +import { selectRecoveryByGuardian } from '@/store/recoverySlice' +import useWallet from '@/hooks/wallets/useWallet' +import useOnboard from '@/hooks/wallets/useOnboard' +import { TxModalContext } from '../..' +import { asError } from '@/services/exceptions/utils' +import { trackError, Errors } from '@/services/exceptions' +import type { RecoverAccountFlowProps } from '.' + +import commonCss from '@/components/tx-flow/common/styles.module.css' + +export function RecoverAccountFlowReview({ params }: { params: RecoverAccountFlowProps }): ReactElement | null { + // Form state + const [isSubmittable, setIsSubmittable] = useState(true) + const [submitError, setSubmitError] = useState() + + // Hooks + const { setTxFlow } = useContext(TxModalContext) + const { safeTx, safeTxError, setSafeTx, setSafeTxError } = useContext(SafeTxContext) + const [decodedData, decodedDataError, decodedDataLoading] = useDecodeTx(safeTx) + const { safe } = useSafeInfo() + const wallet = useWallet() + const onboard = useOnboard() + const recovery = useAppSelector((state) => selectRecoveryByGuardian(state, wallet?.address ?? '')) + + // Proposal + const txCooldown = recovery?.txCooldown?.toNumber() + const newThreshold = Number(params[RecoverAccountFlowFields.threshold]) + const newOwners = params[RecoverAccountFlowFields.owners] + + useEffect(() => { + const transactions = getRecoveryProposalTransactions({ + safe, + newThreshold, + newOwners, + }) + + const promise = transactions.length > 1 ? createMultiSendCallOnlyTx(transactions) : createTx(transactions[0]) + + promise.then(setSafeTx).catch(setSafeTxError) + }, [newThreshold, newOwners, safe, setSafeTx, setSafeTxError]) + + // On modal submit + const onSubmit = async () => { + if (!recovery || !onboard) { + return + } + + setIsSubmittable(false) + setSubmitError(undefined) + + try { + await dispatchRecoveryProposal({ onboard, safe, newThreshold, newOwners, delayModifierAddress: recovery.address }) + } catch (_err) { + const err = asError(_err) + trackError(Errors._810, err) + setIsSubmittable(true) + setSubmitError(err) + return + } + + setTxFlow(undefined) + } + + const submitDisabled = !safeTx || !isSubmittable || !recovery + + return ( + <> + + + This transaction will reset the Account setup, changing the owners + {newThreshold !== safe.threshold ? ' and threshold' : ''}. + + + + + + + + + After recovery, Safe Account transactions will require: + + + {params.threshold} out of {params[RecoverAccountFlowFields.owners].length} owners. + + + + + + + + + + + + + + + + + + {safeTxError && ( + + This recovery will most likely fail. To save gas costs, avoid executing the transaction. + + )} + + {submitError && ( + Error submitting the transaction. Please try again. + )} + + + + + {/* // TODO: Convert txCooldown to days, minutes, seconds when https://github.com/safe-global/safe-wallet-web/pull/2772 is merged */} + Recovery will be {txCooldown === 0 ? 'immediately possible' : `possible ${txCooldown}`} after this transaction + is executed. + + + + + + + {(isOk) => ( + + )} + + + + + ) +} diff --git a/src/components/tx-flow/flows/RecoverAccount/RecoverAccountFlowSetup.tsx b/src/components/tx-flow/flows/RecoverAccount/RecoverAccountFlowSetup.tsx new file mode 100644 index 0000000000..2d67ae8915 --- /dev/null +++ b/src/components/tx-flow/flows/RecoverAccount/RecoverAccountFlowSetup.tsx @@ -0,0 +1,166 @@ +import { + Typography, + Divider, + CardActions, + Button, + SvgIcon, + Grid, + MenuItem, + TextField, + IconButton, + Tooltip, +} from '@mui/material' +import { useForm, FormProvider, useFieldArray, Controller } from 'react-hook-form' +import type { ReactElement } from 'react' + +import TxCard from '../../common/TxCard' +import AddIcon from '@/public/images/common/add.svg' +import DeleteIcon from '@/public/images/common/delete.svg' +import { RecoverAccountFlowFields } from '.' +import AddressBookInput from '@/components/common/AddressBookInput' +import { TOOLTIP_TITLES } from '../../common/constants' +import InfoIcon from '@/public/images/notifications/info.svg' +import useSafeInfo from '@/hooks/useSafeInfo' +import { sameAddress } from '@/utils/addresses' +import type { RecoverAccountFlowProps } from '.' + +import commonCss from '@/components/tx-flow/common/styles.module.css' + +export function RecoverAccountFlowSetup({ + params, + onSubmit, +}: { + params: RecoverAccountFlowProps + onSubmit: (formData: RecoverAccountFlowProps) => void +}): ReactElement { + const { safeAddress } = useSafeInfo() + + const formMethods = useForm({ + defaultValues: params, + mode: 'all', + }) + + const { fields, append, remove } = useFieldArray({ + control: formMethods.control, + name: RecoverAccountFlowFields.owners, + }) + + const owners = formMethods.watch(RecoverAccountFlowFields.owners) + + return ( + +
+ + + Add owner(s) + + + + Set the new owner wallet(s) of this Safe Account and how many need to confirm a transaction before it can be + executed. + + + + {fields.map((field, index) => ( + <> + + { + if (sameAddress(value, safeAddress)) { + return 'The Safe Account cannot own itself' + } + + const isDuplicate = owners.filter((owner) => owner.value === value).length > 1 + if (isDuplicate) { + return 'Already designated to be an owner' + } + }} + /> + + + + {index > 0 && ( + remove(index)}> + + + )} + + + ))} + + + + + + + + Threshold + + + + + + + + + After recovery, Safe Account transactions will require: + + + + + ( + + {fields.map((_, index) => { + const value = index + 1 + return ( + + {value} + + ) + })} + + )} + /> + + + + out of {fields.length} owner(s) + + + + + + + + + +
+ + ) +} diff --git a/src/components/tx-flow/flows/RecoverAccount/index.tsx b/src/components/tx-flow/flows/RecoverAccount/index.tsx new file mode 100644 index 0000000000..af075b6e77 --- /dev/null +++ b/src/components/tx-flow/flows/RecoverAccount/index.tsx @@ -0,0 +1,44 @@ +import type { ReactElement } from 'react' +import type { AddressEx } from '@safe-global/safe-gateway-typescript-sdk' + +import TxLayout from '@/components/tx-flow/common/TxLayout' +import SaveAddressIcon from '@/public/images/common/save-address.svg' +import useTxStepper from '../../useTxStepper' +import { RecoverAccountFlowReview } from './RecoverAccountFlowReview' +import { RecoverAccountFlowSetup } from './RecoverAccountFlowSetup' + +export enum RecoverAccountFlowFields { + owners = 'owners', + threshold = 'threshold', +} + +export type RecoverAccountFlowProps = { + // RHF accept primitive field arrays + [RecoverAccountFlowFields.owners]: Array + [RecoverAccountFlowFields.threshold]: string +} + +export function RecoverAccountFlow(): ReactElement { + const { data, step, nextStep, prevStep } = useTxStepper({ + [RecoverAccountFlowFields.owners]: [{ value: '' }], + [RecoverAccountFlowFields.threshold]: '1', + }) + + const steps = [ + nextStep({ ...data, ...formData })} />, + , + ] + + return ( + + {steps} + + ) +} diff --git a/src/components/tx/SignOrExecuteForm/TxChecks.tsx b/src/components/tx/SignOrExecuteForm/TxChecks.tsx index d1fb2444fa..9ad3f069f0 100644 --- a/src/components/tx/SignOrExecuteForm/TxChecks.tsx +++ b/src/components/tx/SignOrExecuteForm/TxChecks.tsx @@ -6,14 +6,14 @@ import { Redefine, RedefineMessage } from '@/components/tx/security/redefine' import css from './styles.module.css' -const TxChecks = (): ReactElement => { +const TxChecks = ({ isRecovery = false }: { isRecovery?: boolean }): ReactElement => { const { safeTx } = useContext(SafeTxContext) return ( <> Transaction checks - + diff --git a/src/components/tx/security/tenderly/index.tsx b/src/components/tx/security/tenderly/index.tsx index 4f1487205e..fa4ce7c6a3 100644 --- a/src/components/tx/security/tenderly/index.tsx +++ b/src/components/tx/security/tenderly/index.tsx @@ -23,12 +23,18 @@ export type TxSimulationProps = { transactions?: SimulationTxParams['transactions'] gasLimit?: number disabled: boolean + isRecovery?: boolean } // TODO: Investigate resetting on gasLimit change as we are not simulating with the gasLimit of the tx // otherwise remove all usage of gasLimit in simulation. Note: this was previously being done. // TODO: Test this component -const TxSimulationBlock = ({ transactions, disabled, gasLimit }: TxSimulationProps): ReactElement => { +const TxSimulationBlock = ({ + transactions, + disabled, + gasLimit, + isRecovery = false, +}: TxSimulationProps): ReactElement => { const { safe } = useSafeInfo() const wallet = useWallet() const isDarkMode = useDarkMode() @@ -45,7 +51,7 @@ const TxSimulationBlock = ({ transactions, disabled, gasLimit }: TxSimulationPro simulateTransaction({ safe, - executionOwner: wallet.address, + executionOwner: isRecovery ? safe.owners[0].value : wallet.address, transactions, gasLimit, } as SimulationTxParams) diff --git a/src/services/exceptions/ErrorCodes.ts b/src/services/exceptions/ErrorCodes.ts index 90993a46ef..8319df5f75 100644 --- a/src/services/exceptions/ErrorCodes.ts +++ b/src/services/exceptions/ErrorCodes.ts @@ -61,6 +61,7 @@ enum ErrorCodes { _807 = '807: Failed to remove guard', _808 = '808: Failed to get transaction origin', _809 = '809: Failed decoding transaction', + _810 = '810: Error executing a recovery proposal transaction', _900 = '900: Error loading Safe App', _901 = '901: Error processing Safe Apps SDK request', diff --git a/src/services/recovery/__tests__/transaction.test.ts b/src/services/recovery/__tests__/transaction.test.ts new file mode 100644 index 0000000000..faf447c8d8 --- /dev/null +++ b/src/services/recovery/__tests__/transaction.test.ts @@ -0,0 +1,666 @@ +import { faker } from '@faker-js/faker' +import { Interface } from 'ethers/lib/utils' +import { SENTINEL_ADDRESS } from '@safe-global/safe-core-sdk/dist/src/utils/constants' +import { OperationType } from '@safe-global/safe-core-sdk-types' +import * as deployments from '@safe-global/safe-deployments' +import type { SafeInfo } from '@safe-global/safe-gateway-typescript-sdk' + +import { getRecoveryProposalTransaction, getRecoveryProposalTransactions } from '../transaction' + +describe('transaction', () => { + describe('getRecoveryTransactions', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + const encodeFunctionDataSpy = jest.spyOn(Interface.prototype, 'encodeFunctionData') + + describe('when recovering with the same number of owner(s) as the current Safe owner(s)', () => { + describe('with unique owners', () => { + describe('should swap all owners when the threshold remains the same', () => { + it('for singular owners', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(1) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(1) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + }) + + it('for multiple owners', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(3) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(3) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'swapOwner', [oldOwner2, oldOwner3, newOwner3]) + }) + }) + + it('should swap all owners and finally change the threshold if it changes', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + const newThreshold = oldThreshold + 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + expect(transactions).toHaveLength(4) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(4) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'swapOwner', [oldOwner2, oldOwner3, newOwner3]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(4, 'changeThreshold', [newThreshold]) + }) + }) + + describe('with duplicate owners', () => { + describe('should swap all differing owners when the threshold remains the same', () => { + it('for singular owners it should return nothing', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: oldOwner1 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(0) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(0) + }) + + it('for multiple owners', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = oldOwner3 + + const oldThreshold = 2 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(2) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(2) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + }) + }) + + it('should swap all differing owners and finally change the threshold if it changes', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + const newThreshold = oldThreshold + 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: oldOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + expect(transactions).toHaveLength(3) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(3) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'changeThreshold', [newThreshold]) + }) + }) + + it('should change the threshold with the same owners', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + const newThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + expect(transactions).toHaveLength(1) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(1) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'changeThreshold', [newThreshold]) + }) + }) + + describe('when recovering with more owner(s) than the current Safe owner(s)', () => { + describe('with unique owners', () => { + describe('should swap as many owners as possible then add the rest when the threshold remains the same', () => { + it('for singular owners', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(3) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(3) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'addOwnerWithThreshold', [newOwner2, 1]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'addOwnerWithThreshold', [newOwner3, oldThreshold]) + }) + + it('for multiple owners', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(3) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(3) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'addOwnerWithThreshold', [newOwner3, oldThreshold]) + }) + }) + + it('should swap as many owners as possible then add the rest when with a final threshold change if the threshold changes', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + const newThreshold = oldThreshold + 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + expect(transactions).toHaveLength(3) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(3) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'addOwnerWithThreshold', [newOwner3, newThreshold]) + }) + }) + + describe('with duplicates owners', () => { + it('should swap as many differing owners as possible then add the rest when the threshold remains the same', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: oldOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(2) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(2) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'addOwnerWithThreshold', [newOwner2, 1]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'addOwnerWithThreshold', [newOwner3, oldThreshold]) + }) + + it('should swap as many differing owners as possible then add the rest when with a final threshold change if the threshold changes', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + const newThreshold = oldThreshold + 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: oldOwner1 }, { value: newOwner1 }, { value: newOwner2 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + expect(transactions).toHaveLength(2) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(2) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'addOwnerWithThreshold', [newOwner1, 1]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'addOwnerWithThreshold', [newOwner2, newThreshold]) + }) + }) + }) + + describe('when recovering with less owner(s) than the current Safe owner(s)', () => { + describe('with unique owners', () => { + it('should swap as many owners as possible then remove the rest when the threshold remains the same', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }] + + const transactions = getRecoveryProposalTransactions({ + safe, + newThreshold: oldThreshold, + newOwners, + }) + + expect(transactions).toHaveLength(3) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(3) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'removeOwner', [oldOwner2, oldOwner3, oldThreshold]) + }) + + it('should swap as many owners as possible then remove the rest when with a final threshold change if the threshold changes', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + const newThreshold = oldThreshold + 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + expect(transactions).toHaveLength(3) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(3) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [ + SENTINEL_ADDRESS, + oldOwner1, + newOwner1, + ]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'swapOwner', [oldOwner1, oldOwner2, newOwner2]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(3, 'removeOwner', [oldOwner2, oldOwner3, newThreshold]) + }) + }) + + describe('with duplicates owners', () => { + it('should swap as many differing owners as possible then remove the rest when the threshold remains the same', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: oldOwner1 }, { value: newOwner1 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold: oldThreshold, newOwners }) + + expect(transactions).toHaveLength(2) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(2) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [oldOwner1, oldOwner2, newOwner1]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'removeOwner', [oldOwner2, oldOwner3, oldThreshold]) + }) + + it('should swap as many differing owners as possible then remove the rest when with a final threshold change if the threshold changes', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + const newThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: oldOwner1 }, { value: newOwner1 }] + + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + expect(transactions).toHaveLength(2) + + expect(encodeFunctionDataSpy).toHaveBeenCalledTimes(2) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(1, 'swapOwner', [oldOwner1, oldOwner2, newOwner1]) + expect(encodeFunctionDataSpy).toHaveBeenNthCalledWith(2, 'removeOwner', [oldOwner2, oldOwner3, newThreshold]) + }) + }) + }) + }) + + describe('getRecoveryProposalTransaction', () => { + it('should throw an error when no recovery transactions are found', () => { + const safe = { + address: { value: faker.finance.ethereumAddress() }, + owners: [{ value: faker.finance.ethereumAddress() }], + threshold: 1, + } as SafeInfo + + expect(() => + getRecoveryProposalTransaction({ + safe, + newThreshold: safe.threshold, + newOwners: safe.owners, + }), + ).toThrow('No recovery transactions found') + }) + + it('should return the transaction when a single recovery transaction is found', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const newOwner1 = faker.finance.ethereumAddress() + + const oldThreshold = 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }] + + const transaction = getRecoveryProposalTransaction({ + safe, + newThreshold: oldThreshold, + newOwners, + }) + + expect(transaction).toEqual({ + to: safeAddresss, + value: '0', + data: expect.any(String), + operation: OperationType.Call, + }) + }) + + describe('when multiple recovery transactions are found', () => { + it('should return a MetaTransactionData object ', () => { + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + const oldOwner3 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + const newThreshold = oldThreshold + 1 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }, { value: oldOwner3 }], + threshold: oldThreshold, + } as SafeInfo + + const multiSendDeployment = deployments.getMultiSendCallOnlyDeployment()! + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + const transaction = getRecoveryProposalTransaction({ + safe, + newThreshold, + newOwners, + }) + + expect(transaction).toEqual({ + to: multiSendDeployment.defaultAddress, + value: '0', + data: expect.any(String), + operation: OperationType.Call, + }) + }) + + it('should throw an error when MultiSend deployment is not found', () => { + jest.spyOn(deployments, 'getMultiSendCallOnlyDeployment').mockReturnValue(undefined) + + const safeAddresss = faker.finance.ethereumAddress() + + const oldOwner1 = faker.finance.ethereumAddress() + const oldOwner2 = faker.finance.ethereumAddress() + + const newOwner1 = faker.finance.ethereumAddress() + const newOwner2 = faker.finance.ethereumAddress() + const newOwner3 = faker.finance.ethereumAddress() + + const oldThreshold = 2 + + const safe = { + address: { value: safeAddresss }, + owners: [{ value: oldOwner1 }, { value: oldOwner2 }], + threshold: oldThreshold, + } as SafeInfo + + const newOwners = [{ value: newOwner1 }, { value: newOwner2 }, { value: newOwner3 }] + + expect(() => + getRecoveryProposalTransaction({ + safe, + newThreshold: oldThreshold, + newOwners, + }), + ).toThrow('MultiSend deployment not found') + }) + }) + }) +}) diff --git a/src/services/recovery/transaction.ts b/src/services/recovery/transaction.ts new file mode 100644 index 0000000000..83e6786904 --- /dev/null +++ b/src/services/recovery/transaction.ts @@ -0,0 +1,133 @@ +import { Interface } from 'ethers/lib/utils' +import { getMultiSendCallOnlyDeployment, getSafeSingletonDeployment } from '@safe-global/safe-deployments' +import { SENTINEL_ADDRESS } from '@safe-global/safe-core-sdk/dist/src/utils/constants' +import { encodeMultiSendData } from '@safe-global/safe-core-sdk/dist/src/utils/transactions/utils' +import { OperationType } from '@safe-global/safe-core-sdk-types' +import type { MetaTransactionData } from '@safe-global/safe-core-sdk-types' +import type { AddressEx, SafeInfo } from '@safe-global/safe-gateway-typescript-sdk' +import { sameAddress } from '@/utils/addresses' + +export function getRecoveryProposalTransactions({ + safe, + newThreshold, + newOwners, +}: { + safe: SafeInfo + newThreshold: number + newOwners: Array +}): Array { + const INTERMEDIARY_THRESHOLD = 1 + + const safeDeployment = getSafeSingletonDeployment({ network: safe.chainId, version: safe.version ?? undefined }) + + if (!safeDeployment) { + throw new Error('Safe deployment not found') + } + + const safeInterface = new Interface(safeDeployment.abi) + + // Cache owner changes to determine prevOwner + let _owners = [...safe.owners] + + // Don't add/remove same owners + const ownersToAdd = newOwners.filter( + (newOwner) => !_owners.some((oldOwner) => sameAddress(oldOwner.value, newOwner.value)), + ) + const ownersToRemove = safe.owners.filter( + (oldOwner) => !newOwners.some((newOwner) => sameAddress(newOwner.value, oldOwner.value)), + ) + + // Check whether threshold should be changed after owner management + let changeThreshold = newThreshold !== safe.threshold + + const txData: Array = [] + + const length = Math.max(ownersToAdd.length, ownersToRemove.length) + + for (let index = 0; index < length; index++) { + const ownerToAdd = ownersToAdd[index]?.value + const ownerToRemove = ownersToRemove[index]?.value + + const ownerIndex = _owners.findIndex((owner) => sameAddress(owner.value, ownerToRemove)) + const prevOwner = ownerIndex === 0 ? SENTINEL_ADDRESS : _owners[ownerIndex - 1]?.value + + // Swap owner if possible + if (ownerToRemove && ownerToAdd) { + txData.push(safeInterface.encodeFunctionData('swapOwner', [prevOwner, ownerToRemove, ownerToAdd])) + + // Update cache to reflect swap + _owners = _owners.map((owner) => { + if (sameAddress(owner.value, ownerToRemove)) { + return ownersToAdd[index] + } + return owner + }) + continue + } + + // Add or remove owner, finally setting threshold (intermediary value prevents threshold > owner length) + const threshold = index === length - 1 ? newThreshold : INTERMEDIARY_THRESHOLD + + if (!ownerToRemove) { + txData.push(safeInterface.encodeFunctionData('addOwnerWithThreshold', [ownerToAdd, threshold])) + + // Update cache to reflect addition + _owners.push(ownersToAdd[index]) + } else { + txData.push(safeInterface.encodeFunctionData('removeOwner', [prevOwner, ownerToRemove, threshold])) + + // Update cache to reflect removal + _owners = _owners.filter((owner) => !sameAddress(owner.value, ownerToRemove)) + } + + changeThreshold = false + } + + // If only swapOwners exist but there is a threshold change, changeThreshold + if (changeThreshold) { + txData.push(safeInterface.encodeFunctionData('changeThreshold', [newThreshold])) + } + + return txData.map((data) => ({ + to: safe.address.value, + value: '0', + operation: OperationType.Call, + data, + })) +} + +export function getRecoveryProposalTransaction({ + safe, + newThreshold, + newOwners, +}: { + safe: SafeInfo + newThreshold: number + newOwners: Array +}): MetaTransactionData { + const transactions = getRecoveryProposalTransactions({ safe, newThreshold, newOwners }) + + if (transactions.length === 0) { + throw new Error('No recovery transactions found') + } + + if (transactions.length === 1) { + return transactions[0] + } + + const multiSendDeployment = getMultiSendCallOnlyDeployment({ + network: safe.chainId, + version: safe.version ?? undefined, + }) + + if (!multiSendDeployment) { + throw new Error('MultiSend deployment not found') + } + + return { + to: multiSendDeployment.networkAddresses[safe.chainId] ?? multiSendDeployment.defaultAddress, + value: '0', + operation: OperationType.Call, + data: encodeMultiSendData(transactions), + } +} diff --git a/src/services/tx/tx-sender/dispatch.ts b/src/services/tx/tx-sender/dispatch.ts index 570aa0e0ea..98b9ce244c 100644 --- a/src/services/tx/tx-sender/dispatch.ts +++ b/src/services/tx/tx-sender/dispatch.ts @@ -1,4 +1,5 @@ -import type { SafeInfo, TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' +import type { AddressEx, SafeInfo, TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' +import { OperationType } from '@safe-global/safe-core-sdk-types' import type { SafeTransaction, TransactionOptions, TransactionResult } from '@safe-global/safe-core-sdk-types' import type { EthersError } from '@/utils/ethers-utils' import { didReprice, didRevert } from '@/utils/ethers-utils' @@ -22,6 +23,8 @@ import { import { createWeb3 } from '@/hooks/wallets/web3' import { type OnboardAPI } from '@web3-onboard/core' import { asError } from '@/services/exceptions/utils' +import { getRecoveryProposalTransaction } from '@/services/recovery/transaction' +import { getModuleInstance, KnownContracts } from '@gnosis.pm/zodiac' /** * Propose a transaction @@ -400,3 +403,31 @@ export const dispatchBatchExecutionRelay = async ( groupKey, ) } + +export async function dispatchRecoveryProposal({ + onboard, + safe, + newThreshold, + newOwners, + delayModifierAddress, +}: { + onboard: OnboardAPI + safe: SafeInfo + newThreshold: number + newOwners: Array + delayModifierAddress: string +}) { + const wallet = await assertWalletChain(onboard, safe.chainId) + const provider = createWeb3(wallet.provider) + + const { to, value, data } = getRecoveryProposalTransaction({ + safe, + newThreshold, + newOwners, + }) + + const delayModifier = getModuleInstance(KnownContracts.DELAY, delayModifierAddress, provider) + + const signer = provider.getSigner() + await delayModifier.connect(signer).execTransactionFromModule(to, value, data, OperationType.Call) +} diff --git a/src/store/recoverySlice.ts b/src/store/recoverySlice.ts index 1755799cf3..855bc2da0b 100644 --- a/src/store/recoverySlice.ts +++ b/src/store/recoverySlice.ts @@ -3,6 +3,8 @@ import type { TransactionAddedEvent } from '@gnosis.pm/zodiac/dist/cjs/types/Del import type { BigNumber } from 'ethers' import { makeLoadableSlice } from './common' +import { sameAddress } from '@/utils/addresses' +import type { RootState } from '.' export type RecoveryQueueItem = TransactionAddedEvent & { timestamp: number @@ -26,4 +28,11 @@ const { slice, selector } = makeLoadableSlice('recovery', initialState) export const recoverySlice = slice -export const selectRecovery = createSelector(selector, (recovery) => recovery.data) +const selectRecovery = createSelector(selector, (recovery) => recovery.data) + +export const selectRecoveryByGuardian = createSelector( + [selectRecovery, (_: RootState, walletAddress: string) => walletAddress], + (recovery, walletAddress) => { + return recovery.find(({ modules }) => modules.some((module) => sameAddress(module, walletAddress))) + }, +)