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: supplier module gRPC return errors #962

Merged
merged 5 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
39 changes: 26 additions & 13 deletions x/supplier/keeper/msg_server_unstake_supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,45 @@ func (k msgServer) UnstakeSupplier(
logger.Info(fmt.Sprintf("About to unstake supplier with msg: %v", msg))

if err := msg.ValidateBasic(); err != nil {
return nil, err
return nil, status.Error(codes.InvalidArgument, err.Error())
}

// Check if the supplier already exists or not
supplier, isSupplierFound := k.GetSupplier(ctx, msg.OperatorAddress)
supplier, isSupplierFound := k.GetSupplier(ctx, msg.GetOperatorAddress())
if !isSupplierFound {
logger.Info(fmt.Sprintf("Supplier not found. Cannot unstake address %s", msg.OperatorAddress))
return nil, suppliertypes.ErrSupplierNotFound
logger.Info(fmt.Sprintf("Supplier not found. Cannot unstake address %s", msg.GetOperatorAddress()))
return nil, status.Error(
codes.NotFound,
suppliertypes.ErrSupplierNotFound.Wrapf(
"supplier with operator address %q", msg.GetOperatorAddress(),
).Error(),
)
}

// Ensure the singer address matches the owner address or the operator address.
if !supplier.HasOperator(msg.Signer) && !supplier.HasOwner(msg.Signer) {
logger.Error("only the supplier owner or operator is allowed to unstake the supplier")
return nil, sharedtypes.ErrSharedUnauthorizedSupplierUpdate.Wrapf(
"signer %q is not allowed to unstake supplier %v",
msg.Signer,
supplier,
if !supplier.HasOperator(msg.GetSigner()) && !supplier.HasOwner(msg.GetSigner()) {
logger.Info("only the supplier owner or operator is allowed to unstake the supplier")
return nil, status.Error(
codes.PermissionDenied,
sharedtypes.ErrSharedUnauthorizedSupplierUpdate.Wrapf(
"signer %q is not allowed to unstake supplier %+v",
msg.Signer,
supplier,
).Error(),
)
}

logger.Info(fmt.Sprintf("Supplier found. Unstaking supplier for address %s", msg.OperatorAddress))
logger.Info(fmt.Sprintf("Supplier found. Unstaking supplier with operating address %s", msg.GetOperatorAddress()))

// Check if the supplier has already initiated the unstake action.
if supplier.IsUnbonding() {
logger.Warn(fmt.Sprintf("Supplier %s still unbonding from previous unstaking", msg.OperatorAddress))
return nil, suppliertypes.ErrSupplierIsUnstaking
logger.Info(fmt.Sprintf("Supplier %s still unbonding from previous unstaking", msg.GetOperatorAddress()))
return nil, status.Error(
codes.FailedPrecondition,
suppliertypes.ErrSupplierIsUnstaking.Wrapf(
"supplier with operator address %q", msg.GetOperatorAddress(),
).Error(),
)
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
Expand Down
4 changes: 2 additions & 2 deletions x/supplier/keeper/msg_server_unstake_supplier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func TestMsgServer_UnstakeSupplier_FailIfNotStaked(t *testing.T) {
}
_, err := srv.UnstakeSupplier(ctx, unstakeMsg)
require.Error(t, err)
require.ErrorIs(t, err, suppliertypes.ErrSupplierNotFound)
require.ErrorContains(t, err, suppliertypes.ErrSupplierNotFound.Error())

_, isSupplierFound = supplierModuleKeepers.GetSupplier(ctx, supplierOperatorAddr)
require.False(t, isSupplierFound)
Expand Down Expand Up @@ -311,7 +311,7 @@ func TestMsgServer_UnstakeSupplier_FailIfCurrentlyUnstaking(t *testing.T) {
ctx = keepertest.SetBlockHeight(ctx, sdkCtx.BlockHeight()+1)

_, err = srv.UnstakeSupplier(ctx, unstakeMsg)
require.ErrorIs(t, err, suppliertypes.ErrSupplierIsUnstaking)
require.ErrorContains(t, err, suppliertypes.ErrSupplierIsUnstaking.Error())
}

func TestMsgServer_UnstakeSupplier_OperatorCanUnstake(t *testing.T) {
Expand Down
18 changes: 15 additions & 3 deletions x/supplier/keeper/msg_update_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,34 @@ package keeper
import (
"context"

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

"github.com/pokt-network/poktroll/x/supplier/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.ErrSupplierInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority)
return nil, status.Error(
codes.PermissionDenied,
types.ErrSupplierInvalidSigner.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 = status.Error(codes.Internal, err.Error())
bryanchriswhite marked this conversation as resolved.
Show resolved Hide resolved
logger.Error(err.Error())
return nil, status.Error(codes.Internal, err.Error())
}

return &types.MsgUpdateParamsResponse{}, nil
Expand Down
6 changes: 5 additions & 1 deletion x/supplier/keeper/query_supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ func (k Keeper) AllSuppliers(
ctx context.Context,
req *types.QueryAllSuppliersRequest,
) (*types.QueryAllSuppliersResponse, error) {
logger := k.Logger().With("method", "AllSuppliers")

if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}
Expand All @@ -33,7 +35,9 @@ func (k Keeper) AllSuppliers(
func(key []byte, value []byte) error {
var supplier sharedtypes.Supplier
if err := k.cdc.Unmarshal(value, &supplier); err != nil {
return err
err = fmt.Errorf("unmarshaling supplier with key (hex): %x: %+v", key, err)
logger.Error(err.Error())
return status.Error(codes.Internal, err.Error())
}

suppliers = append(suppliers, supplier)
Expand Down