eth/downloader: ignore headers not requested

eth/downloader/downloader.go

@@ -854,7 +854,7 @@ func (d *Downloader) findAncestorSpanSearch(p *peerConnection, mode SyncMode, re
 		case packet := <-d.headerCh:
 			// Discard anything not from the origin peer
 			if packet.PeerId() != p.id {
-				log.Debug("Received headers from incorrect peer", "peer", packet.PeerId())
+				log.Debug("Received headers from incorrect peer [1]", "peer", packet.PeerId())
 				break
 			}
 			// Make sure the peer actually gave something valid
@@ -864,13 +864,19 @@ func (d *Downloader) findAncestorSpanSearch(p *peerConnection, mode SyncMode, re
 				return 0, errEmptyHeaderSet
 			}
 			// Make sure the peer's reply conforms to the request
+			deliveryOk := true
 			for i, header := range headers {
 				expectNumber := from + int64(i)*int64(skip+1)
 				if number := header.Number.Int64(); number != expectNumber {
 					p.log.Warn("Head headers broke chain ordering", "index", i, "requested", expectNumber, "received", number)
-					return 0, fmt.Errorf("%w: %v", errInvalidChain, errors.New("head headers broke chain ordering"))
+					deliveryOk = false
+					break
+					//return 0, fmt.Errorf("%w: %v", errInvalidChain, errors.New("head headers broke chain ordering"))
 				}
 			}
+			if !deliveryOk {
+				break // continue waiting until timeout
+			}
 			// Check if a common ancestor was found
 			finished = true
 			for i := len(headers) - 1; i >= 0; i-- {
@@ -946,21 +952,27 @@ func (d *Downloader) findAncestorBinarySearch(p *peerConnection, mode SyncMode,
 			case packet := <-d.headerCh:
 				// Discard anything not from the origin peer
 				if packet.PeerId() != p.id {
-					log.Debug("Received headers from incorrect peer", "peer", packet.PeerId())
+					log.Debug("Received headers from incorrect peer [2]", "peer", packet.PeerId())
 					break
 				}
 				// Make sure the peer actually gave something valid
 				headers := packet.(*headerPack).headers
 				if len(headers) != 1 {
 					p.log.Warn("Multiple headers for single request", "headers", len(headers))
-					return 0, fmt.Errorf("%w: multiple headers (%d) for single request", errBadPeer, len(headers))
+					break //ignore it
 				}
-				arrived = true
 
 				// Modify the search interval based on the response
 				h := headers[0].Hash()
 				n := headers[0].Number.Uint64()
 
+				// Did the peer deliver what we asked for?
+				if n != check {
+					p.log.Warn("Received non requested header", "number", n, "hash", h, "request", check)
+					break // ignore it
+				}
+				arrived = true
+
 				var known bool
 				switch mode {
 				case FullSync:
@@ -974,11 +986,6 @@ func (d *Downloader) findAncestorBinarySearch(p *peerConnection, mode SyncMode,
 					end = check
 					break
 				}
-				header := d.lightchain.GetHeaderByHash(h) // Independent of sync mode, header surely exists
-				if header.Number.Uint64() != check {
-					p.log.Warn("Received non requested header", "number", header.Number, "hash", header.Hash(), "request", check)
-					return 0, fmt.Errorf("%w: non-requested header (%d)", errBadPeer, header.Number)
-				}
 				start = check
 				hash = h
 

eth/downloader/downloader_test.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"os"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -33,6 +34,7 @@ import (
 	"github.com/ethereum/go-ethereum/eth/protocols/eth"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/event"
+	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/trie"
 )
 
@@ -42,6 +44,7 @@ func init() {
 	lightMaxForkAncestry = 10000
 	blockCacheMaxItems = 1024
 	fsHeaderContCheck = 500 * time.Millisecond
+	ttlLimit = 2 * time.Second
 }
 
 // downloadTester is a test simulator for mocking out local block chain.
@@ -1383,7 +1386,11 @@ func testFailedSyncProgress(t *testing.T, protocol uint, mode SyncMode) {
 
 	// Attempt a full sync with a faulty peer
 	brokenChain := chain.shorten(chain.len())
-	missing := brokenChain.len() / 2
+	// It matters which one we delete here: we want to delete a header which is
+	// not part of the binary search for ancestor, since that will cause
+	// a very early fail. Using length/2 + 1 makes the spot-checking ancestor search
+	// pass, and the gap won't be seen until later during the sync
+	missing := brokenChain.len()/2 + 1
 	delete(brokenChain.headerm, brokenChain.chain[missing])
 	delete(brokenChain.blockm, brokenChain.chain[missing])
 	delete(brokenChain.receiptm, brokenChain.chain[missing])
@@ -1439,6 +1446,9 @@ func TestFakedSyncProgress66Light(t *testing.T) { testFakedSyncProgress(t, eth.E
 
 func testFakedSyncProgress(t *testing.T, protocol uint, mode SyncMode) {
 	t.Parallel()
+	if false {
+		log.Root().SetHandler(log.LvlFilterHandler(log.LvlTrace, log.StreamHandler(os.Stderr, log.TerminalFormat(true))))
+	}
 
 	tester := newTester()
 	defer tester.terminate()

eth/downloader/testchain_test.go

@@ -179,17 +179,20 @@ func (tc *testChain) headersByHash(origin common.Hash, amount int, skip int, rev
 // headersByNumber returns headers from the given number.
 func (tc *testChain) headersByNumber(origin uint64, amount int, skip int, reverse bool) []*types.Header {
 	result := make([]*types.Header, 0, amount)
-
 	if !reverse {
 		for num := origin; num < uint64(len(tc.chain)) && len(result) < amount; num += uint64(skip) + 1 {
 			if header, ok := tc.headerm[tc.chain[int(num)]]; ok {
 				result = append(result, header)
+			} else {
+				break
 			}
 		}
 	} else {
 		for num := int64(origin); num >= 0 && len(result) < amount; num -= int64(skip) + 1 {
 			if header, ok := tc.headerm[tc.chain[int(num)]]; ok {
 				result = append(result, header)
+			} else {
+				break
 			}
 		}
 	}