op-chain-ops: Correctly filter withdrawal hashes

Since we're checking every slot in the legacy message passer, we need to pass in the full list of messages to the message passer for verification purposes then filter out messages from senders other than the L2XDM prior to executing the withdrawals migration. This PR fixes the filtering, and adds a check in the withdrawals post-check step to make sure that no potentially fraudulent withdrawals get migrated.

Fixes CLI-3379

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

@@ -284,11 +284,11 @@ func main() {
 
 				if !isFinalized {
 					// Get the ETH balance of the withdrawal target *before* the finalization
-					targetBalBefore, err := clients.L1Client.BalanceAt(context.Background(), *wd.Target, nil)
+					targetBalBefore, err := clients.L1Client.BalanceAt(context.Background(), wd.XDomainTarget, nil)
 					if err != nil {
 						return err
 					}
-					log.Debug("Balance before finalization", "balance", targetBalBefore, "account", *wd.Target)
+					log.Debug("Balance before finalization", "balance", targetBalBefore, "account", wd.XDomainTarget)
 
 					log.Info("Finalizing withdrawal")
 					receipt, err := finalizeWithdrawalTransaction(contracts, clients, opts, wd, withdrawal)
@@ -369,14 +369,14 @@ func main() {
 					if method != nil {
 						log.Info("withdrawal action", "function", method.Name, "value", wdValue)
 					} else {
-						log.Info("unknown method", "to", wd.Target, "data", hexutil.Encode(wd.Data))
+						log.Info("unknown method", "to", wd.XDomainTarget, "data", hexutil.Encode(wd.XDomainData))
 						if err := writeSuspicious(f, withdrawal, wd, finalizationTrace, i, "unknown method"); err != nil {
 							return err
 						}
 					}
 
 					// check that the user's intents are actually executed
-					if common.HexToAddress(callFrame.To) != *wd.Target {
+					if common.HexToAddress(callFrame.To) != wd.XDomainTarget {
 						log.Info("target mismatch", "index", i)
 
 						if err := writeSuspicious(f, withdrawal, wd, finalizationTrace, i, "target mismatch"); err != nil {
@@ -384,7 +384,7 @@ func main() {
 						}
 						continue
 					}
-					if !bytes.Equal(hexutil.MustDecode(callFrame.Input), wd.Data) {
+					if !bytes.Equal(hexutil.MustDecode(callFrame.Input), wd.XDomainData) {
 						log.Info("calldata mismatch", "index", i)
 
 						if err := writeSuspicious(f, withdrawal, wd, finalizationTrace, i, "calldata mismatch"); err != nil {
@@ -401,7 +401,7 @@ func main() {
 					}
 
 					// Get the ETH balance of the withdrawal target *after* the finalization
-					targetBalAfter, err := clients.L1Client.BalanceAt(context.Background(), *wd.Target, nil)
+					targetBalAfter, err := clients.L1Client.BalanceAt(context.Background(), wd.XDomainTarget, nil)
 					if err != nil {
 						return err
 					}
@@ -621,7 +621,7 @@ func finalizeWithdrawalTransaction(
 	wd *crossdomain.LegacyWithdrawal,
 	withdrawal *crossdomain.Withdrawal,
 ) (*types.Receipt, error) {
-	if wd.Target == nil {
+	if wd.XDomainTarget == (common.Address{}) {
 		return nil, errors.New("withdrawal target is nil, should never happen")
 	}
 
@@ -833,7 +833,7 @@ func newTransactor(ctx *cli.Context) (*bind.TransactOpts, error) {
 // represents the user's intent.
 func findWithdrawalCall(trace *callFrame, wd *crossdomain.LegacyWithdrawal, l1xdm common.Address) *callFrame {
 	isCall := trace.Type == "CALL"
-	isTarget := common.HexToAddress(trace.To) == *wd.Target
+	isTarget := common.HexToAddress(trace.To) == wd.XDomainTarget
 	isFrom := common.HexToAddress(trace.From) == l1xdm
 	if isCall && isTarget && isFrom {
 		return trace

op-chain-ops/crossdomain/encoding.go

@@ -37,8 +37,8 @@ func init() {
 // EncodeCrossDomainMessageV0 will encode the calldata for
 // "relayMessage(address,address,bytes,uint256)",
 func EncodeCrossDomainMessageV0(
-	target *common.Address,
-	sender *common.Address,
+	target common.Address,
+	sender common.Address,
 	message []byte,
 	nonce *big.Int,
 ) ([]byte, error) {
@@ -49,8 +49,8 @@ func EncodeCrossDomainMessageV0(
 // "relayMessage(uint256,address,address,uint256,uint256,bytes)",
 func EncodeCrossDomainMessageV1(
 	nonce *big.Int,
-	sender *common.Address,
-	target *common.Address,
+	sender common.Address,
+	target common.Address,
 	value *big.Int,
 	gasLimit *big.Int,
 	data []byte,

op-chain-ops/crossdomain/hashing.go

@@ -10,8 +10,8 @@ import (
 // HashCrossDomainMessageV0 computes the pre bedrock cross domain messaging
 // hashing scheme.
 func HashCrossDomainMessageV0(
-	target *common.Address,
-	sender *common.Address,
+	target common.Address,
+	sender common.Address,
 	data []byte,
 	nonce *big.Int,
 ) (common.Hash, error) {
@@ -27,8 +27,8 @@ func HashCrossDomainMessageV0(
 // messaging hashing scheme.
 func HashCrossDomainMessageV1(
 	nonce *big.Int,
-	sender *common.Address,
-	target *common.Address,
+	sender common.Address,
+	target common.Address,
 	value *big.Int,
 	gasLimit *big.Int,
 	data []byte,

op-chain-ops/crossdomain/legacy_withdrawal.go

@@ -16,21 +16,28 @@ import (
 
 // LegacyWithdrawal represents a pre bedrock upgrade withdrawal.
 type LegacyWithdrawal struct {
-	Target *common.Address `json:"target"`
-	Sender *common.Address `json:"sender"`
-	Data   hexutil.Bytes   `json:"data"`
-	Nonce  *big.Int        `json:"nonce"`
+	// MessageSender is the caller of the message passer
+	MessageSender common.Address `json:"who"`
+	// XDomainTarget is the L1 target of the withdrawal message
+	XDomainTarget common.Address `json:"target"`
+	// XDomainSender is the L2 withdrawing account
+	XDomainSender common.Address `json:"sender"`
+	// XDomainData represents the calldata of the withdrawal message
+	XDomainData hexutil.Bytes `json:"data"`
+	// XDomainNonce represents the nonce of the withdrawal
+	XDomainNonce *big.Int `json:"nonce"`
 }
 
 var _ WithdrawalMessage = (*LegacyWithdrawal)(nil)
 
 // NewLegacyWithdrawal will construct a LegacyWithdrawal
-func NewLegacyWithdrawal(target, sender *common.Address, data []byte, nonce *big.Int) *LegacyWithdrawal {
+func NewLegacyWithdrawal(msgSender, target, sender common.Address, data []byte, nonce *big.Int) *LegacyWithdrawal {
 	return &LegacyWithdrawal{
-		Target: target,
-		Sender: sender,
-		Data:   data,
-		Nonce:  nonce,
+		MessageSender: msgSender,
+		XDomainTarget: target,
+		XDomainSender: sender,
+		XDomainData:   data,
+		XDomainNonce:  nonce,
 	}
 }
 
@@ -39,7 +46,7 @@ func NewLegacyWithdrawal(target, sender *common.Address, data []byte, nonce *big
 // through the standard optimism cross domain messaging system by hashing in
 // the L2CrossDomainMessenger address.
 func (w *LegacyWithdrawal) Encode() ([]byte, error) {
-	enc, err := EncodeCrossDomainMessageV0(w.Target, w.Sender, []byte(w.Data), w.Nonce)
+	enc, err := EncodeCrossDomainMessageV0(w.XDomainTarget, w.XDomainSender, []byte(w.XDomainData), w.XDomainNonce)
 	if err != nil {
 		return nil, fmt.Errorf("cannot encode LegacyWithdrawal: %w", err)
 	}
@@ -62,9 +69,6 @@ func (w *LegacyWithdrawal) Decode(data []byte) error {
 	}
 
 	msgSender := data[len(data)-len(predeploys.L2CrossDomainMessengerAddr):]
-	if !bytes.Equal(msgSender, predeploys.L2CrossDomainMessengerAddr.Bytes()) {
-		return errors.New("invalid msg.sender")
-	}
 
 	raw := data[4 : len(data)-len(predeploys.L2CrossDomainMessengerAddr)]
 
@@ -97,10 +101,11 @@ func (w *LegacyWithdrawal) Decode(data []byte) error {
 		return errors.New("cannot abi decode nonce")
 	}
 
-	w.Target = &target
-	w.Sender = &sender
-	w.Data = hexutil.Bytes(msgData)
-	w.Nonce = nonce
+	w.MessageSender = common.BytesToAddress(msgSender)
+	w.XDomainTarget = target
+	w.XDomainSender = sender
+	w.XDomainData = msgData
+	w.XDomainNonce = nonce
 	return nil
 }
 
@@ -142,19 +147,15 @@ func (w *LegacyWithdrawal) Value() (*big.Int, error) {
 	value := new(big.Int)
 
 	// Parse the 4byte selector
-	method, err := abi.MethodById(w.Data)
+	method, err := abi.MethodById(w.XDomainData)
 	// If it is an unknown selector, there is no value
 	if err != nil {
 		return value, nil
 	}
 
-	if w.Sender == nil {
-		return nil, errors.New("sender is nil")
-	}
-
-	isFromL2StandardBridge := *w.Sender == predeploys.L2StandardBridgeAddr
+	isFromL2StandardBridge := w.XDomainSender == predeploys.L2StandardBridgeAddr
 	if isFromL2StandardBridge && method.Name == "finalizeETHWithdrawal" {
-		data, err := method.Inputs.Unpack(w.Data[4:])
+		data, err := method.Inputs.Unpack(w.XDomainData[4:])
 		if err != nil {
 			return nil, err
 		}
@@ -177,11 +178,11 @@ func (w *LegacyWithdrawal) Value() (*big.Int, error) {
 // the concept of value or gaslimit, so set them to 0.
 func (w *LegacyWithdrawal) CrossDomainMessage() *CrossDomainMessage {
 	return &CrossDomainMessage{
-		Nonce:    w.Nonce,
-		Sender:   w.Sender,
-		Target:   w.Target,
+		Nonce:    w.XDomainNonce,
+		Sender:   w.XDomainSender,
+		Target:   w.XDomainTarget,
 		Value:    new(big.Int),
 		GasLimit: new(big.Int),
-		Data:     []byte(w.Data),
+		Data:     []byte(w.XDomainData),
 	}
 }

op-chain-ops/crossdomain/legacy_withdrawal_test.go

@@ -133,8 +133,7 @@ func TestWithdrawalLegacyStorageSlot(t *testing.T) {
 
 			// Cast the cross domain message to a withdrawal. Note that
 			// this only works for legacy style messages
-			withdrawal, err := msg.ToWithdrawal()
-			require.Nil(t, err)
+			withdrawal := toWithdrawal(t, common.HexToAddress(call.From), msg)
 
 			// Compute the legacy storage slot for the withdrawal
 			slot, err := withdrawal.StorageSlot()
@@ -160,12 +159,13 @@ func TestWithdrawalLegacyStorageSlot(t *testing.T) {
 }
 
 func FuzzEncodeDecodeLegacyWithdrawal(f *testing.F) {
-	f.Fuzz(func(t *testing.T, _target, _sender, _nonce, data []byte) {
+	f.Fuzz(func(t *testing.T, _msgSender, _target, _sender, _nonce, data []byte) {
+		msgSender := common.BytesToAddress(_msgSender)
 		target := common.BytesToAddress(_target)
 		sender := common.BytesToAddress(_sender)
 		nonce := new(big.Int).SetBytes(_nonce)
 
-		withdrawal := crossdomain.NewLegacyWithdrawal(&target, &sender, data, nonce)
+		withdrawal := crossdomain.NewLegacyWithdrawal(msgSender, target, sender, data, nonce)
 
 		encoded, err := withdrawal.Encode()
 		require.Nil(t, err)
@@ -174,10 +174,10 @@ func FuzzEncodeDecodeLegacyWithdrawal(f *testing.F) {
 		err = w.Decode(encoded)
 		require.Nil(t, err)
 
-		require.Equal(t, withdrawal.Nonce.Uint64(), w.Nonce.Uint64())
-		require.Equal(t, withdrawal.Sender, w.Sender)
-		require.Equal(t, withdrawal.Target, w.Target)
-		require.Equal(t, withdrawal.Data, w.Data)
+		require.Equal(t, withdrawal.XDomainNonce.Uint64(), w.XDomainNonce.Uint64())
+		require.Equal(t, withdrawal.XDomainSender, w.XDomainSender)
+		require.Equal(t, withdrawal.XDomainTarget, w.XDomainTarget)
+		require.Equal(t, withdrawal.XDomainData, w.XDomainData)
 	})
 }
 
@@ -221,8 +221,8 @@ func findCrossDomainMessage(receipt *types.Receipt) (*crossdomain.CrossDomainMes
 		// Parse the legacy event
 		if event.Name == "SentMessage" {
 			e, _ := l2xdm.ParseSentMessage(*log)
-			msg.Target = &e.Target
-			msg.Sender = &e.Sender
+			msg.Target = e.Target
+			msg.Sender = e.Sender
 			msg.Data = e.Message
 			msg.Nonce = e.MessageNonce
 			msg.GasLimit = e.GasLimit
@@ -336,3 +336,30 @@ func readStateDiff(hash string) (stateDiff, error) {
 	}
 	return diff, nil
 }
+
+// ToWithdrawal will turn a CrossDomainMessage into a Withdrawal.
+// This only works for version 0 CrossDomainMessages as not all of
+// the data is present for version 1 CrossDomainMessages to be turned
+// into Withdrawals.
+func toWithdrawal(t *testing.T, msgSender common.Address, c *crossdomain.CrossDomainMessage) *crossdomain.LegacyWithdrawal {
+	version := c.Version()
+	switch version {
+	case 0:
+		if c.Value != nil && c.Value.Cmp(common.Big0) != 0 {
+			t.Fatalf("version 0 messages must have 0 value")
+		}
+		w := &crossdomain.LegacyWithdrawal{
+			MessageSender: msgSender,
+			XDomainTarget: c.Target,
+			XDomainSender: c.Sender,
+			XDomainData:   c.Data,
+			XDomainNonce:  c.Nonce,
+		}
+		return w
+	case 1:
+		t.Fatalf("cannot convert version 1 messages to withdrawals")
+	default:
+		t.Fatalf("unknown message version: %d", version)
+	}
+	return nil
+}

op-chain-ops/crossdomain/message.go

 package crossdomain
 
 import (
-	"errors"
 	"fmt"
 	"math/big"
 
@@ -14,18 +13,18 @@ import (
 // version 1 messages have a value and the most significant
 // byte of the nonce is a 1
 type CrossDomainMessage struct {
-	Nonce    *big.Int        `json:"nonce"`
-	Sender   *common.Address `json:"sender"`
-	Target   *common.Address `json:"target"`
-	Value    *big.Int        `json:"value"`
-	GasLimit *big.Int        `json:"gasLimit"`
-	Data     []byte          `json:"data"`
+	Nonce    *big.Int       `json:"nonce"`
+	Sender   common.Address `json:"sender"`
+	Target   common.Address `json:"target"`
+	Value    *big.Int       `json:"value"`
+	GasLimit *big.Int       `json:"gasLimit"`
+	Data     []byte         `json:"data"`
 }
 
 // NewCrossDomainMessage creates a CrossDomainMessage.
 func NewCrossDomainMessage(
 	nonce *big.Int,
-	sender, target *common.Address,
+	sender, target common.Address,
 	value, gasLimit *big.Int,
 	data []byte,
 ) *CrossDomainMessage {
@@ -77,23 +76,3 @@ func (c *CrossDomainMessage) Hash() (common.Hash, error) {
 func (c *CrossDomainMessage) HashV1() (common.Hash, error) {
 	return HashCrossDomainMessageV1(c.Nonce, c.Sender, c.Target, c.Value, c.GasLimit, c.Data)
 }
-
-// ToWithdrawal will turn a CrossDomainMessage into a Withdrawal.
-// This only works for version 0 CrossDomainMessages as not all of
-// the data is present for version 1 CrossDomainMessages to be turned
-// into Withdrawals.
-func (c *CrossDomainMessage) ToWithdrawal() (WithdrawalMessage, error) {
-	version := c.Version()
-	switch version {
-	case 0:
-		if c.Value != nil && c.Value.Cmp(common.Big0) != 0 {
-			return nil, errors.New("version 0 messages must have 0 value")
-		}
-		w := NewLegacyWithdrawal(c.Target, c.Sender, c.Data, c.Nonce)
-		return w, nil
-	case 1:
-		return nil, errors.New("version 1 messages cannot be turned into withdrawals")
-	default:
-		return nil, fmt.Errorf("unknown version %d", version)
-	}
-}

op-chain-ops/crossdomain/message_test.go

@@ -18,8 +18,8 @@ func TestEncode(t *testing.T) {
 	t.Run("V0", func(t *testing.T) {
 		msg := crossdomain.NewCrossDomainMessage(
 			crossdomain.EncodeVersionedNonce(common.Big0, common.Big0),
-			&common.Address{},
-			&common.Address{19: 0x01},
+			common.Address{},
+			common.Address{19: 0x01},
 			big.NewInt(0),
 			big.NewInt(5),
 			[]byte{},
@@ -37,8 +37,8 @@ func TestEncode(t *testing.T) {
 	t.Run("V1", func(t *testing.T) {
 		msg := crossdomain.NewCrossDomainMessage(
 			crossdomain.EncodeVersionedNonce(common.Big1, common.Big1),
-			&common.Address{19: 0x01},
-			&common.Address{19: 0x02},
+			common.Address{19: 0x01},
+			common.Address{19: 0x02},
 			big.NewInt(100),
 			big.NewInt(555),
 			[]byte{},
@@ -63,8 +63,8 @@ func TestHash(t *testing.T) {
 	t.Run("V0", func(t *testing.T) {
 		msg := crossdomain.NewCrossDomainMessage(
 			crossdomain.EncodeVersionedNonce(common.Big0, common.Big0),
-			&common.Address{},
-			&common.Address{19: 0x01},
+			common.Address{},
+			common.Address{19: 0x01},
 			big.NewInt(10),
 			big.NewInt(5),
 			[]byte{},
@@ -82,8 +82,8 @@ func TestHash(t *testing.T) {
 	t.Run("V1", func(t *testing.T) {
 		msg := crossdomain.NewCrossDomainMessage(
 			crossdomain.EncodeVersionedNonce(common.Big0, common.Big1),
-			&common.Address{},
-			&common.Address{19: 0x01},
+			common.Address{},
+			common.Address{19: 0x01},
 			big.NewInt(0),
 			big.NewInt(5),
 			[]byte{},

op-chain-ops/crossdomain/migrate.go

@@ -19,7 +19,7 @@ var (
 )
 
 // MigrateWithdrawals will migrate a list of pending withdrawals given a StateDB.
-func MigrateWithdrawals(withdrawals []*LegacyWithdrawal, db vm.StateDB, l1CrossDomainMessenger *common.Address, noCheck bool) error {
+func MigrateWithdrawals(withdrawals SafeFilteredWithdrawals, db vm.StateDB, l1CrossDomainMessenger *common.Address, noCheck bool) error {
 	for i, legacy := range withdrawals {
 		legacySlot, err := legacy.StorageSlot()
 		if err != nil {
@@ -66,17 +66,17 @@ func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *com
 	// Migrated withdrawals are specified as version 0. Both the
 	// L2ToL1MessagePasser and the CrossDomainMessenger use the same
 	// versioning scheme. Both should be set to version 0
-	versionedNonce := EncodeVersionedNonce(withdrawal.Nonce, new(big.Int))
+	versionedNonce := EncodeVersionedNonce(withdrawal.XDomainNonce, new(big.Int))
 	// Encode the call to `relayMessage` on the `CrossDomainMessenger`.
 	// The minGasLimit can safely be 0 here.
 	data, err := abi.Pack(
 		"relayMessage",
 		versionedNonce,
-		withdrawal.Sender,
-		withdrawal.Target,
+		withdrawal.XDomainSender,
+		withdrawal.XDomainTarget,
 		value,
 		new(big.Int),
-		[]byte(withdrawal.Data),
+		[]byte(withdrawal.XDomainData),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("cannot abi encode relayMessage: %w", err)

op-chain-ops/crossdomain/migrate_test.go

@@ -17,10 +17,7 @@ func TestMigrateWithdrawal(t *testing.T) {
 	for _, receipt := range receipts {
 		msg, err := findCrossDomainMessage(receipt)
 		require.Nil(t, err)
-		withdrawal, err := msg.ToWithdrawal()
-		require.Nil(t, err)
-		legacyWithdrawal, ok := withdrawal.(*crossdomain.LegacyWithdrawal)
-		require.True(t, ok)
+		legacyWithdrawal := toWithdrawal(t, predeploys.L2CrossDomainMessengerAddr, msg)
 		withdrawals = append(withdrawals, legacyWithdrawal)
 	}
 
@@ -31,7 +28,7 @@ func TestMigrateWithdrawal(t *testing.T) {
 			require.Nil(t, err)
 			require.NotNil(t, withdrawal)
 
-			require.Equal(t, legacy.Nonce.Uint64(), withdrawal.Nonce.Uint64())
+			require.Equal(t, legacy.XDomainNonce.Uint64(), withdrawal.Nonce.Uint64())
 			require.Equal(t, *withdrawal.Sender, predeploys.L2CrossDomainMessengerAddr)
 			require.Equal(t, *withdrawal.Target, l1CrossDomainMessenger)
 		})

op-chain-ops/crossdomain/precheck.go

 package crossdomain
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
-
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/log"
 )
 
+var (
+	ErrUnknownSlotInMessagePasser = errors.New("unknown slot in legacy message passer")
+	ErrMissingSlotInWitness       = errors.New("missing storage slot in witness data")
+)
+
 // PreCheckWithdrawals checks that the given list of withdrawals represents all withdrawals made
 // in the legacy system and filters out any extra withdrawals not included in the legacy system.
-func PreCheckWithdrawals(db *state.StateDB, withdrawals []*LegacyWithdrawal) ([]*LegacyWithdrawal, error) {
+func PreCheckWithdrawals(db *state.StateDB, withdrawals DangerousUnfilteredWithdrawals) (SafeFilteredWithdrawals, error) {
 	// Convert each withdrawal into a storage slot, and build a map of those slots.
 	slotsInp := make(map[common.Hash]*LegacyWithdrawal)
 	for _, wd := range withdrawals {
@@ -26,6 +31,7 @@ func PreCheckWithdrawals(db *state.StateDB, withdrawals []*LegacyWithdrawal) ([]
 
 	// Build a mapping of the slots of all messages actually sent in the legacy system.
 	var count int
+	var innerErr error
 	slotsAct := make(map[common.Hash]bool)
 	err := db.ForEachStorage(predeploys.LegacyMessagePasserAddr, func(key, value common.Hash) bool {
 		// When a message is inserted into the LegacyMessagePasser, it is stored with the value
@@ -33,7 +39,7 @@ func PreCheckWithdrawals(db *state.StateDB, withdrawals []*LegacyWithdrawal) ([]
 		// can safely ignore anything that is not "true".
 		if value != abiTrue {
 			// Should not happen!
-			log.Error("found unknown slot in LegacyMessagePasser", "key", key.String(), "val", value.String())
+			innerErr = fmt.Errorf("%w: key: %s, val: %s", ErrUnknownSlotInMessagePasser, key.String(), value.String())
 			return true
 		}
 
@@ -45,6 +51,9 @@ func PreCheckWithdrawals(db *state.StateDB, withdrawals []*LegacyWithdrawal) ([]
 	if err != nil {
 		return nil, fmt.Errorf("cannot iterate over LegacyMessagePasser: %w", err)
 	}
+	if innerErr != nil {
+		return nil, innerErr
+	}
 
 	// Log the number of messages we found.
 	log.Info("Iterated legacy messages", "count", count)
@@ -53,13 +62,13 @@ func PreCheckWithdrawals(db *state.StateDB, withdrawals []*LegacyWithdrawal) ([]
 	for slot := range slotsAct {
 		_, ok := slotsInp[slot]
 		if !ok {
-			return nil, fmt.Errorf("unknown storage slot in state: %s", slot)
+			return nil, ErrMissingSlotInWitness
 		}
 	}
 
 	// Iterate over the list of input messages and check that we have a known slot for each one.
 	// We'll filter out any extra messages that are not in the legacy system.
-	filtered := make([]*LegacyWithdrawal, 0)
+	filtered := make(SafeFilteredWithdrawals, 0)
 	for slot := range slotsInp {
 		_, ok := slotsAct[slot]
 		if !ok {
@@ -67,7 +76,13 @@ func PreCheckWithdrawals(db *state.StateDB, withdrawals []*LegacyWithdrawal) ([]
 			continue
 		}
 
-		filtered = append(filtered, slotsInp[slot])
+		wd := slotsInp[slot]
+		if wd.MessageSender != predeploys.L2CrossDomainMessengerAddr {
+			log.Info("filtering out message from sender other than the L2XDM", "sender", wd.MessageSender)
+			continue
+		}
+
+		filtered = append(filtered, wd)
 	}
 
 	// At this point, we know that the list of filtered withdrawals MUST be exactly the same as the

op-chain-ops/crossdomain/precheck_test.go

+package crossdomain
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/rawdb"
+	"github.com/ethereum/go-ethereum/core/state"
+	"github.com/ethereum/go-ethereum/trie"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPreCheckWithdrawals_Filtering(t *testing.T) {
+	dbWds := []*LegacyWithdrawal{
+		// Random legacy WD to something other than the L2XDM.
+		{
+			MessageSender: common.Address{19: 0xFF},
+			XDomainTarget: common.Address{19: 0x01},
+			XDomainSender: common.Address{19: 0x02},
+			XDomainData:   []byte{0x01, 0x02, 0x03},
+			XDomainNonce:  big.NewInt(0),
+		},
+		// Random legacy WD to the L2XDM. Should be the only thing
+		// returned by the prechecker.
+		{
+			MessageSender: predeploys.L2CrossDomainMessengerAddr,
+			XDomainTarget: common.Address{19: 0x01},
+			XDomainSender: common.Address{19: 0x02},
+			XDomainData:   []byte{0x01, 0x02, 0x03},
+			XDomainNonce:  big.NewInt(1),
+		},
+	}
+
+	// Add an additional witness to the witnesses list to
+	// test how the prechecker handles witness data that
+	// isn't in state.
+	witnessWds := append([]*LegacyWithdrawal{
+		{
+			MessageSender: common.Address{19: 0xAA},
+			XDomainTarget: common.Address{19: 0x03},
+			XDomainSender: predeploys.L2CrossDomainMessengerAddr,
+			XDomainData:   []byte{0x01, 0x02, 0x03},
+			XDomainNonce:  big.NewInt(0),
+		},
+	}, dbWds...)
+
+	filteredWds, err := runPrecheck(t, dbWds, witnessWds)
+	require.NoError(t, err)
+	require.EqualValues(t, []*LegacyWithdrawal{dbWds[1]}, filteredWds)
+}
+
+func TestPreCheckWithdrawals_InvalidSlotInStorage(t *testing.T) {
+	rawDB := rawdb.NewMemoryDatabase()
+	rawStateDB := state.NewDatabaseWithConfig(rawDB, &trie.Config{
+		Preimages: true,
+		Cache:     1024,
+	})
+	stateDB, err := state.New(common.Hash{}, rawStateDB, nil)
+	require.NoError(t, err)
+
+	// Create account, and set a random storage slot to a value
+	// other than abiTrue.
+	stateDB.CreateAccount(predeploys.LegacyMessagePasserAddr)
+	stateDB.SetState(predeploys.LegacyMessagePasserAddr, common.Hash{0: 0xff}, common.Hash{0: 0xff})
+
+	root, err := stateDB.Commit(false)
+	require.NoError(t, err)
+
+	err = stateDB.Database().TrieDB().Commit(root, true, nil)
+	require.NoError(t, err)
+
+	_, err = PreCheckWithdrawals(stateDB, nil)
+	require.ErrorIs(t, err, ErrUnknownSlotInMessagePasser)
+}
+
+func TestPreCheckWithdrawals_MissingStorageSlot(t *testing.T) {
+	// Add a legacy WD to state that does not appear in witness data.
+	dbWds := []*LegacyWithdrawal{
+		{
+			XDomainTarget: common.Address{19: 0x01},
+			XDomainSender: predeploys.L2CrossDomainMessengerAddr,
+			XDomainData:   []byte{0x01, 0x02, 0x03},
+			XDomainNonce:  big.NewInt(1),
+		},
+	}
+
+	// Create some witness data that includes both a valid
+	// and an invalid witness, but neither of which correspond
+	// to the value above in state.
+	witnessWds := []*LegacyWithdrawal{
+		{
+			XDomainTarget: common.Address{19: 0x01},
+			XDomainSender: common.Address{19: 0x02},
+			XDomainData:   []byte{0x01, 0x02, 0x03},
+			XDomainNonce:  big.NewInt(0),
+		},
+		{
+			XDomainTarget: common.Address{19: 0x03},
+			XDomainSender: predeploys.L2CrossDomainMessengerAddr,
+			XDomainData:   []byte{0x01, 0x02, 0x03},
+			XDomainNonce:  big.NewInt(0),
+		},
+	}
+
+	_, err := runPrecheck(t, dbWds, witnessWds)
+	require.ErrorIs(t, err, ErrMissingSlotInWitness)
+}
+
+func runPrecheck(t *testing.T, dbWds []*LegacyWithdrawal, witnessWds []*LegacyWithdrawal) ([]*LegacyWithdrawal, error) {
+	rawDB := rawdb.NewMemoryDatabase()
+	rawStateDB := state.NewDatabaseWithConfig(rawDB, &trie.Config{
+		Preimages: true,
+		Cache:     1024,
+	})
+	stateDB, err := state.New(common.Hash{}, rawStateDB, nil)
+	require.NoError(t, err)
+
+	stateDB.CreateAccount(predeploys.LegacyMessagePasserAddr)
+	for _, wd := range dbWds {
+		slot, err := wd.StorageSlot()
+		require.NoError(t, err)
+		stateDB.SetState(predeploys.LegacyMessagePasserAddr, slot, abiTrue)
+	}
+
+	root, err := stateDB.Commit(false)
+	require.NoError(t, err)
+
+	err = stateDB.Database().TrieDB().Commit(root, true, nil)
+	require.NoError(t, err)
+
+	return PreCheckWithdrawals(stateDB, witnessWds)
+}

op-chain-ops/crossdomain/types.go

@@ -5,6 +5,15 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 )
 
+// DangerousUnfilteredWithdrawals is a list of raw withdrawal witness
+// data. It has not been filtered for messages from sources other than
+// the
+type DangerousUnfilteredWithdrawals []*LegacyWithdrawal
+
+// SafeFilteredWithdrawals is a list of withdrawals that have been filtered to only include
+// withdrawals that were from the L2XDM.
+type SafeFilteredWithdrawals []*LegacyWithdrawal
+
 var (
 	// Standard ABI types
 	Uint256Type, _ = abi.NewType("uint256", "", nil)

op-chain-ops/crossdomain/withdrawals.go

@@ -87,8 +87,8 @@ func GetPendingWithdrawals(messengers *Messengers, version *big.Int, start, end
 
 		msg := NewCrossDomainMessage(
 			event.MessageNonce,
-			&event.Sender,
-			&event.Target,
+			event.Sender,
+			event.Target,
 			common.Big0,
 			event.GasLimit,
 			event.Message,
@@ -116,10 +116,10 @@ func GetPendingWithdrawals(messengers *Messengers, version *big.Int, start, end
 
 			withdrawal := PendingWithdrawal{
 				LegacyWithdrawal: LegacyWithdrawal{
-					Target: &event.Target,
-					Sender: &event.Sender,
-					Data:   event.Message,
-					Nonce:  event.MessageNonce,
+					XDomainTarget: event.Target,
+					XDomainSender: event.Sender,
+					XDomainData:   event.Message,
+					XDomainNonce:  event.MessageNonce,
 				},
 				TransactionHash: event.Raw.TxHash,
 			}

op-chain-ops/crossdomain/withdrawals_test.go

@@ -146,8 +146,8 @@ func sendCrossDomainMessage(
 		// Parse the legacy event
 		if event.Name == "SentMessage" {
 			e, _ := l2xdm.ParseSentMessage(*log)
-			msg.Target = &e.Target
-			msg.Sender = &e.Sender
+			msg.Target = e.Target
+			msg.Sender = e.Sender
 			msg.Data = e.Message
 			msg.Nonce = e.MessageNonce
 			msg.GasLimit = e.GasLimit
@@ -272,7 +272,7 @@ func TestGetPendingWithdrawals(t *testing.T) {
 	for i, msg := range msgs[3:] {
 		withdrawal := withdrawals[i]
 
-		require.Equal(t, msg.Target, *withdrawal.Target)
-		require.Equal(t, msg.Message, []byte(withdrawal.Data))
+		require.Equal(t, msg.Target, withdrawal.XDomainTarget)
+		require.Equal(t, msg.Message, []byte(withdrawal.XDomainData))
 	}
 }

op-chain-ops/genesis/check.go

@@ -461,7 +461,8 @@ func CheckWithdrawalsAfter(db vm.StateDB, data migration.MigrationData, l1CrossD
 	// First, make a mapping between old withdrawal slots and new ones.
 	// This list can be a superset of what was actually migrated, since
 	// some witness data may references withdrawals that reverted.
-	oldToNew := make(map[common.Hash]common.Hash)
+	oldToNewSlots := make(map[common.Hash]common.Hash)
+	wdsByOldSlot := make(map[common.Hash]*crossdomain.LegacyWithdrawal)
 	for _, wd := range wds {
 		migrated, err := crossdomain.MigrateWithdrawal(wd, l1CrossDomainMessenger)
 		if err != nil {
@@ -477,7 +478,8 @@ func CheckWithdrawalsAfter(db vm.StateDB, data migration.MigrationData, l1CrossD
 			return fmt.Errorf("cannot compute migrated storage slot: %w", err)
 		}
 
-		oldToNew[legacySlot] = migratedSlot
+		oldToNewSlots[legacySlot] = migratedSlot
+		wdsByOldSlot[legacySlot] = wd
 	}
 
 	// Now, iterate over each legacy withdrawal and check if there is a corresponding
@@ -498,17 +500,29 @@ func CheckWithdrawalsAfter(db vm.StateDB, data migration.MigrationData, l1CrossD
 		}
 
 		// Grab the migrated slot.
-		migratedSlot := oldToNew[key]
+		migratedSlot := oldToNewSlots[key]
 		if migratedSlot == (common.Hash{}) {
 			innerErr = fmt.Errorf("no migrated slot found for legacy slot %s", key)
 			return false
 		}
 
-		// Look up the migrated slot in the DB, and make sure it is abiTrue.
+		// Look up the migrated slot in the DB.
 		migratedValue := db.GetState(predeploys.L2ToL1MessagePasserAddr, migratedSlot)
-		if migratedValue != abiTrue {
-			innerErr = fmt.Errorf("expected migrated value to be true, but got %s", migratedValue)
-			return false
+
+		// If the sender is _not_ the L2XDM, the value should not be migrated.
+		wd := wdsByOldSlot[key]
+		if wd.XDomainSender == predeploys.L2CrossDomainMessengerAddr {
+			// Make sure the value is abiTrue if this withdrawal should be migrated.
+			if migratedValue != abiTrue {
+				innerErr = fmt.Errorf("expected migrated value to be true, but got %s", migratedValue)
+				return false
+			}
+		} else {
+			// Otherwise, ensure that withdrawals from senders other than the L2XDM are _not_ migrated.
+			if migratedValue != abiFalse {
+				innerErr = fmt.Errorf("a migration from a sender other than the L2XDM was migrated")
+				return false
+			}
 		}
 
 		return true

op-chain-ops/genesis/db_migration.go

@@ -20,7 +20,8 @@ import (
 )
 
 var (
-	abiTrue = common.Hash{31: 0x01}
+	abiTrue  = common.Hash{31: 0x01}
+	abiFalse = common.Hash{}
 	// BedrockTransitionBlockExtraData represents the extradata
 	// set in the very first bedrock block. This value must be
 	// less than 32 bytes long or it will create an invalid block.
@@ -120,7 +121,7 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 
 	// We now need to check that we have all of the withdrawals that we expect to have. An error
 	// will be thrown if there are any missing messages, and any extra messages will be removed.
-	var filteredWithdrawals []*crossdomain.LegacyWithdrawal
+	var filteredWithdrawals crossdomain.SafeFilteredWithdrawals
 	if !noCheck {
 		log.Info("Checking withdrawals...")
 		filteredWithdrawals, err = crossdomain.PreCheckWithdrawals(db, unfilteredWithdrawals)
@@ -129,7 +130,7 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 		}
 	} else {
 		log.Info("Skipping checking withdrawals")
-		filteredWithdrawals = unfilteredWithdrawals
+		filteredWithdrawals = crossdomain.SafeFilteredWithdrawals(unfilteredWithdrawals)
 	}
 
 	// We also need to verify that we have all of the storage slots for the LegacyERC20ETH contract

op-chain-ops/genesis/migration/types.go

@@ -5,8 +5,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
-
 	"github.com/ethereum-optimism/optimism/op-chain-ops/crossdomain"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -119,12 +117,9 @@ type MigrationData struct {
 	EvmMessages []*SentMessage
 }
 
-func (m *MigrationData) ToWithdrawals() ([]*crossdomain.LegacyWithdrawal, error) {
-	messages := make([]*crossdomain.LegacyWithdrawal, 0)
+func (m *MigrationData) ToWithdrawals() (crossdomain.DangerousUnfilteredWithdrawals, error) {
+	messages := make(crossdomain.DangerousUnfilteredWithdrawals, 0)
 	for _, msg := range m.OvmMessages {
-		if msg.Who != predeploys.L2CrossDomainMessengerAddr {
-			continue
-		}
 		wd, err := msg.ToLegacyWithdrawal()
 		if err != nil {
 			return nil, err
@@ -135,9 +130,6 @@ func (m *MigrationData) ToWithdrawals() ([]*crossdomain.LegacyWithdrawal, error)
 		}
 	}
 	for _, msg := range m.EvmMessages {
-		if msg.Who != predeploys.L2CrossDomainMessengerAddr {
-			continue
-		}
 		wd, err := msg.ToLegacyWithdrawal()
 		if err != nil {
 			return nil, err