From 88c5b563410a074d680402173df2f3e7e6ae89a4 Mon Sep 17 00:00:00 2001 From: Usame Algan <5880855+usame-algan@users.noreply.github.com> Date: Fri, 9 Feb 2024 09:47:27 +0100 Subject: [PATCH] fix: [Counterfactual] Enable addOwner and swapOwner flow for counterfactual safes (#3192) * fix: Enable addOwner and swapOwner flow for counterfactual safes * fix: Add extra gas on top for safety --- .../tx-flow/flows/AddOwner/ReviewOwner.tsx | 10 ++-- .../counterfactual/hooks/useDeployGasLimit.ts | 20 +++++++- src/services/tx/tx-sender/create.ts | 48 +++++++++++++++++-- 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx b/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx index 464d2bf570..2c802bbbb2 100644 --- a/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx +++ b/src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx @@ -1,3 +1,4 @@ +import { useCurrentChain } from '@/hooks/useChains' import { useContext, useEffect } from 'react' import { Typography, Divider, Box, SvgIcon, Paper } from '@mui/material' @@ -20,21 +21,24 @@ export const ReviewOwner = ({ params }: { params: AddOwnerFlowProps | ReplaceOwn const { setSafeTx, setSafeTxError } = useContext(SafeTxContext) const { safe } = useSafeInfo() const { chainId } = safe + const chain = useCurrentChain() const { newOwner, removedOwner, threshold } = params useEffect(() => { + if (!chain) return + const promise = removedOwner - ? createSwapOwnerTx({ + ? createSwapOwnerTx(chain, safe.deployed, { newOwnerAddress: newOwner.address, oldOwnerAddress: removedOwner.address, }) - : createAddOwnerTx({ + : createAddOwnerTx(chain, safe.deployed, { ownerAddress: newOwner.address, threshold, }) promise.then(setSafeTx).catch(setSafeTxError) - }, [removedOwner, newOwner, threshold, setSafeTx, setSafeTxError]) + }, [removedOwner, newOwner, threshold, setSafeTx, setSafeTxError, chain, safe.deployed]) const addAddressBookEntryAndSubmit = () => { if (typeof newOwner.name !== 'undefined') { diff --git a/src/features/counterfactual/hooks/useDeployGasLimit.ts b/src/features/counterfactual/hooks/useDeployGasLimit.ts index 2fae468ca3..bee4b964bb 100644 --- a/src/features/counterfactual/hooks/useDeployGasLimit.ts +++ b/src/features/counterfactual/hooks/useDeployGasLimit.ts @@ -1,3 +1,4 @@ +import { id } from 'ethers' import useAsync from '@/hooks/useAsync' import useChainId from '@/hooks/useChainId' import useOnboard from '@/hooks/wallets/useOnboard' @@ -5,6 +6,21 @@ import { getSafeSDKWithSigner } from '@/services/tx/tx-sender/sdk' import { estimateSafeDeploymentGas, estimateSafeTxGas, estimateTxBaseGas } from '@safe-global/protocol-kit' import type { SafeTransaction } from '@safe-global/safe-core-sdk-types' +const ADD_OWNER_WITH_THRESHOLD_SIG_HASH = id('addOwnerWithThreshold(address,uint256)').slice(0, 10) +const SWAP_OWNER_SIG_HASH = id('swapOwner(address,address,address)').slice(0, 10) + +/** + * The estimation from the protocol-kit is too low for swapOwner and addOwnerWithThreshold, + * so we add a fixed amount + * @param safeTx + */ +const getExtraGasForSafety = (safeTx?: SafeTransaction): bigint => { + return safeTx?.data.data.startsWith(ADD_OWNER_WITH_THRESHOLD_SIG_HASH) || + safeTx?.data.data.startsWith(SWAP_OWNER_SIG_HASH) + ? 25000n + : 0n +} + const useDeployGasLimit = (safeTx?: SafeTransaction) => { const onboard = useOnboard() const chainId = useChainId() @@ -14,13 +30,15 @@ const useDeployGasLimit = (safeTx?: SafeTransaction) => { const sdk = await getSafeSDKWithSigner(onboard, chainId) + const extraGasForSafety = getExtraGasForSafety(safeTx) + const [gas, safeTxGas, safeDeploymentGas] = await Promise.all([ safeTx ? estimateTxBaseGas(sdk, safeTx) : '0', safeTx ? estimateSafeTxGas(sdk, safeTx) : '0', estimateSafeDeploymentGas(sdk), ]) - return BigInt(gas) + BigInt(safeTxGas) + BigInt(safeDeploymentGas) + return BigInt(gas) + BigInt(safeTxGas) + BigInt(safeDeploymentGas) + extraGasForSafety }, [onboard, chainId, safeTx]) return { gasLimit, gasLimitError, gasLimitLoading } diff --git a/src/services/tx/tx-sender/create.ts b/src/services/tx/tx-sender/create.ts index edd5dd6cea..d0b71e067d 100644 --- a/src/services/tx/tx-sender/create.ts +++ b/src/services/tx/tx-sender/create.ts @@ -1,4 +1,7 @@ -import type { TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' +import { LATEST_SAFE_VERSION } from '@/config/constants' +import { getReadOnlyGnosisSafeContract } from '@/services/contracts/safeContracts' +import { SENTINEL_ADDRESS } from '@safe-global/protocol-kit/dist/src/utils/constants' +import type { ChainInfo, TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' import { getTransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' import type { AddOwnerTxParams, RemoveOwnerTxParams, SwapOwnerTxParams } from '@safe-global/protocol-kit' import type { MetaTransactionData, SafeTransaction, SafeTransactionDataPartial } from '@safe-global/safe-core-sdk-types' @@ -28,14 +31,49 @@ export const createRemoveOwnerTx = async (txParams: RemoveOwnerTxParams): Promis return safeSDK.createRemoveOwnerTx(txParams) } -export const createAddOwnerTx = async (txParams: AddOwnerTxParams): Promise => { +export const createAddOwnerTx = async ( + chain: ChainInfo, + isDeployed: boolean, + txParams: AddOwnerTxParams, +): Promise => { const safeSDK = getAndValidateSafeSDK() - return safeSDK.createAddOwnerTx(txParams) + if (isDeployed) return safeSDK.createAddOwnerTx(txParams) + + const contract = await getReadOnlyGnosisSafeContract(chain, LATEST_SAFE_VERSION) + // @ts-ignore TODO: Fix overload issue + const data = contract.encode('addOwnerWithThreshold', [txParams.ownerAddress, txParams.threshold]) + + const tx = { + to: await safeSDK.getAddress(), + value: '0', + data, + } + + return safeSDK.createTransaction({ + transactions: [tx], + }) } -export const createSwapOwnerTx = async (txParams: SwapOwnerTxParams): Promise => { +export const createSwapOwnerTx = async ( + chain: ChainInfo, + isDeployed: boolean, + txParams: SwapOwnerTxParams, +): Promise => { const safeSDK = getAndValidateSafeSDK() - return safeSDK.createSwapOwnerTx(txParams) + if (isDeployed) return safeSDK.createSwapOwnerTx(txParams) + + const contract = await getReadOnlyGnosisSafeContract(chain, LATEST_SAFE_VERSION) + const data = contract.encode('swapOwner', [SENTINEL_ADDRESS, txParams.oldOwnerAddress, txParams.newOwnerAddress]) + + const tx = { + to: await safeSDK.getAddress(), + value: '0', + data, + } + + return safeSDK.createTransaction({ + transactions: [tx], + }) } export const createUpdateThresholdTx = async (threshold: number): Promise => {