op-chain-ops: Fix state migration, add post-checks (#4551)

* op-chain-ops: Fix state migration, add post-checks

This PR fixes a bug in `op-chain-ops` that could cause state to become erroneously erased during the L2 migration. While testing the Goerli migration in check mode, I encountered an issue where `db.GetState` was returning null storage slots for the legacy message passer during the withdrawals portion of the migration. Upon further inspection, I discovered that we were using `db.CreateAccount` in the `setProxies` method. `db.CreateAccount` will erase the state associated with the passed-in address, so I updated `setProxies` to check if the account exists first. I also added a map of predeploys that should _not_ be touched as part of the migration process at all. At the end of the migration process, I iterate over these "untouchable" contracts and make sure that their code matches a preset list of code hashes and sample up to 5,000 storage slots before and after the migration to make sure the match.

Additionally, I found a bug in the witness generation data within `l2geth`. The witness generation code for the message passer does not take reverts into account. There's no easy way to do this, since we don't have access to the call frame in which the revert occurred. To work around this, I added the ability to ignore reverted storage slots in the `ParamsByChainId` structure.

Lastly, I made a few miscellaneous changes to improve our confidence in the migration script:

- I added checks to the OVM ETH migration to ensure that state slots are properly preserved.
- I added a read cache to the underlying state DB so that repeated read calls are faster.

I have verified that this migration script works end-to-end with check mode enabled against a forked Goerli L1.

* Add hash checking in addition to sampling

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

@@ -195,7 +195,7 @@ func main() {
 				return err
 			}
 
-			if err := genesis.CheckMigratedDB(postLDB); err != nil {
+			if err := genesis.PostCheckMigratedDB(postLDB, migrationData, &config.L1CrossDomainMessengerProxy, config.L1ChainID); err != nil {
 				return err
 			}
 

op-chain-ops/ether/migrate.go

@@ -29,7 +29,6 @@ var (
 		// Symbol
 		common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000004"): true,
 		common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000005"): true,
-		// Total supply
 		common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000006"): true,
 	}
 )
@@ -40,7 +39,7 @@ func MigrateLegacyETH(db ethdb.Database, stateDB *state.StateDB, addresses []com
 	// Set of storage slots that we expect to see in the OVM ETH contract.
 	storageSlotsToMigrate := make(map[common.Hash]int)
 	// Chain params to use for integrity checking.
-	params := ParamsByChainID[chainID]
+	params := migration.ParamsByChainID[chainID]
 	if params == nil {
 		return fmt.Errorf("no chain params for %d", chainID)
 	}

op-chain-ops/genesis/db_migration.go

@@ -5,16 +5,14 @@ import (
 	"fmt"
 	"math/big"
 
-	"github.com/ethereum-optimism/optimism/op-chain-ops/ether"
-
 	"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
 	"github.com/ethereum-optimism/optimism/op-chain-ops/crossdomain"
+	"github.com/ethereum-optimism/optimism/op-chain-ops/ether"
 	"github.com/ethereum-optimism/optimism/op-chain-ops/genesis/migration"
 	"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/core/types"
-	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/params"
@@ -70,6 +68,7 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 
 	underlyingDB := state.NewDatabaseWithConfig(ldb, &trie.Config{
 		Preimages: true,
+		Cache:     1024,
 	})
 
 	db, err := state.New(header.Root, underlyingDB, nil)
@@ -78,19 +77,22 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 	}
 
 	// Convert all of the messages into legacy withdrawals
-	withdrawals, err := migrationData.ToWithdrawals()
+	unfilteredWithdrawals, err := migrationData.ToWithdrawals()
 	if err != nil {
 		return nil, fmt.Errorf("cannot serialize withdrawals: %w", err)
 	}
 
+	var filteredWithdrawals []*crossdomain.LegacyWithdrawal
 	if !noCheck {
 		log.Info("Checking withdrawals...")
-		if err := CheckWithdrawals(db, withdrawals); err != nil {
+		filteredWithdrawals, err = PreCheckWithdrawals(db, unfilteredWithdrawals)
+		if err != nil {
 			return nil, fmt.Errorf("withdrawals mismatch: %w", err)
 		}
 		log.Info("Withdrawals accounted for!")
 	} else {
 		log.Info("Skipping checking withdrawals")
+		filteredWithdrawals = unfilteredWithdrawals
 	}
 
 	// Now start the migration
@@ -113,8 +115,12 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 		return nil, fmt.Errorf("cannot set implementations: %w", err)
 	}
 
+	if err := SetLegacyETH(db, storage, immutable); err != nil {
+		return nil, fmt.Errorf("cannot set legacy ETH: %w", err)
+	}
+
 	log.Info("Starting to migrate withdrawals", "no-check", noCheck)
-	err = crossdomain.MigrateWithdrawals(withdrawals, db, &config.L1CrossDomainMessengerProxy, noCheck)
+	err = crossdomain.MigrateWithdrawals(filteredWithdrawals, db, &config.L1CrossDomainMessengerProxy, noCheck)
 	if err != nil {
 		return nil, fmt.Errorf("cannot migrate withdrawals: %w", err)
 	}
@@ -132,7 +138,7 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 	if err != nil {
 		return nil, err
 	}
-	log.Info("committing state DB", "root", newRoot)
+	log.Info("committed state DB", "root", newRoot)
 
 	// Set the amount of gas used so that EIP 1559 starts off stable
 	gasUsed := (uint64)(config.L2GenesisBlockGasLimit) * config.EIP1559Elasticity
@@ -232,47 +238,60 @@ func MigrateDB(ldb ethdb.Database, config *DeployConfig, l1Block *types.Block, m
 	return res, nil
 }
 
-// CheckWithdrawals will ensure that the entire list of withdrawals is being
+// PreCheckWithdrawals will ensure that the entire list of withdrawals is being
 // operated on during the database migration.
-func CheckWithdrawals(db vm.StateDB, withdrawals []*crossdomain.LegacyWithdrawal) error {
+func PreCheckWithdrawals(db *state.StateDB, withdrawals []*crossdomain.LegacyWithdrawal) ([]*crossdomain.LegacyWithdrawal, error) {
 	// Create a mapping of all of their storage slots
-	knownSlots := make(map[common.Hash]bool)
+	slotsWds := make(map[common.Hash]*crossdomain.LegacyWithdrawal)
 	for _, wd := range withdrawals {
 		slot, err := wd.StorageSlot()
 		if err != nil {
-			return fmt.Errorf("cannot check withdrawals: %w", err)
+			return nil, fmt.Errorf("cannot check withdrawals: %w", err)
 		}
-		knownSlots[slot] = true
+
+		slotsWds[slot] = wd
 	}
+
 	// Build a map of all the slots in the LegacyMessagePasser
+	var count int
 	slots := make(map[common.Hash]bool)
 	err := db.ForEachStorage(predeploys.LegacyMessagePasserAddr, func(key, value common.Hash) bool {
 		if value != abiTrue {
 			return false
 		}
 		slots[key] = true
+		count++
 		return true
 	})
+
 	if err != nil {
-		return fmt.Errorf("cannot iterate over LegacyMessagePasser: %w", err)
+		return nil, fmt.Errorf("cannot iterate over LegacyMessagePasser: %w", err)
 	}
 
+	log.Info("iterated legacy messages", "count", count)
+
 	// Check that all of the slots from storage correspond to a known message
 	for slot := range slots {
-		_, ok := knownSlots[slot]
+		_, ok := slotsWds[slot]
 		if !ok {
-			return fmt.Errorf("Unknown storage slot in state: %s", slot)
+			return nil, fmt.Errorf("Unknown storage slot in state: %s", slot)
 		}
 	}
+
+	filtered := make([]*crossdomain.LegacyWithdrawal, 0)
+
 	// Check that all of the input messages are legit
-	for slot := range knownSlots {
+	for slot := range slotsWds {
 		//nolint:staticcheck
 		_, ok := slots[slot]
 		//nolint:staticcheck
 		if !ok {
-			return fmt.Errorf("Unknown input message: %s", slot)
+			log.Info("filtering out unknown input message", "slot", slot.String())
+			continue
 		}
+
+		filtered = append(filtered, slotsWds[slot])
 	}
 
-	return nil
+	return filtered, nil
 }

op-chain-ops/genesis/layer_two.go

@@ -22,22 +22,17 @@ func BuildL2DeveloperGenesis(config *DeployConfig, l1StartBlock *types.Block) (*
 	}
 	SetPrecompileBalances(db)
 
-	return BuildL2Genesis(db, config, l1StartBlock)
-}
-
-// BuildL2Genesis will build the L2 Optimism Genesis Block
-func BuildL2Genesis(db *state.MemoryStateDB, config *DeployConfig, l1Block *types.Block) (*core.Genesis, error) {
-	if err := SetL2Proxies(db); err != nil {
+	storage, err := NewL2StorageConfig(config, l1StartBlock)
+	if err != nil {
 		return nil, err
 	}
 
-	storage, err := NewL2StorageConfig(config, l1Block)
+	immutable, err := NewL2ImmutableConfig(config, l1StartBlock)
 	if err != nil {
 		return nil, err
 	}
 
-	immutable, err := NewL2ImmutableConfig(config, l1Block)
-	if err != nil {
+	if err := SetL2Proxies(db); err != nil {
 		return nil, err
 	}
 
@@ -45,5 +40,9 @@ func BuildL2Genesis(db *state.MemoryStateDB, config *DeployConfig, l1Block *type
 		return nil, err
 	}
 
+	if err := SetDevOnlyL2Implementations(db, storage, immutable); err != nil {
+		return nil, err
+	}
+
 	return db.Genesis(), nil
 }

op-chain-ops/genesis/layer_two_test.go

@@ -56,7 +56,7 @@ func TestBuildL2DeveloperGenesis(t *testing.T) {
 		require.Equal(t, ok, true)
 		require.Greater(t, len(account.Code), 0)
 
-		if name == "GovernanceToken" || name == "LegacyERC20ETH" || name == "ProxyAdmin" {
+		if name == "GovernanceToken" || name == "LegacyERC20ETH" || name == "ProxyAdmin" || name == "WETH9" {
 			continue
 		}
 
@@ -65,7 +65,7 @@ func TestBuildL2DeveloperGenesis(t *testing.T) {
 		require.Equal(t, adminSlot, predeploys.ProxyAdminAddr.Hash())
 		require.Equal(t, account.Code, depB)
 	}
-	require.Equal(t, 2343, len(gen.Alloc))
+	require.Equal(t, 2342, len(gen.Alloc))
 
 	if writeFile {
 		file, _ := json.MarshalIndent(gen, "", " ")
@@ -92,5 +92,5 @@ func TestBuildL2DeveloperGenesisDevAccountsFunding(t *testing.T) {
 
 	gen, err := genesis.BuildL2DeveloperGenesis(config, block)
 	require.NoError(t, err)
-	require.Equal(t, 2321, len(gen.Alloc))
+	require.Equal(t, 2320, len(gen.Alloc))
 }

op-chain-ops/ether/params.go → op-chain-ops/genesis/migration/params.go

-package ether
+package migration
 
 import (
 	"math/big"

op-chain-ops/genesis/setters.go

@@ -14,6 +14,28 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
+// UntouchablePredeploys are addresses in the predeploy namespace
+// that should not be touched by the migration process.
+var UntouchablePredeploys = map[common.Address]bool{
+	predeploys.GovernanceTokenAddr: true,
+	predeploys.WETH9Addr:           true,
+}
+
+// UntouchableCodeHashes contains code hashes of all the contracts
+// that should not be touched by the migration process.
+type ChainHashMap map[uint64]common.Hash
+
+var UntouchableCodeHashes = map[common.Address]ChainHashMap{
+	predeploys.GovernanceTokenAddr: {
+		1: common.HexToHash("0x8551d935f4e67ad3c98609f0d9f0f234740c4c4599f82674633b55204393e07f"),
+		5: common.HexToHash("0xc4a213cf5f06418533e5168d8d82f7ccbcc97f27ab90197c2c051af6a4941cf9"),
+	},
+	predeploys.WETH9Addr: {
+		1: common.HexToHash("0x779bbf2a738ef09d961c945116197e2ac764c1b39304b2b4418cd4e42668b173"),
+		5: common.HexToHash("0x779bbf2a738ef09d961c945116197e2ac764c1b39304b2b4418cd4e42668b173"),
+	},
+}
+
 // FundDevAccounts will fund each of the development accounts.
 func FundDevAccounts(db vm.StateDB) {
 	for _, account := range DevAccounts {
@@ -48,15 +70,15 @@ func setProxies(db vm.StateDB, proxyAdminAddr common.Address, namespace *big.Int
 		bigAddr := new(big.Int).Or(namespace, new(big.Int).SetUint64(i))
 		addr := common.BigToAddress(bigAddr)
 
-		// There is no proxy at the governance token address or
-		// the proxy admin address. LegacyERC20ETH lives in the
-		// 0xDead namespace so it can be ignored here
-		if addr == predeploys.GovernanceTokenAddr || addr == predeploys.ProxyAdminAddr {
+		if UntouchablePredeploys[addr] || addr == predeploys.ProxyAdminAddr {
 			log.Info("Skipping setting proxy", "address", addr)
 			continue
 		}
 
-		db.CreateAccount(addr)
+		if !db.Exist(addr) {
+			db.CreateAccount(addr)
+		}
+
 		db.SetCode(addr, depBytecode)
 		db.SetState(addr, AdminSlot, proxyAdminAddr.Hash())
 		log.Trace("Set proxy", "address", addr, "admin", proxyAdminAddr)
@@ -64,7 +86,16 @@ func setProxies(db vm.StateDB, proxyAdminAddr common.Address, namespace *big.Int
 	return nil
 }
 
-// SetImplementations will set the implmentations of the contracts in the state
+func SetLegacyETH(db vm.StateDB, storage state.StorageConfig, immutable immutables.ImmutableConfig) error {
+	deployResults, err := immutables.BuildOptimism(immutable)
+	if err != nil {
+		return err
+	}
+
+	return setupPredeploy(db, deployResults, storage, "LegacyERC20ETH", predeploys.LegacyERC20ETHAddr, predeploys.LegacyERC20ETHAddr)
+}
+
+// SetImplementations will set the implementations of the contracts in the state
 // and configure the proxies to point to the implementations. It also sets
 // the appropriate storage values for each contract at the proxy address.
 func SetImplementations(db vm.StateDB, storage state.StorageConfig, immutable immutables.ImmutableConfig) error {
@@ -74,47 +105,72 @@ func SetImplementations(db vm.StateDB, storage state.StorageConfig, immutable im
 	}
 
 	for name, address := range predeploys.Predeploys {
-		// Convert the address to the code address unless it is
-		// designed to not be behind a proxy
-		addr, special, err := mapImplementationAddress(address)
+		if UntouchablePredeploys[*address] {
+			continue
+		}
+
+		if *address == predeploys.LegacyERC20ETHAddr {
+			continue
+		}
+
+		codeAddr, err := AddressToCodeNamespace(*address)
 		if err != nil {
+			return fmt.Errorf("error converting to code namespace: %w", err)
+		}
+
+		// Proxy admin is a special case - it needs an impl set, but at its own address
+		if *address == predeploys.ProxyAdminAddr {
+			codeAddr = *address
+		}
+
+		if !db.Exist(codeAddr) {
+			db.CreateAccount(codeAddr)
+		}
+
+		if *address != predeploys.ProxyAdminAddr {
+			db.SetState(*address, ImplementationSlot, codeAddr.Hash())
+		}
+
+		if err := setupPredeploy(db, deployResults, storage, name, *address, codeAddr); err != nil {
 			return err
 		}
 
-		if !special {
-			db.SetState(*address, ImplementationSlot, addr.Hash())
+		code := db.GetCode(codeAddr)
+		if len(code) == 0 {
+			return fmt.Errorf("code not set for %s", name)
 		}
+	}
+	return nil
+}
 
-		// Create the account
-		db.CreateAccount(addr)
+func SetDevOnlyL2Implementations(db vm.StateDB, storage state.StorageConfig, immutable immutables.ImmutableConfig) error {
+	deployResults, err := immutables.BuildOptimism(immutable)
+	if err != nil {
+		return err
+	}
 
-		// Use the genrated bytecode when there are immutables
-		// otherwise use the artifact deployed bytecode
-		if bytecode, ok := deployResults[name]; ok {
-			log.Info("Setting deployed bytecode with immutables", "name", name, "address", addr)
-			db.SetCode(addr, bytecode)
-		} else {
-			depBytecode, err := bindings.GetDeployedBytecode(name)
-			if err != nil {
-				return err
-			}
-			log.Info("Setting deployed bytecode from solc compiler output", "name", name, "address", addr)
-			db.SetCode(addr, depBytecode)
+	for name, address := range predeploys.Predeploys {
+		if !UntouchablePredeploys[*address] {
+			continue
 		}
 
-		// Set the storage values
-		if storageConfig, ok := storage[name]; ok {
-			log.Info("Setting storage", "name", name, "address", *address)
-			if err := state.SetStorage(name, *address, storageConfig, db); err != nil {
-				return err
-			}
+		db.CreateAccount(*address)
+
+		if err := setupPredeploy(db, deployResults, storage, name, *address, *address); err != nil {
+			return err
 		}
 
-		code := db.GetCode(addr)
+		code := db.GetCode(*address)
 		if len(code) == 0 {
 			return fmt.Errorf("code not set for %s", name)
 		}
 	}
+
+	db.CreateAccount(predeploys.LegacyERC20ETHAddr)
+	if err := setupPredeploy(db, deployResults, storage, "LegacyERC20ETH", predeploys.LegacyERC20ETHAddr, predeploys.LegacyERC20ETHAddr); err != nil {
+		return fmt.Errorf("error setting up legacy eth: %w", err)
+	}
+
 	return nil
 }
 
@@ -129,22 +185,28 @@ func SetPrecompileBalances(db vm.StateDB) {
 	}
 }
 
-func mapImplementationAddress(addrP *common.Address) (common.Address, bool, error) {
-	var addr common.Address
-	var err error
-	var special bool
-	switch *addrP {
-	case predeploys.GovernanceTokenAddr:
-		addr = predeploys.GovernanceTokenAddr
-		special = true
-	case predeploys.LegacyERC20ETHAddr:
-		addr = predeploys.LegacyERC20ETHAddr
-		special = true
-	case predeploys.ProxyAdminAddr:
-		addr = predeploys.ProxyAdminAddr
-		special = true
-	default:
-		addr, err = AddressToCodeNamespace(*addrP)
+func setupPredeploy(db vm.StateDB, deployResults immutables.DeploymentResults, storage state.StorageConfig, name string, proxyAddr common.Address, implAddr common.Address) error {
+	// Use the generated bytecode when there are immutables
+	// otherwise use the artifact deployed bytecode
+	if bytecode, ok := deployResults[name]; ok {
+		log.Info("Setting deployed bytecode with immutables", "name", name, "address", implAddr)
+		db.SetCode(implAddr, bytecode)
+	} else {
+		depBytecode, err := bindings.GetDeployedBytecode(name)
+		if err != nil {
+			return err
+		}
+		log.Info("Setting deployed bytecode from solc compiler output", "name", name, "address", implAddr)
+		db.SetCode(implAddr, depBytecode)
 	}
-	return addr, special, err
+
+	// Set the storage values
+	if storageConfig, ok := storage[name]; ok {
+		log.Info("Setting storage", "name", name, "address", proxyAddr)
+		if err := state.SetStorage(name, proxyAddr, storageConfig, db); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }

packages/contracts-bedrock/tasks/check-l2.ts

@@ -41,7 +41,11 @@ const checkPredeploys = async (hre: HardhatRuntimeEnvironment) => {
       throw new Error(`no code found at ${addr}`)
     }
 
-    if (addr === predeploys.GovernanceToken || addr === predeploys.ProxyAdmin) {
+    if (
+      addr === predeploys.GovernanceToken ||
+      addr === predeploys.ProxyAdmin ||
+      addr === predeploys.WETH9
+    ) {
       continue
     }
 
@@ -370,7 +374,6 @@ const check = {
   // - check name
   // - check symbol
   // - check decimals
-  // - is behind a proxy
   WETH9: async (hre: HardhatRuntimeEnvironment) => {
     const WETH9 = await hre.ethers.getContractAt('WETH9', predeploys.WETH9)
 
@@ -385,9 +388,6 @@ const check = {
     const decimals = await WETH9.decimals()
     assert(decimals === 18)
     console.log(`  - decimals: ${decimals}`)
-
-    await checkProxy(hre, 'WETH9')
-    await assertProxy(hre, 'WETH9')
   },
   // GovernanceToken
   // - not behind a proxy

specs/predeploys.md

@@ -46,11 +46,11 @@ indicates when the predeploy was introduced. The possible values are `Legacy`
 or `Bedrock`. Deprecated contracts should not be used.
 
 | Name                          | Address                                    | Introduced | Deprecated | Proxied |
-| ----------------------------- | ------------------------------------------ | ---------- | ---------- | ------- |
+| ----------------------------- | ------------------------------------------ | ---------- | ---------- |---------|
 | LegacyMessagePasser           | 0x4200000000000000000000000000000000000000 | Legacy     | Yes        | Yes     |
 | DeployerWhitelist             | 0x4200000000000000000000000000000000000002 | Legacy     | Yes        | Yes     |
 | LegacyERC20ETH                | 0xDeadDeAddeAddEAddeadDEaDDEAdDeaDDeAD0000 | Legacy     | Yes        | No      |
-| WETH9                         | 0x4200000000000000000000000000000000000006 | Legacy     | No         | Yes     |
+| WETH9                         | 0x4200000000000000000000000000000000000006 | Legacy     | No         | No      |
 | L2CrossDomainMessenger        | 0x4200000000000000000000000000000000000007 | Legacy     | No         | Yes     |
 | L2StandardBridge              | 0x4200000000000000000000000000000000000010 | Legacy     | No         | Yes     |
 | SequencerFeeVault             | 0x4200000000000000000000000000000000000011 | Legacy     | No         | Yes     |