Merge pull request #4885 from spacemeshos/revert-4884-fix-nil-pointer…

…-grpc

Revert "Fix nil pointer dereference"

api/grpcserver/admin_service.go

@@ -13,7 +13,6 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
-	"google.golang.org/protobuf/types/known/durationpb"
 
 	"github.com/spacemeshos/go-spacemesh/checkpoint"
 	"github.com/spacemeshos/go-spacemesh/common/types"
@@ -32,16 +31,14 @@ type AdminService struct {
 	logger  log.Logger
 	db      *sql.Database
 	dataDir string
-	p       peers
 }
 
 // NewAdminService creates a new admin grpc service.
-func NewAdminService(db *sql.Database, dataDir string, lg log.Logger, p peers) *AdminService {
+func NewAdminService(db *sql.Database, dataDir string, lg log.Logger) *AdminService {
 	return &AdminService{
 		logger:  lg,
 		db:      db,
 		dataDir: dataDir,
-		p:       p,
 	}
 }
 
@@ -131,37 +128,3 @@ func (a AdminService) EventsStream(req *pb.EventStreamRequest, stream pb.AdminSe
 		}
 	}
 }
-
-func (a AdminService) PeerInfoStream(_ *empty.Empty, stream pb.AdminService_PeerInfoStreamServer) error {
-	for _, p := range a.p.GetPeers() {
-		select {
-		case <-stream.Context().Done():
-			return nil
-		default:
-			info := a.p.ConnectedPeerInfo(p)
-			// There is no guarantee that the peers originally returned will still
-			// be connected by the time we call ConnectedPeerInfo.
-			if info == nil {
-				continue
-			}
-			connections := make([]*pb.ConnectionInfo, len(info.Connections))
-			for j, c := range info.Connections {
-				connections[j] = &pb.ConnectionInfo{
-					Address:  c.Address.String(),
-					Uptime:   durationpb.New(c.Uptime),
-					Outbound: c.Outbound,
-				}
-			}
-			err := stream.Send(&pb.PeerInfo{
-				Id:          info.ID.String(),
-				Connections: connections,
-				Tags:        info.Tags,
-			})
-			if err != nil {
-				return fmt.Errorf("send to stream: %w", err)
-			}
-		}
-	}
-
-	return nil
-}

api/grpcserver/admin_service_test.go

@@ -56,7 +56,7 @@ func createMesh(tb testing.TB, db *sql.Database) {
 func TestAdminService_Checkpoint(t *testing.T) {
 	db := sql.InMemory()
 	createMesh(t, db)
-	svc := NewAdminService(db, t.TempDir(), logtest.New(t), nil)
+	svc := NewAdminService(db, t.TempDir(), logtest.New(t))
 	t.Cleanup(launchServer(t, cfg, svc))
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
@@ -91,7 +91,7 @@ func TestAdminService_Checkpoint(t *testing.T) {
 
 func TestAdminService_CheckpointError(t *testing.T) {
 	db := sql.InMemory()
-	svc := NewAdminService(db, t.TempDir(), logtest.New(t), nil)
+	svc := NewAdminService(db, t.TempDir(), logtest.New(t))
 	t.Cleanup(launchServer(t, cfg, svc))
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)

api/grpcserver/grpc.go

@@ -19,15 +19,10 @@ type ServiceAPI interface {
 
 // Server is a very basic grpc server.
 type Server struct {
-	Listener string
-	logger   log.Logger
-	// BoundAddress contains the address that the server bound to, useful if
-	// the server uses a dynamic port. It is set during startup and can be
-	// safely accessed after Start has completed (I.E. the returned channel has
-	// been waited on)
-	BoundAddress string
-	GrpcServer   *grpc.Server
-	grp          errgroup.Group
+	Listener   string
+	logger     log.Logger
+	GrpcServer *grpc.Server
+	grp        errgroup.Group
 }
 
 // New creates and returns a new Server with port and interface.
@@ -56,7 +51,6 @@ func (s *Server) Start() error {
 		s.logger.Error("error listening: %v", err)
 		return err
 	}
-	s.BoundAddress = lis.Addr().String()
 	reflection.Register(s.GrpcServer)
 	s.grp.Go(func() error {
 		if err := s.GrpcServer.Serve(lis); err != nil {

api/grpcserver/interface.go

@@ -60,12 +60,6 @@ type peerCounter interface {
 	PeerCount() uint64
 }
 
-// Peers is an api to get peer related info.
-type peers interface {
-	ConnectedPeerInfo(p2p.Peer) *p2p.PeerInfo
-	GetPeers() []p2p.Peer
-}
-
 // genesisTimeAPI is an API to get genesis time and current layer of the system.
 type genesisTimeAPI interface {
 	GenesisTime() time.Time

api/grpcserver/smesher_service.go

@@ -258,7 +258,6 @@ func statusToPbStatus(status *activation.PostSetupStatus) *pb.PostSetupStatus {
 	if status.LastOpts != nil {
 		var providerID *uint32
 		if status.LastOpts.ProviderID.Value() != nil {
-			providerID = new(uint32)
 			*providerID = uint32(*status.LastOpts.ProviderID.Value())
 		}
 

api/grpcserver/smesher_service_test.go

@@ -117,85 +117,3 @@ func TestSmesherService_PostSetupProviders(t *testing.T) {
 	require.EqualValues(t, providers[1].ID, resp.Providers[1].Id)
 	require.Equal(t, uint64(100_000), resp.Providers[1].Performance)
 }
-
-func TestSmesherService_PostSetupStatus(t *testing.T) {
-	t.Run("completed", func(t *testing.T) {
-		ctrl := gomock.NewController(t)
-		postSetupProvider := activation.NewMockpostSetupProvider(ctrl)
-		smeshingProvider := activation.NewMockSmeshingProvider(ctrl)
-		svc := grpcserver.NewSmesherService(postSetupProvider, smeshingProvider, time.Second, activation.DefaultPostSetupOpts(), logtest.New(t).WithName("grpc.Smesher"))
-
-		postSetupProvider.EXPECT().Status().Return(&activation.PostSetupStatus{
-			State:            activation.PostSetupStateComplete,
-			NumLabelsWritten: 1_000,
-			LastOpts:         nil,
-		})
-		resp, err := svc.PostSetupStatus(context.Background(), &emptypb.Empty{})
-		require.NoError(t, err)
-		require.Equal(t, pb.PostSetupStatus_STATE_COMPLETE, resp.Status.State)
-		require.EqualValues(t, 1_000, resp.Status.NumLabelsWritten)
-		require.Nil(t, resp.Status.Opts)
-	})
-
-	t.Run("completed with last Opts", func(t *testing.T) {
-		ctrl := gomock.NewController(t)
-		postSetupProvider := activation.NewMockpostSetupProvider(ctrl)
-		smeshingProvider := activation.NewMockSmeshingProvider(ctrl)
-		svc := grpcserver.NewSmesherService(postSetupProvider, smeshingProvider, time.Second, activation.DefaultPostSetupOpts(), logtest.New(t).WithName("grpc.Smesher"))
-
-		id := activation.PostProviderID{}
-		id.SetInt64(1)
-		opts := activation.PostSetupOpts{
-			DataDir:     "data-dir",
-			NumUnits:    4,
-			MaxFileSize: 1024,
-			ProviderID:  id,
-			Throttle:    true,
-		}
-		postSetupProvider.EXPECT().Status().Return(&activation.PostSetupStatus{
-			State:            activation.PostSetupStateComplete,
-			NumLabelsWritten: 1_000,
-			LastOpts:         &opts,
-		})
-		resp, err := svc.PostSetupStatus(context.Background(), &emptypb.Empty{})
-		require.NoError(t, err)
-		require.Equal(t, pb.PostSetupStatus_STATE_COMPLETE, resp.Status.State)
-		require.EqualValues(t, 1_000, resp.Status.NumLabelsWritten)
-		require.Equal(t, "data-dir", resp.Status.Opts.DataDir)
-		require.EqualValues(t, 4, resp.Status.Opts.NumUnits)
-		require.EqualValues(t, 1024, resp.Status.Opts.MaxFileSize)
-		require.EqualValues(t, 1, *resp.Status.Opts.ProviderId)
-		require.True(t, resp.Status.Opts.Throttle)
-	})
-
-	t.Run("in progress", func(t *testing.T) {
-		ctrl := gomock.NewController(t)
-		postSetupProvider := activation.NewMockpostSetupProvider(ctrl)
-		smeshingProvider := activation.NewMockSmeshingProvider(ctrl)
-		svc := grpcserver.NewSmesherService(postSetupProvider, smeshingProvider, time.Second, activation.DefaultPostSetupOpts(), logtest.New(t).WithName("grpc.Smesher"))
-
-		id := activation.PostProviderID{}
-		id.SetInt64(100)
-		opts := activation.PostSetupOpts{
-			DataDir:     "data-dir",
-			NumUnits:    4,
-			MaxFileSize: 1024,
-			ProviderID:  id,
-			Throttle:    false,
-		}
-		postSetupProvider.EXPECT().Status().Return(&activation.PostSetupStatus{
-			State:            activation.PostSetupStateInProgress,
-			NumLabelsWritten: 1_000,
-			LastOpts:         &opts,
-		})
-		resp, err := svc.PostSetupStatus(context.Background(), &emptypb.Empty{})
-		require.NoError(t, err)
-		require.Equal(t, pb.PostSetupStatus_STATE_IN_PROGRESS, resp.Status.State)
-		require.EqualValues(t, 1_000, resp.Status.NumLabelsWritten)
-		require.Equal(t, "data-dir", resp.Status.Opts.DataDir)
-		require.EqualValues(t, 4, resp.Status.Opts.NumUnits)
-		require.EqualValues(t, 1024, resp.Status.Opts.MaxFileSize)
-		require.EqualValues(t, 100, *resp.Status.Opts.ProviderId)
-		require.False(t, resp.Status.Opts.Throttle)
-	})
-}

go.mod

@@ -35,7 +35,7 @@ require (
 	github.com/pyroscope-io/pyroscope v0.37.2
 	github.com/santhosh-tekuri/jsonschema/v5 v5.3.1
 	github.com/seehuhn/mt19937 v1.0.0
-	github.com/spacemeshos/api/release/go v1.20.0
+	github.com/spacemeshos/api/release/go v1.19.0
 	github.com/spacemeshos/economics v0.1.0
 	github.com/spacemeshos/fixed v0.1.0
 	github.com/spacemeshos/go-scale v1.1.10

go.sum

@@ -606,8 +606,8 @@ github.com/smartystreets/goconvey v1.7.2 h1:9RBaZCeXEQ3UselpuwUQHltGVXvdwm6cv1hg
 github.com/smartystreets/goconvey v1.7.2/go.mod h1:Vw0tHAZW6lzCRk3xgdin6fKYcG+G3Pg9vgXWeJpQFMM=
 github.com/sourcegraph/annotate v0.0.0-20160123013949-f4cad6c6324d/go.mod h1:UdhH50NIW0fCiwBSr0co2m7BnFLdv4fQTgdqdJTHFeE=
 github.com/sourcegraph/syntaxhighlight v0.0.0-20170531221838-bd320f5d308e/go.mod h1:HuIsMU8RRBOtsCgI77wP899iHVBQpCmg4ErYMZB+2IA=
-github.com/spacemeshos/api/release/go v1.20.0 h1:9HEWDTXyE5mSfca8C5tIXauI1igeUO8CLm5LM6X6y0k=
-github.com/spacemeshos/api/release/go v1.20.0/go.mod h1:nC5g7IVRNxF8Kl/Tzvr7cQeYBwHN4sMTVsIEa6Ebtl4=
+github.com/spacemeshos/api/release/go v1.19.0 h1:QPt1nUuSVQ4DfZPNsSAuJpjjYlbS9HJhDunE7K8rn08=
+github.com/spacemeshos/api/release/go v1.19.0/go.mod h1:nC5g7IVRNxF8Kl/Tzvr7cQeYBwHN4sMTVsIEa6Ebtl4=
 github.com/spacemeshos/economics v0.1.0 h1:PJAKbhBKqbbdCYTB29pkmc8sYqK3pKUAiuAvQxuSJEg=
 github.com/spacemeshos/economics v0.1.0/go.mod h1:Bz0wRDwCOUP1A6w3cPW6iuUBGME8Tz48sIriYiohsBg=
 github.com/spacemeshos/fixed v0.1.0 h1:20KIGvxLlAsuidQrvuwwHe6PrvqeTKzbJIsScbmnUPQ=