Skip to content

Commit

Permalink
op-chain-ops: legacy withdrawal script refactoring
Browse files Browse the repository at this point in the history
Clean up the script that is used to migrate legacy withdrawals.
This script is very important for testing the legacy withdrawal
migration.

A bug was introduced with #4911
which was a sherlock audit fix. This was a bug because the change
resulted in the withdrawal hashes changing, meaning that the storage
slots computed client side also changed. This resulted in breaking the
script. This commit reverts this change. The changes landing in
#5470 will cause the
buffer in the gas limit for the  migrated withdrawals to be too small,
so we can reintroduce this code behind a switch with the network.
Its not ideal that the migration code isn't going to match 1:1
with goerli but thats ok because we will be able to thoroughly
test it.
  • Loading branch information
tynes committed Apr 17, 2023
1 parent 62b9850 commit 7bf4d54
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 27 deletions.
91 changes: 83 additions & 8 deletions op-chain-ops/cmd/withdrawals/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ import (
"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/core/state"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/eth"
"github.com/ethereum/go-ethereum/eth/tracers"
"github.com/ethereum/go-ethereum/ethclient"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/rpc"
)

// abiTrue represents the storage representation of the boolean
Expand Down Expand Up @@ -116,6 +119,11 @@ func main() {
Value: "bad-withdrawals.json",
Usage: "Path to write JSON file of bad withdrawals to manually inspect",
},
&cli.StringFlag{
Name: "storage-out",
Value: "storage.txt",
Usage: "Path to write JSON file of L2ToL1MessagePasser storage",
},
},
Action: func(ctx *cli.Context) error {
clients, err := util.NewClients(ctx)
Expand Down Expand Up @@ -163,10 +171,11 @@ func main() {
}

outfile := ctx.String("bad-withdrawals-out")
f, err := os.OpenFile(outfile, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o755)
f, err := os.OpenFile(outfile, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o644)
if err != nil {
return err
}
defer f.Close()

// create a transactor
opts, err := newTransactor(ctx)
Expand All @@ -177,6 +186,28 @@ func main() {
// Need this to compare in event parsing
l1StandardBridgeAddress := common.HexToAddress(ctx.String("l1-standard-bridge-address"))

if storageOutfile := ctx.String("storage-out"); storageOutfile != "" {
ff, err := os.OpenFile(storageOutfile, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o644)
if err != nil {
return err
}
defer ff.Close()

log.Info("Fetching storage for L2ToL1MessagePasser")
if storageRange, err := callStorageRange(clients, predeploys.L2ToL1MessagePasserAddr); err != nil {
log.Info("error getting storage range", "err", err)
} else {
str := ""
for key, value := range storageRange {
str += fmt.Sprintf("%s: %s\n", key.Hex(), value.Hex())
}
_, err = ff.WriteString(str)
if err != nil {
return err
}
}
}

// iterate over all of the withdrawals and submit them
for i, wd := range wds {
log.Info("Processing withdrawal", "index", i)
Expand Down Expand Up @@ -234,7 +265,7 @@ func main() {
// successful messages can be skipped, received messages failed
// their execution and should be replayed
if isSuccessNew {
log.Info("Message already relayed", "index", i, "hash", hash, "slot", slot)
log.Info("Message already relayed", "index", i, "hash", hash.Hex(), "slot", slot.Hex())
continue
}

Expand All @@ -248,7 +279,9 @@ func main() {

// the value should be set to a boolean in storage
if !bytes.Equal(storageValue, abiTrue.Bytes()) {
return fmt.Errorf("storage slot %x not found in state", slot)
log.Info("slot not found in state", "slot", slot.Hex())
continue
//return fmt.Errorf("storage slot %x not found in state", slot)
}

legacySlot, err := wd.StorageSlot()
Expand Down Expand Up @@ -443,10 +476,48 @@ func callTrace(c *util.Clients, receipt *types.Receipt) (callFrame, error) {
Tracer: &tracer,
}
err := c.L1RpcClient.Call(&finalizationTrace, "debug_traceTransaction", receipt.TxHash, traceConfig)
return finalizationTrace, err
}

func callStorageRangeAt(
client *rpc.Client,
blockHash common.Hash,
txIndex int,
addr common.Address,
keyStart hexutil.Bytes,
maxResult int,
) (*eth.StorageRangeResult, error) {
var storageRange *eth.StorageRangeResult
err := client.Call(&storageRange, "debug_storageRangeAt", blockHash, txIndex, addr, keyStart, maxResult)
return storageRange, err
}

func callStorageRange(c *util.Clients, addr common.Address) (state.Storage, error) {
header, err := c.L2Client.HeaderByNumber(context.Background(), nil)
if err != nil {
return finalizationTrace, err
return nil, err
}
return finalizationTrace, err
hash := header.Hash()
keyStart := hexutil.Bytes(common.Hash{}.Bytes())
maxResult := 1000

ret := make(state.Storage)

for {
result, err := callStorageRangeAt(c.L2RpcClient, hash, 0, addr, keyStart, maxResult)
if err != nil {
return nil, err
}
for key, value := range result.Storage {
ret[key] = value.Value
}
if result.NextKey == nil {
break
} else {
keyStart = hexutil.Bytes(result.NextKey.Bytes())
}
}
return ret, nil
}

// handleFinalizeETHWithdrawal will ensure that the calldata is correct
Expand Down Expand Up @@ -709,9 +780,13 @@ func newWithdrawals(ctx *cli.Context, l1ChainID *big.Int) ([]*crossdomain.Legacy
witnessFile := ctx.String("witness-file")

log.Debug("Migration data", "ovm-path", ovmMsgs, "evm-messages", evmMsgs, "witness-file", witnessFile)
ovmMessages, err := crossdomain.NewSentMessageFromJSON(ovmMsgs)
if err != nil {
return nil, err
var ovmMessages []*crossdomain.SentMessage
var err error
if ovmMsgs != "" {
ovmMessages, err = crossdomain.NewSentMessageFromJSON(ovmMsgs)
if err != nil {
return nil, err
}
}

// use empty ovmMessages if its not mainnet. The mainnet messages are
Expand Down
15 changes: 5 additions & 10 deletions op-chain-ops/crossdomain/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,12 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
return w, nil
}

// MigrateWithdrawalGasLimit computes the gas limit for the migrated withdrawal.
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
}
}

// 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
// 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
Expand Down
6 changes: 3 additions & 3 deletions op-chain-ops/crossdomain/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ func TestMigrateWithdrawalGasLimit(t *testing.T) {
},
{
input: []byte{0xff, 0x00},
output: 200_000 + 16 + 4,
output: 200_000 + 16 + 16,
},
{
input: []byte{0x00},
output: 200_000 + 4,
output: 200_000 + 16,
},
{
input: []byte{0x00, 0x00, 0x00},
output: 200_000 + 4 + 4 + 4,
output: 200_000 + 16 + 16 + 16,
},
}

Expand Down
8 changes: 5 additions & 3 deletions packages/sdk/src/utils/message-utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { hashWithdrawal, calldataCost } from '@eth-optimism/core-utils'
import { BigNumber } from 'ethers'
import { hashWithdrawal } from '@eth-optimism/core-utils'
import { BigNumber, utils } from 'ethers'

import { LowLevelMessage } from '../interfaces'

const { hexDataLength } = utils

/**
* Utility for hashing a LowLevelMessage object.
*
Expand All @@ -25,7 +27,7 @@ export const hashLowLevelMessage = (message: LowLevelMessage): string => {
*/
export const migratedWithdrawalGasLimit = (data: string): BigNumber => {
// Compute the gas limit and cap at 25 million
const dataCost = calldataCost(data)
const dataCost = BigNumber.from(hexDataLength(data))
let minGasLimit = dataCost.add(200_000)
if (minGasLimit.gt(25_000_000)) {
minGasLimit = BigNumber.from(25_000_000)
Expand Down
6 changes: 3 additions & 3 deletions packages/sdk/test/utils/message-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ describe('Message Utils', () => {
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) },
{ input: '0xff00', result: BigNumber.from(200_000 + 16 + 16) },
{ input: '0x00', result: BigNumber.from(200_000 + 16) },
{ input: '0x000000', result: BigNumber.from(200_000 + 16 + 16 + 16) },
]

for (const test of tests) {
Expand Down

0 comments on commit 7bf4d54

Please sign in to comment.