Merge pull request #4911 from ethereum-optimism/fix/gaslimit

migration: update migrated withdrawal gas limit

.changeset/poor-buses-hug.md

+---
+'@eth-optimism/sdk': patch
+---
+
+Update migrated withdrawal gaslimit calculation

op-chain-ops/crossdomain/migrate.go

@@ -11,6 +11,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/params"
 )
 
 var (
@@ -82,8 +83,7 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 		return nil, fmt.Errorf("cannot abi encode relayMessage: %w", err)
 	}
 
-	// Set the outer gas limit. This cannot be zero
-	gasLimit := uint64(len(data)*16 + 200_000)
+	gasLimit := MigrateWithdrawalGasLimit(data)
 
 	w := NewWithdrawal(
 		versionedNonce,
@@ -95,3 +95,25 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 	)
 	return w, nil
 }
+
+func MigrateWithdrawalGasLimit(data []byte) uint64 {
+	// Compute the cost of the calldata
+	dataCost := uint64(0)
+	for _, b := range data {
+		if b == 0 {
+			dataCost += params.TxDataZeroGas
+		} else {
+			dataCost += params.TxDataNonZeroGasEIP2028
+		}
+	}
+
+	// Set the outer gas limit. This cannot be zero
+	gasLimit := dataCost + 200_000
+	// Cap the gas limit to be 25 million to prevent creating withdrawals
+	// that go over the block gas limit.
+	if gasLimit > 25_000_000 {
+		gasLimit = 25_000_000
+	}
+
+	return gasLimit
+}

op-chain-ops/crossdomain/migrate_test.go

@@ -2,6 +2,7 @@ package crossdomain_test
 
 import (
 	"fmt"
+	"math/big"
 	"testing"
 
 	"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
@@ -11,6 +12,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var big25Million = big.NewInt(25_000_000)
+
 func TestMigrateWithdrawal(t *testing.T) {
 	withdrawals := make([]*crossdomain.LegacyWithdrawal, 0)
 
@@ -31,6 +34,57 @@ func TestMigrateWithdrawal(t *testing.T) {
 			require.Equal(t, legacy.XDomainNonce.Uint64(), withdrawal.Nonce.Uint64())
 			require.Equal(t, *withdrawal.Sender, predeploys.L2CrossDomainMessengerAddr)
 			require.Equal(t, *withdrawal.Target, l1CrossDomainMessenger)
+			// Always equal to or lower than the cap
+			require.True(t, withdrawal.GasLimit.Cmp(big25Million) <= 0)
 		})
 	}
 }
+
+// TestMigrateWithdrawalGasLimitMax computes the migrated withdrawal
+// gas limit with a very large amount of data. The max value for a migrated
+// withdrawal's gas limit is 25 million.
+func TestMigrateWithdrawalGasLimitMax(t *testing.T) {
+	size := 300_000_000 / 16
+	data := make([]byte, size)
+	for _, i := range data {
+		data[i] = 0xff
+	}
+
+	result := crossdomain.MigrateWithdrawalGasLimit(data)
+	require.Equal(t, result, big25Million.Uint64())
+}
+
+// TestMigrateWithdrawalGasLimit tests an assortment of zero and non zero
+// bytes when computing the migrated withdrawal's gas limit.
+func TestMigrateWithdrawalGasLimit(t *testing.T) {
+	tests := []struct {
+		input  []byte
+		output uint64
+	}{
+		{
+			input:  []byte{},
+			output: 200_000,
+		},
+		{
+			input:  []byte{0xff},
+			output: 200_000 + 16,
+		},
+		{
+			input:  []byte{0xff, 0x00},
+			output: 200_000 + 16 + 4,
+		},
+		{
+			input:  []byte{0x00},
+			output: 200_000 + 4,
+		},
+		{
+			input:  []byte{0x00, 0x00, 0x00},
+			output: 200_000 + 4 + 4 + 4,
+		},
+	}
+
+	for _, test := range tests {
+		result := crossdomain.MigrateWithdrawalGasLimit(test.input)
+		require.Equal(t, test.output, result)
+	}
+}

packages/sdk/src/cross-chain-messenger.ts

@@ -18,12 +18,10 @@ import {
   sleep,
   remove0x,
   toHexString,
-  fromHexString,
   toRpcHexString,
   hashCrossDomainMessage,
   encodeCrossDomainMessageV0,
   encodeCrossDomainMessageV1,
-  L2OutputOracleParameters,
   BedrockOutputData,
   BedrockCrossChainMessageProof,
   decodeVersionedNonce,
@@ -67,6 +65,7 @@ import {
   makeMerkleTreeProof,
   makeStateTrieProof,
   hashLowLevelMessage,
+  migratedWithdrawalGasLimit,
   DEPOSIT_CONFIRMATION_BLOCKS,
   CHAIN_BLOCK_TIMES,
 } from './utils'
@@ -352,12 +351,12 @@ export class CrossChainMessenger {
       }
     }
 
-    const minGasLimit = fromHexString(resolved.message).length * 16 + 200_000
+    const minGasLimit = migratedWithdrawalGasLimit(resolved.message)
 
     return {
       ...resolved,
       value,
-      minGasLimit: BigNumber.from(minGasLimit),
+      minGasLimit,
       messageNonce: encodeVersionedNonce(
         BigNumber.from(1),
         resolved.messageNonce

packages/sdk/src/utils/message-utils.ts

-import { hashWithdrawal } from '@eth-optimism/core-utils'
+import { hashWithdrawal, calldataCost } from '@eth-optimism/core-utils'
+import { BigNumber } from 'ethers'
 
 import { LowLevelMessage } from '../interfaces'
 
@@ -18,3 +19,16 @@ export const hashLowLevelMessage = (message: LowLevelMessage): string => {
     message.message
   )
 }
+
+/**
+ * Compute the min gas limit for a migrated withdrawal.
+ */
+export const migratedWithdrawalGasLimit = (data: string): BigNumber => {
+  // Compute the gas limit and cap at 25 million
+  const dataCost = calldataCost(data)
+  let minGasLimit = dataCost.add(200_000)
+  if (minGasLimit.gt(25_000_000)) {
+    minGasLimit = BigNumber.from(25_000_000)
+  }
+  return minGasLimit
+}

packages/sdk/test/utils/message-utils.spec.ts

+import { BigNumber } from 'ethers'
+
+import { expect } from '../setup'
+import { migratedWithdrawalGasLimit } from '../../src/utils/message-utils'
+
+describe('Message Utils', () => {
+  describe('migratedWithdrawalGasLimit', () => {
+    it('should have a max of 25 million', () => {
+      const data = '0x' + 'ff'.repeat(15_000_000)
+      const result = migratedWithdrawalGasLimit(data)
+      expect(result).to.eq(BigNumber.from(25_000_000))
+    })
+
+    it('should work for mixes of zeros and ones', () => {
+      const tests = [
+        { input: '0x', result: BigNumber.from(200_000) },
+        { input: '0xff', result: BigNumber.from(200_000 + 16) },
+        { input: '0xff00', result: BigNumber.from(200_000 + 16 + 4) },
+        { input: '0x00', result: BigNumber.from(200_000 + 4) },
+        { input: '0x000000', result: BigNumber.from(200_000 + 4 + 4 + 4) },
+      ]
+
+      for (const test of tests) {
+        const result = migratedWithdrawalGasLimit(test.input)
+        expect(result).to.eq(test.result)
+      }
+    })
+  })
+})