diff --git a/beacon-chain/p2p/peers/BUILD.bazel b/beacon-chain/p2p/peers/BUILD.bazel index a8f5c782c3..3e2bfff8f0 100644 --- a/beacon-chain/p2p/peers/BUILD.bazel +++ b/beacon-chain/p2p/peers/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "@com_github_libp2p_go_libp2p_core//peer:go_default_library", "@com_github_multiformats_go_multiaddr//:go_default_library", "@com_github_multiformats_go_multiaddr//net:go_default_library", + "@com_github_pkg_errors//:go_default_library", "@com_github_prysmaticlabs_eth2_types//:go_default_library", "@com_github_prysmaticlabs_go_bitfield//:go_default_library", ], diff --git a/beacon-chain/p2p/peers/status.go b/beacon-chain/p2p/peers/status.go index 2d697bc337..1976e29e53 100644 --- a/beacon-chain/p2p/peers/status.go +++ b/beacon-chain/p2p/peers/status.go @@ -33,6 +33,7 @@ import ( "github.com/libp2p/go-libp2p-core/peer" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" + "github.com/pkg/errors" types "github.com/prysmaticlabs/eth2-types" "github.com/prysmaticlabs/go-bitfield" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" @@ -74,6 +75,11 @@ const ( MaxBackOffDuration = 5000 ) +// ErrNoPeerStatus is returned when there is a map entry for a given peer but there is no chain +// status for that peer. This should happen in rare circumstances only, but is a very possible +// scenario in a chaotic and adversarial network. +var ErrNoPeerStatus = errors.New("no chain status for peer") + // Status is the structure holding the peer status information. type Status struct { ctx context.Context @@ -190,10 +196,17 @@ func (p *Status) SetChainState(pid peer.ID, chainState *pb.Status) { } // ChainState gets the chain state of the given remote peer. -// This can return nil if there is no known chain state for the peer. // This will error if the peer does not exist. +// This will error if there is no known chain state for the peer. func (p *Status) ChainState(pid peer.ID) (*pb.Status, error) { - return p.scorers.PeerStatusScorer().PeerStatus(pid) + s, err := p.scorers.PeerStatusScorer().PeerStatus(pid) + if err != nil { + return nil, err + } + if s == nil { + return nil, ErrNoPeerStatus + } + return s, nil } // IsActive checks if a peers is active and returns the result appropriately. diff --git a/beacon-chain/p2p/peers/status_test.go b/beacon-chain/p2p/peers/status_test.go index ec3f20d48c..c6518ff028 100644 --- a/beacon-chain/p2p/peers/status_test.go +++ b/beacon-chain/p2p/peers/status_test.go @@ -302,6 +302,32 @@ func TestPeerChainState(t *testing.T) { } } +func TestPeerWithNilChainState(t *testing.T) { + maxBadResponses := 2 + p := peers.NewStatus(context.Background(), &peers.StatusConfig{ + PeerLimit: 30, + ScorerParams: &scorers.Config{ + BadResponsesScorerConfig: &scorers.BadResponsesScorerConfig{ + Threshold: maxBadResponses, + }, + }, + }) + + id, err := peer.Decode("16Uiu2HAkyWZ4Ni1TpvDS8dPxsozmHY85KaiFjodQuV6Tz5tkHVeR") + require.NoError(t, err) + address, err := ma.NewMultiaddr("/ip4/213.202.254.180/tcp/13000") + require.NoError(t, err, "Failed to create address") + direction := network.DirInbound + p.Add(new(enr.Record), id, address, direction) + + p.SetChainState(id, nil) + + resChainState, err := p.ChainState(id) + require.Equal(t, peers.ErrNoPeerStatus, err) + var nothing *pb.Status + require.Equal(t, resChainState, nothing) +} + func TestPeerBadResponses(t *testing.T) { maxBadResponses := 2 p := peers.NewStatus(context.Background(), &peers.StatusConfig{