fix: always estimate relay gasLimit (#4679)

Author: iamacook

src/components/new-safe/create/logic/index.test.ts

@@ -10,7 +10,7 @@ import {
   getRedirect,
   createNewUndeployedSafeWithoutSalt,
 } from '@/components/new-safe/create/logic/index'
-import { relayTransaction } from '@safe-global/safe-gateway-typescript-sdk'
+import * as relaying from '@/services/tx/relaying'
 import { toBeHex } from 'ethers'
 import {
   Gnosis_safe__factory,
@@ -21,7 +21,6 @@ import {
   getReadOnlyGnosisSafeContract,
   getReadOnlyProxyFactoryContract,
 } from '@/services/contracts/safeContracts'
-import * as gateway from '@safe-global/safe-gateway-typescript-sdk'
 import { FEATURES, getLatestSafeVersion } from '@/utils/chains'
 import { type FEATURES as GatewayFeatures } from '@safe-global/safe-gateway-typescript-sdk'
 import { chainBuilder } from '@/tests/builders/chains'
@@ -68,13 +67,14 @@ describe('create/logic', () => {
     })
 
     it('returns taskId if create Safe successfully relayed', async () => {
+      const gasLimit = faker.string.numeric()
       const mockSafeProvider = {
         getExternalProvider: jest.fn(),
         getExternalSigner: jest.fn(),
         getChainId: jest.fn().mockReturnValue(BigInt(1)),
       } as unknown as SafeProvider
 
-      jest.spyOn(gateway, 'relayTransaction').mockResolvedValue({ taskId: '0x123' })
+      jest.spyOn(relaying, 'relayTransaction').mockResolvedValue({ taskId: '0x123' })
       jest.spyOn(sdkHelpers, 'getSafeProvider').mockImplementation(() => mockSafeProvider)
 
       jest.spyOn(contracts, 'getReadOnlyFallbackHandlerContract').mockResolvedValue({
@@ -123,20 +123,22 @@ describe('create/logic', () => {
         expectedSaltNonce,
       ])
 
-      const taskId = await relaySafeCreation(mockChainInfo, undeployedSafeProps)
+      const taskId = await relaySafeCreation(mockChainInfo, undeployedSafeProps, gasLimit)
 
       expect(taskId).toEqual('0x123')
-      expect(relayTransaction).toHaveBeenCalledTimes(1)
-      expect(relayTransaction).toHaveBeenCalledWith('1', {
+      expect(relaying.relayTransaction).toHaveBeenCalledTimes(1)
+      expect(relaying.relayTransaction).toHaveBeenCalledWith('1', {
         to: proxyFactoryAddress,
         data: expectedCallData,
         version: latestSafeVersion,
+        gasLimit,
       })
     })
 
     it('should throw an error if relaying fails', () => {
+      const gasLimit = faker.string.numeric()
       const relayFailedError = new Error('Relay failed')
-      jest.spyOn(gateway, 'relayTransaction').mockRejectedValue(relayFailedError)
+      jest.spyOn(relaying, 'relayTransaction').mockRejectedValue(relayFailedError)
 
       const undeployedSafeProps: ReplayedSafeProps = {
         safeAccountConfig: {
@@ -155,7 +157,7 @@ describe('create/logic', () => {
         saltNonce: '69',
       }
 
-      expect(relaySafeCreation(mockChainInfo, undeployedSafeProps)).rejects.toEqual(relayFailedError)
+      expect(relaySafeCreation(mockChainInfo, undeployedSafeProps, gasLimit)).rejects.toEqual(relayFailedError)
     })
   })
   describe('getRedirect', () => {

src/components/new-safe/create/logic/index.ts

@@ -2,7 +2,8 @@ import type { SafeVersion } from '@safe-global/safe-core-sdk-types'
 import { type Eip1193Provider, type Provider } from 'ethers'
 import semverSatisfies from 'semver/functions/satisfies'
 
-import { getSafeInfo, type SafeInfo, type ChainInfo, relayTransaction } from '@safe-global/safe-gateway-typescript-sdk'
+import { getSafeInfo, type SafeInfo, type ChainInfo } from '@safe-global/safe-gateway-typescript-sdk'
+import { relayTransaction } from '@/services/tx/relaying'
 import { getReadOnlyProxyFactoryContract } from '@/services/contracts/safeContracts'
 import type { UrlObject } from 'url'
 import { AppRoutes } from '@/config/routes'
@@ -178,14 +179,19 @@ export const getRedirect = (
   return redirectUrl + `${appendChar}safe=${address}`
 }
 
-export const relaySafeCreation = async (chain: ChainInfo, undeployedSafeProps: UndeployedSafeProps) => {
+export const relaySafeCreation = async (
+  chain: ChainInfo,
+  undeployedSafeProps: UndeployedSafeProps,
+  gasLimit: string | undefined,
+) => {
   const replayedSafeProps = assertNewUndeployedSafeProps(undeployedSafeProps, chain)
   const encodedSafeCreationTx = encodeSafeCreationTx(replayedSafeProps, chain)
 
   const relayResponse = await relayTransaction(chain.chainId, {
     to: replayedSafeProps.factoryAddress,
     data: encodedSafeCreationTx,
     version: replayedSafeProps.safeVersion,
+    gasLimit,
   })
 
   return relayResponse.taskId

src/components/new-safe/create/steps/ReviewStep/index.tsx

@@ -336,7 +336,7 @@ const ReviewStep = ({ data, onSubmit, onBack, setStep }: StepRenderProps<NewSafe
       }
 
       if (willRelay) {
-        const taskId = await relaySafeCreation(chain, props)
+        const taskId = await relaySafeCreation(chain, props, gasLimit?.toString())
         onSubmitCallback(taskId)
       } else {
         await createNewSafe(

src/features/counterfactual/ActivateAccountFlow.tsx

@@ -121,7 +121,7 @@ const ActivateAccountFlow = () => {
 
     try {
       if (willRelay) {
-        const taskId = await relaySafeCreation(chain, undeployedSafe.props)
+        const taskId = await relaySafeCreation(chain, undeployedSafe.props, options?.gasLimit?.toString())
         safeCreationDispatch(SafeCreationEvent.RELAYING, { groupKey: CF_TX_GROUP_KEY, taskId, safeAddress })
 
         onSubmit()

src/services/tx/__tests__/relaying.test.ts

@@ -0,0 +1,66 @@
+import { faker } from '@faker-js/faker'
+import { relayTransaction as _relayTransaction } from '@safe-global/safe-gateway-typescript-sdk'
+
+import { relayTransaction } from '@/services/tx/relaying'
+import { getWeb3ReadOnly } from '@/hooks/wallets/web3'
+
+jest.mock('@/hooks/wallets/web3')
+jest.mock('@safe-global/safe-gateway-typescript-sdk')
+
+describe('relayTransaction', () => {
+  let chainId: string
+  let body: {
+    version: string
+    to: string
+    data: string
+    gasLimit: string | undefined
+  }
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+
+    chainId = faker.string.numeric()
+    body = {
+      version: faker.system.semver(),
+      to: faker.finance.ethereumAddress(),
+      data: faker.string.hexadecimal(),
+      gasLimit: undefined,
+    }
+  })
+
+  it('should relay with the correct parameters when gasLimit is provided', async () => {
+    const bodyWithGasLimit = {
+      ...body,
+      gasLimit: faker.string.numeric(),
+    }
+
+    await relayTransaction(chainId, bodyWithGasLimit)
+
+    expect(_relayTransaction).toHaveBeenCalledWith(chainId, bodyWithGasLimit)
+  })
+
+  it('should estimate gasLimit if not provided and relay with the estimated gasLimit', async () => {
+    const gasLimit = faker.number.bigInt()
+    const mockProvider = {
+      estimateGas: jest.fn().mockResolvedValue(gasLimit),
+    }
+    ;(getWeb3ReadOnly as jest.Mock).mockReturnValue(mockProvider)
+
+    await relayTransaction(chainId, body)
+
+    expect(mockProvider.estimateGas).toHaveBeenCalledWith(body)
+    expect(_relayTransaction).toHaveBeenCalledWith(chainId, { ...body, gasLimit: gasLimit.toString() })
+  })
+
+  it('should relay with undefined gasLimit if estimation fails', async () => {
+    const mockProvider = {
+      estimateGas: jest.fn().mockRejectedValue(new Error('Estimation failed')),
+    }
+    ;(getWeb3ReadOnly as jest.Mock).mockReturnValue(mockProvider)
+
+    await relayTransaction(chainId, body)
+
+    expect(mockProvider.estimateGas).toHaveBeenCalledWith(body)
+    expect(_relayTransaction).toHaveBeenCalledWith(chainId, { ...body, gasLimit: undefined })
+  })
+})

src/services/tx/relaying.ts

@@ -0,0 +1,29 @@
+import { relayTransaction as _relayTransaction } from '@safe-global/safe-gateway-typescript-sdk'
+
+import { getWeb3ReadOnly } from '@/hooks/wallets/web3'
+
+export async function relayTransaction(
+  chainId: string,
+  body: { version: string; to: string; data: string; gasLimit: string | undefined },
+) {
+  /**
+   * Ensures `gasLimit` is estimate so 150k execution overhead buffer can be added by CGW.
+   *
+   * @see https://github.com/safe-global/safe-client-gateway/blob/b7640ed4bd7416ecb7696d21ba105fcb5af52a10/src/datasources/relay-api/gelato-api.service.ts#L62-L75
+   *
+   * - If provided, CGW adds a 150k buffer before submission to Gelato.
+   * - If not provided, no buffer is added, and Gelato estimates the it.
+   *
+   * Note: particularly important on networks like Arbitrum, where estimation is unreliable.
+   */
+  if (!body.gasLimit) {
+    const provider = getWeb3ReadOnly()
+
+    body.gasLimit = await provider
+      ?.estimateGas(body)
+      .then(String)
+      .catch(() => undefined)
+  }
+
+  return _relayTransaction(chainId, body)
+}

src/services/tx/tx-sender/__tests__/ts-sender.test.ts

@@ -22,6 +22,7 @@ import {
   type JsonRpcProvider,
   type JsonRpcSigner,
 } from 'ethers'
+import { faker } from '@faker-js/faker'
 import * as safeContracts from '@/services/contracts/safeContracts'
 
 import * as web3 from '@/hooks/wallets/web3'
@@ -515,6 +516,13 @@ describe('txSender', () => {
 
   describe('dispatchBatchExecutionRelay', () => {
     it('should relay a batch execution', async () => {
+      const gasLimit = faker.number.bigInt()
+      jest.spyOn(web3, 'getWeb3ReadOnly').mockImplementation(() => {
+        return {
+          estimateGas: jest.fn(() => Promise.resolve(gasLimit)),
+        } as unknown as JsonRpcProvider
+      })
+
       const mockMultisendAddress = zeroPadValue('0x1234', 20)
       const safeAddress = toBeHex('0x567', 20)
 

src/services/tx/tx-sender/dispatch.ts

@@ -2,12 +2,8 @@ import type { ConnectedWallet } from '@/hooks/wallets/useOnboard'
 import { isMultisigExecutionInfo } from '@/utils/transaction-guards'
 import { isHardwareWallet, isSmartContractWallet } from '@/utils/wallets'
 import type { MultiSendCallOnlyContractImplementationType } from '@safe-global/protocol-kit'
-import {
-  type ChainInfo,
-  relayTransaction,
-  type SafeInfo,
-  type TransactionDetails,
-} from '@safe-global/safe-gateway-typescript-sdk'
+import { type ChainInfo, type SafeInfo, type TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk'
+import { relayTransaction } from '@/services/tx/relaying'
 import type {
   SafeSignature,
   SafeTransaction,
@@ -559,6 +555,8 @@ export const dispatchBatchExecutionRelay = async (
       to,
       data,
       version: safeVersion,
+      // We have no estimation in place
+      gasLimit: undefined,
     })
   } catch (error) {
     txs.forEach(({ txId }) => {