chore: service module review improvements

proto/poktroll/service/genesis.proto

@@ -1,13 +1,14 @@
 syntax = "proto3";
-
 package poktroll.service;
 
+option go_package = "github.com/pokt-network/poktroll/x/service/types";
+
 import "amino/amino.proto";
 import "gogoproto/gogo.proto";
+
 import "poktroll/service/params.proto";
 import "poktroll/shared/service.proto";
 
-option go_package = "github.com/pokt-network/poktroll/x/service/types";
 
 // GenesisState defines the service module's genesis state.
 message GenesisState {

proto/poktroll/service/params.proto

@@ -1,16 +1,19 @@
 syntax = "proto3";
 package poktroll.service;
 
+option go_package = "github.com/pokt-network/poktroll/x/service/types";
+
 import "amino/amino.proto";
 import "gogoproto/gogo.proto";
 
-option go_package = "github.com/pokt-network/poktroll/x/service/types";
 
 // Params defines the parameters for the module.
 message Params {
   option (amino.name) = "poktroll/x/service/Params";
   option (gogoproto.equal) = true;
 
-
+  // The amount of uPOKT required to add a new service.
+  // This will be deducted from the signer's account balance,
+  // and transferred to the pocket network foundation.
   uint64 add_service_fee = 1 [(gogoproto.moretags) = "yaml:\"add_service_fee\""];
 }

proto/poktroll/service/query.proto

@@ -1,16 +1,16 @@
 syntax = "proto3";
-
 package poktroll.service;
 
+option go_package = "github.com/pokt-network/poktroll/x/service/types";
+
 import "amino/amino.proto";
 import "gogoproto/gogo.proto";
 import "google/api/annotations.proto";
 import "cosmos/base/query/v1beta1/pagination.proto";
+
 import "poktroll/service/params.proto";
 import "poktroll/shared/service.proto";
 
-option go_package = "github.com/pokt-network/poktroll/x/service/types";
-
 // Query defines the gRPC querier service.
 service Query {
 
@@ -22,10 +22,10 @@ service Query {
 
   // Queries a list of Service items.
   rpc Service    (QueryGetServiceRequest) returns (QueryGetServiceResponse) {
-    option (google.api.http).get = "/pokt-network/poktroll/service/service/{index}";
+    option (google.api.http).get = "/pokt-network/poktroll/service/service/{id}";
 
   }
-  rpc ServiceAll (QueryAllServiceRequest) returns (QueryAllServiceResponse) {
+  rpc AllServices (QueryAllServicesRequest) returns (QueryAllServicesResponse) {
     option (google.api.http).get = "/pokt-network/poktroll/service/service";
 
   }
@@ -41,18 +41,19 @@ message QueryParamsResponse {
 }
 
 message QueryGetServiceRequest {
-  string index = 1;
+  // TODO: We could support getting services by name.
+  string id = 1;
 }
 
 message QueryGetServiceResponse {
   poktroll.shared.Service service = 1 [(gogoproto.nullable) = false];
 }
 
-message QueryAllServiceRequest {
+message QueryAllServicesRequest {
   cosmos.base.query.v1beta1.PageRequest pagination = 1;
 }
 
-message QueryAllServiceResponse {
+message QueryAllServicesResponse {
   repeated poktroll.shared.Service service = 1 [(gogoproto.nullable) = false];
   cosmos.base.query.v1beta1.PageResponse pagination = 2;
 }

proto/poktroll/service/tx.proto

@@ -1,35 +1,39 @@
 syntax = "proto3";
-
 package poktroll.service;
 
+option go_package = "github.com/pokt-network/poktroll/x/service/types";
+
 import "amino/amino.proto";
 import "cosmos/msg/v1/msg.proto";
 import "cosmos_proto/cosmos.proto";
 import "gogoproto/gogo.proto";
+
 import "poktroll/service/params.proto";
 import "poktroll/shared/service.proto";
 
-option go_package = "github.com/pokt-network/poktroll/x/service/types";
 
 // Msg defines the Msg service.
 service Msg {
   option (cosmos.msg.v1.service) = true;
-  
+
   // UpdateParams defines a (governance) operation for updating the module
   // parameters. The authority defaults to the x/gov module account.
   rpc UpdateParams (MsgUpdateParams) returns (MsgUpdateParamsResponse);
   rpc AddService   (MsgAddService  ) returns (MsgAddServiceResponse  );
 }
 // MsgUpdateParams is the Msg/UpdateParams request type.
 message MsgUpdateParams {
-  option (cosmos.msg.v1.signer) =                          "authority";
+  option (cosmos.msg.v1.signer) = "authority";
   option           (amino.name) = "poktroll/x/service/MsgUpdateParams";
-  
+
   // authority is the address that controls the module (defaults to x/gov unless overwritten).
   string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 
-  // params defines the module parameters to update.
-
+  // TODO_IMPROVE(#322): The requirement to provide all params is adopted from the
+  // latest Cosmos SDK version. We should look into either improving this ourselves
+  // or seeing if it is on their roadmap.
+
+  // params defines the x/service parameters to update.
   // NOTE: All parameters must be supplied.
   Params params = 2 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true];
 }

proto/poktroll/shared/service.proto

@@ -1,7 +1,7 @@
 syntax = "proto3";
 
 // NOTE that the `shared` package is not a Cosmos module,
-// but rather a manually created package to resolve circular dependencies.
+// but rather a manually created package to resolve circular type dependencies.
 package poktroll.shared;
 
 option go_package = "github.com/pokt-network/poktroll/x/shared/types";
@@ -13,7 +13,7 @@ message Service {
   // For example, what if we want to request a session for a certain service but with some additional configs that identify it?
   string id = 1; // Unique identifier for the service
 
-  // TODO_TECHDEBT: Name is currently unused but acts as a reminder than an optional onchain representation of the service is necessary
+  // TODO_TECHDEBT: Name is currently unused but acts as a reminder that an optional onchain representation of the service is necessary
   string name = 2; // (Optional) Semantic human readable name for the service
 }
 

testutil/keeper/service.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"context"
 	"sync"
 	"testing"
 
@@ -32,7 +33,8 @@ var (
 	mapMu           = sync.RWMutex{}
 )
 
-func ServiceKeeper(t testing.TB) (keeper.Keeper, sdk.Context) {
+func ServiceKeeper(t testing.TB) (keeper.Keeper, context.Context) {
+	t.Helper()
 	storeKey := storetypes.NewKVStoreKey(types.StoreKey)
 
 	db := dbm.NewMemDB()
@@ -48,28 +50,30 @@ func ServiceKeeper(t testing.TB) (keeper.Keeper, sdk.Context) {
 	mockBankKeeper := mocks.NewMockBankKeeper(ctrl)
 	mockBankKeeper.EXPECT().
 		SpendableCoins(gomock.Any(), gomock.Any()).
-		DoAndReturn(func(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins {
-			mapMu.RLock()
-			defer mapMu.RUnlock()
-			if coins, ok := mapAccAddrCoins[addr.String()]; ok {
-				return coins
-			}
-			return sdk.Coins{}
-		}).
-		AnyTimes()
+		DoAndReturn(
+			func(ctx context.Context, addr sdk.AccAddress) sdk.Coins {
+				mapMu.RLock()
+				defer mapMu.RUnlock()
+				if coins, ok := mapAccAddrCoins[addr.String()]; ok {
+					return coins
+				}
+				return sdk.Coins{}
+			},
+		).AnyTimes()
 	mockBankKeeper.EXPECT().
 		SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), types.ModuleName, gomock.Any()).
-		DoAndReturn(func(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error {
-			mapMu.Lock()
-			defer mapMu.Unlock()
-			coins := mapAccAddrCoins[senderAddr.String()]
-			if coins.AmountOf("upokt").GT(amt.AmountOf("upokt")) {
-				mapAccAddrCoins[senderAddr.String()] = coins.Sub(amt...)
-				return nil
-			}
-			return types.ErrServiceNotEnoughFunds
-		}).
-		AnyTimes()
+		DoAndReturn(
+			func(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error {
+				mapMu.Lock()
+				defer mapMu.Unlock()
+				coins := mapAccAddrCoins[senderAddr.String()]
+				if coins.AmountOf("upokt").GT(amt.AmountOf("upokt")) {
+					mapAccAddrCoins[senderAddr.String()] = coins.Sub(amt...)
+					return nil
+				}
+				return types.ErrServiceNotEnoughFunds
+			},
+		).AnyTimes()
 
 	k := keeper.NewKeeper(
 		cdc,
@@ -82,7 +86,7 @@ func ServiceKeeper(t testing.TB) (keeper.Keeper, sdk.Context) {
 	ctx := sdk.NewContext(stateStore, cmtproto.Header{}, false, log.NewNopLogger())
 
 	// Initialize params
-	k.SetParams(ctx, types.DefaultParams())
+	require.NoError(t, k.SetParams(ctx, types.DefaultParams()))
 
 	return k, ctx
 }

x/service/keeper/msg_server.go

@@ -1,8 +1,6 @@
 package keeper
 
-import (
-	"github.com/pokt-network/poktroll/x/service/types"
-)
+import "github.com/pokt-network/poktroll/x/service/types"
 
 type msgServer struct {
 	Keeper

x/service/keeper/msg_server_add_service.go

@@ -10,12 +10,10 @@ import (
 	"github.com/pokt-network/poktroll/x/service/types"
 )
 
-// AddService handles MsgAddService and adds a service to the network storing
-// it in the service keeper's store using the provided ID from the message.
-// If the transaction's signer does not have enough funds (upokt) to cover the
-// AddServiceFee service module's governance parameter, it will not be able to
-// add the service. If it does, the fee will be deducted and debited to the
-// service module's account, then the service will be added on-chain.
+// AddService adds a service to the network.
+// The operation checks if the signer has enough funds (upokt) to pay the AddServiceFee.
+// If funds are insufficient, the service won't be added. Otherwise, the fee is transferred from
+// the signer to the service module's account, afterwards the service will be present on-chain.
 func (k msgServer) AddService(
 	goCtx context.Context,
 	msg *types.MsgAddService,