separate handling of upgrade config

core/blockchain.go

@@ -349,6 +349,32 @@ type BlockChain struct {
 	txIndexTailLock sync.Mutex
 }
 
+func CheckUpgradesCompatible(db ethdb.Database, config *params.ChainConfig, lastAcceptedHash common.Hash) error {
+	stored := rawdb.ReadCanonicalHash(db, 0)
+	if (stored == common.Hash{}) {
+		return errors.New("missing genesis block")
+	}
+	if lastAcceptedHash == (common.Hash{}) || lastAcceptedHash == stored {
+		return nil
+	}
+
+	lastBlock := ReadBlockByHash(db, lastAcceptedHash)
+	if lastBlock == nil {
+		return errors.New("missing last accepted block")
+	}
+	head := lastBlock.Header()
+	storedcfg := rawdb.ReadChainConfig(db, stored)
+	storedUpgrades := rawdb.ReadUpgradeConfig(db, stored)
+	storedcfg.UpgradeConfig = *storedUpgrades
+	compatErr := storedcfg.CheckUpgradesCompatible(&config.UpgradeConfig, lastBlock.Time())
+	if compatErr != nil && ((head.Number.Uint64() != 0 && compatErr.RewindToBlock != 0) || (head.Time != 0 && compatErr.RewindToTime != 0)) {
+		log.Error("Incompatible chain upgrades detected", "err", compatErr)
+		return compatErr
+	}
+
+	return nil
+}
+
 // NewBlockChain returns a fully initialised block chain using information
 // available in the database. It initialises the default Ethereum Validator and
 // Processor.
