eth/downloader, eth/protocols/snap: snap v2 catchup failure (#35321)

Fixes https://github.com/ethereum/go-ethereum/issues/35319

eth/protocols/snap/syncv2.go

@@ -798,11 +798,22 @@ func catchUpExceedsRetention(prev, curr *types.Header) bool {
 // diffs to roll flat state forward.
 func (s *syncerV2) catchUp(target *types.Header, cancel chan struct{}) error {
 	s.lock.RLock()
-	from := s.pivot.Number.Uint64() + 1
+	prev := s.pivot
-	to := target.Number.Uint64()
 	s.lock.RUnlock()
+
+	var (
+		from = prev.Number.Uint64() + 1
+		to   = target.Number.Uint64()
+	)
 	log.Info("Starting BAL catch-up", "from", from, "to", to, "blocks", to-from+1)
 
+	// Resolve the hash chain of the whole gap upfront by walking the parent
+	// hashes backward from the trusted target header.
+	hashes, err := s.gapHashChain(prev, target)
+	if err != nil {
+		return err
+	}
+
 	s.lock.Lock()
 	s.accessListTotal = to - from + 1
 	s.accessListSynced = 0
@@ -819,26 +830,26 @@ func (s *syncerV2) catchUp(target *types.Header, cancel chan struct{}) error {
 		if end > to {
 			end = to
 		}
-		// Collect block hashes and headers for this window.
+		// Collect the headers for this window.
 		var (
-			hashes  = make([]common.Hash, 0, end-start+1)
+			winHashes = hashes[start-from : end-from+1]
-			headers = make(map[common.Hash]*types.Header, end-start+1)
+			headers   = make(map[common.Hash]*types.Header, len(winHashes))
 		)
-		for num := start; num <= end; num++ {
+		for i, hash := range winHashes {
-			hash := rawdb.ReadCanonicalHash(s.db, num)
+			num := start + uint64(i)
-			if hash == (common.Hash{}) {
+			if num == to {
-				return fmt.Errorf("missing canonical hash for block %d during catch-up", num)
+				headers[hash] = target
+				continue
 			}
-			header := rawdb.ReadHeader(s.db, hash, num)
+			header := s.gapHeader(num, hash)
 			if header == nil {
 				return fmt.Errorf("missing header for block %d (hash %v) during catch-up", num, hash)
 			}
-			hashes = append(hashes, hash)
 			headers[hash] = header
 		}
 
 		// Fetch this window's BALs from peers.
-		rawBALs, err := s.fetchAccessLists(hashes, headers, cancel)
+		rawBALs, err := s.fetchAccessLists(winHashes, headers, cancel)
 		if err != nil {
 			return err
 		}
@@ -851,7 +862,7 @@ func (s *syncerV2) catchUp(target *types.Header, cancel chan struct{}) error {
 			default:
 			}
 			num := start + uint64(i)
-			hash := hashes[i]
+			hash := winHashes[i]
 
 			// Decode the raw RLP into a BAL.
 			var (
@@ -883,6 +894,44 @@ func (s *syncerV2) catchUp(target *types.Header, cancel chan struct{}) error {
 	return nil
 }
 
+// gapHashChain resolves the hashes of the catch-up gap (prev, target] by
+// walking the parent hashes backward from the trusted target header.
+// Out-of-memory is not expected as only the 32 bytes hashes are kept.
+func (s *syncerV2) gapHashChain(prev, target *types.Header) ([]common.Hash, error) {
+	var (
+		from   = prev.Number.Uint64() + 1
+		to     = target.Number.Uint64()
+		hashes = make([]common.Hash, to-from+1)
+		want   = target.ParentHash
+	)
+	hashes[to-from] = target.Hash()
+	for num := to - 1; num >= from; num-- {
+		header := s.gapHeader(num, want)
+		if header == nil {
+			return nil, fmt.Errorf("missing header for block %d (hash %v) during catch-up", num, want)
+		}
+		hashes[num-from] = want
+		want = header.ParentHash
+	}
+	if want != prev.Hash() {
+		return nil, fmt.Errorf("catch-up target %d (hash %v) does not descend from pivot %d (hash %v)", to, target.Hash(), prev.Number, prev.Hash())
+	}
+	return hashes, nil
+}
+
+// gapHeader reads the header of a gap block with the given number and
+// expected hash. Two data sources are checked: skeleton or the canonical
+// header, as the headers will be kept in skeleton until the full block
+// is fully downloaded and inserted.
+func (s *syncerV2) gapHeader(num uint64, hash common.Hash) *types.Header {
+	if header := rawdb.ReadSkeletonHeader(s.db, num); header != nil && header.Hash() == hash {
+		return header
+	}
+	// If the header is outside the skeleton scope, read it from
+	// canonical source.
+	return rawdb.ReadHeader(s.db, hash, num)
+}
+
 // fetchAccessLists fetches BALs for the given block hashes from
 // remote peers. It runs its own event loop to assign requests
 // to idle peers and process responses asynchronously. Each BAL is verified

eth/protocols/snap/syncv2_test.go

@@ -1797,8 +1797,11 @@ func testCatchUpPersistsIncrementally(t *testing.T, scheme string) {
 
 	// Build three sequential BAL blocks (A+1, A+2, A+3). The first two touch
 	// goodAddr, the third touches corruptAddr so that block's apply fails
-	// once we've corrupted that account's snapshot.
+	// once we've corrupted that account's snapshot. The headers link up via
+	// their parent hashes, as the catch-up walks the chain backward from the
+	// target down to the previous pivot.
 	blocks := make([]balBlock, 3)
+	parent := pivotAHeader.Hash()
 	for i := 0; i < 3; i++ {
 		blockNum := numA + uint64(i) + 1
 		target := goodAddr
@@ -1819,7 +1822,8 @@ func testCatchUpPersistsIncrementally(t *testing.T, scheme string) {
 		}
 		balHash := b.Hash()
 		header := &types.Header{
-			Number: new(big.Int).SetUint64(blockNum), Difficulty: common.Big0,
+			ParentHash: parent,
+			Number:     new(big.Int).SetUint64(blockNum), Difficulty: common.Big0,
 			BaseFee: common.Big0, WithdrawalsHash: &emptyHash,
 			BlobGasUsed: &zero, ExcessBlobGas: &zero,
 			ParentBeaconRoot: &emptyHash, RequestsHash: &emptyHash,
@@ -1828,6 +1832,7 @@ func testCatchUpPersistsIncrementally(t *testing.T, scheme string) {
 		rawdb.WriteHeader(db, header)
 		rawdb.WriteCanonicalHash(db, header.Hash(), blockNum)
 		blocks[i] = balBlock{header: header, bal: buf.Bytes()}
+		parent = header.Hash()
 	}
 
 	// First sync: complete sync to A so persisted state has pivot=A,
@@ -1929,12 +1934,15 @@ func testCatchUpWindowed(t *testing.T, scheme string) {
 	rawdb.WriteCanonicalHash(db, pivotA.Hash(), numA)
 
 	// Build a 5-block gap, each block bumping targetAddr's balance. The last
-	// block's balance is the expected final state.
+	// block's balance is the expected final state. The headers link up via
+	// their parent hashes, as the catch-up walks the chain backward from the
+	// target down to the previous pivot.
 	const gap = 5
 	var (
 		lastHeader  *types.Header
 		lastBalance *uint256.Int
 		balsByHash  = make(map[common.Hash]rlp.RawValue, gap)
+		parent      = pivotA.Hash()
 	)
 	for i := 0; i < gap; i++ {
 		blockNum := numA + uint64(i) + 1
@@ -1952,7 +1960,8 @@ func testCatchUpWindowed(t *testing.T, scheme string) {
 		}
 		balHash := b.Hash()
 		header := &types.Header{
-			Number: new(big.Int).SetUint64(blockNum), Difficulty: common.Big0,
+			ParentHash: parent,
+			Number:     new(big.Int).SetUint64(blockNum), Difficulty: common.Big0,
 			BaseFee: common.Big0, WithdrawalsHash: &emptyHash,
 			BlobGasUsed: &zero, ExcessBlobGas: &zero,
 			ParentBeaconRoot: &emptyHash, RequestsHash: &emptyHash,
@@ -1962,6 +1971,7 @@ func testCatchUpWindowed(t *testing.T, scheme string) {
 		rawdb.WriteCanonicalHash(db, header.Hash(), blockNum)
 		balsByHash[header.Hash()] = buf.Bytes()
 		lastHeader, lastBalance = header, balance
+		parent = header.Hash()
 	}
 
 	// Seed sync to A: persisted state ends with pivot=A and full flat state.
@@ -3008,7 +3018,8 @@ func testCatchUpAppliesStorageBALs(t *testing.T, scheme string) {
 	rawdb.WriteCanonicalHash(db, hdrA.Hash(), numA)
 
 	hdrB := &types.Header{
-		Number: new(big.Int).SetUint64(numA + 1), Root: rootB, Difficulty: common.Big0,
+		ParentHash: hdrA.Hash(),
+		Number:     new(big.Int).SetUint64(numA + 1), Root: rootB, Difficulty: common.Big0,
 		BaseFee: common.Big0, WithdrawalsHash: &emptyH,
 		BlobGasUsed: &zero, ExcessBlobGas: &zero,
 		ParentBeaconRoot: &emptyH, RequestsHash: &emptyH,