Merge pull request #5501 from ethereum-optimism/fix/withdrawal-gas-limit

fix: withdrawal gas limit

.changeset/rare-moose-itch.md

@@ -0,0 +1,5 @@
+---
+'@eth-optimism/sdk': patch
+---
+
+Update the migrated withdrawal gas limit for non goerli networks

op-chain-ops/cmd/check-migration/main.go

@@ -181,6 +181,7 @@ func main() {
 				migrationData,
 				&config.L1CrossDomainMessengerProxy,
 				config.L1ChainID,
+				config.L2ChainID,
 				config.FinalSystemOwner,
 				config.ProxyAdminOwner,
 				&derive.L1BlockInfo{

op-chain-ops/cmd/op-migrate/main.go

@@ -223,6 +223,7 @@ func main() {
 				migrationData,
 				&config.L1CrossDomainMessengerProxy,
 				config.L1ChainID,
+				config.L2ChainID,
 				config.FinalSystemOwner,
 				config.ProxyAdminOwner,
 				&derive.L1BlockInfo{

op-chain-ops/cmd/withdrawals/main.go

@@ -141,6 +141,10 @@ func main() {
 			if err != nil {
 				return err
 			}
+			l2ChainID, err := clients.L2Client.ChainID(context.Background())
+			if err != nil {
+				return err
+			}
 
 			// create the set of withdrawals
 			wds, err := newWithdrawals(ctx, l1ChainID)
@@ -212,7 +216,7 @@ func main() {
 				log.Info("Processing withdrawal", "index", i)
 
 				// migrate the withdrawal
-				withdrawal, err := crossdomain.MigrateWithdrawal(wd, &l1xdmAddr)
+				withdrawal, err := crossdomain.MigrateWithdrawal(wd, &l1xdmAddr, l2ChainID)
 				if err != nil {
 					return err
 				}

op-chain-ops/crossdomain/migrate.go

@@ -20,7 +20,13 @@ var (
 )
 
 // MigrateWithdrawals will migrate a list of pending withdrawals given a StateDB.
-func MigrateWithdrawals(withdrawals SafeFilteredWithdrawals, db vm.StateDB, l1CrossDomainMessenger *common.Address, noCheck bool) error {
+func MigrateWithdrawals(
+	withdrawals SafeFilteredWithdrawals,
+	db vm.StateDB,
+	l1CrossDomainMessenger *common.Address,
+	noCheck bool,
+	chainID *big.Int,
+) error {
 	for i, legacy := range withdrawals {
 		legacySlot, err := legacy.StorageSlot()
 		if err != nil {
@@ -34,7 +40,7 @@ func MigrateWithdrawals(withdrawals SafeFilteredWithdrawals, db vm.StateDB, l1Cr
 			}
 		}
 
-		withdrawal, err := MigrateWithdrawal(legacy, l1CrossDomainMessenger)
+		withdrawal, err := MigrateWithdrawal(legacy, l1CrossDomainMessenger, chainID)
 		if err != nil {
 			return err
 		}
@@ -52,7 +58,11 @@ func MigrateWithdrawals(withdrawals SafeFilteredWithdrawals, db vm.StateDB, l1Cr
 
 // MigrateWithdrawal will turn a LegacyWithdrawal into a bedrock
 // style Withdrawal.
-func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *common.Address) (*Withdrawal, error) {
+func MigrateWithdrawal(
+	withdrawal *LegacyWithdrawal,
+	l1CrossDomainMessenger *common.Address,
+	chainID *big.Int,
+) (*Withdrawal, error) {
 	// Attempt to parse the value
 	value, err := withdrawal.Value()
 	if err != nil {
@@ -83,7 +93,7 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 		return nil, fmt.Errorf("cannot abi encode relayMessage: %w", err)
 	}
 
-	gasLimit := MigrateWithdrawalGasLimit(data)
+	gasLimit := MigrateWithdrawalGasLimit(data, chainID)
 
 	w := NewWithdrawal(
 		versionedNonce,
@@ -97,13 +107,21 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 }
 
 // MigrateWithdrawalGasLimit computes the gas limit for the migrated withdrawal.
-func MigrateWithdrawalGasLimit(data []byte) uint64 {
+// The chain id is used to determine the overhead.
+func MigrateWithdrawalGasLimit(data []byte, chainID *big.Int) uint64 {
 	// Compute the upper bound on the gas limit. This could be more
 	// accurate if individual 0 bytes and non zero bytes were accounted
 	// for.
 	dataCost := uint64(len(data)) * params.TxDataNonZeroGasEIP2028
+
+	// Goerli has a lower gas limit than other chains.
+	overhead := uint64(200_000)
+	if chainID.Cmp(big.NewInt(420)) != 0 {
+		overhead = 1_000_000
+	}
+
 	// Set the outer gas limit. This cannot be zero
-	gasLimit := dataCost + 200_000
+	gasLimit := dataCost + overhead
 	// Cap the gas limit to be 25 million to prevent creating withdrawals
 	// that go over the block gas limit.
 	if gasLimit > 25_000_000 {

op-chain-ops/crossdomain/migrate_test.go

@@ -12,7 +12,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var big25Million = big.NewInt(25_000_000)
+var (
+	big25Million     = big.NewInt(25_000_000)
+	bigGoerliChainID = big.NewInt(420)
+)
 
 func TestMigrateWithdrawal(t *testing.T) {
 	withdrawals := make([]*crossdomain.LegacyWithdrawal, 0)
@@ -27,7 +30,7 @@ func TestMigrateWithdrawal(t *testing.T) {
 	l1CrossDomainMessenger := common.HexToAddress("0x25ace71c97B33Cc4729CF772ae268934F7ab5fA1")
 	for i, legacy := range withdrawals {
 		t.Run(fmt.Sprintf("test%d", i), func(t *testing.T) {
-			withdrawal, err := crossdomain.MigrateWithdrawal(legacy, &l1CrossDomainMessenger)
+			withdrawal, err := crossdomain.MigrateWithdrawal(legacy, &l1CrossDomainMessenger, bigGoerliChainID)
 			require.Nil(t, err)
 			require.NotNil(t, withdrawal)
 
@@ -50,7 +53,7 @@ func TestMigrateWithdrawalGasLimitMax(t *testing.T) {
 		data[i] = 0xff
 	}
 
-	result := crossdomain.MigrateWithdrawalGasLimit(data)
+	result := crossdomain.MigrateWithdrawalGasLimit(data, bigGoerliChainID)
 	require.Equal(t, result, big25Million.Uint64())
 }
 
@@ -84,7 +87,7 @@ func TestMigrateWithdrawalGasLimit(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		result := crossdomain.MigrateWithdrawalGasLimit(test.input)
+		result := crossdomain.MigrateWithdrawalGasLimit(test.input, bigGoerliChainID)
 		require.Equal(t, test.output, result)
 	}
 }

op-chain-ops/genesis/check.go

@@ -101,6 +101,7 @@ func PostCheckMigratedDB(
 	migrationData crossdomain.MigrationData,
 	l1XDM *common.Address,
 	l1ChainID uint64,
+	l2ChainID uint64,
 	finalSystemOwner common.Address,
 	proxyAdminOwner common.Address,
 	info *derive.L1BlockInfo,
@@ -163,7 +164,7 @@ func PostCheckMigratedDB(
 	}
 	log.Info("checked legacy eth")
 
-	if err := CheckWithdrawalsAfter(db, migrationData, l1XDM); err != nil {
+	if err := CheckWithdrawalsAfter(db, migrationData, l1XDM, new(big.Int).SetUint64(l2ChainID)); err != nil {
 		return err
 	}
 	log.Info("checked withdrawals")
@@ -557,7 +558,7 @@ func PostCheckL1Block(db *state.StateDB, info *derive.L1BlockInfo) error {
 	return nil
 }
 
-func CheckWithdrawalsAfter(db *state.StateDB, data crossdomain.MigrationData, l1CrossDomainMessenger *common.Address) error {
+func CheckWithdrawalsAfter(db *state.StateDB, data crossdomain.MigrationData, l1CrossDomainMessenger *common.Address, l2ChainID *big.Int) error {
 	wds, invalidMessages, err := data.ToWithdrawals()
 	if err != nil {
 		return err
@@ -570,7 +571,7 @@ func CheckWithdrawalsAfter(db *state.StateDB, data crossdomain.MigrationData, l1
 	wdsByOldSlot := make(map[common.Hash]*crossdomain.LegacyWithdrawal)
 	invalidMessagesByOldSlot := make(map[common.Hash]crossdomain.InvalidMessage)
 	for _, wd := range wds {
-		migrated, err := crossdomain.MigrateWithdrawal(wd, l1CrossDomainMessenger)
+		migrated, err := crossdomain.MigrateWithdrawal(wd, l1CrossDomainMessenger, l2ChainID)
 		if err != nil {
 			return err
 		}

op-chain-ops/genesis/db_migration.go

@@ -186,7 +186,8 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 	// the LegacyMessagePasser contract. Here we operate on the list of withdrawals that we
 	// previously filtered and verified.
 	log.Info("Starting to migrate withdrawals", "no-check", noCheck)
-	err = crossdomain.MigrateWithdrawals(filteredWithdrawals, db, &config.L1CrossDomainMessengerProxy, noCheck)
+	l2ChainID := new(big.Int).SetUint64(config.L2ChainID)
+	err = crossdomain.MigrateWithdrawals(filteredWithdrawals, db, &config.L1CrossDomainMessengerProxy, noCheck, l2ChainID)
 	if err != nil {
 		return nil, fmt.Errorf("cannot migrate withdrawals: %w", err)
 	}

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

@@ -26,6 +26,7 @@ import {
   BedrockCrossChainMessageProof,
   decodeVersionedNonce,
   encodeVersionedNonce,
+  getChainId,
 } from '@eth-optimism/core-utils'
 import { getContractInterface, predeploys } from '@eth-optimism/contracts'
 import * as rlp from 'rlp'
@@ -403,7 +404,8 @@ export class CrossChainMessenger {
     let gasLimit: BigNumber
     let messageNonce: BigNumber
     if (version.eq(0)) {
-      gasLimit = migratedWithdrawalGasLimit(encoded)
+      const chainID = await getChainId(this.l2Provider)
+      gasLimit = migratedWithdrawalGasLimit(encoded, chainID)
       messageNonce = resolved.messageNonce
     } else {
       const receipt = await this.l2Provider.getTransactionReceipt(

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

@@ -41,10 +41,17 @@ export const hashMessageHash = (messageHash: string): string => {
 /**
  * Compute the min gas limit for a migrated withdrawal.
  */
-export const migratedWithdrawalGasLimit = (data: string): BigNumber => {
+export const migratedWithdrawalGasLimit = (
+  data: string,
+  chainID: number
+): BigNumber => {
   // Compute the gas limit and cap at 25 million
   const dataCost = BigNumber.from(hexDataLength(data)).mul(16)
-  let minGasLimit = dataCost.add(200_000)
+  let overhead = 200_000
+  if (chainID !== 420) {
+    overhead = 1_000_000
+  }
+  let minGasLimit = dataCost.add(overhead)
   if (minGasLimit.gt(25_000_000)) {
     minGasLimit = BigNumber.from(25_000_000)
   }

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

@@ -7,11 +7,13 @@ import {
   hashMessageHash,
 } from '../../src/utils/message-utils'
 
+const goerliChainID = 420
+
 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)
+      const result = migratedWithdrawalGasLimit(data, goerliChainID)
       expect(result).to.eq(BigNumber.from(25_000_000))
     })
 
@@ -25,7 +27,7 @@ describe('Message Utils', () => {
       ]
 
       for (const test of tests) {
-        const result = migratedWithdrawalGasLimit(test.input)
+        const result = migratedWithdrawalGasLimit(test.input, goerliChainID)
         expect(result).to.eq(test.result)
       }
     })