Fix Sentry server initialization

cmd/sentry/download/sentry.go

@@ -762,6 +762,7 @@ func (ss *SentryServerImpl) HandShake(context.Context, *emptypb.Empty) (*proto_s
 	}
 	return reply, nil
 }
+
 func (ss *SentryServerImpl) SetStatus(_ context.Context, statusData *proto_sentry.StatusData) (*proto_sentry.SetStatusReply, error) {
 	genesisHash := gointerfaces.ConvertH256ToHash(statusData.ForkData.Genesis)
 
@@ -783,15 +784,19 @@ func (ss *SentryServerImpl) SetStatus(_ context.Context, statusData *proto_sentr
 			}
 		}
 
-		ss.P2pServer, err = makeP2PServer(*ss.p2p, genesisHash, ss.Protocol)
+		srv, err := makeP2PServer(*ss.p2p, genesisHash, ss.Protocol)
 		if err != nil {
 			return reply, err
 		}
+
 		// Add protocol
-		if err := ss.P2pServer.Start(); err != nil {
+		if err = srv.Start(); err != nil {
 			return reply, fmt.Errorf("could not start server: %w", err)
 		}
+
+		ss.P2pServer = srv
 	}
+
 	ss.P2pServer.LocalNode().Set(eth.CurrentENREntryFromForks(statusData.ForkData.Forks, genesisHash, statusData.MaxBlock))
 	if ss.statusData == nil || statusData.MaxBlock != 0 {
 		// Not overwrite statusData if the message contains zero MaxBlock (comes from standalone transaction pool)

cmd/sentry/download/sentry_test.go

@@ -148,3 +148,32 @@ func testForkIDSplit(t *testing.T, protocol uint) {
 		}
 	}
 }
+
+func TestSentryServerImpl_SetStatusInitPanic(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("panic during server initialization")
+		}
+	}()
+
+	configNoFork := &params.ChainConfig{HomesteadBlock: big.NewInt(1), ChainID: big.NewInt(1)}
+	dbNoFork := memdb.NewTestDB(t)
+	gspecNoFork := &core.Genesis{Config: configNoFork}
+	genesisNoFork := gspecNoFork.MustCommit(dbNoFork)
+	ss := &SentryServerImpl{p2p: &p2p.Config{}}
+
+	_, err := ss.SetStatus(context.Background(), &proto_sentry.StatusData{
+		ForkData: &proto_sentry.Forks{Genesis: gointerfaces.ConvertHashToH256(genesisNoFork.Hash())},
+	})
+	if err == nil {
+		t.Fatalf("error expected")
+	}
+
+	// Should not panic here.
+	_, err = ss.SetStatus(context.Background(), &proto_sentry.StatusData{
+		ForkData: &proto_sentry.Forks{Genesis: gointerfaces.ConvertHashToH256(genesisNoFork.Hash())},
+	})
+	if err == nil {
+		t.Fatalf("error expected")
+	}
+}

p2p/server.go

@@ -245,8 +245,8 @@ type transport interface {
 	// handshake has completed. The code uses conn.id to track this
 	// by setting it to a non-nil value after the encryption handshake.
 	MsgReadWriter
-	// transports must provide Close because we use MsgPipe in some of
-	// the tests. Closing the actual network connection doesn't do
+	// transports must provide Close because we use MsgPipe in some
+	// tests. Closing the actual network connection doesn't do
 	// anything in those tests because MsgPipe doesn't use it.
 	close(err error)
 }
@@ -383,7 +383,7 @@ func (srv *Server) RemoveTrustedPeer(node *enode.Node) {
 	}
 }
 
-// SubscribePeers subscribes the given channel to peer events
+// SubscribeEvents subscribes the given channel to peer events.
 func (srv *Server) SubscribeEvents(ch chan *PeerEvent) event.Subscription {
 	return srv.peerFeed.Subscribe(ch)
 }
@@ -451,13 +451,13 @@ func (srv *Server) Running() bool {
 
 // Start starts running the server.
 // Servers can not be re-used after stopping.
-func (srv *Server) Start() (err error) {
+func (srv *Server) Start() error {
 	srv.lock.Lock()
 	defer srv.lock.Unlock()
 	if srv.running {
 		return errors.New("server already running")
 	}
-	srv.running = true
+
 	srv.log = srv.Config.Logger
 	if srv.log == nil {
 		srv.log = log.Root()
@@ -501,6 +501,7 @@ func (srv *Server) Start() (err error) {
 	}
 	srv.setupDialScheduler()
 
+	srv.running = true
 	srv.loopWG.Add(1)
 	go srv.run()
 	return nil