diff --git a/.changeset/poor-buses-hug.md b/.changeset/poor-buses-hug.md new file mode 100644 index 000000000000..66db062bbf53 --- /dev/null +++ b/.changeset/poor-buses-hug.md @@ -0,0 +1,5 @@ +--- +'@eth-optimism/sdk': patch +--- + +Update migrated withdrawal gaslimit calculation diff --git a/op-chain-ops/crossdomain/migrate.go b/op-chain-ops/crossdomain/migrate.go index b88074d212b7..741f353fb9e0 100644 --- a/op-chain-ops/crossdomain/migrate.go +++ b/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 +} diff --git a/op-chain-ops/crossdomain/migrate_test.go b/op-chain-ops/crossdomain/migrate_test.go index bcd3711ed889..45a604eaeaae 100644 --- a/op-chain-ops/crossdomain/migrate_test.go +++ b/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) + } +} diff --git a/packages/sdk/src/cross-chain-messenger.ts b/packages/sdk/src/cross-chain-messenger.ts index 59379b15a126..6287c809cf37 100644 --- a/packages/sdk/src/cross-chain-messenger.ts +++ b/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 diff --git a/packages/sdk/src/utils/message-utils.ts b/packages/sdk/src/utils/message-utils.ts index 210bf88296e8..323734b1fdf0 100644 --- a/packages/sdk/src/utils/message-utils.ts +++ b/packages/sdk/src/utils/message-utils.ts @@ -1,4 +1,5 @@ -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 +} diff --git a/packages/sdk/test/utils/message-utils.spec.ts b/packages/sdk/test/utils/message-utils.spec.ts new file mode 100644 index 000000000000..718b7f6d4a59 --- /dev/null +++ b/packages/sdk/test/utils/message-utils.spec.ts @@ -0,0 +1,29 @@ +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) + } + }) + }) +})