Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Safe creation/batch execution gas estimation #2232

Merged
merged 3 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/components/new-safe/create/steps/ReviewStep/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { ExecutionMethodSelector, ExecutionMethod } from '@/components/tx/Execut
import { useLeastRemainingRelays } from '@/hooks/useRemainingRelays'
import classnames from 'classnames'
import { hasRemainingRelays } from '@/utils/relaying'
import { BigNumber } from 'ethers'

const ReviewStep = ({ data, onSubmit, onBack, setStep }: StepRenderProps<NewSafeFormData>) => {
const isWrongChain = useIsWrongChain()
Expand Down Expand Up @@ -56,8 +57,15 @@ const ReviewStep = ({ data, onSubmit, onBack, setStep }: StepRenderProps<NewSafe
const { gasLimit } = useEstimateSafeCreationGas(safeParams)

const totalFee =
gasLimit && maxFeePerGas && maxPriorityFeePerGas
? formatVisualAmount(maxFeePerGas.add(maxPriorityFeePerGas).mul(gasLimit), chain?.nativeCurrency.decimals)
gasLimit && maxFeePerGas
? formatVisualAmount(
maxFeePerGas
.add(
maxPriorityFeePerGas || BigNumber.from(0), // EIP-1559 disabled
)
.mul(gasLimit),
chain?.nativeCurrency.decimals,
)
: '> 0.001'

const handleBack = () => {
Expand Down
13 changes: 11 additions & 2 deletions src/components/new-safe/create/steps/StatusStep/useSafeCreation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { useAppDispatch } from '@/store'
import { closeByGroupKey } from '@/store/notificationsSlice'
import { CREATE_SAFE_EVENTS, trackEvent } from '@/services/analytics'
import { waitForCreateSafeTx } from '@/services/tx/txMonitor'
import useGasPrice from '@/hooks/useGasPrice'
import { shouldSetGasPrice } from '@/utils/transactions'

export enum SafeCreationStatus {
AWAITING,
Expand Down Expand Up @@ -48,6 +50,7 @@ export const useSafeCreation = (
const provider = useWeb3()
const web3ReadOnly = useWeb3ReadOnly()
const chain = useCurrentChain()
const { maxFeePerGas } = useGasPrice()

const createSafeCallback = useCallback(
async (txHash: string, tx: PendingSafeTx) => {
Expand All @@ -59,7 +62,7 @@ export const useSafeCreation = (
)

const handleCreateSafe = useCallback(async () => {
if (!pendingSafe || !provider || !chain || !wallet || isCreating) return
if (!pendingSafe || !provider || !chain || !wallet || isCreating || !maxFeePerGas) return
iamacook marked this conversation as resolved.
Show resolved Hide resolved

setIsCreating(true)
dispatch(closeByGroupKey({ groupKey: SAFE_CREATION_ERROR_KEY }))
Expand Down Expand Up @@ -87,7 +90,12 @@ export const useSafeCreation = (
chain.chainId,
)

await createNewSafe(provider, safeParams)
await createNewSafe(provider, {
...safeParams,
options: {
gasPrice: shouldSetGasPrice(chain, wallet) ? maxFeePerGas.toString() : undefined,
},
})
setStatus(SafeCreationStatus.SUCCESS)
}
} catch (err) {
Expand All @@ -107,6 +115,7 @@ export const useSafeCreation = (
createSafeCallback,
dispatch,
isCreating,
maxFeePerGas,
pendingSafe,
provider,
setPendingSafe,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Errors, logError } from '@/services/exceptions'
import ErrorMessage from '@/components/tx/ErrorMessage'
import type { BatchExecuteData } from '@/components/tx/modals/BatchExecuteModal/index'
import DecodedTxs from '@/components/tx/modals/BatchExecuteModal/DecodedTxs'
import { getMultiSendTxs, getTxsWithDetails } from '@/utils/transactions'
import { getMultiSendTxs, getTxsWithDetails, shouldSetGasPrice } from '@/utils/transactions'
import { TxSimulation } from '@/components/tx/TxSimulation'
import { useRelaysBySafe } from '@/hooks/useRemainingRelays'
import { ExecutionMethod, ExecutionMethodSelector } from '../../ExecutionMethodSelector'
Expand All @@ -21,6 +21,8 @@ import useOnboard from '@/hooks/wallets/useOnboard'
import { WrongChainWarning } from '@/components/tx/WrongChainWarning'
import { useWeb3 } from '@/hooks/wallets/web3'
import { hasRemainingRelays } from '@/utils/relaying'
import useGasPrice from '@/hooks/useGasPrice'
import useWallet from '@/hooks/wallets/useWallet'

const ReviewBatchExecute = ({ data, onSubmit }: { data: BatchExecuteData; onSubmit: (data: null) => void }) => {
const [isSubmittable, setIsSubmittable] = useState<boolean>(true)
Expand All @@ -29,6 +31,8 @@ const ReviewBatchExecute = ({ data, onSubmit }: { data: BatchExecuteData; onSubm
const chain = useCurrentChain()
const { safe } = useSafeInfo()
const [relays] = useRelaysBySafe()
const wallet = useWallet()
const { maxFeePerGas } = useGasPrice()

// Chain has relaying feature and available relays
const canRelay = hasRemainingRelays(relays)
Expand Down Expand Up @@ -58,7 +62,8 @@ const ReviewBatchExecute = ({ data, onSubmit }: { data: BatchExecuteData; onSubm
}, [txsWithDetails, multiSendTxs])

const onExecute = async () => {
if (!onboard || !multiSendTxData || !multiSendContract || !txsWithDetails) return
if (!onboard || !multiSendTxData || !multiSendContract || !txsWithDetails || !chain || !wallet || !maxFeePerGas)
iamacook marked this conversation as resolved.
Show resolved Hide resolved
return

await dispatchBatchExecution(
txsWithDetails,
Expand All @@ -67,6 +72,7 @@ const ReviewBatchExecute = ({ data, onSubmit }: { data: BatchExecuteData; onSubm
onboard,
safe.chainId,
safe.address.value,
shouldSetGasPrice(chain, wallet) ? maxFeePerGas : undefined,
)
onSubmit(null)
}
Expand Down Expand Up @@ -100,7 +106,7 @@ const ReviewBatchExecute = ({ data, onSubmit }: { data: BatchExecuteData; onSubm
}
}

const submitDisabled = loading || !isSubmittable
const submitDisabled = loading || !isSubmittable || !maxFeePerGas

return (
<div>
Expand Down
8 changes: 6 additions & 2 deletions src/services/tx/tx-sender/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { didReprice, didRevert } from '@/utils/ethers-utils'
import type MultiSendCallOnlyEthersContract from '@safe-global/safe-ethers-lib/dist/src/contracts/MultiSendCallOnly/MultiSendCallOnlyEthersContract'
import type { SpendingLimitTxParams } from '@/components/tx/modals/TokenTransferModal/ReviewSpendingLimitTx'
import { getSpendingLimitContract } from '@/services/contracts/spendingLimitContracts'
import type { ContractTransaction } from 'ethers'
import type { BigNumber, ContractTransaction } from 'ethers'
import type { RequestId } from '@safe-global/safe-apps-sdk'
import proposeTx from '../proposeTransaction'
import { txDispatch, TxEvent } from '../txEvents'
Expand Down Expand Up @@ -174,6 +174,7 @@ export const dispatchBatchExecution = async (
onboard: OnboardAPI,
chainId: SafeInfo['chainId'],
safeAddress: string,
gasPrice?: BigNumber,
) => {
const groupKey = multiSendTxData

Expand All @@ -183,7 +184,10 @@ export const dispatchBatchExecution = async (
const wallet = await assertWalletChain(onboard, chainId)

const provider = createWeb3(wallet.provider)
result = await multiSendContract.contract.connect(provider.getSigner()).multiSend(multiSendTxData)
result = await multiSendContract.contract
.connect(provider.getSigner())
.multiSend(multiSendTxData, { gasPrice: gasPrice?.toString() })

txs.forEach(({ txId }) => {
txDispatch(TxEvent.EXECUTING, { txId, groupKey })
})
Expand Down
16 changes: 16 additions & 0 deletions src/utils/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import { Multi_send__factory } from '@/types/contracts'
import { ethers } from 'ethers'
import { type BaseTransaction } from '@safe-global/safe-apps-sdk'
import { id } from 'ethers/lib/utils'
import chains from '@/config/chains'
import { WALLET_KEYS } from '@/hooks/wallets/consts'
import type { ConnectedWallet } from '@/services/onboard'

export const makeTxFromDetails = (txDetails: TransactionDetails): Transaction => {
const getMissingSigners = ({
Expand Down Expand Up @@ -266,3 +269,16 @@ export const decodeMultiSendTxs = (encodedMultiSendData: string): BaseTransactio

return txs
}

/**
* Transactions are underpriced on Ledger if the gasPrice is not set
*/
export const shouldSetGasPrice = (chain: ChainInfo, wallet: ConnectedWallet): boolean => {
const POLYGON_CHAIN_ID = chains.matic

const isEIP1559 = hasFeature(chain, FEATURES.EIP1559)
const isPolygon = chains[chain.shortName] === POLYGON_CHAIN_ID
const isLedger = wallet.label.toUpperCase() === WALLET_KEYS.LEDGER

return !isEIP1559 && isLedger && isPolygon
}