From 56ff16f61f5c66a2089ca4967d940cf71a9b466e Mon Sep 17 00:00:00 2001 From: krish-z <122767080+krish-nr@users.noreply.github.com> Date: Tue, 27 Aug 2024 17:42:30 +0800 Subject: [PATCH] (op-geth) Add extra error info when meet an unexpected el sync (#132) --- eth/catalyst/api.go | 3 +++ eth/catalyst/api_test.go | 12 +++++++----- eth/downloader/skeleton.go | 6 +++++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 31da11533b..e8703f7532 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -722,6 +722,9 @@ func (api *ConsensusAPI) delayPayloadImport(block *types.Block) (engine.PayloadS // 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(), "reason", err) + if errors.Is(err, downloader.ErrForcedNeeded) { + return engine.PayloadStatusV1{Status: engine.SYNCING}, 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 diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 8b6a4f7169..64907080af 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -20,6 +20,7 @@ import ( "bytes" "context" crand "crypto/rand" + "errors" "fmt" "math/big" "math/rand" @@ -30,6 +31,8 @@ import ( "github.com/stretchr/testify/require" + "github.com/mattn/go-colorable" + "github.com/ethereum/go-ethereum/beacon/engine" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -50,7 +53,6 @@ import ( "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" "github.com/ethereum/go-ethereum/trie" - "github.com/mattn/go-colorable" ) var ( @@ -715,7 +717,7 @@ func TestEmptyBlocks(t *testing.T) { payload := getNewPayload(t, api, commonAncestor, nil) status, err := api.NewPayloadV1(*payload) - if err != nil { + if err != nil && !errors.Is(err, downloader.ErrForcedNeeded) { t.Fatal(err) } if status.Status != engine.VALID { @@ -731,7 +733,7 @@ func TestEmptyBlocks(t *testing.T) { payload = setBlockhash(payload) // Now latestValidHash should be the common ancestor status, err = api.NewPayloadV1(*payload) - if err != nil { + if err != nil && !errors.Is(err, downloader.ErrForcedNeeded) { t.Fatal(err) } if status.Status != engine.INVALID { @@ -749,7 +751,7 @@ func TestEmptyBlocks(t *testing.T) { payload = setBlockhash(payload) // Now latestValidHash should be the common ancestor status, err = api.NewPayloadV1(*payload) - if err != nil { + if err != nil && !errors.Is(err, downloader.ErrForcedNeeded) { t.Fatal(err) } if status.Status != engine.SYNCING { @@ -861,7 +863,7 @@ func TestTrickRemoteBlockCache(t *testing.T) { // feed the payloads to node B for _, payload := range invalidChain { status, err := apiB.NewPayloadV1(*payload) - if err != nil { + if err != nil && !errors.Is(err, downloader.ErrForcedNeeded) { panic(err) } if status.Status == engine.VALID { diff --git a/eth/downloader/skeleton.go b/eth/downloader/skeleton.go index 59d03c954f..129ffb7f61 100644 --- a/eth/downloader/skeleton.go +++ b/eth/downloader/skeleton.go @@ -82,6 +82,10 @@ var errChainGapped = errors.New("chain gapped") // of the current sync cycle is forked with the one advertised by consensus client. var errChainForked = errors.New("chain forked") +// ErrForcedNeeded is a public error to signal that the header chain +// of the current sync cycle needs a forced flag at startup. +var ErrForcedNeeded = errors.New("forced head needed for startup") + // maxBlockNumGapTolerance is the max gap tolerance by peer var maxBlockNumGapTolerance = uint64(30) @@ -269,7 +273,7 @@ func (s *skeleton) startup() { // New head announced, start syncing to it, looping every time a current // cycle is terminated due to a chain event (head reorg, old chain merge). if !event.force { - event.errc <- errors.New("forced head needed for startup") + event.errc <- ErrForcedNeeded continue } event.errc <- nil // forced head accepted for startup