eth/downloader: fix case where skeleton reorgs below the filled block

eth/downloader/downloader_test.go

@@ -1311,3 +1311,84 @@ func testBeaconSync(t *testing.T, protocol uint, mode SyncMode) {
 		})
 	}
 }
+
+// Tests that synchronisation progress (origin block number and highest block
+// number) is tracked and updated correctly in case of manual head reversion
+func TestBeaconForkedSyncProgress68Full(t *testing.T) {
+	testBeaconForkedSyncProgress(t, eth.ETH68, FullSync)
+}
+func TestBeaconForkedSyncProgress68Snap(t *testing.T) {
+	testBeaconForkedSyncProgress(t, eth.ETH68, SnapSync)
+}
+func TestBeaconForkedSyncProgress68Light(t *testing.T) {
+	testBeaconForkedSyncProgress(t, eth.ETH68, LightSync)
+}
+
+func testBeaconForkedSyncProgress(t *testing.T, protocol uint, mode SyncMode) {
+	success := make(chan struct{})
+	tester := newTesterWithNotification(t, func() {
+		success <- struct{}{}
+	})
+	defer tester.terminate()
+
+	chainA := testChainForkLightA.shorten(len(testChainBase.blocks) + MaxHeaderFetch)
+	chainB := testChainForkLightB.shorten(len(testChainBase.blocks) + MaxHeaderFetch)
+
+	// Set a sync init hook to catch progress changes
+	starting := make(chan struct{})
+	progress := make(chan struct{})
+
+	tester.downloader.syncInitHook = func(origin, latest uint64) {
+		starting <- struct{}{}
+		<-progress
+	}
+	checkProgress(t, tester.downloader, "pristine", ethereum.SyncProgress{})
+
+	// Synchronise with one of the forks and check progress
+	tester.newPeer("fork A", protocol, chainA.blocks[1:])
+	pending := new(sync.WaitGroup)
+	pending.Add(1)
+	go func() {
+		defer pending.Done()
+		if err := tester.downloader.BeaconSync(mode, chainA.blocks[len(chainA.blocks)-1].Header(), nil); err != nil {
+			panic(fmt.Sprintf("failed to beacon sync: %v", err))
+		}
+	}()
+
+	<-starting
+	progress <- struct{}{}
+	select {
+	case <-success:
+		checkProgress(t, tester.downloader, "initial", ethereum.SyncProgress{
+			HighestBlock: uint64(len(chainA.blocks) - 1),
+			CurrentBlock: uint64(len(chainA.blocks) - 1),
+		})
+	case <-time.NewTimer(time.Second * 3).C:
+		t.Fatalf("Failed to sync chain in three seconds")
+	}
+
+	// Set the head to a second fork
+	tester.newPeer("fork B", protocol, chainB.blocks[1:])
+	pending.Add(1)
+	go func() {
+		defer pending.Done()
+		if err := tester.downloader.BeaconSync(mode, chainB.blocks[len(chainB.blocks)-1].Header(), nil); err != nil {
+			panic(fmt.Sprintf("failed to beacon sync: %v", err))
+		}
+	}()
+
+	<-starting
+	progress <- struct{}{}
+
+	// reorg below available state causes the state sync to rewind to genesis
+	select {
+	case <-success:
+		checkProgress(t, tester.downloader, "initial", ethereum.SyncProgress{
+			HighestBlock:  uint64(len(chainB.blocks) - 1),
+			CurrentBlock:  uint64(len(chainB.blocks) - 1),
+			StartingBlock: 0,
+		})
+	case <-time.NewTimer(time.Second * 3).C:
+		t.Fatalf("Failed to sync chain in three seconds")
+	}
+}

eth/downloader/skeleton.go

@@ -1132,6 +1132,16 @@ func (s *skeleton) cleanStales(filled *types.Header) error {
 	if number+1 == s.progress.Subchains[0].Tail {
 		return nil
 	}
+	// If the latest fill was on a different subchain, it means the backfiller
+	// was interrupted before it got to do any meaningful work, no cleanup
+	header := rawdb.ReadSkeletonHeader(s.db, filled.Number.Uint64())
+	if header == nil {
+		log.Debug("Filled header outside of skeleton range", "number", number, "head", s.progress.Subchains[0].Head, "tail", s.progress.Subchains[0].Tail)
+		return nil
+	} else if header.Hash() != filled.Hash() {
+		log.Debug("Filled header on different sidechain", "number", number, "filled", filled.Hash(), "skeleton", header.Hash())
+		return nil
+	}
 	var (
 		start uint64
 		end   uint64