From 71817f318edc8c37161f38aa7a0407d4efc2429f Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Tue, 28 Nov 2023 15:38:30 +0800 Subject: [PATCH] eth/catalyst, eth/downloader: expose more sync information (#28584) This change exposes more information from sync module internally --- eth/catalyst/api.go | 7 ++--- eth/downloader/skeleton.go | 46 ++++++++++++++++----------------- eth/downloader/skeleton_test.go | 12 ++++----- trie/verkle_test.go | 8 +++--- 4 files changed, 37 insertions(+), 36 deletions(-) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index d1e1991414..37b0248f28 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -611,7 +611,8 @@ func (api *ConsensusAPI) delayPayloadImport(block *types.Block) (engine.PayloadS // Although we don't want to trigger a sync, if there is one already in // progress, try to extend if with the current payload request to relieve // some strain from the forkchoice update. - if err := api.eth.Downloader().BeaconExtend(api.eth.SyncMode(), block.Header()); err == nil { + err := api.eth.Downloader().BeaconExtend(api.eth.SyncMode(), block.Header()) + if err == nil { log.Debug("Payload accepted for sync extension", "number", block.NumberU64(), "hash", block.Hash()) return engine.PayloadStatusV1{Status: engine.SYNCING}, nil } @@ -623,12 +624,12 @@ func (api *ConsensusAPI) delayPayloadImport(block *types.Block) (engine.PayloadS // In full sync mode, failure to import a well-formed block can only mean // that the parent state is missing and the syncer rejected extending the // current cycle with the new payload. - log.Warn("Ignoring payload with missing parent", "number", block.NumberU64(), "hash", block.Hash(), "parent", block.ParentHash()) + log.Warn("Ignoring payload with missing parent", "number", block.NumberU64(), "hash", block.Hash(), "parent", block.ParentHash(), "reason", err) } else { // In non-full sync mode (i.e. snap sync) all payloads are rejected until // snap sync terminates as snap sync relies on direct database injections // and cannot afford concurrent out-if-band modifications via imports. - log.Warn("Ignoring payload while snap syncing", "number", block.NumberU64(), "hash", block.Hash()) + log.Warn("Ignoring payload while snap syncing", "number", block.NumberU64(), "hash", block.Hash(), "reason", err) } return engine.PayloadStatusV1{Status: engine.SYNCING}, nil } diff --git a/eth/downloader/skeleton.go b/eth/downloader/skeleton.go index 4f1f462048..f40ca24d99 100644 --- a/eth/downloader/skeleton.go +++ b/eth/downloader/skeleton.go @@ -69,9 +69,17 @@ var errSyncReorged = errors.New("sync reorged") // might still be propagating. var errTerminated = errors.New("terminated") -// errReorgDenied is returned if an attempt is made to extend the beacon chain -// with a new header, but it does not link up to the existing sync. -var errReorgDenied = errors.New("non-forced head reorg denied") +// errChainReorged is an internal helper error to signal that the header chain +// of the current sync cycle was (partially) reorged. +var errChainReorged = errors.New("chain reorged") + +// errChainGapped is an internal helper error to signal that the header chain +// of the current sync cycle is gaped with the one advertised by consensus client. +var errChainGapped = errors.New("chain gapped") + +// errChainForked is an internal helper error to signal that the header chain +// of the current sync cycle is forked with the one advertised by consensus client. +var errChainForked = errors.New("chain forked") func init() { // Tuning parameters is nice, but the scratch space must be assignable in @@ -271,9 +279,9 @@ func (s *skeleton) startup() { newhead, err := s.sync(head) switch { case err == errSyncLinked: - // Sync cycle linked up to the genesis block. Tear down the loop - // and restart it so, it can properly notify the backfiller. Don't - // account a new head. + // Sync cycle linked up to the genesis block, or the existent chain + // segment. Tear down the loop and restart it so, it can properly + // notify the backfiller. Don't account a new head. head = nil case err == errSyncMerged: @@ -457,15 +465,16 @@ func (s *skeleton) sync(head *types.Header) (*types.Header, error) { // we don't seamlessly integrate reorgs to keep things simple. If the // network starts doing many mini reorgs, it might be worthwhile handling // a limited depth without an error. - if reorged := s.processNewHead(event.header, event.final, event.force); reorged { + if err := s.processNewHead(event.header, event.final); err != nil { // If a reorg is needed, and we're forcing the new head, signal // the syncer to tear down and start over. Otherwise, drop the // non-force reorg. if event.force { event.errc <- nil // forced head reorg accepted + log.Info("Restarting sync cycle", "reason", err) return event.header, errSyncReorged } - event.errc <- errReorgDenied + event.errc <- err continue } event.errc <- nil // head extension accepted @@ -610,7 +619,7 @@ func (s *skeleton) saveSyncStatus(db ethdb.KeyValueWriter) { // accepts and integrates it into the skeleton or requests a reorg. Upon reorg, // the syncer will tear itself down and restart with a fresh head. It is simpler // to reconstruct the sync state than to mutate it and hope for the best. -func (s *skeleton) processNewHead(head *types.Header, final *types.Header, force bool) bool { +func (s *skeleton) processNewHead(head *types.Header, final *types.Header) error { // If a new finalized block was announced, update the sync process independent // of what happens with the sync head below if final != nil { @@ -631,26 +640,17 @@ func (s *skeleton) processNewHead(head *types.Header, final *types.Header, force // once more, ignore it instead of tearing down sync for a noop. if lastchain.Head == lastchain.Tail { if current := rawdb.ReadSkeletonHeader(s.db, number); current.Hash() == head.Hash() { - return false + return nil } } // Not a noop / double head announce, abort with a reorg - if force { - log.Warn("Beacon chain reorged", "tail", lastchain.Tail, "head", lastchain.Head, "newHead", number) - } - return true + return fmt.Errorf("%w, tail: %d, head: %d, newHead: %d", errChainReorged, lastchain.Tail, lastchain.Head, number) } if lastchain.Head+1 < number { - if force { - log.Warn("Beacon chain gapped", "head", lastchain.Head, "newHead", number) - } - return true + return fmt.Errorf("%w, head: %d, newHead: %d", errChainGapped, lastchain.Head, number) } if parent := rawdb.ReadSkeletonHeader(s.db, number-1); parent.Hash() != head.ParentHash { - if force { - log.Warn("Beacon chain forked", "ancestor", number-1, "hash", parent.Hash(), "want", head.ParentHash) - } - return true + return fmt.Errorf("%w, ancestor: %d, hash: %s, want: %s", errChainForked, number-1, parent.Hash(), head.ParentHash) } // New header seems to be in the last subchain range. Unwind any extra headers // from the chain tip and insert the new head. We won't delete any trimmed @@ -666,7 +666,7 @@ func (s *skeleton) processNewHead(head *types.Header, final *types.Header, force if err := batch.Write(); err != nil { log.Crit("Failed to write skeleton sync status", "err", err) } - return false + return nil } // assignTasks attempts to match idle peers to pending header retrievals. diff --git a/eth/downloader/skeleton_test.go b/eth/downloader/skeleton_test.go index c31007765a..aceadd00d3 100644 --- a/eth/downloader/skeleton_test.go +++ b/eth/downloader/skeleton_test.go @@ -434,7 +434,7 @@ func TestSkeletonSyncExtend(t *testing.T) { newstate: []*subchain{ {Head: 49, Tail: 49}, }, - err: errReorgDenied, + err: errChainReorged, }, // Initialize a sync and try to extend it with a number-wise sequential // header, but a hash wise non-linking one. @@ -444,7 +444,7 @@ func TestSkeletonSyncExtend(t *testing.T) { newstate: []*subchain{ {Head: 49, Tail: 49}, }, - err: errReorgDenied, + err: errChainForked, }, // Initialize a sync and try to extend it with a non-linking future block. { @@ -453,7 +453,7 @@ func TestSkeletonSyncExtend(t *testing.T) { newstate: []*subchain{ {Head: 49, Tail: 49}, }, - err: errReorgDenied, + err: errChainGapped, }, // Initialize a sync and try to extend it with a past canonical block. { @@ -462,7 +462,7 @@ func TestSkeletonSyncExtend(t *testing.T) { newstate: []*subchain{ {Head: 50, Tail: 50}, }, - err: errReorgDenied, + err: errChainReorged, }, // Initialize a sync and try to extend it with a past sidechain block. { @@ -471,7 +471,7 @@ func TestSkeletonSyncExtend(t *testing.T) { newstate: []*subchain{ {Head: 50, Tail: 50}, }, - err: errReorgDenied, + err: errChainReorged, }, } for i, tt := range tests { @@ -487,7 +487,7 @@ func TestSkeletonSyncExtend(t *testing.T) { skeleton.Sync(tt.head, nil, true) <-wait - if err := skeleton.Sync(tt.extend, nil, false); err != tt.err { + if err := skeleton.Sync(tt.extend, nil, false); !errors.Is(err, tt.err) { t.Errorf("test %d: extension failure mismatch: have %v, want %v", i, err, tt.err) } skeleton.Terminate() diff --git a/trie/verkle_test.go b/trie/verkle_test.go index 44fb7dc29e..bd31ea3879 100644 --- a/trie/verkle_test.go +++ b/trie/verkle_test.go @@ -31,24 +31,24 @@ import ( var ( accounts = map[common.Address]*types.StateAccount{ - common.Address{1}: { + {1}: { Nonce: 100, Balance: big.NewInt(100), CodeHash: common.Hash{0x1}.Bytes(), }, - common.Address{2}: { + {2}: { Nonce: 200, Balance: big.NewInt(200), CodeHash: common.Hash{0x2}.Bytes(), }, } storages = map[common.Address]map[common.Hash][]byte{ - common.Address{1}: { + {1}: { common.Hash{10}: []byte{10}, common.Hash{11}: []byte{11}, common.MaxHash: []byte{0xff}, }, - common.Address{2}: { + {2}: { common.Hash{20}: []byte{20}, common.Hash{21}: []byte{21}, common.MaxHash: []byte{0xff},