From a34b9e0b1dcdcf3ed382d11f949c24c8e3464f5f Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Wed, 2 Sep 2026 11:12:13 +0800 Subject: [PATCH] fix(eth/downloader): cap the common ancestor search at the local chain head, close #2533 Side chain blocks are written by hash without state, so a node whose head sits below them still answers HasBlock for the whole segment; findAncestor then resolved the ancestor above its own head and the sync resumed there, stranding the range in between. On XDPoS this stalls the head for good: epoch-switch header verification needs the gap block's masternode snapshot, which is only written when the gap block imports with state. Cap both searches at the local head. calculateRequestSpan now always asks for two samples spaced one skipped header apart, taking its top sample from the highest block below the remote head and clamping it down to the local head, so the window never spends a round trip on a block the head guard would reject; findAncestor bounds the binary search with the same limit. The window keeps its count when 'from' is clamped up to zero, which can top it out above the head, so usableAsAncestor still rejects those candidates and keeps a span hit from being returned above the head. A hit on the lower sample no longer ends the search: the true fork can sit on the skipped block, so the hit seeds the binary search with the gap under the next sample, clamped by the local head and by the remote height. Covered by blocks-above-head tests in full and fast sync (stubs stored by hash, carrying receipts in fast sync but never entering the canonical chain, so the snap head stays pinned), span candidate rejection and the head-0 and head-2 window clamps, the gap refinement, a remote sitting at genesis, a binary-search fork variant, per-mode head-guard boundaries, the write-without-state semantics the bug rests on, and the per-case request span and window max. --- eth/downloader/downloader.go | 196 ++++---- eth/downloader/downloader_test.go | 772 ++++++++++++++++++++++++++++-- 2 files changed, 847 insertions(+), 121 deletions(-) diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 05cd1bd8fe78..6e280b76eb1d 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -668,58 +668,72 @@ func (d *Downloader) fetchHeight(p *peerConnection, hash common.Hash) (*types.He } } +// Sampling parameters of the fixed ancestor span request: two samples spaced +// one skipped header apart, spanning three consecutive blocks and sampling +// only its two ends. +const ( + spanSampleCount = 2 // samples per span request + spanSampleSkip = 1 // headers skipped between two samples +) + // calculateRequestSpan calculates what headers to request from a peer when trying to determine the // common ancestor. -// It returns parameters to be used for peer.RequestHeadersByNumber: +// The request shape is fixed at spanSampleCount samples spaced spanSampleSkip +// skipped headers apart; the function returns the parameters to be used for +// peer.RequestHeadersByNumber: // // from - starting block number -// count - number of headers to request -// skip - number of headers to skip +// max - the highest block the search should consider from the response, +// i.e. the top of the acceptance window; it can top out past the peer's +// head when 'from' is clamped up to zero (see the clamp below) // -// and also returns 'max', the last block which is expected to be returned by the remote peers, -// given the (from,count,skip) -func calculateRequestSpan(remoteHeight, localHeight uint64) (int64, int, int, uint64) { - var ( - from int - count int - MaxCount = MaxHeaderFetch / 16 - ) - // requestHead is the highest block that we will ask for. If requestHead is not offset, - // the highest block that we will get is 16 blocks back from head, which means we - // will fetch 14 or 15 blocks unnecessarily in the case the height difference - // between us and the peer is 1-2 blocks, which is most common - requestHead := int(remoteHeight) - 1 - if requestHead < 0 { - requestHead = 0 - } - // requestBottom is the lowest block we want included in the query - // Ideally, we want to include the one just below our own head - requestBottom := int(localHeight - 1) - if requestBottom < 0 { - requestBottom = 0 - } - totalSpan := requestHead - requestBottom - span := 1 + totalSpan/MaxCount - if span < 2 { - span = 2 - } - if span > 16 { - span = 16 - } - - count = 1 + totalSpan/span - if count > MaxCount { - count = MaxCount - } - if count < 2 { - count = 2 - } - from = requestHead - (count-1)*span - if from < 0 { - from = 0 - } - max := from + (count-1)*span - return int64(from), count, span - 1, uint64(max) +// The sampling top is the highest block below the remote head, capped at +// the local head, because usableAsAncestor rejects every candidate above it and +// samples there are wasted round trips. The window spans three +// consecutive blocks and samples only its two ends, so a common ancestor at +// the top sample, at the lower one, or on the skipped middle between them +// all yields a hit in the first round trip. A hit at the top sample is +// returned as is, while a hit at the lower one leaves the true ancestor +// somewhere in the two-block gap under the top sample, and a single +// binary-search probe at the skipped middle resolves it whichever block +// it is. When the capped top sits below 2, the raw start would be negative, +// so 'from' is clamped up to zero and the fixed two-sample window is left +// as is, topping it out past the sampling top; candidates above the local +// head are rejected by usableAsAncestor's head guard. +func calculateRequestSpan(remoteHeight, localHeight uint64) (int64, uint64) { + // The remote head itself was already fetched to learn the remote height, + // so the sampling top starts at the highest block below it. The local + // head cap on it is described in the doc comment. max(remoteHeight, 1)-1 + // saturates the decrement at zero, avoiding the uint64 underflow a raw + // remoteHeight-1 would hit at genesis. + spanTop := min(max(remoteHeight, 1)-1, localHeight) + // The capped top and the block two below it. The skipped middle block is + // deliberate; see the doc comment for the geometry, the clamp, and the + // refinement trade-off. + width := (spanSampleCount - 1) * (spanSampleSkip + 1) + from := max(int(spanTop)-width, 0) + return int64(from), uint64(from + width) +} + +// usableAsAncestor reports whether a remote block may be used as the common ancestor. +// Blocks above the local head are rejected even when their body is on disk: +// side chain blocks written ahead of the head are known by hash, and accepting +// one as the ancestor skips the range the chain still has to import. The span +// search's acceptance window follows the raw request, so it can top out above +// the head when 'from' is clamped up to zero; this guard is what keeps those +// above-head candidates out of the ancestor search. +func (d *Downloader) usableAsAncestor(mode SyncMode, hash common.Hash, number, localHeight uint64) bool { + if number > localHeight { + return false + } + switch mode { + case FullSync: + return d.blockchain.HasBlock(hash, number) + case FastSync: + return d.blockchain.HasFastBlock(hash, number) + default: + return d.lightchain.HasHeader(hash, number) + } } // findAncestor tries to locate the common ancestor link of the local chain and @@ -748,10 +762,18 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header) if localHeight >= MaxForkAncestry { floor = int64(localHeight - MaxForkAncestry) } - from, count, skip, max := calculateRequestSpan(remoteHeight, localHeight) + // The common ancestor can never sit above the local head, and + // usableAsAncestor rejects every candidate above it, so cap both the span + // search and the binary search there. Otherwise a head far below the + // remote, or stale side chain segments stored above it, burn the whole + // search on candidates that cannot be accepted instead of anchoring in + // one round trip. + ancestorLimitExclusive := localHeight + 1 + + from, spanMax := calculateRequestSpan(remoteHeight, localHeight) - p.log.Trace("Span searching for common ancestor", "count", count, "from", from, "skip", skip) - go p.peer.RequestHeadersByNumber(uint64(from), count, skip, false) + p.log.Trace("Span searching for common ancestor", "count", spanSampleCount, "from", from, "skip", spanSampleSkip) + go p.peer.RequestHeadersByNumber(uint64(from), spanSampleCount, spanSampleSkip, false) // Wait for the remote response to the head fetch number, hash := uint64(0), common.Hash{} @@ -778,7 +800,7 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header) } // Make sure the peer's reply conforms to the request for i, header := range headers { - expectNumber := from + int64(i)*int64((skip+1)) + expectNumber := from + int64(i)*int64(spanSampleSkip+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")) @@ -788,23 +810,14 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header) finished = true for i := len(headers) - 1; i >= 0; i-- { // Skip any headers that underflow/overflow our requested set - if headers[i].Number.Int64() < from || headers[i].Number.Uint64() > max { + if headers[i].Number.Int64() < from || headers[i].Number.Uint64() > spanMax { continue } // Otherwise check if we already know the header or not h := headers[i].Hash() n := headers[i].Number.Uint64() - var known bool - switch mode { - case FullSync: - known = d.blockchain.HasBlock(h, n) - case FastSync: - known = d.blockchain.HasFastBlock(h, n) - default: - known = d.lightchain.HasHeader(h, n) - } - if known { + if d.usableAsAncestor(mode, h, n, localHeight) { number, hash = n, h break } @@ -819,19 +832,48 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header) // Out of bounds delivery, ignore } } - // If the head fetch already found an ancestor, return + // A span hit is only the highest SAMPLED common block: the search steps + // over skip headers, so the true fork can sit in the unsampled gap below + // the next sample (e.g. at localHeight-1 when the head-capped window is + // {head-2, head}, or at the head itself when the clamped window missed it + // and the hit fell back to genesis). A hit at the local head or at the + // top of the window leaves no gap to close and is returned as is; any + // other hit seeds the binary search below with the gap bounds, the hit + // being the known-common lower bound and the next sample the + // known-rejected upper one. + // floor starts at -1 and stays there while the local chain is shorter + // than MaxForkAncestry, so it must be clamped to zero before the + // unsigned conversion: uint64(-1) would silently become MaxUint64 and + // leave the binary search correct only through unsigned wraparound. + start, end := uint64(0), min(remoteHeight, ancestorLimitExclusive) + if floor > 0 { + start = uint64(floor) + } if !hash.IsZero() { if int64(number) <= floor { p.log.Warn("Ancestor below allowance", "number", number, "hash", hash, "allowance", floor) return 0, errInvalidAncestor } - p.log.Debug("Found common ancestor", "number", number, "hash", hash) - return number, nil - } - // Ancestor not found, we need to binary search over our chain - start, end := uint64(0), remoteHeight - if floor > 0 { - start = uint64(floor) + if gap := number + spanSampleSkip + 1; number < localHeight && gap <= spanMax { + // The next sample is the known-rejected upper bound of the + // refinement. The clamped window keeps its count, so it can top + // out past the local head and past the peer's; probe neither: + // usableAsAncestor rejects the former, and the latter comes back + // empty and fails the sync. The local-head clamp is explicit so + // the not-above-the-head invariant does not hinge on the branch + // guard plus the fixed skip arithmetic. + // The remote-head bound is exclusive, so the peer's head itself stays + // probeable; compare before incrementing, because a head advertised at + // MaxUint64 would wrap remoteHeight+1 to zero and silently collapse + // the refinement interval to an empty range. + start, end = number, min(gap, ancestorLimitExclusive) + if remoteHeight < end { + end = remoteHeight + 1 + } + } else { + p.log.Debug("Found common ancestor", "number", number, "hash", hash) + return number, nil + } } p.log.Trace("Binary searching for common ancestor", "start", start, "end", end) @@ -868,16 +910,10 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header) h := headers[0].Hash() n := headers[0].Number.Uint64() - var known bool - switch mode { - case FullSync: - known = d.blockchain.HasBlock(h, n) - case FastSync: - known = d.blockchain.HasFastBlock(h, n) - default: - known = d.lightchain.HasHeader(h, n) - } - if !known { + // The search is capped at the local head, so an honest peer + // can never offer a probe above it; any probe that is not a + // known block narrows the search below it. + if !d.usableAsAncestor(mode, h, n, localHeight) { end = check break } diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index f7195caf0dbb..02f4ab9b3733 100644 --- a/eth/downloader/downloader_test.go +++ b/eth/downloader/downloader_test.go @@ -20,6 +20,7 @@ import ( "encoding/json" "errors" "fmt" + "math" "math/big" "slices" "strings" @@ -322,6 +323,30 @@ func (dl *downloadTester) InsertChain(blocks types.Blocks) (i int, err error) { return len(blocks), nil } +// writeBlockWithoutState mirrors core.BlockChain.writeBlockWithoutState: the block +// becomes known by hash, but without state it cannot become the chain head. The +// parent must have been imported first (e.g. via InsertChain), as the simulated +// chain only tracks the total difficulty of blocks it already knows. +// +// The block is deliberately kept out of ownHashes, the tester's canonical hash +// chain: a real node writes such blocks as non-canonical side blocks, so the +// head getters keep reporting the imported head. Raising CurrentSnapBlock in +// particular would inflate the local height findAncestor derives in fast sync +// and mask a sync that resumes above the head. +func (dl *downloadTester) writeBlockWithoutState(block *types.Block) error { + dl.lock.Lock() + defer dl.lock.Unlock() + + parentTd, ok := dl.ownChainTd[block.ParentHash()] + if !ok { + return fmt.Errorf("parent block %s not imported", block.ParentHash()) + } + dl.ownHeaders[block.Hash()] = block.Header() + dl.ownBlocks[block.Hash()] = block + dl.ownChainTd[block.Hash()] = new(big.Int).Add(parentTd, block.Difficulty()) + return nil +} + // InsertReceiptChain injects a new batch of receipts into the simulated chain. func (dl *downloadTester) InsertReceiptChain(blocks types.Blocks, receipts []types.Receipts) (i int, err error) { dl.lock.Lock() @@ -397,6 +422,10 @@ type downloadTesterPeer struct { id string chain *testChain missingStates map[common.Hash]bool // State entries that fast sync should not return + // requestHook, when non-nil, observes every numbered header request just + // before the fake peer serves it, letting tests assert which heights the + // downloader actually probes. + requestHook func(origin uint64, amount int, skip int, reverse bool) } // Head constructs a function to retrieve a peer's current head hash @@ -426,6 +455,9 @@ func (dlp *downloadTesterPeer) RequestHeadersByNumber(origin uint64, amount int, if reverse { panic("reverse header requests not supported") } + if dlp.requestHook != nil { + dlp.requestHook(origin, amount, skip, reverse) + } result := dlp.chain.headersByNumber(origin, amount, skip) go dlp.dl.downloader.DeliverHeaders(dlp.id, result) @@ -552,6 +584,636 @@ func testCanonicalSynchronisation(t *testing.T, protocol int, mode SyncMode) { assertOwnChain(t, tester, chain.len()) } +// TestFindAncestorIgnoresBlocksAboveHead checks that blocks stored ahead of the +// chain head are not accepted as the common ancestor through the span search. +// Side chain blocks are written by hash without state, so accepting one +// resumes the sync above the head and leaves the range in between permanently +// unimported. The request span is capped at the local head, and any header it +// still samples past the requested top is rejected by the usableAsAncestor head +// guard, so the sync anchors at or below the head. +func TestFindAncestorIgnoresBlocksAboveHead(t *testing.T) { + t.Parallel() + testFindAncestorIgnoresBlocksAboveHead(t, FullSync, 64, 32, 32) +} + +// TestFindAncestorIgnoresBlocksAboveHeadFast runs the blocks-above-head +// scenario in fast sync, where the ancestor search derives the local height +// from the snap block: stubs stored ahead of the head must neither raise the +// reported snap head nor be accepted as the common ancestor. +func TestFindAncestorIgnoresBlocksAboveHeadFast(t *testing.T) { + t.Parallel() + testFindAncestorIgnoresBlocksAboveHead(t, FastSync, 128, 32, 32) +} + +// testFindAncestorIgnoresBlocksAboveHead runs the blocks-above-head scenario: +// the local head sits at headHeight while the rest of the chainLen-block chain +// is only known by hash (plus receipts on the fast sync path), the way a side +// chain segment is written, and the sync must resume from the head instead of +// resuming above it. A high head (headHeight 32) keeps the span window at or +// below the head: the span search never samples above it. A head of 1 is the +// one height where the clamped window {0, 2} tops the request out at the +// first block past the head; that header is served and rejected by +// usableAsAncestor's head guard, and the genesis hit the span still finds is +// refined over the unsampled gap, so the sync anchors at the head anyway. +// wantOrigin pins the expected anchor exactly: the head itself. +// +// The scenario runs in full and fast sync. FastSync derives the local height +// from the snap block, so it also pins that stubs stored ahead of the head do +// not raise the reported snap head. Fast sync knowledge is modeled through +// receipts, so the fast sync runs give the stubs receipts as well: the blocks +// above the head count as known, so usableAsAncestor's head guard is what keeps +// them out, not a missing-receipt lookup. Its chain is kept long enough +// (chainLen > headHeight+fsMinFullBlocks) for the fast sync pivot to land +// above the origin; a chain at or below fsMinFullBlocks would force the +// origin back to genesis instead. +func testFindAncestorIgnoresBlocksAboveHead(t *testing.T, mode SyncMode, chainLen int, headHeight, wantOrigin uint64) { + t.Helper() + + tester := newTester() + defer tester.terminate() + + chain := testChainBase.shorten(chainLen) + tester.newPeer("peer", xdc165, chain) + + imported := make([]*types.Block, 0, headHeight) + for i := uint64(1); i <= headHeight; i++ { + imported = append(imported, chain.blockm[chain.chain[i]]) + } + if _, err := tester.InsertChain(imported); err != nil { + t.Fatalf("failed to import blocks: %v", err) + } + stubs := make(types.Blocks, 0, uint64(chain.len())-headHeight-1) + for i := headHeight + 1; i < uint64(chain.len()); i++ { + block := chain.blockm[chain.chain[i]] + if err := tester.writeBlockWithoutState(block); err != nil { + t.Fatalf("failed to write block without state: %v", err) + } + stubs = append(stubs, block) + } + if mode == FastSync { + // The fake chain reports fast-sync knowledge through receipts, so the + // whole stored prefix must carry them the way a fast-synced local chain + // has them: the imported prefix keeps the head syncable, and the stubs + // get receipts too so the blocks above the head count as known. + // usableAsAncestor's head guard is then what keeps them out; receipt-less + // stubs would be dropped as unknown instead, so a sync that lost the + // guard would still anchor below the head and mask the regression. + blocks := append(append(types.Blocks{}, imported...), stubs...) + receipts := make([]types.Receipts, len(blocks)) + for i := range receipts { + receipts[i] = types.Receipts{} + } + if _, err := tester.InsertReceiptChain(blocks, receipts); err != nil { + t.Fatalf("failed to insert receipts: %v", err) + } + } + if have := tester.CurrentBlock().Number.Uint64(); have != headHeight { + t.Fatalf("unexpected head: have %d want %d", have, headHeight) + } + + // Record the first numbered header request the sync issues: inside + // findAncestor that is always the span search request. The requested + // range must top out at the head, otherwise the span burns its samples + // on blocks usableAsAncestor rejects anyway. Also record the very last + // numbered request to pin the sync's completion: a graceful fetchHeaders + // run terminates with an empty probe at the first height past the remote + // head, so a run that silently stalls after the ancestor search stops + // requesting earlier. + var ( + spanRequested bool + spanOrigin uint64 + spanLast uint64 + lastRequest uint64 + ) + tester.peers["peer"].requestHook = func(origin uint64, amount int, skip int, reverse bool) { + if !spanRequested { + spanRequested = true + spanOrigin = origin + spanLast = origin + uint64((amount-1)*(skip+1)) + } + lastRequest = origin + } + + var origin uint64 + tester.downloader.syncInitHook = func(from, _ uint64) { origin = from } + + if err := tester.sync("peer", nil, mode); err != nil { + t.Fatalf("failed to synchronise blocks: %v", err) + } + if !spanRequested { + t.Fatalf("no numbered header request issued, span search not exercised") + } + // The span request must top out exactly at the head. The single + // exception is the clamped window of calculateRequestSpan: with the + // head at 0 or 1 the window is {0, 2} and tops the request out at the + // first block past the head (pinned in TestRemoteHeaderRequestSpan); + // those headers are served and considered, but rejected by usableAsAncestor's + // head guard, so the exception is tied to the head while the request + // stays anchored at genesis. + wantSpanLast := max(headHeight, 2) + if spanLast != wantSpanLast { + t.Fatalf("span request tops out at the wrong height: last %d, want %d, head %d", spanLast, wantSpanLast, headHeight) + } + if spanLast > headHeight && spanOrigin != 0 { + t.Fatalf("span request tops out above the head without a below-head anchor: origin %d, head %d", spanOrigin, headHeight) + } + // Whatever the head height, the sync must anchor at the head itself: the + // span hit is refined over the unsampled gap below the next sample, so a + // genesis hit with the head at 1 still converges on the head. Pin the + // anchor exactly: an ancestor at or below the head is not enough, a + // regression re-anchoring lower (e.g. at genesis) must fail as well. + if origin != wantOrigin { + t.Fatalf("sync resumed at the wrong ancestor: origin %d, want %d (head %d)", origin, wantOrigin, headHeight) + } + // Anchoring is not enough: the sync must actually reach the remote head. + // fetchHeaders terminates with an empty numbered probe at the first + // height past the remote head, so a run that silently stalls after the + // ancestor search ends its request stream earlier and is caught here. + // (CurrentBlock cannot serve this purpose: every header the sync fetches + // is already stored by hash, so the simulated chain's head never moves.) + if remoteHeight := uint64(chain.len() - 1); lastRequest != remoteHeight+1 { + t.Fatalf("sync did not fetch up to the remote head: last numbered request at %d, want %d", lastRequest, remoteHeight+1) + } +} + +// TestFindAncestorSpanRejectsAboveHeadCandidate drives the one height where +// the clamped span window tops the request out at the first block past the +// head: with the local head at height 1 the peer is asked for +// block 2 as well, which the test stores by hash the way a side chain segment +// is written. The span's acceptance window follows the raw request top, so +// block 2 is served and considered; usableAsAncestor's head guard is what rejects +// it, and the genesis hit the span still finds is refined over the unsampled +// gap, so the sync anchors at the head. +func TestFindAncestorSpanRejectsAboveHeadCandidate(t *testing.T) { + t.Parallel() + testFindAncestorIgnoresBlocksAboveHead(t, FullSync, 64, 1, 1) +} + +// TestFindAncestorSpanRejectsAboveHeadCandidateFast runs the same clamped +// window overshoot in fast sync: with the local head at height 1 the span +// request still tops out at the first block past the head, and that stub carries +// receipts, so it counts as known. usableAsAncestor's head guard is what rejects +// it, and the genesis hit the span still finds is refined over the unsampled +// gap: the sync must anchor at the head instead of resuming above it. +func TestFindAncestorSpanRejectsAboveHeadCandidateFast(t *testing.T) { + t.Parallel() + testFindAncestorIgnoresBlocksAboveHead(t, FastSync, 128, 1, 1) +} + +// TestFindAncestorSpanAnchorsAtHeadZero pins the genesis boundary of the span +// search: with the local head at genesis, the genesis hit is the head itself +// and must be returned as is. Blocks stored above the head are rejected by +// usableAsAncestor's head guard, so the sync must neither resume above the head +// nor search below a hit that is already exact. +func TestFindAncestorSpanAnchorsAtHeadZero(t *testing.T) { + t.Parallel() + testFindAncestorIgnoresBlocksAboveHead(t, FullSync, 64, 0, 0) +} + +// TestFindAncestorSpanAnchorsAtHeadTwo pins the first height where the span +// window {head-2, head} samples the head itself: the hit is exact, and +// the sync must anchor at the head instead of drifting to a lower sample. +func TestFindAncestorSpanAnchorsAtHeadTwo(t *testing.T) { + t.Parallel() + testFindAncestorIgnoresBlocksAboveHead(t, FullSync, 64, 2, 2) +} + +// TestFindAncestorSpanRefinesBelowLowestSample drives the reorg boundary the +// head-capped span window creates: with the local head sitting on the first +// block the peer's fork replaces, the window {head-2, head} rejects the head +// sample and hits its lower sample, while the true common ancestor sits on +// the unsampled head-1. The span hit must be refined over the unsampled gap, +// so the sync anchors at head-1 instead of re-importing the head block it +// already has. +func TestFindAncestorSpanRefinesBelowLowestSample(t *testing.T) { + t.Parallel() + + const ( + forkPoint = 10 // first block the peer's chain replaces + headHeight = 10 // the local head sits on the replaced block + chainLen = 64 + ) + + tester := newTester() + defer tester.terminate() + + // Peer on a heavy fork sharing blocks 0..forkPoint-1 with the base chain + // and replacing everything from forkPoint up, so the sync must reorg onto + // it from the common ancestor just below the local head. + fork := testChainBase.shorten(forkPoint).makeFork(chainLen-forkPoint, true, 7) + tester.newPeer("peer", xdc165, fork) + + // Sanity check the span window the call site issues: it samples the local + // head (the fork replaced it) and head-2 (still common), leaving the true + // ancestor at head-1 unsampled between the two samples. The remote head + // is the top of the peer's chain; the function caps the sampling top at + // the local head. + from, max := calculateRequestSpan(uint64(chainLen-1), headHeight) + if from != int64(headHeight-2) || max != headHeight { + t.Fatalf("unexpected span window: from %d max %d", from, max) + } + + // Import the common prefix plus the first replaced block, so the local + // head sits exactly on a block the peer's chain no longer contains. + imported := make([]*types.Block, 0, headHeight) + for i := uint64(1); i <= headHeight; i++ { + imported = append(imported, testChainBase.blockm[testChainBase.chain[i]]) + } + if _, err := tester.InsertChain(imported); err != nil { + t.Fatalf("failed to import blocks: %v", err) + } + if have := tester.CurrentBlock().Number.Uint64(); have != headHeight { + t.Fatalf("unexpected head: have %d want %d", have, headHeight) + } + + // Record every numbered header request the ancestor search issues: the + // span window and the refinement probe must both stay at or below the + // head, anything above can never be accepted as the common ancestor. + var ( + searching = true + origin uint64 + requested []uint64 + ) + tester.peers["peer"].requestHook = func(from uint64, amount int, skip int, reverse bool) { + if searching { + requested = append(requested, from) + } + } + tester.downloader.syncInitHook = func(from, _ uint64) { + searching = false + origin = from + } + + if err := tester.sync("peer", nil, FullSync); err != nil { + t.Fatalf("failed to synchronise blocks: %v", err) + } + if len(requested) < 2 { + t.Fatalf("span search was not refined over the unsampled gap: %d request(s)", len(requested)) + } + for i, probe := range requested { + if probe > headHeight { + t.Fatalf("ancestor search request #%d probes above the head: %d > %d", i, probe, headHeight) + } + } + if origin != forkPoint-1 { + t.Fatalf("sync resumed at the wrong ancestor: origin %d want %d", origin, forkPoint-1) + } + if have := tester.CurrentBlock().Number.Uint64(); have != uint64(chainLen-1) { + t.Fatalf("unexpected head after sync: have %d want %d", have, chainLen-1) + } +} + +// TestFindAncestorSpanRemoteAtGenesis pins the remote-head clamp of the gap +// refinement: a peer whose head is the genesis block can only reach the sync +// through a direct Synchronise call (the sync entry gate requires a TD +// advantage), yet it exercises a real boundary of the ancestor search. Its +// clamped span window {0, 2} hits at genesis and refines to the interval +// [0, 1]; the refinement must stop at the peer's head instead of probing +// past it, because an empty reply to a probe above the head fails the whole +// sync rather than just burning a round trip. +func TestFindAncestorSpanRemoteAtGenesis(t *testing.T) { + t.Parallel() + + tester := newTester() + defer tester.terminate() + + // Peer holding nothing but the genesis block, sharing it with the base + // chain the local chain extends. + peerChain := testChainBase.shorten(1) + tester.newPeer("peer", xdc165, peerChain) + + // Advance the local chain one block past the peer's head. + imported := []*types.Block{testChainBase.blockm[testChainBase.chain[1]]} + if _, err := tester.InsertChain(imported); err != nil { + t.Fatalf("failed to import blocks: %v", err) + } + if have := tester.CurrentBlock().Number.Uint64(); have != 1 { + t.Fatalf("unexpected head: have %d want 1", have) + } + + // Record every numbered header request the ancestor search issues: the + // span request at genesis is fine, but a second one could only be a + // binary-search probe above the peer's head, which the peer cannot + // serve and which fails the sync outright. + var ( + searching = true + origin uint64 + requested []uint64 + ) + tester.peers["peer"].requestHook = func(from uint64, amount int, skip int, reverse bool) { + if searching { + requested = append(requested, from) + } + } + tester.downloader.syncInitHook = func(from, _ uint64) { + searching = false + origin = from + } + + if err := tester.sync("peer", nil, FullSync); err != nil { + t.Fatalf("failed to synchronise blocks: %v", err) + } + // The genesis span hit must refine to [0, 1] without a single probe + // past the peer's head: one request, then straight to the sync with + // the genesis anchor. + if len(requested) != 1 || requested[0] != 0 { + t.Fatalf("ancestor search probed past the genesis peer: requests %v", requested) + } + if origin != 0 { + t.Fatalf("sync resumed at the wrong ancestor: origin %d want 0", origin) + } +} + +// TestFindAncestorSpanRemoteHeadAtMaxUint64 pins the overflow boundary of +// the gap refinement's remote-head clamp: the remote head number reaches +// findAncestor through fetchHeight, which validates the response's shape +// but not its height, so a misbehaving peer can advertise a head at +// math.MaxUint64. The refinement's exclusive remote bound must not wrap +// remoteHeight+1 to zero: a zero end silently skips the binary search and +// anchors the sync one block lower than the search actually resolved. +func TestFindAncestorSpanRemoteHeadAtMaxUint64(t *testing.T) { + t.Parallel() + + const ( + forkPoint = 10 // first block the peer's chain replaces + headHeight = 10 // the local head sits on the replaced block + chainLen = 64 + ) + + tester := newTester() + defer tester.terminate() + + // Peer on a heavy fork sharing blocks 0..forkPoint-1 with the base chain: + // the span window {head-2, head} then rejects the head sample and hits its + // lower sample, seeding the gap refinement the overflow must not skip. + fork := testChainBase.shorten(forkPoint).makeFork(chainLen-forkPoint, true, 7) + if err := tester.newPeer("peer", xdc165, fork); err != nil { + t.Fatalf("failed to create peer: %v", err) + } + + // Import the common prefix plus the first replaced block, so the local + // head sits exactly on a block the peer's chain no longer contains. + imported := make([]*types.Block, 0, headHeight) + for i := uint64(1); i <= headHeight; i++ { + imported = append(imported, testChainBase.blockm[testChainBase.chain[i]]) + } + if _, err := tester.InsertChain(imported); err != nil { + t.Fatalf("failed to import blocks: %v", err) + } + if have := tester.CurrentBlock().Number.Uint64(); have != headHeight { + t.Fatalf("unexpected head: have %d want %d", have, headHeight) + } + + peer := tester.downloader.peers.Peer("peer") + if peer == nil { + t.Fatal("peer not registered") + } + atomic.StoreUint32(&tester.downloader.mode, uint32(FullSync)) + + // DeliverHeaders drops replies unless a sync is marked active, so arm the + // cancellation channel the way Synchronise would before driving the search + // directly. + tester.downloader.cancelLock.Lock() + tester.downloader.cancelCh = make(chan struct{}) + tester.downloader.cancelLock.Unlock() + + // fetchHeight never validates the advertised height, so hand the search + // a head at the wrap boundary directly. + remoteHead := &types.Header{Number: new(big.Int).SetUint64(math.MaxUint64)} + ancestor, err := tester.downloader.findAncestor(peer, remoteHead) + if err != nil { + t.Fatalf("failed to find ancestor: %v", err) + } + if want := uint64(headHeight - 1); ancestor != want { + t.Fatalf("gap refinement skipped: ancestor %d want %d", ancestor, want) + } +} + +// TestFindAncestorBinarySearchIgnoresBlocksAboveHead checks the same guarantee +// as TestFindAncestorIgnoresBlocksAboveHead, but through the binary search. +// The request span samples only at or below the local head, so with a peer +// whose chain forks below the sampled range the span search finds nothing and +// the binary search takes over: capped at the local head, it must stop at the +// true common ancestor instead of a side chain block stored ahead of the head. +// The search is also required to never request a height above the head: such +// a probe can never be accepted as the common ancestor and would only burn a +// round trip on every rejected candidate before converging on the same one. +func TestFindAncestorBinarySearchIgnoresBlocksAboveHead(t *testing.T) { + t.Parallel() + + const ( + forkPoint = 35 // first block the peer's chain replaces + headHeight = 37 + chainLen = 64 + ) + + tester := newTester() + defer tester.terminate() + + // Peer on a heavy fork sharing blocks 0..forkPoint-1 with the base chain + // and replacing everything from forkPoint up, so the sync can reorg onto it. + fork := testChainBase.shorten(forkPoint).makeFork(chainLen-forkPoint, true, 7) + tester.newPeer("peer", xdc165, fork) + + // Sanity check the request span: the call site samples only at or below + // the head, and every sample must be replaced by the fork, otherwise + // the span search would resolve the ancestor itself. The remote head is + // the top of the peer's chain; the function caps the sampling top at + // the local head. + from, max := calculateRequestSpan(uint64(chainLen-1), headHeight) + if max != headHeight { + t.Fatalf("span search samples above the head: max %d want %d", max, headHeight) + } + for i := 0; i < spanSampleCount; i++ { + num := int(from) + i*(spanSampleSkip+1) + if num < 0 || num > int(headHeight) { + t.Fatalf("span sample #%d out of the head-capped range: %d", i, num) + } + if fork.blockm[fork.chain[num]].Hash() == testChainBase.blockm[testChainBase.chain[num]].Hash() { + t.Fatalf("span sample #%d is common to both chains, binary search not exercised", num) + } + } + + // Import the common prefix up to the head, then store the rest of the base + // chain by hash without state, the way a side chain segment is written. + imported := make([]*types.Block, 0, headHeight) + for i := uint64(1); i <= headHeight; i++ { + imported = append(imported, testChainBase.blockm[testChainBase.chain[i]]) + } + if _, err := tester.InsertChain(imported); err != nil { + t.Fatalf("failed to import blocks: %v", err) + } + for i := uint64(headHeight + 1); i < uint64(chainLen); i++ { + if err := tester.writeBlockWithoutState(testChainBase.blockm[testChainBase.chain[i]]); err != nil { + t.Fatalf("failed to write block without state: %v", err) + } + } + if have := tester.CurrentBlock().Number.Uint64(); have != headHeight { + t.Fatalf("unexpected head: have %d want %d", have, headHeight) + } + + // Record every numbered header request issued while the ancestor search + // runs, that is everything before the post-search init hook fires: the + // span search samples only at or below the head and the binary search + // probes below it, so any request above the head means one of the caps + // regressed. Recording stops at the hook because the header fetch phase + // that follows legitimately reaches above the head. + var ( + searching = true + origin uint64 + requested []uint64 + ) + tester.peers["peer"].requestHook = func(from uint64, amount int, skip int, reverse bool) { + if searching { + requested = append(requested, from) + } + } + tester.downloader.syncInitHook = func(from, _ uint64) { + searching = false + origin = from + } + + if err := tester.sync("peer", nil, FullSync); err != nil { + t.Fatalf("failed to synchronise blocks: %v", err) + } + if len(requested) < 2 { + t.Fatalf("span search resolved the ancestor, binary search not exercised: %d request(s)", len(requested)) + } + for i, probe := range requested { + if probe > headHeight { + t.Fatalf("ancestor search request #%d probes above the head: %d > %d", i, probe, headHeight) + } + } + if origin != forkPoint-1 { + t.Fatalf("sync resumed at the wrong ancestor: origin %d want %d", origin, forkPoint-1) + } + if have := tester.CurrentBlock().Number.Uint64(); have != uint64(chainLen-1) { + t.Fatalf("unexpected head after sync: have %d want %d", have, chainLen-1) + } +} + +// TestWriteBlockWithoutStateRequiresImportedParent asserts that the helper +// reports an unimported parent instead of silently depending on call order +// and dereferencing a missing total difficulty. +func TestWriteBlockWithoutStateRequiresImportedParent(t *testing.T) { + t.Parallel() + + tester := newTester() + defer tester.terminate() + + chain := testChainBase.shorten(3) + // Block at height 2 has an unimported parent, as only the genesis is + // known to the simulated chain. + block := chain.blockm[chain.chain[2]] + if err := tester.writeBlockWithoutState(block); err == nil { + t.Fatalf("writeBlockWithoutState accepted block %s with unimported parent %s", block.Hash(), block.ParentHash()) + } + if _, ok := tester.ownChainTd[block.Hash()]; ok { + t.Fatalf("writeBlockWithoutState recorded a total difficulty for block %s despite the unimported parent", block.Hash()) + } +} + +// TestWriteBlockWithoutStateDoesNotAdvanceHeads pins the side chain semantics +// of the helper: a block stored by hash without state is known to the chain +// but never enters the canonical hash chain, so the head getters keep +// reporting the imported head. CurrentSnapBlock in particular feeds the local +// height findAncestor derives in fast sync; letting stubs raise it would mask +// a sync that resumes above the head. +func TestWriteBlockWithoutStateDoesNotAdvanceHeads(t *testing.T) { + t.Parallel() + + tester := newTester() + defer tester.terminate() + + const headHeight = uint64(4) + chain := testChainBase.shorten(int(headHeight) + 4) + + imported := make([]*types.Block, 0, headHeight) + for i := uint64(1); i <= headHeight; i++ { + imported = append(imported, chain.blockm[chain.chain[i]]) + } + if _, err := tester.InsertChain(imported); err != nil { + t.Fatalf("failed to import blocks: %v", err) + } + for i := headHeight + 1; i < uint64(chain.len()); i++ { + if err := tester.writeBlockWithoutState(chain.blockm[chain.chain[i]]); err != nil { + t.Fatalf("failed to write block without state: %v", err) + } + } + + heads := []struct { + name string + have uint64 + }{ + {"CurrentHeader", tester.CurrentHeader().Number.Uint64()}, + {"CurrentBlock", tester.CurrentBlock().Number.Uint64()}, + {"CurrentSnapBlock", tester.CurrentSnapBlock().Number.Uint64()}, + } + for _, head := range heads { + if head.have != headHeight { + t.Errorf("%s reports %d, want the imported head %d", head.name, head.have, headHeight) + } + } +} + +// TestUsableAsAncestorBoundaries checks the head-height guard of usableAsAncestor in +// every sync mode: a block at the local head is an acceptable ancestor, while +// a block one above the head is rejected even when it is known by hash. Side +// chain blocks are written by hash without state, so knowledge above the head +// is exactly what used to let the sync resume above itself. The span search +// caps its acceptance window below the head, so this white-box pin is the +// guard's only coverage; the end-to-end tests exercise the cap instead. +func TestUsableAsAncestorBoundaries(t *testing.T) { + t.Parallel() + + tester := newTester() + defer tester.terminate() + + const headHeight = 4 + chain := testChainBase.shorten(headHeight + 2) + + imported := make([]*types.Block, 0, headHeight) + for i := 1; i <= headHeight; i++ { + imported = append(imported, chain.blockm[chain.chain[i]]) + } + if _, err := tester.InsertChain(imported); err != nil { + t.Fatalf("failed to import blocks: %v", err) + } + above := chain.blockm[chain.chain[headHeight+1]] + if err := tester.writeBlockWithoutState(above); err != nil { + t.Fatalf("failed to write block without state: %v", err) + } + + // The fake chain reports fast-sync knowledge through receipts, so give the + // head and the above-head block receipts the way a fast-synced segment + // would have them. This makes the above-head block known in every mode, so + // only the height guard can reject it. + receiptBlocks := make(types.Blocks, 0, headHeight+1) + receiptBlocks = append(receiptBlocks, imported...) + receiptBlocks = append(receiptBlocks, above) + receipts := make([]types.Receipts, 0, headHeight+1) + for i := 0; i <= headHeight; i++ { + receipts = append(receipts, types.Receipts{}) + } + if _, err := tester.InsertReceiptChain(receiptBlocks, receipts); err != nil { + t.Fatalf("failed to insert receipts: %v", err) + } + if have := tester.CurrentBlock().Number.Uint64(); have != headHeight { + t.Fatalf("unexpected head: have %d want %d", have, headHeight) + } + + downloader := tester.downloader + head := chain.blockm[chain.chain[headHeight]] + for _, mode := range []SyncMode{FullSync, FastSync, LightSync} { + if !downloader.usableAsAncestor(mode, head.Hash(), headHeight, headHeight) { + t.Errorf("mode %s: block at the head rejected as ancestor", mode) + } + if downloader.usableAsAncestor(mode, above.Hash(), headHeight+1, headHeight) { + t.Errorf("mode %s: block above the head accepted as ancestor", mode) + } + } +} + // Tests that if a large batch of blocks are being downloaded, it is throttled // until the cached blocks are retrieved. func TestThrottling100Full(t *testing.T) { testThrottling(t, xdc100, FullSync) } @@ -1761,67 +2423,95 @@ func (ftp *floodingTestPeer) RequestHeadersByNumber(from uint64, count, skip int return nil } -// TestRemoteHeaderRequestSpan tests remote header request span. +// TestRemoteHeaderRequestSpan tests the header request span calculated from +// the remote head height and the local head, the exact pair of heights +// findAncestor passes, so the table pins the wire requests the call site can +// really issue, with the sampling top derived one below the remote head and +// capped at the local head. func TestRemoteHeaderRequestSpan(t *testing.T) { testCases := []struct { remoteHeight uint64 localHeight uint64 - expected []int + samples []int + max uint64 }{ - // Remote is way higher. We should ask for the remote head and go backwards - {1500, 1000, - []int{1323, 1339, 1355, 1371, 1387, 1403, 1419, 1435, 1451, 1467, 1483, 1499}, - }, - {15000, 13006, - []int{14823, 14839, 14855, 14871, 14887, 14903, 14919, 14935, 14951, 14967, 14983, 14999}, - }, + // The remote head height is the exact first argument findAncestor + // passes, so every row is a call-site reachable input. The sampling + // top is derived one block below it and capped at the local head + // inside the function. + // Remote is way higher: the cap pulls the request down to the two + // blocks just below the local head. + {remoteHeight: 1500, localHeight: 1000, samples: []int{998, 1000}, max: 1000}, + {remoteHeight: 15000, localHeight: 13006, samples: []int{13004, 13006}, max: 13006}, //Remote is pretty close to us. We don't have to fetch as many - {1200, 1150, - []int{1149, 1154, 1159, 1164, 1169, 1174, 1179, 1184, 1189, 1194, 1199}, - }, + {remoteHeight: 1200, localHeight: 1150, samples: []int{1148, 1150}, max: 1150}, // Remote is equal to us (so on a fork with higher td) // We should get the closest couple of ancestors - {1500, 1500, - []int{1497, 1499}, - }, + {remoteHeight: 1500, localHeight: 1500, samples: []int{1497, 1499}, max: 1499}, // We're higher than the remote! Odd - {1000, 1500, - []int{997, 999}, - }, - // Check some weird edgecases that it behaves somewhat rationally - {0, 1500, - []int{0, 2}, - }, - {6000000, 0, - []int{5999823, 5999839, 5999855, 5999871, 5999887, 5999903, 5999919, 5999935, 5999951, 5999967, 5999983, 5999999}, - }, - {0, 0, - []int{0, 2}, - }, - } - reqs := func(from, count, span int) []int { + {remoteHeight: 1000, localHeight: 1500, samples: []int{997, 999}, max: 999}, + // Check some weird edgecases that it behaves somewhat rationally. When + // the raw start is negative it is clamped up to zero while the count is + // kept, so the wire request can top out past the sampling top and max + // follows the raw window; candidates above the local head are rejected + // by usableAsAncestor, not by the window. + {remoteHeight: 1, localHeight: 1500, samples: []int{0, 2}, max: 2}, + // The cap pulls the sampling top all the way down to the genesis head. + {remoteHeight: 6000000, localHeight: 0, samples: []int{0, 2}, max: 2}, + // A remote at genesis skips the below-head decrement and derives the + // same clamped window. + {remoteHeight: 0, localHeight: 0, samples: []int{0, 2}, max: 2}, + // The clamped window: with the remote head at 2 the sampling top + // lands on 1, so the raw start is negative, 'from' pins to zero and + // the kept count tops the window out one block past the sampling + // top. max follows the raw window in that case; anything above the + // local head is rejected by usableAsAncestor, not by the window. Note the + // head-at-1 entry: the clamped window cannot sample the head itself, + // so findAncestor recovers it by refining the genesis hit over the + // unsampled gap (TestFindAncestorSpanRejectsAboveHeadCandidate). + {remoteHeight: 2, localHeight: 1, samples: []int{0, 2}, max: 2}, + // The local head itself is a legitimate ancestor sample even when + // the sampling top sits below it (here the top is head-1, as whenever + // the remote is at the local height): the raw start is negative, and + // keeping the count after clamping 'from' to zero is what tops the + // window out at the head. Anything above the head is still rejected + // by usableAsAncestor, not by the window. + {remoteHeight: 2, localHeight: 2, samples: []int{0, 2}, max: 2}, + // With the remote exactly one block ahead the sampling top lands on + // the head without the cap being involved: 'from' stays non-negative + // and the window tops out at the head itself, so the head is a + // direct sample. + {remoteHeight: 3, localHeight: 2, samples: []int{0, 2}, max: 2}, + // The cap fires with room to spare: the top pins to the head with + // 'from' landing exactly at zero, unclamped. + {remoteHeight: 4, localHeight: 2, samples: []int{0, 2}, max: 2}, + } + reqs := func(from int) []int { var r []int num := from - for len(r) < count { + for len(r) < spanSampleCount { r = append(r, num) - num += span + 1 + num += spanSampleSkip + 1 } return r } for i, tt := range testCases { - from, count, span, max := calculateRequestSpan(tt.remoteHeight, tt.localHeight) - data := reqs(int(from), count, span) - - if max != uint64(data[len(data)-1]) { - t.Errorf("test %d: wrong last value %d != %d", i, data[len(data)-1], max) + from, max := calculateRequestSpan(tt.remoteHeight, tt.localHeight) + data := reqs(int(from)) + + // Cross-check the returned max against the request window itself: + // max must stay in lockstep with the issued request, since findAncestor + // uses it to accept or discard the served headers. + if max != tt.max || max != uint64(data[len(data)-1]) { + t.Errorf("test %d: wrong max %d != %d, last sample %d", i, max, tt.max, data[len(data)-1]) } failed := false - if len(data) != len(tt.expected) { + if len(data) != len(tt.samples) { failed = true - t.Errorf("test %d: length wrong, expected %d got %d", i, len(tt.expected), len(data)) + t.Errorf("test %d: length wrong, expected %d got %d", i, len(tt.samples), len(data)) } else { for j, n := range data { - if n != tt.expected[j] { + if n != tt.samples[j] { failed = true break } @@ -1829,7 +2519,7 @@ func TestRemoteHeaderRequestSpan(t *testing.T) { } if failed { res := strings.Replace(fmt.Sprint(data), " ", ",", -1) - exp := strings.Replace(fmt.Sprint(tt.expected), " ", ",", -1) + exp := strings.Replace(fmt.Sprint(tt.samples), " ", ",", -1) fmt.Printf("got: %v\n", res) fmt.Printf("exp: %v\n", exp) t.Errorf("test %d: wrong values", i)