p2p: Return error if no ChainState found for a peer (#9502)

* p2p: Return error if no ChainState found for a peer

* add commentary

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",
     ],

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.

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{