Skip to content

Commit

Permalink
fix: [Counterfactual] Show notification when submitting counterfactua…
Browse files Browse the repository at this point in the history
…l tx (#3226)

* feat: Show notification when submitting counterfactual tx and break down costs

* fix: Use actual gas estimation for execution

* fix: Adjust wording

* fix: Adjust wording

* fix: Adjust wording and write tests for useDeployGasLimit

* fix: spread deploymentTx so that signing works again

* fix: Dispatch success event, adjust notification wording

* fix: Show success notification when deploying safe separately
  • Loading branch information
usame-algan authored Feb 12, 2024
1 parent a315552 commit 32747ba
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 29 deletions.
4 changes: 4 additions & 0 deletions src/components/dashboard/index.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import useOnboard from '@/hooks/wallets/useOnboard'
import type { ReactElement } from 'react'
import dynamic from 'next/dynamic'
import { Grid } from '@mui/material'
Expand All @@ -17,8 +18,11 @@ const RecoveryWidget = dynamic(() => import('@/features/recovery/components/Reco

const Dashboard = (): ReactElement => {
const router = useRouter()
const onboard = useOnboard()
const { [CREATION_MODAL_QUERY_PARM]: showCreationModal = '' } = router.query

console.log(onboard)

const supportsRecovery = useIsRecoverySupported()
const [recovery] = useRecovery()
const showRecoveryWidget = supportsRecovery && !recovery
Expand Down
21 changes: 15 additions & 6 deletions src/features/counterfactual/ActivateAccount.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import ErrorMessage from '@/components/tx/ErrorMessage'
import { ExecutionMethod, ExecutionMethodSelector } from '@/components/tx/ExecutionMethodSelector'
import useDeployGasLimit from '@/features/counterfactual/hooks/useDeployGasLimit'
import { removeUndeployedSafe, selectUndeployedSafe } from '@/features/counterfactual/store/undeployedSafesSlice'
import { CF_TX_GROUP_KEY, showSubmitNotification } from '@/features/counterfactual/utils'
import useChainId from '@/hooks/useChainId'
import { useCurrentChain } from '@/hooks/useChains'
import useGasPrice, { getTotalFeeFormatted } from '@/hooks/useGasPrice'
Expand All @@ -22,6 +23,7 @@ import useWallet from '@/hooks/wallets/useWallet'
import { useWeb3 } from '@/hooks/wallets/web3'
import { asError } from '@/services/exceptions/utils'
import { isSocialLoginWallet } from '@/services/mpc/SocialLoginModule'
import { txDispatch, TxEvent } from '@/services/tx/txEvents'
import { waitForCreateSafeTx } from '@/services/tx/txMonitor'
import { useAppDispatch, useAppSelector } from '@/store'
import { hasFeature } from '@/utils/chains'
Expand All @@ -44,12 +46,12 @@ const useActivateAccount = () => {
? {
maxFeePerGas: maxFeePerGas?.toString(),
maxPriorityFeePerGas: maxPriorityFeePerGas?.toString(),
gasLimit: gasLimit?.toString(),
gasLimit: gasLimit?.totalGas.toString(),
}
: { gasPrice: maxFeePerGas?.toString(), gasLimit: gasLimit?.toString() }
: { gasPrice: maxFeePerGas?.toString(), gasLimit: gasLimit?.totalGas.toString() }

const totalFee = getTotalFeeFormatted(maxFeePerGas, maxPriorityFeePerGas, gasLimit, chain)
const walletCanPay = useWalletCanPay({ gasLimit, maxFeePerGas, maxPriorityFeePerGas })
const totalFee = getTotalFeeFormatted(maxFeePerGas, maxPriorityFeePerGas, gasLimit?.totalGas, chain)
const walletCanPay = useWalletCanPay({ gasLimit: gasLimit?.totalGas, maxFeePerGas, maxPriorityFeePerGas })

return { options, totalFee, walletCanPay }
}
Expand Down Expand Up @@ -81,6 +83,12 @@ const ActivateAccountFlow = () => {

const onSuccess = () => {
dispatch(removeUndeployedSafe({ chainId, address: safeAddress }))
txDispatch(TxEvent.SUCCESS, { groupKey: CF_TX_GROUP_KEY })
}

const onSubmit = (txHash?: string) => {
showSubmitNotification(dispatch, chain, txHash)
setTxFlow(undefined)
}

const createSafe = async () => {
Expand All @@ -98,6 +106,8 @@ const ActivateAccountFlow = () => {
Number(undeployedSafe.safeDeploymentConfig?.saltNonce!),
)

onSubmit()

waitForCreateSafeTx(taskId, (status) => {
if (status === SafeCreationStatus.SUCCESS) {
onSuccess()
Expand All @@ -108,6 +118,7 @@ const ActivateAccountFlow = () => {
safeAccountConfig: undeployedSafe.safeAccountConfig,
saltNonce: undeployedSafe.safeDeploymentConfig?.saltNonce,
options,
callback: onSubmit,
})
onSuccess()
}
Expand All @@ -117,8 +128,6 @@ const ActivateAccountFlow = () => {
setSubmitError(err)
return
}

setTxFlow(undefined)
}

const submitDisabled = !isSubmittable
Expand Down
33 changes: 29 additions & 4 deletions src/features/counterfactual/CounterfactualForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ import useDeployGasLimit from '@/features/counterfactual/hooks/useDeployGasLimit
import { removeUndeployedSafe } from '@/features/counterfactual/store/undeployedSafesSlice'
import { deploySafeAndExecuteTx } from '@/features/counterfactual/utils'
import useChainId from '@/hooks/useChainId'
import { getTotalFeeFormatted } from '@/hooks/useGasPrice'
import useSafeInfo from '@/hooks/useSafeInfo'
import useWalletCanPay from '@/hooks/useWalletCanPay'
import useOnboard from '@/hooks/wallets/useOnboard'
import useWallet from '@/hooks/wallets/useWallet'
import { useAppDispatch } from '@/store'
import madProps from '@/utils/mad-props'
import React, { type ReactElement, type SyntheticEvent, useContext, useState } from 'react'
import { CircularProgress, Box, Button, CardActions, Divider } from '@mui/material'
import { CircularProgress, Box, Button, CardActions, Divider, Alert } from '@mui/material'
import classNames from 'classnames'

import ErrorMessage from '@/components/tx/ErrorMessage'
Expand Down Expand Up @@ -46,6 +47,7 @@ export const CounterfactualForm = ({
}): ReactElement => {
const wallet = useWallet()
const onboard = useOnboard()
const chain = useCurrentChain()
const chainId = useChainId()
const dispatch = useAppDispatch()
const { safeAddress } = useSafeInfo()
Expand All @@ -61,7 +63,7 @@ export const CounterfactualForm = ({

// Estimate gas limit
const { gasLimit, gasLimitError } = useDeployGasLimit(safeTx)
const [advancedParams, setAdvancedParams] = useAdvancedParams(gasLimit)
const [advancedParams, setAdvancedParams] = useAdvancedParams(gasLimit?.totalGas)

// On modal submit
const handleSubmit = async (e: SyntheticEvent) => {
Expand Down Expand Up @@ -91,12 +93,11 @@ export const CounterfactualForm = ({
return
}

// TODO: Show a success or status screen
setTxFlow(undefined)
}

const walletCanPay = useWalletCanPay({
gasLimit,
gasLimit: gasLimit?.totalGas,
maxFeePerGas: advancedParams.maxFeePerGas,
maxPriorityFeePerGas: advancedParams.maxPriorityFeePerGas,
})
Expand All @@ -113,6 +114,30 @@ export const CounterfactualForm = ({
return (
<>
<form onSubmit={handleSubmit}>
<Alert severity="info" sx={{ mb: 2 }}>
Executing this transaction will also activate your account. Additional network fees will apply. Base fee is{' '}
<strong>
{getTotalFeeFormatted(
advancedParams.maxFeePerGas,
advancedParams.maxPriorityFeePerGas,
BigInt(gasLimit?.baseGas || '0') + BigInt(gasLimit?.safeTxGas || '0'),
chain,
)}{' '}
{chain?.nativeCurrency.symbol}
</strong>
, one time activation fee is{' '}
<strong>
{getTotalFeeFormatted(
advancedParams.maxFeePerGas,
advancedParams.maxPriorityFeePerGas,
BigInt(gasLimit?.safeDeploymentGas || '0'),
chain,
)}{' '}
{chain?.nativeCurrency.symbol}
</strong>
. This is an estimation and the final cost can be higher.
</Alert>

<div className={classNames(css.params)}>
<AdvancedParams
willExecute
Expand Down
65 changes: 65 additions & 0 deletions src/features/counterfactual/__tests__/useDeployGasLimit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import useDeployGasLimit from '@/features/counterfactual/hooks/useDeployGasLimit'
import * as onboard from '@/hooks/wallets/useOnboard'
import * as sdk from '@/services/tx/tx-sender/sdk'
import { safeTxBuilder } from '@/tests/builders/safeTx'
import * as protocolKit from '@safe-global/protocol-kit'
import { renderHook } from '@/tests/test-utils'
import { waitFor } from '@testing-library/react'
import type { OnboardAPI } from '@web3-onboard/core'

describe('useDeployGasLimit hook', () => {
it('returns undefined in onboard is not initialized', () => {
jest.spyOn(onboard, 'default').mockReturnValue(undefined)
const { result } = renderHook(() => useDeployGasLimit())

expect(result.current.gasLimit).toBeUndefined()
})

it('returns safe deployment gas estimation', async () => {
const mockGas = '100'
jest.spyOn(onboard, 'default').mockImplementation(jest.fn(() => ({} as OnboardAPI)))
jest.spyOn(sdk, 'getSafeSDKWithSigner').mockImplementation(jest.fn())
const mockEstimateSafeDeploymentGas = jest
.spyOn(protocolKit, 'estimateSafeDeploymentGas')
.mockReturnValue(Promise.resolve(mockGas))

const { result } = renderHook(() => useDeployGasLimit())

await waitFor(() => {
expect(mockEstimateSafeDeploymentGas).toHaveBeenCalled()
expect(result.current.gasLimit?.safeDeploymentGas).toEqual(mockGas)
})
})

it('does not estimate baseGas and safeTxGas if there is no safeTx and returns 0 for them instead', async () => {
jest.spyOn(onboard, 'default').mockImplementation(jest.fn(() => ({} as OnboardAPI)))
jest.spyOn(sdk, 'getSafeSDKWithSigner').mockImplementation(jest.fn())
jest.spyOn(protocolKit, 'estimateSafeDeploymentGas').mockReturnValue(Promise.resolve('100'))

const mockEstimateTxBaseGas = jest.spyOn(protocolKit, 'estimateTxBaseGas')
const mockEstimateSafeTxGas = jest.spyOn(protocolKit, 'estimateSafeTxGas')

const { result } = renderHook(() => useDeployGasLimit())

await waitFor(() => {
expect(mockEstimateTxBaseGas).not.toHaveBeenCalled()
expect(mockEstimateSafeTxGas).not.toHaveBeenCalled()
expect(result.current.gasLimit?.baseGas).toEqual('0')
expect(result.current.gasLimit?.safeTxGas).toEqual('0')
})
})

it('returns the totalFee', async () => {
jest.spyOn(onboard, 'default').mockImplementation(jest.fn(() => ({} as OnboardAPI)))
jest.spyOn(sdk, 'getSafeSDKWithSigner').mockImplementation(jest.fn())
jest.spyOn(protocolKit, 'estimateSafeDeploymentGas').mockReturnValue(Promise.resolve('100'))
jest.spyOn(protocolKit, 'estimateTxBaseGas').mockReturnValue(Promise.resolve('100'))
jest.spyOn(protocolKit, 'estimateSafeTxGas').mockReturnValue(Promise.resolve('100'))

const { result } = renderHook(() => useDeployGasLimit(safeTxBuilder().build()))

await waitFor(() => {
expect(result.current.gasLimit?.totalGas).toEqual(300n)
})
})
})
35 changes: 24 additions & 11 deletions src/features/counterfactual/hooks/useDeployGasLimit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,38 @@ const getExtraGasForSafety = (safeTx?: SafeTransaction): bigint => {
: 0n
}

type DeployGasLimitProps = {
baseGas: string
safeTxGas: string
safeDeploymentGas: string
totalGas: bigint
}

const useDeployGasLimit = (safeTx?: SafeTransaction) => {
const onboard = useOnboard()
const chainId = useChainId()

const [gasLimit, gasLimitError, gasLimitLoading] = useAsync<bigint | undefined>(async () => {
if (!onboard) return
const [gasLimit, gasLimitError, gasLimitLoading] = useAsync<DeployGasLimitProps | undefined>(
async () => {
if (!onboard) return

const sdk = await getSafeSDKWithSigner(onboard, chainId)

const sdk = await getSafeSDKWithSigner(onboard, chainId)
const extraGasForSafety = getExtraGasForSafety(safeTx)

const extraGasForSafety = getExtraGasForSafety(safeTx)
const [baseGas, safeTxGas, safeDeploymentGas] = await Promise.all([
safeTx ? estimateTxBaseGas(sdk, safeTx) : '0',
safeTx ? estimateSafeTxGas(sdk, safeTx) : '0',
estimateSafeDeploymentGas(sdk),
])

const [gas, safeTxGas, safeDeploymentGas] = await Promise.all([
safeTx ? estimateTxBaseGas(sdk, safeTx) : '0',
safeTx ? estimateSafeTxGas(sdk, safeTx) : '0',
estimateSafeDeploymentGas(sdk),
])
const totalGas = BigInt(baseGas) + BigInt(safeTxGas) + BigInt(safeDeploymentGas) + extraGasForSafety

return BigInt(gas) + BigInt(safeTxGas) + BigInt(safeDeploymentGas) + extraGasForSafety
}, [onboard, chainId, safeTx])
return { baseGas, safeTxGas, safeDeploymentGas, totalGas }
},
[onboard, chainId, safeTx],
false,
)

return { gasLimit, gasLimitError, gasLimitLoading }
}
Expand Down
34 changes: 26 additions & 8 deletions src/features/counterfactual/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import { txDispatch, TxEvent } from '@/services/tx/txEvents'
import type { AppDispatch } from '@/store'
import { addOrUpdateSafe } from '@/store/addedSafesSlice'
import { upsertAddressBookEntry } from '@/store/addressBookSlice'
import { showNotification } from '@/store/notificationsSlice'
import { defaultSafeInfo } from '@/store/safeInfoSlice'
import { getBlockExplorerLink } from '@/utils/chains'
import { didReprice, didRevert, type EthersError } from '@/utils/ethers-utils'
import { assertOnboard, assertTx, assertWallet } from '@/utils/helpers'
import type { DeploySafeProps, PredictedSafeProps } from '@safe-global/protocol-kit'
Expand Down Expand Up @@ -47,6 +49,8 @@ export const getUndeployedSafeInfo = (undeployedSafe: PredictedSafeProps, addres
})
}

export const CF_TX_GROUP_KEY = 'cf-tx'

export const dispatchTxExecutionAndDeploySafe = async (
safeTx: SafeTransaction,
txOptions: TransactionOptions,
Expand All @@ -55,8 +59,7 @@ export const dispatchTxExecutionAndDeploySafe = async (
onSuccess?: () => void,
) => {
const sdkUnchecked = await getUncheckedSafeSDK(onboard, chainId)
const eventParams = { groupKey: 'cf-tx' }
const safeAddress = await sdkUnchecked.getAddress()
const eventParams = { groupKey: CF_TX_GROUP_KEY }

let result: ContractTransactionResponse | undefined
try {
Expand All @@ -68,8 +71,11 @@ export const dispatchTxExecutionAndDeploySafe = async (

const deploymentTx = await sdkUnchecked.wrapSafeTransactionIntoDeploymentBatch(signedTx, txOptions)

// We need to estimate the actual gasLimit after the user has signed since it is more accurate than what useDeployGasLimit returns
const gas = await signer.estimateGas({ data: deploymentTx.data, value: deploymentTx.value, to: deploymentTx.to })

// @ts-ignore TODO: Check why TransactionResponse type doesn't work
result = await signer.sendTransaction(deploymentTx)
result = await signer.sendTransaction({ ...deploymentTx, gasLimit: gas })
txDispatch(TxEvent.EXECUTING, eventParams)
} catch (error) {
txDispatch(TxEvent.FAILED, { ...eventParams, error: asError(error) })
Expand All @@ -86,21 +92,20 @@ export const dispatchTxExecutionAndDeploySafe = async (
} else if (didRevert(receipt)) {
txDispatch(TxEvent.REVERTED, { ...eventParams, error: new Error('Transaction reverted by EVM') })
} else {
txDispatch(TxEvent.PROCESSED, { ...eventParams, safeAddress })
txDispatch(TxEvent.SUCCESS, eventParams)
onSuccess?.()
}
})
.catch((err) => {
const error = err as EthersError

if (didReprice(error)) {
txDispatch(TxEvent.PROCESSED, { ...eventParams, safeAddress })
txDispatch(TxEvent.SUCCESS, eventParams)
onSuccess?.()
} else {
txDispatch(TxEvent.FAILED, { ...eventParams, error: asError(error) })
}
})
.finally(() => {
onSuccess?.()
})

return result!.hash
}
Expand Down Expand Up @@ -196,3 +201,16 @@ export const createCounterfactualSafe = (
query: { safe: `${chain.shortName}:${safeAddress}`, [CREATION_MODAL_QUERY_PARM]: true },
})
}

export const showSubmitNotification = (dispatch: AppDispatch, chain?: ChainInfo, txHash?: string) => {
const link = chain && txHash ? getBlockExplorerLink(chain, txHash) : undefined
dispatch(
showNotification({
variant: 'info',
groupKey: CF_TX_GROUP_KEY,
message: 'Safe Account activation in progress',
detailedMessage: 'Your Safe Account will be deployed onchain after the transaction is executed.',
link: link ? { href: link.href, title: link.title } : undefined,
}),
)
}

0 comments on commit 32747ba

Please sign in to comment.