From 2a6fd50ebe761621de9669488b419443dc9bee82 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Fri, 22 Jan 2021 09:40:02 +0100 Subject: [PATCH 1/2] eth/downloader: ignore headers not requested --- eth/downloader/downloader.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index b8cb48914e6b..a644d7256624 100644 --- a/eth/downloader/downloader.go +++ b/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 From e244c7e0b3df95f05d61e268a0ca4cb5eb0e4bcb Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 2 Feb 2021 09:45:08 +0100 Subject: [PATCH 2/2] eth/downloader: fixes to tests --- eth/downloader/downloader_test.go | 12 +++++++++++- eth/downloader/testchain_test.go | 5 ++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index 1140a444c1c5..acfed5d3f381 100644 --- a/eth/downloader/downloader_test.go +++ b/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() diff --git a/eth/downloader/testchain_test.go b/eth/downloader/testchain_test.go index 2d7b4d1f1035..fb9b0737a203 100644 --- a/eth/downloader/testchain_test.go +++ b/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 } } }