eth/downloader: disable snap mode after committing pivot block (#35402)

This PR improves the sync mode management, disabling the snap mode once 
the pivot block is committed. 

Originally the mode will only be flipped from snap to full once the entire sync 
cycle is completed. However, it's theoretically possible to cancel the sync cycle
after committing the pivot but before inserting the remaining blocks. It drags
the following sync cycle back to the snap mode.

The snap mode should only be used if the head state is missing and unrecoverable. 
Enabling the snap mode for multiple times should be strictly prevented.

What's more, it brings a fix to pivot block management, that once the
pivot block is committed, the pivot block marker should be no longer advanced,
preventing the situation that the real pivot state is below the pivot marker.

eth/downloader/beaconsync.go

@@ -306,9 +306,13 @@ func (d *Downloader) fetchHeaders(from uint64) error {
 		// If the pivot became stale (older than 2*64-8 (bit of wiggle room)),
 		// move it ahead to HEAD-64.
 		//
+		// Note the pivot is only moved before its commitment. Once the pivot
+		// is committed, the chain obtains a stateful head and pivot block should
+		// be on longer advanced.
+		//
 		// The state syncer is consulted first before the pivot movement.
 		d.pivotLock.Lock()
-		if d.pivotHeader != nil && d.snapSyncer.FrozenPivot() == nil {
+		if d.pivotHeader != nil && !d.committed.Load() && d.snapSyncer.FrozenPivot() == nil {
 			if head.Number.Uint64() > d.pivotHeader.Number.Uint64()+2*uint64(fsMinFullBlocks)-8 {
 				// Retrieve the next pivot header, either from skeleton chain
 				// or the filled chain

eth/downloader/downloader.go

@@ -387,10 +387,14 @@ func (d *Downloader) synchronise(beaconPing chan struct{}) (err error) {
 	// Obtain the synchronized used in this cycle
 	mode := d.moder.get(true)
 	defer func() {
+		// The snap-sync mode is usually already disabled right after the pivot
+		// commitment; this is the fallback for the cycles terminating without
+		// a pivot block (e.g. a short chain fully imported from genesis).
 		if err == nil && mode == ethconfig.SnapSync {
-			d.moder.disableSnap()
+			if d.moder.disableSnap() {
 				log.Info("Disabled snap-sync after the initial sync cycle")
 			}
+		}
 	}()
 
 	// Disable chain mutations when snap sync is selected, ensuring the
@@ -1106,6 +1110,13 @@ func (d *Downloader) commitPivotBlock(result *fetchResult) error {
 		return err
 	}
 	d.committed.Store(true)
+
+	// The chain has obtained a stateful head by committing the pivot block,
+	// the mission of the snap sync is regarded as accomplished and the mode
+	// is flipped to full-sync.
+	if d.moder.disableSnap() {
+		log.Info("Disabled snap-sync after pivot commitment", "number", block.Number(), "hash", block.Hash())
+	}
 	return nil
 }
 

eth/downloader/syncmode.go

@@ -107,9 +107,15 @@ func (m *syncModer) get(report bool) ethconfig.SyncMode {
 	return ethconfig.FullSync
 }
 
-// disableSnap disables the snap sync mode, usually it's called after a successful snap sync.
-func (m *syncModer) disableSnap() {
+// disableSnap disables the snap sync mode and reports whether the mode was
+// actually flipped.
+func (m *syncModer) disableSnap() bool {
 	m.lock.Lock()
+	defer m.lock.Unlock()
+
+	if m.mode == ethconfig.FullSync {
+		return false
+	}
 	m.mode = ethconfig.FullSync
-	m.lock.Unlock()
+	return true
 }