eth/protocols/snap: unfreeze the committed pivot instead of dropping the journal on load

eth/protocols/snap/syncv2.go

@@ -448,19 +448,10 @@ func newSyncerV2(db ethdb.Database, scheme string) *syncerV2 {
 	if raw := rawdb.ReadSnapshotSyncStatus(db); len(raw) > 0 && raw[0] == syncProgressVersion {
 		var progress syncProgressV2
 		if err := json.Unmarshal(raw[1:], &progress); err == nil {
-			// A completed journal whose pivot block was already committed is
-			// a dead leftover. Drop it before FrozenPivot hands the stale
-			// pivot back to the downloader. The flat state stays, on a
-			// full-sync node it is the live snapshot.
-			if progress.Phase == phaseComplete && progress.Pivot != nil && isPivotCommitted(db, progress.Pivot) {
-				log.Info("Dropping journal of a completed snap sync", "pivot", progress.Pivot.Number)
-				rawdb.DeleteSnapshotSyncStatus(db)
-			} else {
 			s.pivot = progress.Pivot
 			s.setPhase(progress.Phase)
 		}
 	}
-	}
 	return s
 }
 
@@ -564,11 +555,10 @@ func (s *syncerV2) Sync(target *types.Header, cancel chan struct{}) error {
 	// If the pivot of a completed sync was already committed, full sync has
 	// been mutating the flat state since. Nothing here can be resumed or
 	// rolled forward, snap sync was re-enabled, so wipe and start fresh.
-	// newSyncerV2 already drops such a journal on load, so this branch only
+	// FrozenPivot refuses to freeze such a pivot, so the downloader resumes
-	// fires for an in-process re-enable, where the syncer was built before
+	// against a fresh target and lands here to do the cleanup. Same-pivot
-	// the sync completed so the load-time check never saw a committed journal.
+	// retries return through the skip above on purpose, a cycle that dies
-	// Same-pivot retries return through the skip above on purpose, a cycle
+	// right after the commit just needs to recommit.
-	// that dies right after the commit just needs to recommit.
 	if prevPivot != nil && s.getPhase() == phaseComplete && isPivotCommitted(s.db, prevPivot) {
 		log.Warn("Reenabled snap sync over a completed one, restarting from scratch", "oldpivot", prevPivot.Number, "target", target.Number)
 		s.resetSyncState()
@@ -701,6 +691,12 @@ func (s *syncerV2) FrozenPivot() *types.Header {
 	}
 	s.lock.RLock()
 	defer s.lock.RUnlock()
+	// A committed pivot from a completed sync is a dead leftover, not a
+	// freeze. Handing it back would pin a re-enabled sync to the old pivot,
+	// so report it as unfrozen and let the next Sync wipe the journal.
+	if s.getPhase() == phaseComplete && s.pivot != nil && isPivotCommitted(s.db, s.pivot) {
+		return nil
+	}
 	return s.pivot
 }
 

eth/protocols/snap/syncv2_test.go

@@ -2432,40 +2432,42 @@ func testCommittedPivotRetrySkips(t *testing.T, scheme string) {
 	}
 }
 
-// TestReenableDropsCompletedJournal covers the constructor. A restart with
+// TestCommittedPivotNotFrozen covers FrozenPivot. A pivot from a crash
-// a committed completed journal must drop it before FrozenPivot hands the
+// before the commit must stay frozen so the downloader resumes against it.
-// stale pivot to the downloader. A journal from a crash before the commit
+// Once the pivot block is committed and the head has moved on, FrozenPivot
-// stays.
+// must report it as unfrozen so a re-enabled sync is not pinned to it. The
-func TestReenableDropsCompletedJournal(t *testing.T) {
+// journal stays on disk either way, the next Sync cleans it.
+func TestCommittedPivotNotFrozen(t *testing.T) {
 	t.Parallel()
-	testReenableDropsCompletedJournal(t, rawdb.HashScheme)
+	testCommittedPivotNotFrozen(t, rawdb.HashScheme)
-	testReenableDropsCompletedJournal(t, rawdb.PathScheme)
+	testCommittedPivotNotFrozen(t, rawdb.PathScheme)
 }
 
-func testReenableDropsCompletedJournal(t *testing.T, scheme string) {
+func testCommittedPivotNotFrozen(t *testing.T, scheme string) {
 	fix := seedCompletedSync(t, scheme)
 
-	// Crash before the commit, the journal survives and the pivot stays
+	// Crash before the commit, the head never reached the pivot, so it
-	// frozen.
+	// stays frozen for the resume.
 	head50 := fix.mkHeader(50, common.HexToHash("0x50"), nil)
 	rawdb.WriteHeadBlockHash(fix.db, head50.Hash())
 	syncer := newSyncerV2(fix.db, fix.nodeScheme)
 	if frozen := syncer.FrozenPivot(); frozen == nil || frozen.Hash() != fix.pivotA.Hash() {
-		t.Fatal("expected journal to survive a restart before the pivot commit")
+		t.Fatal("expected the pivot to stay frozen before the commit")
 	}
 
-	// Commit the pivot and advance the head, a restart now drops the
+	// Commit the pivot and advance the head, the pivot is now a dead
-	// journal.
+	// leftover and must not be reported as frozen.
 	header103 := fix.mkHeader(103, common.HexToHash("0x67"), nil)
 	rawdb.WriteHeadBlockHash(fix.db, header103.Hash())
 	syncer = newSyncerV2(fix.db, fix.nodeScheme)
 	if frozen := syncer.FrozenPivot(); frozen != nil {
-		t.Fatalf("expected journal drop to release the pivot freeze, got %v", frozen.Number)
+		t.Fatalf("expected the committed pivot to be unfrozen, got %v", frozen.Number)
 	}
-	if raw := rawdb.ReadSnapshotSyncStatus(fix.db); len(raw) != 0 {
+	// The journal stays on disk until a re-enabled Sync wipes it, and the
-		t.Fatal("expected completed journal to be dropped from disk")
+	// flat state is untouched, on a full-sync node it is the live snapshot.
+	if raw := rawdb.ReadSnapshotSyncStatus(fix.db); len(raw) == 0 {
+		t.Fatal("expected the journal to stay on disk")
 	}
-	// The flat state stays, on a full-sync node it is the live snapshot.
 	assertAccountBalance(t, fix.db, fix.hashY, 50)
 }