Skip to content

Commit

Permalink
fix: [Counterfactual] Enable addOwner and swapOwner flow for counterf…
Browse files Browse the repository at this point in the history
…actual safes (safe-global#3192)

* fix: Enable addOwner and swapOwner flow for counterfactual safes

* fix: Add extra gas on top for safety
  • Loading branch information
usame-algan authored Feb 9, 2024
1 parent 3d14ef7 commit 88c5b56
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 9 deletions.
10 changes: 7 additions & 3 deletions src/components/tx-flow/flows/AddOwner/ReviewOwner.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useCurrentChain } from '@/hooks/useChains'
import { useContext, useEffect } from 'react'
import { Typography, Divider, Box, SvgIcon, Paper } from '@mui/material'

Expand All @@ -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') {
Expand Down
20 changes: 19 additions & 1 deletion src/features/counterfactual/hooks/useDeployGasLimit.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,26 @@
import { id } from 'ethers'
import useAsync from '@/hooks/useAsync'
import useChainId from '@/hooks/useChainId'
import useOnboard from '@/hooks/wallets/useOnboard'
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()
Expand All @@ -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 }
Expand Down
48 changes: 43 additions & 5 deletions src/services/tx/tx-sender/create.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -28,14 +31,49 @@ export const createRemoveOwnerTx = async (txParams: RemoveOwnerTxParams): Promis
return safeSDK.createRemoveOwnerTx(txParams)
}

export const createAddOwnerTx = async (txParams: AddOwnerTxParams): Promise<SafeTransaction> => {
export const createAddOwnerTx = async (
chain: ChainInfo,
isDeployed: boolean,
txParams: AddOwnerTxParams,
): Promise<SafeTransaction> => {
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<SafeTransaction> => {
export const createSwapOwnerTx = async (
chain: ChainInfo,
isDeployed: boolean,
txParams: SwapOwnerTxParams,
): Promise<SafeTransaction> => {
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<SafeTransaction> => {
Expand Down

0 comments on commit 88c5b56

Please sign in to comment.