From 80662702734cda3923095a3cbbe526704aeaacf5 Mon Sep 17 00:00:00 2001 From: krish Date: Mon, 24 Jul 2023 10:12:34 +0800 Subject: [PATCH 1/4] feat: merge PR #105 --- eth/handler.go | 4 +++- eth/protocols/snap/handler.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/eth/handler.go b/eth/handler.go index 77ac554ce3..bcca1ff3b1 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -168,8 +168,10 @@ func newHandler(config *handlerConfig) (*handler, error) { log.Warn("Switch sync mode from full sync to snap sync") } } else { - if h.chain.CurrentBlock().Number.Uint64() > 0 { + blockNumber := h.chain.CurrentBlock().Number + if blockNumber.Uint64() > 0 && (!config.Chain.Config().IsOptimism() || blockNumber.Cmp(config.Chain.Config().BedrockBlock) != 0) { // Print warning log if database is not empty to run snap sync. + // For OP chains, snap sync from bedrock block is allowed. log.Warn("Switch sync mode from snap sync to full sync") } else { // If snap sync was requested and our database is empty, grant it diff --git a/eth/protocols/snap/handler.go b/eth/protocols/snap/handler.go index d7c9400440..e8ed0fd3eb 100644 --- a/eth/protocols/snap/handler.go +++ b/eth/protocols/snap/handler.go @@ -469,7 +469,7 @@ func ServiceGetByteCodesQuery(chain *core.BlockChain, req *GetByteCodesPacket) [ // Peers should not request the empty code, but if they do, at // least sent them back a correct response without db lookups codes = append(codes, []byte{}) - } else if blob, err := chain.ContractCodeWithPrefix(hash); err == nil { + } else if blob, err := chain.ContractCode(hash); err == nil { codes = append(codes, blob) bytes += uint64(len(blob)) } From df40134ce0ab86263db501fd5dfe44073ca6bbc7 Mon Sep 17 00:00:00 2001 From: Krish Date: Tue, 9 Jan 2024 22:11:45 +0800 Subject: [PATCH 2/4] fix: peer loop ignore 'invalid batch anchor' error and not quit with peer disconnect --- eth/downloader/skeleton.go | 18 +++++++++++++----- eth/protocols/eth/handler.go | 7 ++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/eth/downloader/skeleton.go b/eth/downloader/skeleton.go index 12eb5700f8..369156dae7 100644 --- a/eth/downloader/skeleton.go +++ b/eth/downloader/skeleton.go @@ -73,6 +73,14 @@ var errTerminated = errors.New("terminated") // with a new header, but it does not link up to the existing sync. var errReorgDenied = errors.New("non-forced head reorg denied") +var ( + ErrNoHeadersDelivered = errors.New("no headers delivered") + ErrInvalidHeaderBatchAnchor = errors.New("invalid header batch anchor") + ErrNotEnoughNonGenesisHeaders = errors.New("not enough non-genesis headers delivered") + ErrNotEnoughGenesisHeaders = errors.New("not enough genesis headers delivered") + ErrInvalidHashProgression = errors.New("invalid hash progression") +) + func init() { // Tuning parameters is nice, but the scratch space must be assignable in // full to peers. It's a useless cornercase to support a dangling half-group. @@ -786,25 +794,25 @@ func (s *skeleton) executeTask(peer *peerConnection, req *headerRequest) { case len(headers) == 0: // No headers were delivered, reject the response and reschedule peer.log.Debug("No headers delivered") - res.Done <- errors.New("no headers delivered") + res.Done <- ErrNoHeadersDelivered s.scheduleRevertRequest(req) case headers[0].Number.Uint64() != req.head: // Header batch anchored at non-requested number peer.log.Debug("Invalid header response head", "have", headers[0].Number, "want", req.head) - res.Done <- errors.New("invalid header batch anchor") + res.Done <- ErrInvalidHeaderBatchAnchor s.scheduleRevertRequest(req) case req.head >= requestHeaders && len(headers) != requestHeaders: // Invalid number of non-genesis headers delivered, reject the response and reschedule peer.log.Debug("Invalid non-genesis header count", "have", len(headers), "want", requestHeaders) - res.Done <- errors.New("not enough non-genesis headers delivered") + res.Done <- ErrNotEnoughNonGenesisHeaders s.scheduleRevertRequest(req) case req.head < requestHeaders && uint64(len(headers)) != req.head: // Invalid number of genesis headers delivered, reject the response and reschedule peer.log.Debug("Invalid genesis header count", "have", len(headers), "want", headers[0].Number.Uint64()) - res.Done <- errors.New("not enough genesis headers delivered") + res.Done <- ErrNotEnoughGenesisHeaders s.scheduleRevertRequest(req) default: @@ -813,7 +821,7 @@ func (s *skeleton) executeTask(peer *peerConnection, req *headerRequest) { for i := 0; i < len(headers)-1; i++ { if headers[i].ParentHash != headers[i+1].Hash() { peer.log.Debug("Invalid hash progression", "index", i, "wantparenthash", headers[i].ParentHash, "haveparenthash", headers[i+1].Hash()) - res.Done <- errors.New("invalid hash progression") + res.Done <- ErrInvalidHashProgression s.scheduleRevertRequest(req) return } diff --git a/eth/protocols/eth/handler.go b/eth/protocols/eth/handler.go index 2f2dd1cf6a..592e307618 100644 --- a/eth/protocols/eth/handler.go +++ b/eth/protocols/eth/handler.go @@ -17,6 +17,7 @@ package eth import ( + "errors" "fmt" "math/big" "time" @@ -24,6 +25,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/eth/downloader" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/p2p/enode" @@ -153,7 +155,10 @@ func nodeInfo(chain *core.BlockChain, network uint64) *NodeInfo { // connection is torn down. func Handle(backend Backend, peer *Peer) error { for { - if err := handleMessage(backend, peer); err != nil { + if err := handleMessage(backend, peer); errors.Is(err, downloader.ErrInvalidHeaderBatchAnchor) { + //ignore invalid header anchor + peer.Log().Warn("Message handling failed with invalid batch request anchor") + } else if err != nil { peer.Log().Debug("Message handling failed in `eth`", "err", err) return err } From 5a43edcc9c14eb2e184852a16e5942d9cb3c95f1 Mon Sep 17 00:00:00 2001 From: Krish Date: Thu, 18 Jan 2024 16:42:41 +0800 Subject: [PATCH 3/4] fix: add 'noheaders' as ignored error --- eth/downloader/skeleton.go | 19 ++++++------------- eth/etherror/errors.go | 11 +++++++++++ eth/protocols/eth/handler.go | 13 +++++++++---- 3 files changed, 26 insertions(+), 17 deletions(-) create mode 100644 eth/etherror/errors.go diff --git a/eth/downloader/skeleton.go b/eth/downloader/skeleton.go index 369156dae7..754281a6a1 100644 --- a/eth/downloader/skeleton.go +++ b/eth/downloader/skeleton.go @@ -27,6 +27,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/eth/etherror" "github.com/ethereum/go-ethereum/eth/protocols/eth" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" @@ -73,14 +74,6 @@ var errTerminated = errors.New("terminated") // with a new header, but it does not link up to the existing sync. var errReorgDenied = errors.New("non-forced head reorg denied") -var ( - ErrNoHeadersDelivered = errors.New("no headers delivered") - ErrInvalidHeaderBatchAnchor = errors.New("invalid header batch anchor") - ErrNotEnoughNonGenesisHeaders = errors.New("not enough non-genesis headers delivered") - ErrNotEnoughGenesisHeaders = errors.New("not enough genesis headers delivered") - ErrInvalidHashProgression = errors.New("invalid hash progression") -) - func init() { // Tuning parameters is nice, but the scratch space must be assignable in // full to peers. It's a useless cornercase to support a dangling half-group. @@ -794,25 +787,25 @@ func (s *skeleton) executeTask(peer *peerConnection, req *headerRequest) { case len(headers) == 0: // No headers were delivered, reject the response and reschedule peer.log.Debug("No headers delivered") - res.Done <- ErrNoHeadersDelivered + res.Done <- etherror.ErrNoHeadersDelivered s.scheduleRevertRequest(req) case headers[0].Number.Uint64() != req.head: // Header batch anchored at non-requested number peer.log.Debug("Invalid header response head", "have", headers[0].Number, "want", req.head) - res.Done <- ErrInvalidHeaderBatchAnchor + res.Done <- etherror.ErrInvalidHeaderBatchAnchor s.scheduleRevertRequest(req) case req.head >= requestHeaders && len(headers) != requestHeaders: // Invalid number of non-genesis headers delivered, reject the response and reschedule peer.log.Debug("Invalid non-genesis header count", "have", len(headers), "want", requestHeaders) - res.Done <- ErrNotEnoughNonGenesisHeaders + res.Done <- etherror.ErrNotEnoughNonGenesisHeaders s.scheduleRevertRequest(req) case req.head < requestHeaders && uint64(len(headers)) != req.head: // Invalid number of genesis headers delivered, reject the response and reschedule peer.log.Debug("Invalid genesis header count", "have", len(headers), "want", headers[0].Number.Uint64()) - res.Done <- ErrNotEnoughGenesisHeaders + res.Done <- etherror.ErrNotEnoughGenesisHeaders s.scheduleRevertRequest(req) default: @@ -821,7 +814,7 @@ func (s *skeleton) executeTask(peer *peerConnection, req *headerRequest) { for i := 0; i < len(headers)-1; i++ { if headers[i].ParentHash != headers[i+1].Hash() { peer.log.Debug("Invalid hash progression", "index", i, "wantparenthash", headers[i].ParentHash, "haveparenthash", headers[i+1].Hash()) - res.Done <- ErrInvalidHashProgression + res.Done <- etherror.ErrInvalidHashProgression s.scheduleRevertRequest(req) return } diff --git a/eth/etherror/errors.go b/eth/etherror/errors.go new file mode 100644 index 0000000000..ff66bd2cd3 --- /dev/null +++ b/eth/etherror/errors.go @@ -0,0 +1,11 @@ +package etherror + +import "errors" + +var ( + ErrNoHeadersDelivered = errors.New("no headers delivered") + ErrInvalidHeaderBatchAnchor = errors.New("invalid header batch anchor") + ErrNotEnoughNonGenesisHeaders = errors.New("not enough non-genesis headers delivered") + ErrNotEnoughGenesisHeaders = errors.New("not enough genesis headers delivered") + ErrInvalidHashProgression = errors.New("invalid hash progression") +) diff --git a/eth/protocols/eth/handler.go b/eth/protocols/eth/handler.go index 592e307618..d8955d96ab 100644 --- a/eth/protocols/eth/handler.go +++ b/eth/protocols/eth/handler.go @@ -25,7 +25,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/eth/downloader" + "github.com/ethereum/go-ethereum/eth/etherror" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/p2p/enode" @@ -155,10 +155,15 @@ func nodeInfo(chain *core.BlockChain, network uint64) *NodeInfo { // connection is torn down. func Handle(backend Backend, peer *Peer) error { for { - if err := handleMessage(backend, peer); errors.Is(err, downloader.ErrInvalidHeaderBatchAnchor) { - //ignore invalid header anchor + err := handleMessage(backend, peer) + switch { + case errors.Is(err, etherror.ErrInvalidHeaderBatchAnchor): + // ignore invalid header anchor peer.Log().Warn("Message handling failed with invalid batch request anchor") - } else if err != nil { + case errors.Is(err, etherror.ErrNoHeadersDelivered): + // ignore no headers delivered + peer.Log().Warn("Message handling failed with no headers") + case err != nil: peer.Log().Debug("Message handling failed in `eth`", "err", err) return err } From 7a10addd99b9f39a94127982ff19295fd8756c4b Mon Sep 17 00:00:00 2001 From: Krish Date: Tue, 30 Jan 2024 18:20:18 +0800 Subject: [PATCH 4/4] config: remove no-headers as it may leads to a dead peer not removing --- eth/downloader/skeleton.go | 9 ++++++++- eth/etherror/errors.go | 1 + eth/protocols/eth/handler.go | 16 ++++++++++------ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/eth/downloader/skeleton.go b/eth/downloader/skeleton.go index 754281a6a1..5ad8834d05 100644 --- a/eth/downloader/skeleton.go +++ b/eth/downloader/skeleton.go @@ -74,6 +74,9 @@ var errTerminated = errors.New("terminated") // with a new header, but it does not link up to the existing sync. var errReorgDenied = errors.New("non-forced head reorg denied") +// maxBlockNumGapTolerance is the max gap tolerance by peer +var maxBlockNumGapTolerance = uint64(30) + func init() { // Tuning parameters is nice, but the scratch space must be assignable in // full to peers. It's a useless cornercase to support a dangling half-group. @@ -793,7 +796,11 @@ func (s *skeleton) executeTask(peer *peerConnection, req *headerRequest) { case headers[0].Number.Uint64() != req.head: // Header batch anchored at non-requested number peer.log.Debug("Invalid header response head", "have", headers[0].Number, "want", req.head) - res.Done <- etherror.ErrInvalidHeaderBatchAnchor + if req.head-headers[0].Number.Uint64() < maxBlockNumGapTolerance { + res.Done <- etherror.ErrHeaderBatchAnchorLow + } else { + res.Done <- etherror.ErrInvalidHeaderBatchAnchor + } s.scheduleRevertRequest(req) case req.head >= requestHeaders && len(headers) != requestHeaders: diff --git a/eth/etherror/errors.go b/eth/etherror/errors.go index ff66bd2cd3..ce14cf22e2 100644 --- a/eth/etherror/errors.go +++ b/eth/etherror/errors.go @@ -8,4 +8,5 @@ var ( ErrNotEnoughNonGenesisHeaders = errors.New("not enough non-genesis headers delivered") ErrNotEnoughGenesisHeaders = errors.New("not enough genesis headers delivered") ErrInvalidHashProgression = errors.New("invalid hash progression") + ErrHeaderBatchAnchorLow = errors.New("header batch anchor is lower than requested") ) diff --git a/eth/protocols/eth/handler.go b/eth/protocols/eth/handler.go index d8955d96ab..5643e6b767 100644 --- a/eth/protocols/eth/handler.go +++ b/eth/protocols/eth/handler.go @@ -157,12 +157,16 @@ func Handle(backend Backend, peer *Peer) error { for { err := handleMessage(backend, peer) switch { - case errors.Is(err, etherror.ErrInvalidHeaderBatchAnchor): - // ignore invalid header anchor - peer.Log().Warn("Message handling failed with invalid batch request anchor") - case errors.Is(err, etherror.ErrNoHeadersDelivered): - // ignore no headers delivered - peer.Log().Warn("Message handling failed with no headers") + // TODO: currently no headers not ignored as it may leads to a dead peer not removing as expected + /* + case errors.Is(err, etherror.ErrNoHeadersDelivered): + // ignore no headers delivered + peer.Log().Warn("Message handling failed with no headers") + */ + case errors.Is(err, etherror.ErrHeaderBatchAnchorLow): + // ignore lower header anchor within tolerance + peer.Log().Warn("Message handling failed with lower batch anchor") + case err != nil: peer.Log().Debug("Message handling failed in `eth`", "err", err) return err