Skip to content

Commit

Permalink
fix: ignore errors that caused by gap to keep peer connection (bnb-ch…
Browse files Browse the repository at this point in the history
  • Loading branch information
krish-nr authored Feb 1, 2024
1 parent 9114a97 commit 5d346de
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 6 deletions.
18 changes: 13 additions & 5 deletions eth/downloader/skeleton.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -73,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.
Expand Down Expand Up @@ -786,25 +790,29 @@ 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 <- 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 <- errors.New("invalid header batch anchor")
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:
// 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 <- 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 <- errors.New("not enough genesis headers delivered")
res.Done <- etherror.ErrNotEnoughGenesisHeaders
s.scheduleRevertRequest(req)

default:
Expand All @@ -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 <- etherror.ErrInvalidHashProgression
s.scheduleRevertRequest(req)
return
}
Expand Down
12 changes: 12 additions & 0 deletions eth/etherror/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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")
ErrHeaderBatchAnchorLow = errors.New("header batch anchor is lower than requested")
)
16 changes: 15 additions & 1 deletion eth/protocols/eth/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@
package eth

import (
"errors"
"fmt"
"math/big"
"time"

"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/etherror"
"github.com/ethereum/go-ethereum/metrics"
"github.com/ethereum/go-ethereum/p2p"
"github.com/ethereum/go-ethereum/p2p/enode"
Expand Down Expand Up @@ -153,7 +155,19 @@ 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 {
err := handleMessage(backend, peer)
switch {
// 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
}
Expand Down

0 comments on commit 5d346de

Please sign in to comment.