Commit 090644ae authored by OptimismBot's avatar OptimismBot Committed by GitHub

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

fix: withdrawal gas limit
parents 582f7d7d afc2ab8c
---
'@eth-optimism/sdk': patch
---
Update the migrated withdrawal gas limit for non goerli networks
......@@ -181,6 +181,7 @@ func main() {
migrationData,
&config.L1CrossDomainMessengerProxy,
config.L1ChainID,
config.L2ChainID,
config.FinalSystemOwner,
config.ProxyAdminOwner,
&derive.L1BlockInfo{
......
......@@ -223,6 +223,7 @@ func main() {
migrationData,
&config.L1CrossDomainMessengerProxy,
config.L1ChainID,
config.L2ChainID,
config.FinalSystemOwner,
config.ProxyAdminOwner,
&derive.L1BlockInfo{
......
......@@ -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
}
......
......@@ -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 {
......
......@@ -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)
}
}
......@@ -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
}
......
......@@ -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)
}
......
......@@ -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(
......
......@@ -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)
}
......
......@@ -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)
}
})
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment