diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 3c53caa2cfaa..1ae7f9d8c57f 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -1361,6 +1361,11 @@ func (d *Downloader) processHeaders(origin uint64, pivot uint64, td *big.Int) er rollback []*types.Header rollbackErr error mode = d.getMode() + // Highest header written to the light chain this cycle. Block imports + // move the header head back to the inserted block, so CurrentHeader + // can trail the headers the peer already delivered. A bailing peer + // never advances it, keeping the stalling-peer detection intact. + lastInserted *types.Header ) defer func() { if len(rollback) > 0 { @@ -1433,6 +1438,9 @@ func (d *Downloader) processHeaders(origin uint64, pivot uint64, td *big.Int) er // peer gave us something useful, we're already happy/progressed (above check). if mode == FastSync || mode == LightSync { head := d.lightchain.CurrentHeader() + if lastInserted != nil && lastInserted.Number.Uint64() > head.Number.Uint64() { + head = lastInserted + } if td.Cmp(d.lightchain.GetTd(head.Hash(), head.Number.Uint64())) > 0 { return errStallingPeer } @@ -1483,6 +1491,7 @@ func (d *Downloader) processHeaders(origin uint64, pivot uint64, td *big.Int) er if len(rollback) > fsHeaderSafetyNet { rollback = append(rollback[:0], rollback[len(rollback)-fsHeaderSafetyNet:]...) } + lastInserted = chunk[len(chunk)-1] } // Unless we're doing light chains, schedule the headers for associated content retrieval if mode == FullSync || mode == FastSync { diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index c8b0ba2da5d3..c872c27ac393 100644 --- a/eth/downloader/downloader_test.go +++ b/eth/downloader/downloader_test.go @@ -75,6 +75,17 @@ type downloadTester struct { insertHeaderChainHook func([]*types.Header) error + // headHeaderCap, when non-zero, caps the height reported by CurrentHeader. + // It models the real chain, where importing blocks moves the header head + // back to the block being inserted. It must be set below the length of the + // peer chain being synced; a value at or above the chain head simply + // disables the cap and no longer models the lagging head. + // + // The cap is static, while the real lag is a transient window around the + // block being imported, but the simplified model is enough to reproduce + // the stall misdetection in the terminating header batch. + headHeaderCap uint64 + // configOverride, when non-nil, is returned by Config() instead of the // default TestChainConfig. Used by tests that require XDPoS to be active. configOverride *params.ChainConfig @@ -187,6 +198,9 @@ func (dl *downloadTester) CurrentHeader() *types.Header { for i := len(dl.ownHashes) - 1; i >= 0; i-- { if header := dl.ownHeaders[dl.ownHashes[i]]; header != nil { + if dl.headHeaderCap != 0 && header.Number.Uint64() > dl.headHeaderCap { + continue + } return header } } @@ -1136,6 +1150,33 @@ func testHighTDStarvationAttack(t *testing.T, protocol int, mode SyncMode) { tester.terminate() } +// Tests that a header head lagging behind the headers the peer already delivered +// is not mistaken for a stalling peer. Importing the post-pivot blocks moves the +// header head back to the block being inserted, so it can trail the synced head +// while the terminating header batch is processed. Both fast and light sync run +// the lag-sensitive check, hence both modes are covered. +func TestFastSyncHeaderHeadLag100(t *testing.T) { testHeaderHeadLag(t, xdc100, FastSync) } +func TestFastSyncHeaderHeadLag164(t *testing.T) { testHeaderHeadLag(t, xdc164, FastSync) } +func TestFastSyncHeaderHeadLag165(t *testing.T) { testHeaderHeadLag(t, xdc165, FastSync) } +func TestLightSyncHeaderHeadLag164(t *testing.T) { testHeaderHeadLag(t, xdc164, LightSync) } +func TestLightSyncHeaderHeadLag165(t *testing.T) { testHeaderHeadLag(t, xdc165, LightSync) } + +func testHeaderHeadLag(t *testing.T, protocol int, mode SyncMode) { + t.Parallel() + + tester := newTester() + defer tester.terminate() + + chain := testChainBase.shorten(blockCacheMaxItems - 15) + tester.headHeaderCap = uint64(chain.len()) - 4 + tester.newPeer("peer", protocol, chain) + + if err := tester.sync("peer", nil, mode); err != nil { + t.Fatalf("failed to synchronise blocks: %v", err) + } + assertOwnChain(t, tester, chain.len()) +} + // Tests that misbehaving peers are disconnected, whilst behaving ones are not. func TestBlockHeaderAttackerDropping100(t *testing.T) { testBlockHeaderAttackerDropping(t, xdc100) } func TestBlockHeaderAttackerDropping164(t *testing.T) { testBlockHeaderAttackerDropping(t, xdc164) }