diff --git a/core/blockchain.go b/core/blockchain.go index ca337fb42a4f..1874de9a9667 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1441,7 +1441,12 @@ func (bc *BlockChain) InsertReceiptChain(blockChain types.Blocks, receiptChain [ head := blockChain[len(blockChain)-1] if td := bc.GetTd(head.Hash(), head.NumberU64()); td != nil { // Rewind may have occurred, skip in that case currentSnapBlock := bc.CurrentSnapBlock() - if bc.GetTd(currentSnapBlock.Hash(), currentSnapBlock.Number.Uint64()).Cmp(td) < 0 { + snapTd := bc.GetTd(currentSnapBlock.Hash(), currentSnapBlock.Number.Uint64()) + if snapTd == nil { + // See writeBlockWithState: a missing local TD is treated as zero. + snapTd = new(big.Int) + } + if snapTd.Cmp(td) < 0 { rawdb.WriteHeadFastBlockHash(bc.db, head.Hash()) bc.currentSnapBlock.Store(head.Header()) headFastBlockGauge.Update(int64(head.NumberU64())) @@ -1481,6 +1486,98 @@ func (bc *BlockChain) writeBlockWithoutState(block *types.Block, td *big.Int) (e return nil } +// RepairMissingTd reconstructs and persists the total difficulty entries for +// the canonical chain segment leading to the given head, which may be missing +// on legacy XDPoS chaindata that predates the TD index. It walks the canonical +// parents from the head down to the nearest ancestor with a known TD entry +// (or genesis, which always has one) and writes back the accumulated values. +// +// Total difficulty values are immutable chain facts, so the repair does not +// need to hold the chain mutex: concurrent inserts can only ever write the +// same value for the same block, and the head is snapshotted at entry. The +// method returns the head's total difficulty, or nil if the head is unknown +// or the node is shutting down. +func (bc *BlockChain) RepairMissingTd(headHash common.Hash, headNumber uint64) *big.Int { + start := time.Now() + + // Fast path: nothing to repair. + if td := bc.GetTd(headHash, headNumber); td != nil { + return td + } + // Walk the canonical parent chain collecting the segment with missing TDs, + // stopping at the first ancestor with a known entry. + type missingTd struct { + hash common.Hash + number uint64 + difficulty *big.Int + } + var ( + segment []missingTd + base *big.Int + visited int + ) + for hash, number := headHash, headNumber; ; { + header := bc.GetHeader(hash, number) + if header == nil { + return nil + } + if td := bc.GetTd(hash, number); td != nil { + base = td + break + } + segment = append(segment, missingTd{hash: hash, number: number, difficulty: header.Difficulty}) + visited++ + if visited%100000 == 0 { + log.Info("Repairing missing total difficulties", "blocks", visited, "number", number, "head", headNumber, "elapsed", common.PrettyDuration(time.Since(start))) + } + if number == 0 { + // Genesis reached without any known TD: anchor at zero so the + // genesis entry becomes its own difficulty. + base = new(big.Int) + break + } + hash, number = header.ParentHash, number-1 + } + // Reconstruct the TDs from the anchor back towards the head, persisting + // them in chunks to bound memory usage. + const tdWriteChunk = 50000 + var ( + batch = bc.db.NewBatch() + written int + acc = new(big.Int).Set(base) + ) + flush := func() { + if written == 0 { + return + } + if err := batch.Write(); err != nil { + log.Crit("Failed to write repaired total difficulties", "err", err) + } + batch = bc.db.NewBatch() + written = 0 + } + for i := len(segment) - 1; i >= 0; i-- { + if bc.insertStopped() { + // Shutting down: persist the partial result (entries are immutable + // facts) and report the repair as incomplete. + flush() + log.Warn("Total difficulty repair interrupted", "blocks", len(segment)-i-1, "head", headNumber) + return nil + } + entry := segment[i] + acc.Add(acc, entry.difficulty) + rawdb.WriteTd(batch, entry.hash, entry.number, new(big.Int).Set(acc)) + bc.hc.tdCache.Add(entry.hash, new(big.Int).Set(acc)) + written++ + if written >= tdWriteChunk { + flush() + } + } + flush() + log.Info("Repaired missing total difficulties", "blocks", len(segment), "head", headNumber, "elapsed", common.PrettyDuration(time.Since(start))) + return acc +} + // WriteBlockWithState writes the block and all associated state to the database. func (bc *BlockChain) WriteBlockWithState(block *types.Block, receipts []*types.Receipt, state *state.StateDB, tradingState *tradingstate.TradingStateDB, lendingState *lendingstate.LendingStateDB) (status WriteStatus, err error) { if !bc.chainmu.TryLock() { @@ -1505,6 +1602,13 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. // Make sure no inconsistent state is leaked during insertion currentBlock := bc.CurrentBlock() localTd := bc.GetTd(currentBlock.Hash(), currentBlock.Number.Uint64()) + if localTd == nil { + // Legacy XDPoS chaindata may predate the TD index, leaving the + // current head without a TD entry. The sync layer repairs the entry + // before downloading, but a still-missing value is treated as zero + // (i.e. lower than any real TD) to avoid a nil comparison panic. + localTd = new(big.Int) + } externTd := new(big.Int).Add(block.Difficulty(), ptd) // Irrelevant of the canonical status, write the block itself to the database. @@ -2106,6 +2210,10 @@ func (bc *BlockChain) insertSidechain(block *types.Block, it *insertIterator) (i // If the externTd was larger than our local TD, we now need to reimport the previous // blocks to regenerate the required state localTd := bc.GetTd(bc.CurrentBlock().Hash(), current) + if localTd == nil { + // See writeBlockWithState: a missing local TD is treated as zero. + localTd = new(big.Int) + } if localTd.Cmp(externTd) > 0 { log.Info("Sidechain written to disk", "start", it.first().NumberU64(), "end", it.previous().Number, "sidetd", externTd, "localtd", localTd) return it.index, nil, nil, err @@ -2236,6 +2344,10 @@ func (bc *BlockChain) getResultBlock(block *types.Block, verifiedM2 bool) (*Resu // until the competitor TD goes above the canonical TD currentBlock := bc.CurrentBlock() localTd := bc.GetTd(currentBlock.Hash(), currentBlock.Number.Uint64()) + if localTd == nil { + // See writeBlockWithState: a missing local TD is treated as zero. + localTd = new(big.Int) + } externTd := new(big.Int).Add(bc.GetTd(block.ParentHash(), block.NumberU64()-1), block.Difficulty()) if localTd.Cmp(externTd) > 0 { return nil, err diff --git a/core/blockchain_test.go b/core/blockchain_test.go index dc747e5e2190..0ec7b4762c67 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -2890,3 +2890,143 @@ func TestDeleteCreateRevert(t *testing.T) { t.Fatalf("block %d: failed to insert into chain: %v", n, err) } } + +// TestWriteHeaderMissingLocalTd exercises the defensive nil handling in +// HeaderChain.WriteHeader: a fork rooted below the head must be importable +// when the current head's total difficulty entry is missing (legacy chaindata +// predating the TD index), without panicking in the fork-choice comparison. +func TestWriteHeaderMissingLocalTd(t *testing.T) { + engine := ethash.NewFaker() + db, _, blockchain, err := newCanonical(engine, 5, false) + if err != nil { + t.Fatalf("failed to create canonical header chain: %v", err) + } + defer blockchain.Stop() + + // Build a side fork rooted at block 3, so the fork parent has a TD entry + // while the canonical head's entry has been removed below. + fork := makeHeaderChain(blockchain.chainConfig, blockchain.GetHeaderByNumber(3), 2, engine, db, forkSeed) + + head := blockchain.CurrentHeader() + rawdb.DeleteTd(blockchain.ChainDb(), head.Hash(), head.Number.Uint64()) + blockchain.hc.tdCache.Purge() + + if _, err := blockchain.InsertHeaderChain(fork, 1); err != nil { + t.Fatalf("failed to insert fork with missing local TD: %v", err) + } + if got := blockchain.CurrentHeader().Hash(); got != fork[len(fork)-1].Hash() { + t.Errorf("canonical head mismatch: have %v, want %v", got, fork[len(fork)-1].Hash()) + } +} + +// TestWriteBlockWithStateMissingLocalTd exercises the defensive nil handling +// in writeBlockWithState: importing a fork rooted below the head must not +// panic when the current head's TD entry is missing (legacy chaindata +// predating the TD index). +func TestWriteBlockWithStateMissingLocalTd(t *testing.T) { + engine := ethash.NewFaker() + db, _, blockchain, err := newCanonical(engine, 5, true) + if err != nil { + t.Fatalf("failed to create canonical block chain: %v", err) + } + defer blockchain.Stop() + + fork := makeBlockChain(blockchain.chainConfig, blockchain.GetBlockByNumber(3), 2, engine, db, forkSeed) + + head := blockchain.CurrentBlock() + rawdb.DeleteTd(blockchain.ChainDb(), head.Hash(), head.Number.Uint64()) + blockchain.hc.tdCache.Purge() + + if _, err := blockchain.InsertChain(fork); err != nil { + t.Fatalf("failed to insert fork with missing local TD: %v", err) + } + if got := blockchain.CurrentBlock().Hash(); got != fork[len(fork)-1].Hash() { + t.Errorf("canonical head mismatch: have %v, want %v", got, fork[len(fork)-1].Hash()) + } +} + +// TestRepairMissingTd verifies that RepairMissingTd rebuilds the missing TD +// entries along the canonical chain leading to the head, stopping at the +// nearest ancestor with a known TD, and that the operation is idempotent. +func TestRepairMissingTd(t *testing.T) { + engine := ethash.NewFaker() + _, _, blockchain, err := newCanonical(engine, 8, true) + if err != nil { + t.Fatalf("failed to create canonical block chain: %v", err) + } + defer blockchain.Stop() + + head := blockchain.CurrentBlock() + want := blockchain.GetTd(head.Hash(), head.Number.Uint64()) + if want == nil { + t.Fatalf("expected a TD entry before deletion") + } + // Simulate legacy chaindata: drop the TD entries of the last three blocks. + for num := head.Number.Uint64(); num > head.Number.Uint64()-3; num-- { + block := blockchain.GetBlockByNumber(num) + rawdb.DeleteTd(blockchain.ChainDb(), block.Hash(), num) + } + blockchain.hc.tdCache.Purge() + + got := blockchain.RepairMissingTd(head.Hash(), head.Number.Uint64()) + if got == nil || got.Cmp(want) != 0 { + t.Fatalf("repaired head TD mismatch: have %v, want %v", got, want) + } + // The repaired head TD must be persisted. + if td := blockchain.GetTd(head.Hash(), head.Number.Uint64()); td == nil || td.Cmp(want) != 0 { + t.Fatalf("head TD not persisted after repair: have %v, want %v", td, want) + } + // The intermediate ancestors must have been repaired as well. + for num := head.Number.Uint64() - 2; num <= head.Number.Uint64(); num++ { + block := blockchain.GetBlockByNumber(num) + if td := blockchain.GetTd(block.Hash(), num); td == nil { + t.Errorf("ancestor %d TD not repaired", num) + } + } + // A second call is a no-op returning the same value. + if again := blockchain.RepairMissingTd(head.Hash(), head.Number.Uint64()); again == nil || again.Cmp(want) != 0 { + t.Fatalf("idempotent repair mismatch: have %v, want %v", again, want) + } +} + +// TestRepairMissingTdHeaderChain verifies repair on a header-only chain, the +// shape used by the snap-sync header phase. +func TestRepairMissingTdHeaderChain(t *testing.T) { + engine := ethash.NewFaker() + _, _, blockchain, err := newCanonical(engine, 8, false) + if err != nil { + t.Fatalf("failed to create canonical header chain: %v", err) + } + defer blockchain.Stop() + + head := blockchain.CurrentHeader() + want := blockchain.GetTd(head.Hash(), head.Number.Uint64()) + if want == nil { + t.Fatalf("expected a TD entry before deletion") + } + for num := head.Number.Uint64(); num > head.Number.Uint64()-3; num-- { + header := blockchain.GetHeaderByNumber(num) + rawdb.DeleteTd(blockchain.ChainDb(), header.Hash(), num) + } + blockchain.hc.tdCache.Purge() + + got := blockchain.RepairMissingTd(head.Hash(), head.Number.Uint64()) + if got == nil || got.Cmp(want) != 0 { + t.Fatalf("repaired head TD mismatch: have %v, want %v", got, want) + } +} + +// TestRepairMissingTdUnknownHead verifies that RepairMissingTd reports failure +// for an unknown head instead of panicking or corrupting the database. +func TestRepairMissingTdUnknownHead(t *testing.T) { + engine := ethash.NewFaker() + _, _, blockchain, err := newCanonical(engine, 2, true) + if err != nil { + t.Fatalf("failed to create canonical block chain: %v", err) + } + defer blockchain.Stop() + + if td := blockchain.RepairMissingTd(common.Hash{1}, 12345); td != nil { + t.Fatalf("expected nil for unknown head, got %v", td) + } +} diff --git a/core/headerchain.go b/core/headerchain.go index 2fa0a4d635d4..9f20f0e7fb1f 100644 --- a/core/headerchain.go +++ b/core/headerchain.go @@ -147,6 +147,13 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er return NonStatTy, consensus.ErrUnknownAncestor } localTd := hc.GetTd(hc.currentHeaderHash, hc.CurrentHeader().Number.Uint64()) + if localTd == nil { + // Legacy XDPoS chaindata may predate the TD index, leaving the + // current head without a TD entry. The sync layer repairs the entry + // before downloading, but a still-missing value is treated as zero + // (i.e. lower than any real TD) to avoid a nil comparison panic. + localTd = new(big.Int) + } externTd := new(big.Int).Add(header.Difficulty, ptd) // Irrelevant of the canonical status, write the td and header to the database diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 1ae7f9d8c57f..c9ba6ac18f2f 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -126,6 +126,11 @@ type Downloader struct { synchronising int32 notified int32 committed int32 + // missingTdWarned is set after the missing-TD warning was logged once and is + // deliberately never reset, so the warning fires at most once per downloader + // lifetime. missingTdCounter keeps counting every occurrence, leaving + // operators an ongoing signal for a persistent condition. + missingTdWarned uint32 // Pivot block configuration (set before sync starts) pivotNumber uint64 // Fixed pivot block number (0 = use default calculation) @@ -1352,6 +1357,41 @@ func (d *Downloader) fetchParts(deliveryCh chan dataPack, deliver func(dataPack) } } +// warnMissingTd reports a missing local total difficulty entry that made the +// stalling-peer check unverifiable, causing the peer to be treated as +// stalling. Header insertion writes TDs atomically with the headers, so this +// only happens when the peer delivered nothing and the local head predates +// the TD index (legacy XDPoS chaindata). The occurrence is counted on every +// cycle so operators can still observe a persistent condition, but the +// warning is logged only once per downloader lifetime to avoid spamming each +// sync cycle. +func (d *Downloader) warnMissingTd(number uint64, hash common.Hash) { + missingTdCounter.Inc(1) + if atomic.CompareAndSwapUint32(&d.missingTdWarned, 0, 1) { + log.Warn("Treating peer as stalling, local TD missing", "number", number, "hash", hash) + } else { + log.Debug("Treating peer as stalling, local TD missing", "number", number, "hash", hash) + } +} + +// checkStallingPeer returns errStallingPeer if the promised total difficulty +// exceeds the local TD of the given head, i.e. the peer bailed out of +// delivering the chain it promised. A missing local TD entry (legacy XDPoS +// chaindata predating the TD index) makes the promise unverifiable; the +// condition is reported via warnMissingTd and conservatively treated as +// stalling, since returning success would silently accept an unverifiable TD +// promise (and td.Cmp(nil) would panic). +func (d *Downloader) checkStallingPeer(td, localTD *big.Int, head *types.Header) error { + if localTD == nil { + d.warnMissingTd(head.Number.Uint64(), head.Hash()) + return errStallingPeer + } + if td.Cmp(localTD) > 0 { + return errStallingPeer + } + return nil +} + // processHeaders takes batches of retrieved headers from an input channel and // keeps processing and scheduling them into the header chain and downloader's // queue until the stream ends or a failure occurs. @@ -1425,8 +1465,15 @@ func (d *Downloader) processHeaders(origin uint64, pivot uint64, td *big.Int) er // R: Nothing to give if mode != LightSync { head := d.blockchain.CurrentBlock() - if !gotHeaders && td.Cmp(d.blockchain.GetTd(head.Hash(), head.Number.Uint64())) > 0 { - return errStallingPeer + if !gotHeaders { + // A missing TD is only possible on legacy XDPoS chaindata that + // predates the TD index; upstream geth assumes it always exists. + // This branch only runs when the peer delivered nothing, so + // treating the unverifiable promise as stalling cannot penalise + // a peer that fed headers. + if err := d.checkStallingPeer(td, d.blockchain.GetTd(head.Hash(), head.Number.Uint64()), head); err != nil { + return err + } } } // If fast or light syncing, ensure promised headers are indeed delivered. This is @@ -1441,8 +1488,14 @@ func (d *Downloader) processHeaders(origin uint64, pivot uint64, td *big.Int) er 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 + // Header insertion writes the TD atomically with the header, so a + // missing TD only triggers when no headers were delivered this + // cycle and the current head predates the TD index (legacy XDPoS + // chaindata); the peer is then treated as stalling. The post-pivot + // attack detection is unaffected whenever the peer delivered + // anything. + if err := d.checkStallingPeer(td, d.lightchain.GetTd(head.Hash(), head.Number.Uint64()), head); err != nil { + return err } } // Disable any rollback and return diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index c872c27ac393..8a75314fd3f9 100644 --- a/eth/downloader/downloader_test.go +++ b/eth/downloader/downloader_test.go @@ -75,6 +75,12 @@ type downloadTester struct { insertHeaderChainHook func([]*types.Header) error + // missingTdLookups counts GetTd lookups that return nil, i.e. the hits of + // the nil-TD guard in the stalling-peer checks. Counting per tester keeps + // test assertions independent of the shared process-wide metric, which + // parallel test siblings would otherwise advance as well. + missingTdLookups uint64 + // 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 @@ -256,7 +262,11 @@ func (dl *downloadTester) GetTd(hash common.Hash, number uint64) *big.Int { // ancients or own blocks). // This method assumes that the caller holds at least the read-lock (dl.lock) func (dl *downloadTester) getTd(hash common.Hash) *big.Int { - return dl.ownChainTd[hash] + td := dl.ownChainTd[hash] + if td == nil { + atomic.AddUint64(&dl.missingTdLookups, 1) + } + return td } // InsertHeaderChain injects a new batch of headers into the simulated chain. @@ -1150,6 +1160,64 @@ func testHighTDStarvationAttack(t *testing.T, protocol int, mode SyncMode) { tester.terminate() } +// Tests that a missing local TD for the chain head neither crashes the +// stalling-peer checks nor silently accepts an unverifiable TD promise. +// GetTd returns nil when the TD is absent from the database (e.g. legacy +// chaindata), which td.Cmp(nil) would turn into a panic. The promised TD +// cannot be checked against a local value, so the peer is conservatively +// treated as stalling and the sync fails instead of succeeding. +func TestMissingLocalTd100Full(t *testing.T) { testMissingLocalTd(t, xdc100, FullSync) } +func TestMissingLocalTd100Fast(t *testing.T) { testMissingLocalTd(t, xdc100, FastSync) } +func TestMissingLocalTd164Full(t *testing.T) { testMissingLocalTd(t, xdc164, FullSync) } +func TestMissingLocalTd164Fast(t *testing.T) { testMissingLocalTd(t, xdc164, FastSync) } +func TestMissingLocalTd164Light(t *testing.T) { testMissingLocalTd(t, xdc164, LightSync) } +func TestMissingLocalTd165Full(t *testing.T) { testMissingLocalTd(t, xdc165, FullSync) } +func TestMissingLocalTd165Fast(t *testing.T) { testMissingLocalTd(t, xdc165, FastSync) } +func TestMissingLocalTd165Light(t *testing.T) { testMissingLocalTd(t, xdc165, LightSync) } + +func testMissingLocalTd(t *testing.T, protocol int, mode SyncMode) { + t.Parallel() + + tester := newTester() + defer tester.terminate() + + // Drop the TD of the genesis head, simulating chaindata without a TD entry + // for the header the stalling-peer checks resolve to. The peer chain must + // stay genesis-only: the harness derives TDs from the parent header on + // insertion, so any delivered header would make InsertHeaderChain panic on + // the missing parent TD. + delete(tester.ownChainTd, tester.genesis.Hash()) + + chain := testChainBase.shorten(1) + tester.newPeer("peer", protocol, chain) + // The promised TD is set far above the local genesis TD, so the + // stalling-peer check must fire even though the local TD is unknown. + // Sync twice: the second cycle repeats the missing-TD path and must + // not panic or change the outcome. Each cycle must advance the + // tester's nil-TD lookup count, proving the guard was taken instead of + // the comparison crashing. Counting on the tester itself keeps the + // assertion independent of the shared process-wide metric, which + // parallel tests would otherwise advance as well. + for i := 0; i < 2; i++ { + tdBefore := atomic.LoadUint64(&tester.missingTdLookups) + if err := tester.sync("peer", big.NewInt(1000000), mode); !errors.Is(err, errStallingPeer) { + t.Fatalf("Synchronisation error mismatch (cycle %d): have %v, want errStallingPeer", i, err) + } + if atomic.LoadUint64(&tester.missingTdLookups) == tdBefore { + t.Fatalf("Missing TD guard not taken in cycle %d: nil-TD lookup count did not advance", i) + } + } + // The public synchronisation entry point drops the peer on + // errStallingPeer, so a lying peer cannot be re-selected by later + // forced syncs. + if err := tester.downloader.Synchronise("peer", tester.genesis.Hash(), big.NewInt(1000000), FullSync); !errors.Is(err, errStallingPeer) { + t.Fatalf("Synchronisation error mismatch (public entry): have %v, want errStallingPeer", err) + } + if _, ok := tester.peers["peer"]; ok { + t.Fatalf("peer not dropped after stalling detection") + } +} + // 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 diff --git a/eth/downloader/metrics.go b/eth/downloader/metrics.go index 4f49c4ed6bfc..0d7673beafb6 100644 --- a/eth/downloader/metrics.go +++ b/eth/downloader/metrics.go @@ -42,4 +42,6 @@ var ( stateDropMeter = metrics.NewRegisteredMeter("eth/downloader/states/drop", nil) throttleCounter = metrics.NewRegisteredCounter("eth/downloader/throttle", nil) + + missingTdCounter = metrics.NewRegisteredCounter("eth/downloader/headers/missingtd", nil) ) diff --git a/eth/handler.go b/eth/handler.go index 1d2b63c17931..d96654a1c889 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -74,6 +74,11 @@ type ProtocolManager struct { snapSync uint32 // Flag whether snap sync is enabled (gets disabled if we already have blocks) acceptTxs uint32 // Flag whether we're considered synchronised (enables transaction processing) + // missingTdWarned is set after the missing-TD warning was logged once and + // is deliberately never reset, so the warning fires at most once per + // protocol manager lifetime. + missingTdWarned uint32 + txpool txPool orderpool orderPool lendingpool lendingPool diff --git a/eth/helper_test.go b/eth/helper_test.go index 62a380db83c3..db8be1f83242 100644 --- a/eth/helper_test.go +++ b/eth/helper_test.go @@ -26,6 +26,7 @@ import ( "math/big" "sort" "sync" + "sync/atomic" "testing" "github.com/XinFinOrg/XDPoSChain/common" @@ -56,6 +57,38 @@ var ( // with the given number of blocks already known, and potential notification // channels for different events. func newTestProtocolManager(mode downloader.SyncMode, blocks int, generator func(int, *core.BlockGen), newtx chan<- []*types.Transaction) (*ProtocolManager, ethdb.Database, error) { + pm, db, err := buildTestProtocolManager(mode, blocks, generator, newtx) + if err != nil { + return nil, nil, err + } + pm.Start(1000) + return pm, db, nil +} + +// newTestProtocolManagerPassive is like newTestProtocolManager, except that the +// background loops are not started. The manager only serves protocol requests +// and never initiates a synchronization of its own, which makes it usable as +// the remote side of a synchronization test. +// +// The returned manager must not be torn down with ProtocolManager.Stop: its +// event subscriptions are nil without Start and would crash the teardown. +// Terminating the downloader is sufficient cleanup. +func newTestProtocolManagerPassive(mode downloader.SyncMode, blocks int, generator func(int, *core.BlockGen), newtx chan<- []*types.Transaction) (*ProtocolManager, ethdb.Database, error) { + pm, db, err := buildTestProtocolManager(mode, blocks, generator, newtx) + if err != nil { + return nil, nil, err + } + // The background loops are intentionally left unstarted, but the peer + // limit must be set so that handle accepts incoming connections. + pm.maxPeers = 1000 + return pm, db, nil +} + +// buildTestProtocolManager creates a new protocol manager for testing purposes, +// with the given number of blocks already known. The background loops are left +// unstarted, so the caller can choose between newTestProtocolManager and +// newTestProtocolManagerPassive. +func buildTestProtocolManager(mode downloader.SyncMode, blocks int, generator func(int, *core.BlockGen), newtx chan<- []*types.Transaction) (*ProtocolManager, ethdb.Database, error) { var ( evmux = new(event.TypeMux) engine = ethash.NewFaker() @@ -78,7 +111,6 @@ func newTestProtocolManager(mode downloader.SyncMode, blocks int, generator func if err != nil { return nil, nil, err } - pm.Start(1000) return pm, db, nil } @@ -94,6 +126,108 @@ func newTestProtocolManagerMust(t *testing.T, mode downloader.SyncMode, blocks i return pm, db } +// newTestProtocolManagerPassiveMust creates a new passive protocol manager for +// testing purposes, with the given number of blocks already known, and +// potential notification channels for different events. In case of an error, +// the constructor force-fails the test. +func newTestProtocolManagerPassiveMust(t *testing.T, mode downloader.SyncMode, blocks int, generator func(int, *core.BlockGen), newtx chan<- []*types.Transaction) (*ProtocolManager, ethdb.Database) { + pm, db, err := newTestProtocolManagerPassive(mode, blocks, generator, newtx) + if err != nil { + t.Fatalf("Failed to create protocol manager: %v", err) + } + return pm, db +} + +// newTestProtocolManagerWithMissingHeadTd creates a protocol manager whose +// current head has no TD entry in the database, simulating legacy XDPoS +// chaindata that predates the TD index. The chain is imported normally, the +// head TD is then deleted, and a fresh blockchain is loaded from the mutated +// database so the in-memory TD cache cannot mask the missing entry. +func newTestProtocolManagerWithMissingHeadTd(t *testing.T, mode downloader.SyncMode, blocks int) (*ProtocolManager, ethdb.Database) { + var ( + evmux = new(event.TypeMux) + engine = ethash.NewFaker() + db = rawdb.NewMemoryDatabase() + gspec = &core.Genesis{ + Alloc: types.GenesisAlloc{testBank: {Balance: new(big.Int).SetUint64(10000000000000000000)}}, + Config: params.TestChainConfig, + } + ) + genesis := gspec.MustCommit(db) + blockchain, err := core.NewBlockChain(db, nil, gspec, engine, vm.Config{}) + if err != nil { + t.Fatalf("Failed to create test blockchain: %v", err) + } + chain, _ := core.GenerateChain(gspec.Config, genesis, ethash.NewFaker(), db, blocks, nil) + if _, err := blockchain.InsertChain(chain); err != nil { + t.Fatalf("Failed to insert test chain: %v", err) + } + head := blockchain.CurrentBlock() + rawdb.DeleteTd(db, head.Hash(), head.Number.Uint64()) + + // Reload a fresh blockchain from the mutated database: the previous + // instance cached the head TD in memory, which would mask the deletion. + blockchain, err = core.NewBlockChain(db, nil, gspec, engine, vm.Config{}) + if err != nil { + t.Fatalf("Failed to reload test blockchain: %v", err) + } + pm, err := NewProtocolManager(gspec.Config, mode, ethconfig.Defaults.NetworkId, evmux, &testTxPool{pool: make(map[common.Hash]*types.Transaction)}, engine, blockchain, db) + if err != nil { + t.Fatalf("Failed to create protocol manager: %v", err) + } + pm.Start(1000) + return pm, db +} + +// newTestProtocolManagerWithUnrepairableSnapTd creates a fast sync protocol +// manager whose snap head TD is missing and cannot be reconstructed. The fast +// head points at a block whose TD entry and parent header were deleted from +// the database, so RepairMissingTd walks into a dead end and fails while the +// canonical block head keeps its TD entry. Fast sync mode is re-enabled +// explicitly: NewProtocolManager disables it on non-empty chains, and the +// snap branch under test is only reachable with fast sync active. +func newTestProtocolManagerWithUnrepairableSnapTd(t *testing.T) (*ProtocolManager, ethdb.Database) { + var ( + evmux = new(event.TypeMux) + engine = ethash.NewFaker() + db = rawdb.NewMemoryDatabase() + gspec = &core.Genesis{ + Alloc: types.GenesisAlloc{testBank: {Balance: new(big.Int).SetUint64(10000000000000000000)}}, + Config: params.TestChainConfig, + } + ) + genesis := gspec.MustCommit(db) + blockchain, err := core.NewBlockChain(db, nil, gspec, engine, vm.Config{}) + if err != nil { + t.Fatalf("Failed to create test blockchain: %v", err) + } + chain, _ := core.GenerateChain(gspec.Config, genesis, ethash.NewFaker(), db, 512, nil) + if _, err := blockchain.InsertChain(chain); err != nil { + t.Fatalf("Failed to insert test chain: %v", err) + } + // Point the fast head at block 256 and make its TD irreparable: the TD + // entry is gone and so is the parent's header, so the repair walk fails + // at block 255 while the block head (512) keeps its TD entry. + snap := chain[255] + rawdb.WriteHeadFastBlockHash(db, snap.Hash()) + rawdb.DeleteTd(db, snap.Hash(), snap.NumberU64()) + rawdb.DeleteHeader(db, snap.ParentHash(), snap.NumberU64()-1) + + // Reload a fresh blockchain from the mutated database so the in-memory + // caches cannot mask the deleted entries. + blockchain, err = core.NewBlockChain(db, nil, gspec, engine, vm.Config{}) + if err != nil { + t.Fatalf("Failed to reload test blockchain: %v", err) + } + pm, err := NewProtocolManager(gspec.Config, downloader.FastSync, ethconfig.Defaults.NetworkId, evmux, &testTxPool{pool: make(map[common.Hash]*types.Transaction)}, engine, blockchain, db) + if err != nil { + t.Fatalf("Failed to create protocol manager: %v", err) + } + atomic.StoreUint32(&pm.snapSync, 1) + pm.Start(1000) + return pm, db +} + // testTxPool is a fake, helper transaction pool for testing purposes type testTxPool struct { txFeed event.Feed diff --git a/eth/metrics.go b/eth/metrics.go index 8b71ec401b70..2047c0166ffe 100644 --- a/eth/metrics.go +++ b/eth/metrics.go @@ -54,6 +54,10 @@ var ( miscInTrafficMeter = metrics.NewRegisteredMeter("eth/misc/in/traffic", nil) miscOutPacketsMeter = metrics.NewRegisteredMeter("eth/misc/out/packets", nil) miscOutTrafficMeter = metrics.NewRegisteredMeter("eth/misc/out/traffic", nil) + + missingTdCounter = metrics.NewRegisteredCounter("eth/sync/missingtd", nil) + + missingTdRepairedCounter = metrics.NewRegisteredCounter("eth/sync/missingtd_repaired", nil) ) // meteredMsgReadWriter is a wrapper around a p2p.MsgReadWriter, capable of diff --git a/eth/sync.go b/eth/sync.go index faa410d6491b..df80bc3f98c1 100644 --- a/eth/sync.go +++ b/eth/sync.go @@ -223,17 +223,43 @@ func (pm *ProtocolManager) syncStatusLogger() { } } +// warnMissingTd reports a missing total difficulty entry for the given chain +// head after the sync-layer repair attempt has failed. Legacy XDPoS chaindata +// predates the TD index, so the local TD may be unknown; synchronise tries to +// reconstruct it via RepairMissingTd first and only falls back to skipping +// the sync threshold checks if that fails. The occurrence is counted on every +// call so operators can observe a persistent condition, while the warning +// fires once per protocol manager lifetime to avoid spamming the logs. +func (pm *ProtocolManager) warnMissingTd(number uint64, hash common.Hash) { + missingTdCounter.Inc(1) + if atomic.CompareAndSwapUint32(&pm.missingTdWarned, 0, 1) { + log.Warn("Missing local total difficulty, sync threshold checks skipped", "number", number, "hash", hash) + } +} + // synchronise tries to sync up our local block chain with a remote peer. func (pm *ProtocolManager) synchronise(peer *peer) { // Short circuit if no peers are available if peer == nil { return } - // Make sure the peer's TD is higher than our own + // Make sure the peer's TD is higher than our own. A missing local TD + // (legacy XDPoS chaindata predating the TD index) is reconstructed from + // the canonical chain first, so the comparison can run against a real + // value. If the reconstruction fails, the cycle is skipped; the condition + // is reported once per node lifetime, see warnMissingTd. currentBlock := pm.blockchain.CurrentBlock() td := pm.blockchain.GetTd(currentBlock.Hash(), currentBlock.Number.Uint64()) pHead, pTd := peer.Head() - if pTd.Cmp(td) <= 0 { + if td == nil { + if repaired := pm.blockchain.RepairMissingTd(currentBlock.Hash(), currentBlock.Number.Uint64()); repaired != nil { + td = repaired + missingTdRepairedCounter.Inc(1) + } else { + pm.warnMissingTd(currentBlock.Number.Uint64(), currentBlock.Hash()) + } + } + if td == nil || pTd.Cmp(td) <= 0 { return } // Otherwise try to sync with the downloader @@ -253,7 +279,21 @@ func (pm *ProtocolManager) synchronise(peer *peer) { if mode == downloader.FastSync { // Make sure the peer's total difficulty we are synchronizing is higher. - if pm.blockchain.GetTdByHash(pm.blockchain.CurrentSnapBlock().Hash()).Cmp(pTd) >= 0 { + // A missing local TD is reconstructed first, mirroring the head TD + // handling above. If the reconstruction fails, the cycle is skipped: + // proceeding would hand the downloader an unverifiable TD promise and + // it would conservatively treat the peer as stalling. + snapHead := pm.blockchain.CurrentSnapBlock() + snapTd := pm.blockchain.GetTdByHash(snapHead.Hash()) + if snapTd == nil { + if repaired := pm.blockchain.RepairMissingTd(snapHead.Hash(), snapHead.Number.Uint64()); repaired != nil { + snapTd = repaired + missingTdRepairedCounter.Inc(1) + } else { + pm.warnMissingTd(snapHead.Number.Uint64(), snapHead.Hash()) + } + } + if snapTd == nil || snapTd.Cmp(pTd) >= 0 { return } } diff --git a/eth/sync_test.go b/eth/sync_test.go index 647a3a8a2b6f..24a62cbdc89d 100644 --- a/eth/sync_test.go +++ b/eth/sync_test.go @@ -70,3 +70,168 @@ func testFastSyncDisabling(t *testing.T, protocol int) { } } } + +// Tests that ProtocolManager.synchronise does not panic when the local head +// has no TD entry in the database (legacy XDPoS chaindata predating the TD +// index). The head TD is reconstructed from the canonical chain before the +// threshold check; since the peer is at the same height, the sync then returns +// early without a cycle. +func TestMissingHeadTdFullSync100(t *testing.T) { testMissingHeadTdFullSync(t, xdc100) } +func TestMissingHeadTdFullSync164(t *testing.T) { testMissingHeadTdFullSync(t, xdc164) } +func TestMissingHeadTdFullSync165(t *testing.T) { testMissingHeadTdFullSync(t, xdc165) } + +func testMissingHeadTdFullSync(t *testing.T, protocol int) { + t.Parallel() + + // Create a node whose chain is complete but whose head TD is missing, + // simulating legacy chaindata, and a passive peer with the same chain. + pm, _ := newTestProtocolManagerWithMissingHeadTd(t, downloader.FullSync, 512) + defer pm.Stop() + + peerPM, _ := newTestProtocolManagerPassiveMust(t, downloader.FullSync, 512, nil, nil) + // The passive peer manager never started, so Stop is unsafe on it; see + // newTestProtocolManagerPassive. Terminating the downloader suffices. + defer peerPM.downloader.Terminate() + + io1, io2 := p2p.MsgPipe() + defer io1.Close() + defer io2.Close() + + go pm.handle(pm.newPeer(protocol, p2p.NewPeer(enode.ID{}, "peer", nil), io1, pm.txpool.Get)) + go peerPM.handle(peerPM.newPeer(protocol, p2p.NewPeer(enode.ID{}, "victim", nil), io2, peerPM.txpool.Get)) + + // Drive the sync loop manually until the head TD has been repaired. The + // missing TD must not panic the entry checks, and once reconstructed the + // peer is not ahead of the node, so synchronise returns without a cycle. + head := pm.blockchain.CurrentBlock() + deadline := time.After(15 * time.Second) + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + + for pm.blockchain.GetTd(head.Hash(), head.Number.Uint64()) == nil { + select { + case <-deadline: + t.Fatalf("head TD not repaired by synchronise") + case <-ticker.C: + pm.synchronise(pm.peers.BestPeer()) + } + } +} + +// Tests that ProtocolManager.synchronise does not panic in fast sync mode when +// the local snap head has no TD entry in the database. The entry is repaired +// before the threshold checks, so header insertion succeeds and the fast sync +// completes with the peer still connected. +func TestMissingHeadTdFastSync100(t *testing.T) { testMissingHeadTdFastSync(t, xdc100) } +func TestMissingHeadTdFastSync164(t *testing.T) { testMissingHeadTdFastSync(t, xdc164) } +func TestMissingHeadTdFastSync165(t *testing.T) { testMissingHeadTdFastSync(t, xdc165) } + +func testMissingHeadTdFastSync(t *testing.T, protocol int) { + t.Parallel() + + // Create an empty fast sync node with the genesis TD missing and a passive + // peer carrying the full chain. + pm, _ := newTestProtocolManagerWithMissingHeadTd(t, downloader.FastSync, 0) + defer pm.Stop() + + peerPM, _ := newTestProtocolManagerPassiveMust(t, downloader.FullSync, 512, nil, nil) + // The passive peer manager never started, so Stop is unsafe on it; see + // newTestProtocolManagerPassive. Terminating the downloader suffices. + defer peerPM.downloader.Terminate() + + io1, io2 := p2p.MsgPipe() + defer io1.Close() + defer io2.Close() + + go pm.handle(pm.newPeer(protocol, p2p.NewPeer(enode.ID{}, "peer", nil), io1, pm.txpool.Get)) + go peerPM.handle(peerPM.newPeer(protocol, p2p.NewPeer(enode.ID{}, "victim", nil), io2, peerPM.txpool.Get)) + + // Drive the sync loop manually until the initial sync completes. The + // missing TD must not panic the fast sync entry checks; with the entry + // repaired, the header phase succeeds and the sync finishes. + deadline := time.After(15 * time.Second) + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + + for atomic.LoadUint32(&pm.acceptTxs) == 0 { + select { + case <-deadline: + t.Fatalf("fast sync not completed with repaired head TD") + case <-ticker.C: + pm.synchronise(pm.peers.BestPeer()) + } + } + if atomic.LoadUint32(&pm.snapSync) == 1 { + t.Fatalf("fast sync still enabled after successful sync") + } + if pm.peers.Len() == 0 { + t.Fatalf("peer dropped after successful sync with repaired head TD") + } +} + +// Tests that ProtocolManager.synchronise skips the fast sync cycle when the +// snap head TD is missing and cannot be repaired, instead of proceeding into +// the downloader with an unverifiable TD promise. The peer is ahead of the +// node, so the cycle would otherwise start; skipping keeps the peer connected +// and the node unmarked. +func TestMissingSnapTdRepairFailed100(t *testing.T) { testMissingSnapTdRepairFailed(t, xdc100) } +func TestMissingSnapTdRepairFailed164(t *testing.T) { testMissingSnapTdRepairFailed(t, xdc164) } +func TestMissingSnapTdRepairFailed165(t *testing.T) { testMissingSnapTdRepairFailed(t, xdc165) } + +func testMissingSnapTdRepairFailed(t *testing.T, protocol int) { + t.Parallel() + + pm, _ := newTestProtocolManagerWithUnrepairableSnapTd(t) + defer pm.Stop() + + peerPM, _ := newTestProtocolManagerPassiveMust(t, downloader.FullSync, 576, nil, nil) + // The passive peer manager never started, so Stop is unsafe on it; see + // newTestProtocolManagerPassive. Terminating the downloader suffices. + defer peerPM.downloader.Terminate() + + io1, io2 := p2p.MsgPipe() + defer io1.Close() + defer io2.Close() + + go pm.handle(pm.newPeer(protocol, p2p.NewPeer(enode.ID{}, "peer", nil), io1, pm.txpool.Get)) + go peerPM.handle(peerPM.newPeer(protocol, p2p.NewPeer(enode.ID{}, "victim", nil), io2, peerPM.txpool.Get)) + + // Wait for the handshake so the peer's advertised head and TD are set + // before synchronise runs the head TD comparison. + deadline := time.After(15 * time.Second) + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + for { + peer := pm.peers.BestPeer() + if peer != nil { + if _, td := peer.Head(); td != nil { + break + } + } + select { + case <-deadline: + t.Fatalf("peer handshake not completed") + case <-ticker.C: + } + } + + // synchronise must skip the cycle at the unrepairable snap TD guard. The + // call is synchronous, so afterwards no downloader cycle may have run: + // the node stays unmarked and the peer stays connected. + done := make(chan struct{}) + go func() { + pm.synchronise(pm.peers.BestPeer()) + close(done) + }() + select { + case <-done: + case <-time.After(15 * time.Second): + t.Fatalf("synchronise did not return within 15s") + } + if atomic.LoadUint32(&pm.acceptTxs) == 1 { + t.Fatalf("node marked synchronised despite unrepairable snap TD") + } + if pm.peers.Len() == 0 { + t.Fatalf("peer dropped despite unrepairable snap TD") + } +}