diff --git a/x/supplier/keeper/msg_server_unstake_supplier.go b/x/supplier/keeper/msg_server_unstake_supplier.go index 17cfda752..2ac70a39b 100644 --- a/x/supplier/keeper/msg_server_unstake_supplier.go +++ b/x/supplier/keeper/msg_server_unstake_supplier.go @@ -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) diff --git a/x/supplier/keeper/msg_server_unstake_supplier_test.go b/x/supplier/keeper/msg_server_unstake_supplier_test.go index fc844e4e0..d96a433fd 100644 --- a/x/supplier/keeper/msg_server_unstake_supplier_test.go +++ b/x/supplier/keeper/msg_server_unstake_supplier_test.go @@ -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) @@ -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) { diff --git a/x/supplier/keeper/msg_update_params.go b/x/supplier/keeper/msg_update_params.go index ed841b914..4fa159dff 100644 --- a/x/supplier/keeper/msg_update_params.go +++ b/x/supplier/keeper/msg_update_params.go @@ -2,6 +2,10 @@ package keeper import ( "context" + "fmt" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/pokt-network/poktroll/x/supplier/types" ) @@ -10,15 +14,24 @@ 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 = fmt.Errorf("unable to set params: %w", err) + logger.Error(err.Error()) + return nil, status.Error(codes.Internal, err.Error()) } return &types.MsgUpdateParamsResponse{}, nil diff --git a/x/supplier/keeper/query_supplier.go b/x/supplier/keeper/query_supplier.go index 92112827a..3d370a8fa 100644 --- a/x/supplier/keeper/query_supplier.go +++ b/x/supplier/keeper/query_supplier.go @@ -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") } @@ -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)