From 9259f6a8066547114b9f7acb8eb852ed8d7f6b2c Mon Sep 17 00:00:00 2001 From: Bryan White Date: Fri, 22 Nov 2024 11:58:18 +0100 Subject: [PATCH 1/4] fix: gateway module gRPC status error returns --- x/gateway/keeper/msg_server_stake_gateway.go | 2 +- x/gateway/keeper/msg_server_unstake_gateway.go | 13 +++++++++---- x/gateway/keeper/msg_update_params.go | 13 ++++++++++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/x/gateway/keeper/msg_server_stake_gateway.go b/x/gateway/keeper/msg_server_stake_gateway.go index 8a6ce25ab..3f3f1cc14 100644 --- a/x/gateway/keeper/msg_server_stake_gateway.go +++ b/x/gateway/keeper/msg_server_stake_gateway.go @@ -90,7 +90,7 @@ func (k msgServer) StakeGateway( if err != nil { // TODO_TECHDEBT(#384): determine whether to continue using cosmos logger for debug level. logger.Error(fmt.Sprintf("could not escrowed %v coins from %q to %q module account due to %v", coinsToEscrow, gatewayAddress, types.ModuleName, err)) - return nil, err + return nil, status.Error(codes.Internal, err.Error()) } // Update the Gateway in the store diff --git a/x/gateway/keeper/msg_server_unstake_gateway.go b/x/gateway/keeper/msg_server_unstake_gateway.go index 2e09bec19..49ebc71ac 100644 --- a/x/gateway/keeper/msg_server_unstake_gateway.go +++ b/x/gateway/keeper/msg_server_unstake_gateway.go @@ -30,7 +30,7 @@ func (k msgServer) UnstakeGateway( logger.Info(fmt.Sprintf("About to unstake gateway with msg: %v", msg)) if err := msg.ValidateBasic(); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } // Check if the gateway already exists or not @@ -38,7 +38,12 @@ func (k msgServer) UnstakeGateway( gateway, isGatewayFound := k.GetGateway(ctx, msg.Address) if !isGatewayFound { logger.Info(fmt.Sprintf("Gateway not found. Cannot unstake address %s", msg.Address)) - return nil, types.ErrGatewayNotFound + return nil, status.Error( + codes.NotFound, + types.ErrGatewayNotFound.Wrapf( + "gateway address %s", msg.Address, + ).Error(), + ) } logger.Info(fmt.Sprintf("Gateway found. Unstaking gateway for address %s", msg.Address)) @@ -46,14 +51,14 @@ func (k msgServer) UnstakeGateway( gatewayAddress, err := sdk.AccAddressFromBech32(msg.Address) if err != nil { logger.Error(fmt.Sprintf("could not parse address %s", msg.Address)) - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } // Send the coins from the gateway pool back to the gateway err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, gatewayAddress, []sdk.Coin{*gateway.Stake}) if err != nil { logger.Error(fmt.Sprintf("could not send %v coins from %s module to %s account due to %v", gateway.Stake, gatewayAddress, types.ModuleName, err)) - return nil, err + return nil, status.Error(codes.Internal, err.Error()) } // Update the Gateway in the store diff --git a/x/gateway/keeper/msg_update_params.go b/x/gateway/keeper/msg_update_params.go index d8039b896..2c16b0bf7 100644 --- a/x/gateway/keeper/msg_update_params.go +++ b/x/gateway/keeper/msg_update_params.go @@ -4,6 +4,8 @@ import ( "context" sdk "github.com/cosmos/cosmos-sdk/types" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/pokt-network/poktroll/x/gateway/types" ) @@ -13,16 +15,21 @@ func (k msgServer) UpdateParams( req *types.MsgUpdateParams, ) (*types.MsgUpdateParamsResponse, error) { 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.ErrGatewayInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority) + return nil, status.Error( + codes.PermissionDenied, + types.ErrGatewayInvalidSigner.Wrapf( + "invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority, + ).Error(), + ) } ctx := sdk.UnwrapSDKContext(goCtx) // NOTE(#322): Omitted parameters will be set to their zero value. if err := k.SetParams(ctx, req.Params); err != nil { - return nil, err + return nil, status.Error(codes.Internal, err.Error()) } return &types.MsgUpdateParamsResponse{}, nil From 62de1ed0163a7a537a02cba1e811f4bae8201c45 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 25 Nov 2024 14:09:03 +0100 Subject: [PATCH 2/4] fix: outstanding gateway module non-status (gRPC) errors --- x/gateway/keeper/query_gateway.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x/gateway/keeper/query_gateway.go b/x/gateway/keeper/query_gateway.go index 0b6d2ab2a..a550ea169 100644 --- a/x/gateway/keeper/query_gateway.go +++ b/x/gateway/keeper/query_gateway.go @@ -14,6 +14,8 @@ import ( ) func (k Keeper) AllGateways(ctx context.Context, req *types.QueryAllGatewaysRequest) (*types.QueryAllGatewaysResponse, error) { + logger := k.Logger().With("method", "AllGateways") + if req == nil { return nil, status.Error(codes.InvalidArgument, "invalid request") } @@ -26,7 +28,8 @@ func (k Keeper) AllGateways(ctx context.Context, req *types.QueryAllGatewaysRequ pageRes, err := query.Paginate(gatewayStore, req.Pagination, func(key []byte, value []byte) error { var gateway types.Gateway if err := k.cdc.Unmarshal(value, &gateway); err != nil { - return err + logger.Error(fmt.Sprintf("unmarshaling gateway with key (hex): %x", key)) + return status.Error(codes.Internal, err.Error()) } gateways = append(gateways, gateway) From a3ca541dd7cf73b8bb6f06fa7573066c4c3aed3c Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 25 Nov 2024 15:05:52 +0100 Subject: [PATCH 3/4] chore: self-review improvements --- x/gateway/keeper/msg_server_unstake_gateway.go | 7 ++++--- x/gateway/keeper/msg_update_params.go | 2 ++ x/gateway/keeper/query_gateway.go | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/x/gateway/keeper/msg_server_unstake_gateway.go b/x/gateway/keeper/msg_server_unstake_gateway.go index 49ebc71ac..d8c757713 100644 --- a/x/gateway/keeper/msg_server_unstake_gateway.go +++ b/x/gateway/keeper/msg_server_unstake_gateway.go @@ -41,7 +41,7 @@ func (k msgServer) UnstakeGateway( return nil, status.Error( codes.NotFound, types.ErrGatewayNotFound.Wrapf( - "gateway address %s", msg.Address, + "gateway with address %s", msg.Address, ).Error(), ) } @@ -50,14 +50,15 @@ func (k msgServer) UnstakeGateway( // Retrieve the address of the gateway gatewayAddress, err := sdk.AccAddressFromBech32(msg.Address) if err != nil { - logger.Error(fmt.Sprintf("could not parse address %s", msg.Address)) + logger.Info(fmt.Sprintf("could not parse address %s", msg.Address)) return nil, status.Error(codes.InvalidArgument, err.Error()) } // Send the coins from the gateway pool back to the gateway err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, gatewayAddress, []sdk.Coin{*gateway.Stake}) if err != nil { - logger.Error(fmt.Sprintf("could not send %v coins from %s module to %s account due to %v", gateway.Stake, gatewayAddress, types.ModuleName, err)) + err = fmt.Errorf("could not send %v coins from %s module to %s account due to %v", gateway.Stake, gatewayAddress, types.ModuleName, err) + logger.Error(err.Error()) return nil, status.Error(codes.Internal, err.Error()) } diff --git a/x/gateway/keeper/msg_update_params.go b/x/gateway/keeper/msg_update_params.go index 2c16b0bf7..965017953 100644 --- a/x/gateway/keeper/msg_update_params.go +++ b/x/gateway/keeper/msg_update_params.go @@ -29,6 +29,8 @@ func (k msgServer) UpdateParams( ctx := sdk.UnwrapSDKContext(goCtx) // NOTE(#322): Omitted parameters will be set to their zero value. if err := k.SetParams(ctx, req.Params); err != nil { + err = fmt.Errorf("unable to set params: %w", err) + logger.Error(err.Error()) return nil, status.Error(codes.Internal, err.Error()) } diff --git a/x/gateway/keeper/query_gateway.go b/x/gateway/keeper/query_gateway.go index a550ea169..8420bde17 100644 --- a/x/gateway/keeper/query_gateway.go +++ b/x/gateway/keeper/query_gateway.go @@ -28,7 +28,7 @@ func (k Keeper) AllGateways(ctx context.Context, req *types.QueryAllGatewaysRequ pageRes, err := query.Paginate(gatewayStore, req.Pagination, func(key []byte, value []byte) error { var gateway types.Gateway if err := k.cdc.Unmarshal(value, &gateway); err != nil { - logger.Error(fmt.Sprintf("unmarshaling gateway with key (hex): %x", key)) + logger.Error(fmt.Sprintf("unmarshaling gateway with key (hex): %x: %+v", key, err)) return status.Error(codes.Internal, err.Error()) } From c1cc4ed4a7c5f1375358f885e6478ab99c3b4b38 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 25 Nov 2024 15:20:01 +0100 Subject: [PATCH 4/4] fix: failing tests --- x/gateway/keeper/msg_server_unstake_gateway_test.go | 2 +- x/gateway/keeper/msg_update_params.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/x/gateway/keeper/msg_server_unstake_gateway_test.go b/x/gateway/keeper/msg_server_unstake_gateway_test.go index 4c977ef3c..64ad5cadf 100644 --- a/x/gateway/keeper/msg_server_unstake_gateway_test.go +++ b/x/gateway/keeper/msg_server_unstake_gateway_test.go @@ -66,7 +66,7 @@ func TestMsgServer_UnstakeGateway_FailIfNotStaked(t *testing.T) { unstakeMsg := &types.MsgUnstakeGateway{Address: addr} _, err := srv.UnstakeGateway(ctx, unstakeMsg) require.Error(t, err) - require.ErrorIs(t, err, types.ErrGatewayNotFound) + require.ErrorContains(t, err, types.ErrGatewayNotFound.Error()) _, isGatewayFound = k.GetGateway(ctx, addr) require.False(t, isGatewayFound) diff --git a/x/gateway/keeper/msg_update_params.go b/x/gateway/keeper/msg_update_params.go index 965017953..9b13b4d17 100644 --- a/x/gateway/keeper/msg_update_params.go +++ b/x/gateway/keeper/msg_update_params.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" sdk "github.com/cosmos/cosmos-sdk/types" "google.golang.org/grpc/codes" @@ -14,6 +15,8 @@ func (k msgServer) UpdateParams( goCtx context.Context, req *types.MsgUpdateParams, ) (*types.MsgUpdateParamsResponse, error) { + logger := k.Logger().With("method", "UpdateParams") + if err := req.ValidateBasic(); err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) }