From 5b6571fe7413c8cb836daf3379bdc9a76a5d6ebf Mon Sep 17 00:00:00 2001
From: Gary Rong <garyrong0905@gmail.com>
Date: Tue, 12 Dec 2023 20:27:14 +0800
Subject: [PATCH] eth/downloader: polish the logic in cleanup

---
 eth/downloader/skeleton.go | 64 +++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 35 deletions(-)

diff --git a/eth/downloader/skeleton.go b/eth/downloader/skeleton.go
index 9674e00054..944bda1f4e 100644
--- a/eth/downloader/skeleton.go
+++ b/eth/downloader/skeleton.go
@@ -1120,52 +1120,46 @@ func (s *skeleton) cleanStales(filled *types.Header) error {
 	number := filled.Number.Uint64()
 	log.Trace("Cleaning stale beacon headers", "filled", number, "hash", filled.Hash())
 
-	// If the filled header is below and discontinuous with the linked subchain,
-	// something's corrupted internally. Report and error and refuse to do anything.
+	// If the filled header is below the linked subchain, something's corrupted
+	// internally. Report and error and refuse to do anything.
 	if number+1 < s.progress.Subchains[0].Tail {
 		return fmt.Errorf("filled header below beacon header tail: %d < %d", number, s.progress.Subchains[0].Tail)
 	}
-	// If nothing is filled, don't bother to do cleanup.
+	// If nothing in subchain is filled, don't bother to do cleanup.
 	if number+1 == s.progress.Subchains[0].Tail {
 		return nil
 	}
-	// Determine the new tail based on the given filled header. If it's still
-	// in the range of skeleton chain, use the next header adjacent to the
-	// filled one (it's expected to link with the filled one, otherwise
-	// something corrupted); otherwise use the filled one if the whole skeleton
-	// chain is consumed.
-	newTail := filled
-	if number < s.progress.Subchains[0].Head {
-		newTail = rawdb.ReadSkeletonHeader(s.db, number+1)
-		if newTail.ParentHash != filled.Hash() {
-			return fmt.Errorf("filled header is discontinuous with subchain: %d %s", number, filled.Hash())
-		}
-	}
-	// Subchain seems trimmable, push the tail forward up to the last
-	// filled header and delete everything before it - if available. In
-	// case we filled past the head, recreate the subchain with a new
-	// head to keep it consistent with the data on disk.
 	var (
-		start = s.progress.Subchains[0].Tail // start deleting from the first known header
-		end   = newTail.Number.Uint64()      // delete skeleton headers before the new tail
+		start uint64
+		end   uint64
 		batch = s.db.NewBatch()
 	)
-	s.progress.Subchains[0].Tail = newTail.Number.Uint64()
-	s.progress.Subchains[0].Next = newTail.ParentHash
+	if number < s.progress.Subchains[0].Head {
+		// The skeleton chain is partially consumed, set the new tail as filled+1.
+		tail := rawdb.ReadSkeletonHeader(s.db, number+1)
+		if tail.ParentHash != filled.Hash() {
+			return fmt.Errorf("filled header is discontinuous with subchain: %d %s", number, filled.Hash())
+		}
+		start, end = s.progress.Subchains[0].Tail, number+1 // remove headers in [tail, filled]
+		s.progress.Subchains[0].Tail = tail.Number.Uint64()
+		s.progress.Subchains[0].Next = tail.ParentHash
+	} else {
+		// The skeleton chain is fully consumed, set both head and tail as filled.
+		start, end = s.progress.Subchains[0].Tail, filled.Number.Uint64() // remove headers in [tail, filled)
+		s.progress.Subchains[0].Tail = filled.Number.Uint64()
+		s.progress.Subchains[0].Next = filled.ParentHash
 
-	// If more headers were filled than available, push the entire subchain
-	// forward to keep tracking the node's block imports.
-	//
-	// Note that the new tail will be the filled one in this case, which is
-	// unexpected, but we cannot do anything else to improve the situation.
-	if s.progress.Subchains[0].Head < number {
-		end = s.progress.Subchains[0].Head + 1 // delete the entire original range, including the head
-		s.progress.Subchains[0].Head = number  // assign a new head (tail is already assigned to this)
+		// If more headers were filled than available, push the entire subchain
+		// forward to keep tracking the node's block imports.
+		if number > s.progress.Subchains[0].Head {
+			end = s.progress.Subchains[0].Head + 1 // delete the entire original range, including the head
+			s.progress.Subchains[0].Head = number  // assign a new head (tail is already assigned to this)
 
-		// The entire original skeleton chain was deleted and a new one
-		// defined. Make sure the new single-header chain gets pushed to
-		// disk to keep internal state consistent.
-		rawdb.WriteSkeletonHeader(batch, filled)
+			// The entire original skeleton chain was deleted and a new one
+			// defined. Make sure the new single-header chain gets pushed to
+			// disk to keep internal state consistent.
+			rawdb.WriteSkeletonHeader(batch, filled)
+		}
 	}
 	// Execute the trimming and the potential rewiring of the progress
 	s.saveSyncStatus(batch)