@@ -372,6 +398,13 @@ func NewBlockChain(
 	if err != nil {
 		return nil, err
 	}
+	if !skipChainConfigCheckCompatible {
+		if err := CheckUpgradesCompatible(db, genesis.Config, lastAcceptedHash); err != nil {
+			return nil, err
+		}
+	}
+	rawdb.WriteUpgradeConfig(db, genesis.ToBlock().Hash(), chainConfig.UpgradeConfig)
+
 	log.Info("")
 	log.Info(strings.Repeat("-", 153))
 	for _, line := range strings.Split(chainConfig.Description(), "\n") {

core/genesis.go

@@ -27,6 +27,7 @@
 package core
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -182,6 +183,7 @@ func SetupGenesisBlock(
 		rawdb.WriteChainConfig(db, stored, newcfg)
 		return newcfg, stored, nil
 	}
+	storedData, _ := json.Marshal(storedcfg)
 	// Check config compatibility and write the config. Compatibility errors
 	// are returned to the caller unless we're already at block zero.
 	// we use last accepted block for cfg compatibility check. Note this allows
@@ -194,22 +196,15 @@ func SetupGenesisBlock(
 	if lastBlock == nil {
 		return newcfg, common.Hash{}, errors.New("missing last accepted block")
 	}
-	height := lastBlock.NumberU64()
-	timestamp := lastBlock.Time()
-	if skipChainConfigCheckCompatible {
-		log.Info("skipping verifying activated network upgrades on chain config")
-	} else {
-		compatErr := storedcfg.CheckCompatible(newcfg, height, timestamp)
-		if compatErr != nil && ((height != 0 && compatErr.RewindToBlock != 0) || (timestamp != 0 && compatErr.RewindToTime != 0)) {
-			storedData, _ := storedcfg.ToWithUpgradesJSON().MarshalJSON()
-			newData, _ := newcfg.ToWithUpgradesJSON().MarshalJSON()
-			log.Error("found mismatch between config on database vs. new config", "storedConfig", string(storedData), "newConfig", string(newData), "err", compatErr)
-			return newcfg, stored, compatErr
-		}
+	head := lastBlock.Header()
+	compatErr := storedcfg.CheckCompatible(newcfg, head.Number.Uint64(), head.Time)
+	if compatErr != nil && ((head.Number.Uint64() != 0 && compatErr.RewindToBlock != 0) || (head.Time != 0 && compatErr.RewindToTime != 0)) {
+		return newcfg, stored, compatErr
+	}
+	// Don't overwrite if the old is identical to the new
+	if newData, _ := json.Marshal(newcfg); !bytes.Equal(storedData, newData) {
+		rawdb.WriteChainConfig(db, stored, newcfg)
 	}
-	// Required to write the chain config to disk to ensure both the chain config and upgrade bytes are persisted to disk.
-	// Note: this intentionally removes an extra check from upstream.
-	rawdb.WriteChainConfig(db, stored, newcfg)
 	return newcfg, stored, nil
 }
 

core/genesis_test.go

@@ -320,16 +320,18 @@ func TestGenesisWriteUpgradesRegression(t *testing.T) {
 	trieDB := triedb.NewDatabase(db, triedb.HashDefaults)
 	genesisBlock := genesis.MustCommit(db, trieDB)
 
-	_, _, err := SetupGenesisBlock(db, trieDB, genesis, genesisBlock.Hash(), false)
+	chain, err := NewBlockChain(db, DefaultCacheConfig, genesis, dummy.NewFullFaker(), vm.Config{}, genesisBlock.Hash(), false)
 	require.NoError(err)
+	chain.Stop()
 
 	genesis.Config.UpgradeConfig.PrecompileUpgrades = []params.PrecompileUpgrade{
 		{
 			Config: deployerallowlist.NewConfig(utils.NewUint64(51), nil, nil, nil),
 		},
 	}
-	_, _, err = SetupGenesisBlock(db, trieDB, genesis, genesisBlock.Hash(), false)
+	chain, err = NewBlockChain(db, DefaultCacheConfig, genesis, dummy.NewFullFaker(), vm.Config{}, genesisBlock.Hash(), false)
 	require.NoError(err)
+	chain.Stop()
 
 	timestamp := uint64(100)
 	lastAcceptedBlock := types.NewBlock(&types.Header{
@@ -343,7 +345,7 @@ func TestGenesisWriteUpgradesRegression(t *testing.T) {
 
 	// Attempt restart after the chain has advanced past the activation of the precompile upgrade.
 	// This tests a regression where the UpgradeConfig would not be written to disk correctly.
-	_, _, err = SetupGenesisBlock(db, trieDB, genesis, lastAcceptedBlock.Hash(), false)
+	err = CheckUpgradesCompatible(db, genesis.Config, lastAcceptedBlock.Hash())
 	require.NoError(err)
 }
 

core/rawdb/accessors_metadata.go

@@ -75,12 +75,17 @@ func ReadChainConfig(db ethdb.KeyValueReader, hash common.Hash) *params.ChainCon
 		return nil
 	}
 
+	return &config
+}
+
+func ReadUpgradeConfig(db ethdb.KeyValueReader, hash common.Hash) *params.UpgradeConfig {
+	var config params.UpgradeConfig
 	// Read the upgrade config for this chain config
-	data, _ = db.Get(upgradeConfigKey(hash))
+	data, _ := db.Get(upgradeConfigKey(hash))
 	if len(data) == 0 {
 		return &config // return early if no upgrade config is found
 	}
-	if err := json.Unmarshal(data, &config.UpgradeConfig); err != nil {
+	if err := json.Unmarshal(data, &config); err != nil {
 		log.Error("Invalid upgrade config JSON", "err", err)
 		return nil
 	}
@@ -100,9 +105,11 @@ func WriteChainConfig(db ethdb.KeyValueWriter, hash common.Hash, cfg *params.Cha
 	if err := db.Put(configKey(hash), data); err != nil {
 		log.Crit("Failed to store chain config", "err", err)
 	}
+}
 
+func WriteUpgradeConfig(db ethdb.KeyValueWriter, hash common.Hash, cfg params.UpgradeConfig) {
 	// Write the upgrade config for this chain config
-	data, err = json.Marshal(cfg.UpgradeConfig)
+	data, err := json.Marshal(cfg)
 	if err != nil {
 		log.Crit("Failed to JSON encode upgrade config", "err", err)
 	}

params/config.go

@@ -526,6 +526,10 @@ func (c *ChainConfig) checkCompatible(newcfg *ChainConfig, headNumber *big.Int,
 		return err
 	}
 
+	return nil
+}
+
+func (c *ChainConfig) CheckUpgradesCompatible(newcfg *UpgradeConfig, headTimestamp uint64) *ConfigCompatError {
 	// Check that the precompiles on the new config are compatible with the existing precompile config.
 	if err := c.CheckPrecompilesCompatible(newcfg.PrecompileUpgrades, headTimestamp); err != nil {
 		return err

params/precompile_upgrade_test.go

@@ -279,7 +279,11 @@ func (tt *upgradeCompatibilityTest) run(t *testing.T, chainConfig ChainConfig) {
 		newCfg := chainConfig
 		newCfg.UpgradeConfig = *upgrade
 
-		err := chainConfig.checkCompatible(&newCfg, nil, tt.startTimestamps[i])
+		err := chainConfig.CheckCompatible(&newCfg, 0, tt.startTimestamps[i])
+		if err == nil {
+			// if the chain configs were compatible, check the upgrade config
+			err = chainConfig.CheckUpgradesCompatible(&newCfg.UpgradeConfig, tt.startTimestamps[i])
+		}
 
 		// if this is not the final upgradeBytes, continue applying
 		// the next upgradeBytes. (only check the result on the last apply)