Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Code Health] fix: service module gRPC status error returns #959

Merged
merged 2 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 37 additions & 21 deletions x/service/keeper/msg_server_add_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/pokt-network/poktroll/telemetry"
"github.com/pokt-network/poktroll/x/service/types"
Expand Down Expand Up @@ -33,61 +35,75 @@ func (k msgServer) AddService(
// Validate the message.
if err := msg.ValidateBasic(); err != nil {
logger.Error(fmt.Sprintf("Adding service failed basic validation: %v", err))
return nil, err
return nil, status.Error(codes.InvalidArgument, err.Error())
}

// Check if the service already exists or not.
foundService, found := k.GetService(ctx, msg.Service.Id)
if found {
if foundService.OwnerAddress != msg.Service.OwnerAddress {
logger.Error(fmt.Sprintf("Owner address of existing service (%q) does not match the owner address %q", foundService.OwnerAddress, msg.OwnerAddress))
return nil, types.ErrServiceInvalidOwnerAddress.Wrapf(
"existing owner address %q does not match the new owner address %q",
foundService.OwnerAddress, msg.Service.OwnerAddress,
return nil, status.Error(
codes.InvalidArgument,
types.ErrServiceInvalidOwnerAddress.Wrapf(
"existing owner address %q does not match the new owner address %q",
foundService.OwnerAddress, msg.Service.OwnerAddress,
).Error(),
)
}
return nil, types.ErrServiceAlreadyExists.Wrapf(
"TODO_MAINNET(@red-0ne): This is an ephemeral state of the code. Once we s/AddService/UpdateService/g, add the business logic here for updates here.",
return nil, status.Error(
codes.FailedPrecondition,
types.ErrServiceAlreadyExists.Wrapf(
"TODO_MAINNET(@red-0ne): This is an ephemeral state of the code. Once we s/AddService/UpdateService/g, add the business logic here for updates here.",
).Error(),
)
}

// Retrieve the address of the actor adding the service; the owner of the service.
serviceOwnerAddr, err := sdk.AccAddressFromBech32(msg.OwnerAddress)
if err != nil {
logger.Error(fmt.Sprintf("could not parse address %s", msg.OwnerAddress))
return nil, types.ErrServiceInvalidAddress.Wrapf(
"%s is not in Bech32 format", msg.OwnerAddress,
return nil, status.Error(
codes.InvalidArgument,
types.ErrServiceInvalidAddress.Wrapf(
"%s is not in Bech32 format", msg.OwnerAddress,
).Error(),
)
}

// Check the actor has sufficient funds to pay for the add service fee.
accCoins := k.bankKeeper.SpendableCoins(ctx, serviceOwnerAddr)
if accCoins.Len() == 0 {
logger.Error(fmt.Sprintf("%s doesn't have any funds to add service: %v", serviceOwnerAddr, err))
return nil, types.ErrServiceNotEnoughFunds.Wrapf(
"account has no spendable coins",
return nil, status.Error(
codes.FailedPrecondition,
types.ErrServiceNotEnoughFunds.Wrapf(
"account has no spendable coins",
).Error(),
)
}

// Check the balance of upokt is enough to cover the AddServiceFee.
accBalance := accCoins.AmountOf("upokt")
addServiceFee := k.GetParams(ctx).AddServiceFee
if accBalance.LTE(addServiceFee.Amount) {
logger.Error(fmt.Sprintf("%s doesn't have enough funds to add service: %v", serviceOwnerAddr, err))
return nil, types.ErrServiceNotEnoughFunds.Wrapf(
"account has %s, but the service fee is %s",
accBalance, k.GetParams(ctx).AddServiceFee,
return nil, status.Error(
codes.FailedPrecondition,
types.ErrServiceNotEnoughFunds.Wrapf(
"account has %s, but the service fee is %s",
accBalance, k.GetParams(ctx).AddServiceFee,
).Error(),
)
}

// Deduct the service fee from the actor's balance.
serviceFee := sdk.NewCoins(*addServiceFee)
err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, serviceOwnerAddr, types.ModuleName, serviceFee)
if err != nil {
logger.Error(fmt.Sprintf("Failed to deduct service fee from actor's balance: %v", err))
return nil, types.ErrServiceFailedToDeductFee.Wrapf(
"account has %s, failed to deduct %s",
accBalance, k.GetParams(ctx).AddServiceFee,
logger.Error(fmt.Sprintf("Failed to deduct service fee from actor's balance: %+v", err))
return nil, status.Error(
codes.Internal,
types.ErrServiceFailedToDeductFee.Wrapf(
"account has %s, failed to deduct %s",
accBalance, k.GetParams(ctx).AddServiceFee,
).Error(),
)
}

Expand Down
19 changes: 16 additions & 3 deletions x/service/keeper/msg_update_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,33 @@ package keeper

import (
"context"
"fmt"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/pokt-network/poktroll/x/service/types"
)

func (k msgServer) UpdateParams(ctx context.Context, req *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
logger := k.Logger().With("method", "UpdateParams")

if err := req.ValidateBasic(); err != nil {
return nil, err
return nil, status.Error(codes.InvalidArgument, err.Error())
}
if k.GetAuthority() != req.Authority {
return nil, types.ErrServiceInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority)
return nil, status.Error(
codes.PermissionDenied,
types.ErrServiceInvalidSigner.Wrapf(
"invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority,
).Error(),
)
}

if err := k.SetParams(ctx, req.Params); err != nil {
return nil, err
err = fmt.Errorf("unable to set params: %w", err)
logger.Error(err.Error())
return nil, status.Error(codes.Internal, err.Error())
}

return &types.MsgUpdateParamsResponse{}, nil
Expand Down
7 changes: 6 additions & 1 deletion x/service/keeper/query_relay_mining_difficulty.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"context"
"fmt"

"cosmossdk.io/store/prefix"
"github.com/cosmos/cosmos-sdk/runtime"
Expand All @@ -13,6 +14,8 @@ import (
)

func (k Keeper) RelayMiningDifficultyAll(ctx context.Context, req *types.QueryAllRelayMiningDifficultyRequest) (*types.QueryAllRelayMiningDifficultyResponse, error) {
logger := k.Logger().With("method", "RelayMiningDifficultyAll")

if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -25,7 +28,9 @@ func (k Keeper) RelayMiningDifficultyAll(ctx context.Context, req *types.QueryAl
pageRes, err := query.Paginate(relayMiningDifficultyStore, req.Pagination, func(key []byte, value []byte) error {
var relayMiningDifficulty types.RelayMiningDifficulty
if err := k.cdc.Unmarshal(value, &relayMiningDifficulty); err != nil {
return err
err = fmt.Errorf("unable to unmarshal relayMiningDifficulty with key (hex): %x: %w", key, err)
logger.Error(err.Error())
return status.Error(codes.Internal, err.Error())
}

relayMiningDifficulties = append(relayMiningDifficulties, relayMiningDifficulty)
Expand Down
7 changes: 6 additions & 1 deletion x/service/keeper/query_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"context"
"fmt"

"cosmossdk.io/store/prefix"
"github.com/cosmos/cosmos-sdk/runtime"
Expand All @@ -15,6 +16,8 @@ import (

// AllServices queries all services.
func (k Keeper) AllServices(ctx context.Context, req *types.QueryAllServicesRequest) (*types.QueryAllServicesResponse, error) {
logger := k.Logger().With("method", "AllServices")

if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -27,7 +30,9 @@ func (k Keeper) AllServices(ctx context.Context, req *types.QueryAllServicesRequ
pageRes, err := query.Paginate(serviceStore, req.Pagination, func(key []byte, value []byte) error {
var service sharedtypes.Service
if err := k.cdc.Unmarshal(value, &service); err != nil {
return err
err = fmt.Errorf("unable to unmarshal service with key (hex): %x: %w", key, err)
logger.Error(err.Error())
return status.Error(codes.Internal, err.Error())
}

services = append(services, service)
Expand Down
Loading