From 004c5afdfaba3d7122e3cba78e7f8ad17ac598c4 Mon Sep 17 00:00:00 2001
From: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com>
Date: Mon, 30 Jan 2023 15:42:23 +0100
Subject: [PATCH] Avoid unnecessary unwind+rewind (#6712)

On
[withdrawal-mainnet-shadowfork-1](https://withdrawal-mainnet-shadowfork-1.ethpandaops.io/)
erigon was unnecessarily re-executing blocks after 16m (snapshot)
multiple times. That was likely due to CL issuing `forkchoiceUpdated`
pointing to an old block for some reason. This PR introduces a
protection against such inefficiency.
---
 eth/stagedsync/stage.go         |  7 +++++
 eth/stagedsync/stage_headers.go | 52 +++++++++++++++++++++------------
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/eth/stagedsync/stage.go b/eth/stagedsync/stage.go
index 839b20911..5169c6273 100644
--- a/eth/stagedsync/stage.go
+++ b/eth/stagedsync/stage.go
@@ -64,6 +64,13 @@ func (s *StageState) ExecutionAt(db kv.Getter) (uint64, error) {
 	return execution, err
 }
 
+// IntermediateHashesAt gets the current state of the "IntermediateHashes" stage.
+// A block is fully validated after the IntermediateHashes stage is passed successfully.
+func (s *StageState) IntermediateHashesAt(db kv.Getter) (uint64, error) {
+	progress, err := stages.GetStageProgress(db, stages.IntermediateHashes)
+	return progress, err
+}
+
 // Unwinder allows the stage to cause an unwind.
 type Unwinder interface {
 	// UnwindTo begins staged sync unwind to the specified block.
diff --git a/eth/stagedsync/stage_headers.go b/eth/stagedsync/stage_headers.go
index 992b5de13..d78f02c5a 100644
--- a/eth/stagedsync/stage_headers.go
+++ b/eth/stagedsync/stage_headers.go
@@ -290,24 +290,41 @@ func startHandlingForkChoice(
 	headerHash := forkChoice.HeadBlockHash
 	log.Debug(fmt.Sprintf("[%s] Handling fork choice", s.LogPrefix()), "headerHash", headerHash)
 
-	currentHeadHash := rawdb.ReadHeadHeaderHash(tx)
-	if currentHeadHash == headerHash { // no-op
-		log.Debug(fmt.Sprintf("[%s] Fork choice no-op", s.LogPrefix()))
+	canonical, err := rawdb.IsCanonicalHash(tx, headerHash)
+	if err != nil {
+		log.Warn(fmt.Sprintf("[%s] Fork choice err (IsCanonicalHash)", s.LogPrefix()), "err", err)
 		cfg.hd.BeaconRequestList.Remove(requestId)
-		canonical, err := writeForkChoiceHashes(forkChoice, s, tx, cfg)
+		return nil, err
+	}
+	if canonical {
+		headerNumber := rawdb.ReadHeaderNumber(tx, headerHash)
+		ihProgress, err := s.IntermediateHashesAt(tx)
 		if err != nil {
-			log.Warn(fmt.Sprintf("[%s] Fork choice err", s.LogPrefix()), "err", err)
+			log.Warn(fmt.Sprintf("[%s] Fork choice err (IntermediateHashesAt)", s.LogPrefix()), "err", err)
+			cfg.hd.BeaconRequestList.Remove(requestId)
 			return nil, err
 		}
-		if canonical {
-			return &engineapi.PayloadStatus{
-				Status:          remote.EngineStatus_VALID,
-				LatestValidHash: currentHeadHash,
-			}, nil
-		} else {
-			return &engineapi.PayloadStatus{
-				CriticalError: &privateapi.InvalidForkchoiceStateErr,
-			}, nil
+		if ihProgress >= *headerNumber {
+			// FCU points to a canonical and fully validated block in the past.
+			// Treat it as a no-op to avoid unnecessary unwind of block execution and other stages
+			// with subsequent rewind on a newer FCU.
+			log.Debug(fmt.Sprintf("[%s] Fork choice no-op", s.LogPrefix()))
+			cfg.hd.BeaconRequestList.Remove(requestId)
+			canonical, err = writeForkChoiceHashes(forkChoice, s, tx, cfg)
+			if err != nil {
+				log.Warn(fmt.Sprintf("[%s] Fork choice err", s.LogPrefix()), "err", err)
+				return nil, err
+			}
+			if canonical {
+				return &engineapi.PayloadStatus{
+					Status:          remote.EngineStatus_VALID,
+					LatestValidHash: headerHash,
+				}, nil
+			} else {
+				return &engineapi.PayloadStatus{
+					CriticalError: &privateapi.InvalidForkchoiceStateErr,
+				}, nil
+			}
 		}
 	}
 
@@ -338,7 +355,6 @@ func startHandlingForkChoice(
 		if err := cfg.forkValidator.FlushExtendingFork(tx, cfg.notifications.Accumulator); err != nil {
 			return nil, err
 		}
-		cfg.hd.BeaconRequestList.Remove(requestId)
 		canonical, err := writeForkChoiceHashes(forkChoice, s, tx, cfg)
 		if err != nil {
 			log.Warn(fmt.Sprintf("[%s] Fork choice err", s.LogPrefix()), "err", err)
@@ -382,7 +398,7 @@ func startHandlingForkChoice(
 		if err = fixCanonicalChain(s.LogPrefix(), logEvery, headerNumber, headerHash, tx, cfg.blockReader); err != nil {
 			return nil, err
 		}
-		if err = rawdb.WriteHeadHeaderHash(tx, forkChoice.HeadBlockHash); err != nil {
+		if err = rawdb.WriteHeadHeaderHash(tx, headerHash); err != nil {
 			return nil, err
 		}
 
@@ -394,13 +410,11 @@ func startHandlingForkChoice(
 		if err := s.Update(tx, headerNumber); err != nil {
 			return nil, err
 		}
-		// Referesh currentHeadHash
-		currentHeadHash = rawdb.ReadHeadHeaderHash(tx)
 
 		if canonical {
 			return &engineapi.PayloadStatus{
 				Status:          remote.EngineStatus_VALID,
-				LatestValidHash: currentHeadHash,
+				LatestValidHash: headerHash,
 			}, nil
 		} else {
 			return &engineapi.PayloadStatus{