fix: correctly select consensus engine during EuclidV2 transition (#1130)

* fix: correctly select consensus engine during EuclidV2 transition

* correctly backdate transition blocks

* nit

* correctly unset fields

* nit

Author: Thegaram

consensus/clique/clique.go

@@ -507,13 +507,29 @@ func (c *Clique) verifySeal(snap *Snapshot, header *types.Header, parents []*typ
 	return nil
 }
 
+func (c *Clique) CalcTimestamp(parent *types.Header) uint64 {
+	timestamp := parent.Time + c.config.Period
+
+	// If RelaxedPeriod is enabled, always set the header timestamp to now (ie the time we start building it) as
+	// we don't know when it will be sealed
+	if c.config.RelaxedPeriod || timestamp < uint64(time.Now().Unix()) {
+		timestamp = uint64(time.Now().Unix())
+	}
+
+	return timestamp
+}
+
 // Prepare implements consensus.Engine, preparing all the consensus fields of the
 // header for running the transactions on top.
-func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error {
+func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header, timeOverride *uint64) error {
 	// If the block isn't a checkpoint, cast a random vote (good enough for now)
 	header.Coinbase = common.Address{}
 	header.Nonce = types.BlockNonce{}
 
+	// Unset EuclidV2-related fields
+	header.BlockSignature = nil
+	header.IsEuclidV2 = false
+
 	number := header.Number.Uint64()
 	// Assemble the voting snapshot to check which votes make sense
 	snap, err := c.snapshot(chain, number-1, header.ParentHash, nil)
@@ -568,11 +584,10 @@ func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header
 	if parent == nil {
 		return consensus.ErrUnknownAncestor
 	}
-	header.Time = parent.Time + c.config.Period
-	// If RelaxedPeriod is enabled, always set the header timestamp to now (ie the time we start building it) as
-	// we don't know when it will be sealed
-	if c.config.RelaxedPeriod || header.Time < uint64(time.Now().Unix()) {
-		header.Time = uint64(time.Now().Unix())
+	if timeOverride != nil {
+		header.Time = *timeOverride
+	} else {
+		header.Time = c.CalcTimestamp(parent)
 	}
 	return nil
 }

consensus/consensus.go

@@ -79,7 +79,7 @@ type Engine interface {
 
 	// Prepare initializes the consensus fields of a block header according to the
 	// rules of a particular engine. The changes are executed inline.
-	Prepare(chain ChainHeaderReader, header *types.Header) error
+	Prepare(chain ChainHeaderReader, header *types.Header, timeOverride *uint64) error
 
 	// Finalize runs any post-transaction state modifications (e.g. block rewards)
 	// but does not assemble the block.
@@ -111,6 +111,10 @@ type Engine interface {
 	// that a new block should have.
 	CalcDifficulty(chain ChainHeaderReader, time uint64, parent *types.Header) *big.Int
 
+	// CalcTimestamp predicts the next block's timestamp based on its parent.
+	// Note, this method is only called from UpgradableEngine.
+	CalcTimestamp(parent *types.Header) uint64
+
 	// APIs returns the RPC APIs this consensus engine provides.
 	APIs(chain ChainHeaderReader) []rpc.API
 

consensus/ethash/consensus.go

@@ -579,9 +579,13 @@ func (ethash *Ethash) verifySeal(chain consensus.ChainHeaderReader, header *type
 	return nil
 }
 
+func (ue *Ethash) CalcTimestamp(parent *types.Header) uint64 {
+	panic("Called CalcTimestamp on Ethash, not implemented")
+}
+
 // Prepare implements consensus.Engine, initializing the difficulty field of a
 // header to conform to the ethash protocol. The changes are done inline.
-func (ethash *Ethash) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error {
+func (ethash *Ethash) Prepare(chain consensus.ChainHeaderReader, header *types.Header, timeOverride *uint64) error {
 	parent := chain.GetHeader(header.ParentHash, header.Number.Uint64()-1)
 	if parent == nil {
 		return consensus.ErrUnknownAncestor

consensus/system_contract/consensus.go

@@ -225,9 +225,21 @@ func (s *SystemContract) VerifyUncles(chain consensus.ChainReader, block *types.
 	return nil
 }
 
+func (s *SystemContract) CalcTimestamp(parent *types.Header) uint64 {
+	timestamp := parent.Time + s.config.Period
+
+	// If RelaxedPeriod is enabled, always set the header timestamp to now (ie the time we start building it) as
+	// we don't know when it will be sealed
+	if s.config.RelaxedPeriod || timestamp < uint64(time.Now().Unix()) {
+		timestamp = uint64(time.Now().Unix())
+	}
+
+	return timestamp
+}
+
 // Prepare initializes the consensus fields of a block header according to the
 // rules of a particular engine. Update only timestamp and prepare ExtraData for Signature
-func (s *SystemContract) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error {
+func (s *SystemContract) Prepare(chain consensus.ChainHeaderReader, header *types.Header, timeOverride *uint64) error {
 	// Make sure unused fields are empty
 	header.Coinbase = common.Address{}
 	header.Nonce = types.BlockNonce{}
@@ -243,11 +255,10 @@ func (s *SystemContract) Prepare(chain consensus.ChainHeaderReader, header *type
 	if parent == nil {
 		return consensus.ErrUnknownAncestor
 	}
-	header.Time = parent.Time + s.config.Period
-	// If RelaxedPeriod is enabled, always set the header timestamp to now (ie the time we start building it) as
-	// we don't know when it will be sealed
-	if s.config.RelaxedPeriod || header.Time < uint64(time.Now().Unix()) {
-		header.Time = uint64(time.Now().Unix())
+	if timeOverride != nil {
+		header.Time = *timeOverride
+	} else {
+		header.Time = s.CalcTimestamp(parent)
 	}
 
 	// Difficulty must be 1

consensus/wrapper/consensus.go

@@ -39,8 +39,8 @@ func NewUpgradableEngine(isUpgraded func(uint64) bool, clique consensus.Engine,
 }
 
 // chooseEngine returns the appropriate consensus engine based on the header's timestamp.
-func (ue *UpgradableEngine) chooseEngine(header *types.Header) consensus.Engine {
-	if ue.isUpgraded(header.Time) {
+func (ue *UpgradableEngine) chooseEngine(timestamp uint64) consensus.Engine {
+	if ue.isUpgraded(timestamp) {
 		return ue.system
 	}
 	return ue.clique
@@ -51,12 +51,12 @@ func (ue *UpgradableEngine) chooseEngine(header *types.Header) consensus.Engine
 
 // Author returns the author of the block based on the header.
 func (ue *UpgradableEngine) Author(header *types.Header) (common.Address, error) {
-	return ue.chooseEngine(header).Author(header)
+	return ue.chooseEngine(header.Time).Author(header)
 }
 
 // VerifyHeader checks whether a header conforms to the consensus rules of the engine.
 func (ue *UpgradableEngine) VerifyHeader(chain consensus.ChainHeaderReader, header *types.Header, seal bool) error {
-	return ue.chooseEngine(header).VerifyHeader(chain, header, seal)
+	return ue.chooseEngine(header.Time).VerifyHeader(chain, header, seal)
 }
 
 // VerifyHeaders verifies a batch of headers concurrently. In our use-case,
@@ -72,8 +72,8 @@ func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, hea
 	}
 
 	// Choose engine for the first and last header.
-	firstEngine := ue.chooseEngine(headers[0])
-	lastEngine := ue.chooseEngine(headers[len(headers)-1])
+	firstEngine := ue.chooseEngine(headers[0].Time)
+	lastEngine := ue.chooseEngine(headers[len(headers)-1].Time)
 
 	// If the first header is system, then all headers must be system.
 	if firstEngine == ue.system {
@@ -89,7 +89,7 @@ func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, hea
 	// a single switchover, find the first header that uses system.
 	splitIndex := 0
 	for i, header := range headers {
-		if ue.chooseEngine(header) == ue.system {
+		if ue.chooseEngine(header.Time) == ue.system {
 			splitIndex = i
 			break
 		}
@@ -151,34 +151,77 @@ func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, hea
 	return abort, results
 }
 
+func (ue *UpgradableEngine) CalcTimestamp(parent *types.Header) uint64 {
+	panic("Called CalcTimestamp on UpgradableEngine, not implemented")
+}
+
 // Prepare prepares a block header for sealing.
-func (ue *UpgradableEngine) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error {
-	return ue.chooseEngine(header).Prepare(chain, header)
+func (ue *UpgradableEngine) Prepare(chain consensus.ChainHeaderReader, header *types.Header, timeOverride *uint64) error {
+	// Override provided => select engine based on override timestamp.
+	if timeOverride != nil {
+		return ue.chooseEngine(*timeOverride).Prepare(chain, header, timeOverride)
+	}
+
+	parent := chain.GetHeader(header.ParentHash, header.Number.Uint64()-1)
+	if parent == nil {
+		return consensus.ErrUnknownAncestor
+	}
+
+	// Check if parent is pre- or post-EuclidV2.
+	if ue.chooseEngine(parent.Time) == ue.clique {
+		// This is either a normal Clique block, or the EuclidV2 transition block.
+
+		time := ue.clique.CalcTimestamp(parent)
+
+		if ue.chooseEngine(time) == ue.clique {
+			// We're still in Clique mode.
+			// Note: We must provide timestamp override to avoid the edge case
+			// where we slip over into EuclidV2 in this next call.
+			return ue.clique.Prepare(chain, header, &time)
+		} else {
+			// This is the EuclidV2 transition block.
+			return ue.system.Prepare(chain, header, &time)
+		}
+	} else {
+		// We are already post EuclidV2, in SystemContract mode.
+
+		time := ue.system.CalcTimestamp(parent)
+
+		if ue.chooseEngine(time) == ue.clique {
+			// Somehow we slipped back to Clique, override with a known post-EuclidV2 timestamp.
+			// Note: This should not happen in practice.
+			log.Error("Parent is post-EuclidV2 but SystemContract set pre-EuclidV2 timestamp, overriding", "blockNumber", header.Number, "parentTime", parent.Time, "systemContractTime", time)
+			return ue.system.Prepare(chain, header, &parent.Time)
+		} else {
+			// We are already in SystemContract mode.
+			return ue.system.Prepare(chain, header, &time)
+		}
+	}
 }
 
 // Seal instructs the engine to start sealing a block.
 func (ue *UpgradableEngine) Seal(chain consensus.ChainHeaderReader, block *types.Block, results chan<- *types.Block, stop <-chan struct{}) error {
-	return ue.chooseEngine(block.Header()).Seal(chain, block, results, stop)
+	return ue.chooseEngine(block.Time()).Seal(chain, block, results, stop)
 }
 
 // CalcDifficulty calculates the block difficulty if applicable.
 func (ue *UpgradableEngine) CalcDifficulty(chain consensus.ChainHeaderReader, time uint64, parent *types.Header) *big.Int {
-	return ue.chooseEngine(parent).CalcDifficulty(chain, time, parent)
+	return ue.chooseEngine(parent.Time).CalcDifficulty(chain, time, parent)
 }
 
 // Finalize finalizes the block, applying any post-transaction rules.
 func (ue *UpgradableEngine) Finalize(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, txs []*types.Transaction, uncles []*types.Header) {
-	ue.chooseEngine(header).Finalize(chain, header, state, txs, uncles)
+	ue.chooseEngine(header.Time).Finalize(chain, header, state, txs, uncles)
 }
 
 // FinalizeAndAssemble finalizes and assembles a new block.
 func (ue *UpgradableEngine) FinalizeAndAssemble(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, txs []*types.Transaction, uncles []*types.Header, receipts []*types.Receipt) (*types.Block, error) {
-	return ue.chooseEngine(header).FinalizeAndAssemble(chain, header, state, txs, uncles, receipts)
+	return ue.chooseEngine(header.Time).FinalizeAndAssemble(chain, header, state, txs, uncles, receipts)
 }
 
 // VerifyUncles verifies that no uncles are attached to the block.
 func (ue *UpgradableEngine) VerifyUncles(chain consensus.ChainReader, block *types.Block) error {
-	return ue.chooseEngine(block.Header()).VerifyUncles(chain, block)
+	return ue.chooseEngine(block.Time()).VerifyUncles(chain, block)
 }
 
 // APIs returns any RPC APIs exposed by the consensus engine.
@@ -203,7 +246,7 @@ func (ue *UpgradableEngine) Close() error {
 
 // SealHash returns the hash of a block prior to it being sealed.
 func (ue *UpgradableEngine) SealHash(header *types.Header) common.Hash {
-	return ue.chooseEngine(header).SealHash(header)
+	return ue.chooseEngine(header.Time).SealHash(header)
 }
 
 // Authorize injects a private key into the consensus engine to mint new blocks

core/blockchain.go

@@ -1832,16 +1832,13 @@ func (bc *BlockChain) BuildAndWriteBlock(parentBlock *types.Block, header *types
 	//  This should be done with https://github.com/scroll-tech/go-ethereum/pull/913.
 
 	if sign {
-		// remember the time as Clique will override it
+		// Prevent Engine from overriding timestamp.
 		originalTime := header.Time
 
-		err = bc.engine.Prepare(bc, header)
+		err = bc.engine.Prepare(bc, header, &originalTime)
 		if err != nil {
 			return nil, NonStatTy, fmt.Errorf("error preparing block %d: %w", tempBlock.Number().Uint64(), err)
 		}
-
-		// we want to re-sign the block: set time to original value again.
-		header.Time = originalTime
 	}
 
 	// finalize and assemble block as fullBlock: replicates consensus.FinalizeAndAssemble()

eth/catalyst/api.go

@@ -150,7 +150,7 @@ func (api *consensusAPI) AssembleBlock(params assembleBlockParams) (*executableD
 		parentL1BaseFee := fees.GetL1BaseFee(stateDb)
 		header.BaseFee = misc.CalcBaseFee(config, parent.Header(), parentL1BaseFee)
 	}
-	err = api.eth.Engine().Prepare(bc, header)
+	err = api.eth.Engine().Prepare(bc, header, nil)
 	if err != nil {
 		return nil, err
 	}

miner/scroll_worker.go

@@ -447,29 +447,11 @@ func (w *worker) updateSnapshot() {
 // collectPendingL1Messages reads pending L1 messages from the database.
 // It returns a list of L1 messages that can be included in the block. Depending on the current
 // block time, it reads L1 messages from either L1MessageQueueV1 or L1MessageQueueV2.
-// It also makes sure that all L1 messages V1 are consumed before we activate EuclidV2 fork by backdating the block's time
-// to the parent block's timestamp.
 func (w *worker) collectPendingL1Messages(startIndex uint64) []types.L1MessageTx {
 	maxCount := w.chainConfig.Scroll.L1Config.NumL1MessagesPerBlock
 
 	// If we are on EuclidV2, we need to read L1 messages from L1MessageQueueV2.
 	if w.chainConfig.IsEuclidV2(w.current.header.Time) {
-		parent := w.chain.CurrentHeader()
-
-		// w.current would be the first block in the EuclidV2 fork
-		if !w.chainConfig.IsEuclidV2(parent.Time) {
-			// We need to make sure that all the L1 messages V1 are consumed before we activate EuclidV2 as with EuclidV2
-			// only L1 messages V2 are allowed.
-			l1MessagesV1 := rawdb.ReadL1MessagesV1From(w.eth.ChainDb(), startIndex, maxCount)
-			if len(l1MessagesV1) > 0 {
-				// backdate the block to the parent block's timestamp -> not yet EuclidV2
-				// TODO: might need to re-run Prepare here
-				log.Warn("Back-labeling header timestamp to ensure it precedes the EuclidV2 transition", "blockNumber", w.current.header.Number, "oldTime", w.current.header.Time, "newTime", parent.Time)
-				w.current.header.Time = parent.Time
-				return l1MessagesV1
-			}
-		}
-
 		return rawdb.ReadL1MessagesV2From(w.eth.ChainDb(), startIndex, maxCount)
 	}
 
@@ -512,6 +494,11 @@ func (w *worker) newWork(now time.Time, parentHash common.Hash, reorging bool, r
 		header.Coinbase = w.coinbase
 	}
 
+	var nextL1MsgIndex uint64
+	if dbVal := rawdb.ReadFirstQueueIndexNotInL2Block(w.eth.ChainDb(), header.ParentHash); dbVal != nil {
+		nextL1MsgIndex = *dbVal
+	}
+
 	if w.config.SigningDisabled {
 		// Need to make sure to set difficulty so that a new canonical chain is detected in Blockchain
 		header.Difficulty = new(big.Int).SetUint64(1)
@@ -520,15 +507,40 @@ func (w *worker) newWork(now time.Time, parentHash common.Hash, reorging bool, r
 		header.Nonce = types.BlockNonce{}
 	} else {
 		prepareStart := time.Now()
-		if err := w.engine.Prepare(w.chain, header); err != nil {
+		// Note: this call will set header.Time, among other fields.
+		if err := w.engine.Prepare(w.chain, header, nil); err != nil {
 			return fmt.Errorf("failed to prepare header for mining: %w", err)
 		}
 		prepareTimer.UpdateSince(prepareStart)
-	}
 
-	var nextL1MsgIndex uint64
-	if dbVal := rawdb.ReadFirstQueueIndexNotInL2Block(w.eth.ChainDb(), header.ParentHash); dbVal != nil {
-		nextL1MsgIndex = *dbVal
+		if w.chainConfig.IsEuclidV2(header.Time) && !w.chainConfig.IsEuclidV2(parent.Time()) {
+			// We found a potential EuclidV2 transition block.
+			// We need to make sure that all the L1 messages V1 are consumed before we activate EuclidV2,
+			// since we can only include MessageQueueV2 messages after EuclidV2.
+			l1MessagesV1 := rawdb.ReadL1MessagesV1From(w.eth.ChainDb(), nextL1MsgIndex, 1)
+			if len(l1MessagesV1) > 0 {
+				// Reset Extra (it was unset by SystemContract)
+				header.Extra = w.extra
+
+				// Backdate the block to the parent block's timestamp -> not yet EuclidV2
+				parentTime := parent.Time()
+				log.Warn("Backdating header timestamp to ensure it precedes the EuclidV2 transition", "blockNumber", header.Number, "oldTime", header.Time, "newTime", parentTime)
+
+				// Run Prepare again, this time we provide a timestamp override, so it will use Clique.
+				// Note: Clique should correctly unset or overwrite any fields previously set by SystemConfig,
+				// with the exception of Extra that was reset above.
+				prepareStart := time.Now()
+				if err := w.engine.Prepare(w.chain, header, &parentTime); err != nil {
+					return fmt.Errorf("failed to prepare header for mining: %w", err)
+				}
+				prepareTimer.UpdateSince(prepareStart)
+			} else {
+				// Only print log if we are the sequencer -- otherwise we will print confusing logs for the pending block.
+				if w.isRunning() {
+					log.Info("All MessageQueueV1 messages processed, creating EuclidV2 transition block", "blockNumber", header.Number, "blockTime", header.Time, "firstV2MsgIndex", nextL1MsgIndex)
+				}
+			}
+		}
 	}
 
 	vmConfig := *w.chain.GetVMConfig()

params/version.go

@@ -24,7 +24,7 @@ import (
 const (
 	VersionMajor = 5         // Major version component of the current release
 	VersionMinor = 8         // Minor version component of the current release
-	VersionPatch = 19        // Patch version component of the current release
+	VersionPatch = 20        // Patch version component of the current release
 	VersionMeta  = "mainnet" // Version metadata to append to the version string
 )