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: application module gRPC status error returns #954

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
55 changes: 40 additions & 15 deletions x/application/keeper/msg_server_delegate_to_gateway.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"
apptypes "github.com/pokt-network/poktroll/x/application/types"
Expand All @@ -20,44 +22,66 @@ func (k msgServer) DelegateToGateway(ctx context.Context, msg *apptypes.MsgDeleg
)

logger := k.Logger().With("method", "DelegateToGateway")
logger.Info(fmt.Sprintf("About to delegate application to gateway with msg: %v", msg))
logger.Info(fmt.Sprintf("About to delegate application to gateway with msg: %+v", msg))

if err := msg.ValidateBasic(); err != nil {
logger.Error(fmt.Sprintf("Delegation Message failed basic validation: %v", err))
return nil, err
logger.Info(fmt.Sprintf("Delegation Message failed basic validation: %s", err))
return nil, status.Error(codes.InvalidArgument, err.Error())
}

// Retrieve the application from the store
app, found := k.GetApplication(ctx, msg.AppAddress)
app, found := k.GetApplication(ctx, msg.GetAppAddress())
if !found {
logger.Info(fmt.Sprintf("Application not found with address [%s]", msg.AppAddress))
return nil, apptypes.ErrAppNotFound.Wrapf("application not found with address: %s", msg.AppAddress)
return nil, status.Error(
codes.NotFound,
apptypes.ErrAppNotFound.Wrapf(
"application with address: %s", msg.GetAppAddress(),
).Error(),
)
}
logger.Info(fmt.Sprintf("Application found with address [%s]", msg.AppAddress))

// Check if the gateway is staked
if _, found := k.gatewayKeeper.GetGateway(ctx, msg.GatewayAddress); !found {
logger.Info(fmt.Sprintf("Gateway not found with address [%s]", msg.GatewayAddress))
return nil, apptypes.ErrAppGatewayNotFound.Wrapf("gateway not found with address: %s", msg.GatewayAddress)
if _, found := k.gatewayKeeper.GetGateway(ctx, msg.GetGatewayAddress()); !found {
logger.Info(fmt.Sprintf("Gateway not found with address [%s]", msg.GetGatewayAddress()))
return nil, status.Error(
codes.NotFound,
apptypes.ErrAppGatewayNotFound.Wrapf(
"gateway with address: %q", msg.GetGatewayAddress(),
).Error(),
)
}

// Ensure the application is not already delegated to the maximum number of gateways
maxDelegatedParam := k.GetParams(ctx).MaxDelegatedGateways
if uint64(len(app.DelegateeGatewayAddresses)) >= maxDelegatedParam {
logger.Info(fmt.Sprintf("Application already delegated to maximum number of gateways: %d", maxDelegatedParam))
return nil, apptypes.ErrAppMaxDelegatedGateways.Wrapf("application already delegated to %d gateways", maxDelegatedParam)
return nil, status.Error(
codes.FailedPrecondition,
apptypes.ErrAppMaxDelegatedGateways.Wrapf(
"application with address %q already delegated to %d (max) gateways",
msg.GetAppAddress(), maxDelegatedParam,
).Error(),
)
}

// Check if the application is already delegated to the gateway
for _, gatewayAddr := range app.DelegateeGatewayAddresses {
if gatewayAddr == msg.GatewayAddress {
if gatewayAddr == msg.GetGatewayAddress() {
logger.Info(fmt.Sprintf("Application already delegated to gateway with address [%s]", msg.GatewayAddress))
return nil, apptypes.ErrAppAlreadyDelegated.Wrapf("application already delegated to gateway with address: %s", msg.GatewayAddress)
return nil, status.Error(
codes.AlreadyExists,
apptypes.ErrAppAlreadyDelegated.Wrapf(
"application with address %q already delegated to gateway with address: %q",
msg.GetAppAddress(), msg.GetGatewayAddress(),
).Error(),
)
}
}

// Update the application with the new delegatee public key
app.DelegateeGatewayAddresses = append(app.DelegateeGatewayAddresses, msg.GatewayAddress)
app.DelegateeGatewayAddresses = append(app.DelegateeGatewayAddresses, msg.GetGatewayAddress())
logger.Info("Successfully added delegatee public key to application")

// Update the application store with the new delegation
Expand All @@ -72,12 +96,13 @@ func (k msgServer) DelegateToGateway(ctx context.Context, msg *apptypes.MsgDeleg
Application: &app,
SessionEndHeight: sessionEndHeight,
}
logger.Info(fmt.Sprintf("Emitting application redelegation event %v", event))
logger.Info(fmt.Sprintf("Emitting application redelegation event %+v", event))

sdkCtx := sdk.UnwrapSDKContext(ctx)
if err := sdkCtx.EventManager().EmitTypedEvent(event); err != nil {
logger.Error(fmt.Sprintf("Failed to emit application redelegation event: %v", err))
return nil, err
err = fmt.Errorf("failed to emit application redelegation event: %w", err)
logger.Error(err.Error())
return nil, status.Error(codes.Internal, err.Error())
}

isSuccessful = true
Expand Down
6 changes: 3 additions & 3 deletions x/application/keeper/msg_server_delegate_to_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func TestMsgServer_DelegateToGateway_FailDuplicate(t *testing.T) {

// Attempt to delegate the application to the gateway again
_, err = srv.DelegateToGateway(ctx, delegateMsg2)
require.ErrorIs(t, err, apptypes.ErrAppAlreadyDelegated)
require.ErrorContains(t, err, apptypes.ErrAppAlreadyDelegated.Error())

events = sdkCtx.EventManager().Events()
require.Equal(t, 0, len(events))
Expand Down Expand Up @@ -221,7 +221,7 @@ func TestMsgServer_DelegateToGateway_FailGatewayNotStaked(t *testing.T) {

// Attempt to delegate the application to the unstaked gateway
_, err = srv.DelegateToGateway(ctx, delegateMsg)
require.ErrorIs(t, err, apptypes.ErrAppGatewayNotFound)
require.ErrorContains(t, err, apptypes.ErrAppGatewayNotFound.Error())
foundApp, isAppFound := k.GetApplication(ctx, appAddr)
require.True(t, isAppFound)
require.Equal(t, 0, len(foundApp.DelegateeGatewayAddresses))
Expand Down Expand Up @@ -312,7 +312,7 @@ func TestMsgServer_DelegateToGateway_FailMaxReached(t *testing.T) {

// Attempt to delegate the application when the max is already reached
_, err = srv.DelegateToGateway(ctx, delegateMsg)
require.ErrorIs(t, err, apptypes.ErrAppMaxDelegatedGateways)
require.ErrorContains(t, err, apptypes.ErrAppMaxDelegatedGateways.Error())

events = sdkCtx.EventManager().Events()
filteredEvents = testevents.FilterEvents[*apptypes.EventRedelegation](t, events)
Expand Down
40 changes: 27 additions & 13 deletions x/application/keeper/msg_server_undelegate_from_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"slices"

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

"github.com/pokt-network/poktroll/telemetry"
apptypes "github.com/pokt-network/poktroll/x/application/types"
Expand All @@ -20,32 +22,43 @@ func (k msgServer) UndelegateFromGateway(ctx context.Context, msg *apptypes.MsgU
)

logger := k.Logger().With("method", "UndelegateFromGateway")
logger.Info(fmt.Sprintf("About to undelegate application from gateway with msg: %v", msg))
logger.Info(fmt.Sprintf("About to undelegate application from gateway with msg: %+v", msg))

// Basic validation of the message
if err := msg.ValidateBasic(); err != nil {
logger.Error(fmt.Sprintf("Undelegation Message failed basic validation: %v", err))
return nil, err
logger.Info(fmt.Sprintf("Undelegation Message failed basic validation: %s", err))
return nil, status.Error(codes.InvalidArgument, err.Error())
}

// Retrieve the application from the store
foundApp, isAppFound := k.GetApplication(ctx, msg.AppAddress)
foundApp, isAppFound := k.GetApplication(ctx, msg.GetAppAddress())
if !isAppFound {
logger.Info(fmt.Sprintf("Application not found with address [%s]", msg.AppAddress))
return nil, apptypes.ErrAppNotFound.Wrapf("application not found with address: %s", msg.AppAddress)
logger.Info(fmt.Sprintf("Application not found with address [%s]", msg.GetAppAddress()))
return nil, status.Error(
codes.NotFound,
apptypes.ErrAppNotFound.Wrapf(
"application with address: %q", msg.GetAppAddress(),
).Error(),
)
}
logger.Info(fmt.Sprintf("Application found with address [%s]", msg.AppAddress))
logger.Info(fmt.Sprintf("Application found with address [%s]", msg.GetAppAddress()))

// Check if the application is already delegated to the gateway
foundIdx := -1
for i, gatewayAddr := range foundApp.DelegateeGatewayAddresses {
if gatewayAddr == msg.GatewayAddress {
if gatewayAddr == msg.GetGatewayAddress() {
foundIdx = i
}
}
if foundIdx == -1 {
logger.Info(fmt.Sprintf("Application not delegated to gateway with address [%s]", msg.GatewayAddress))
return nil, apptypes.ErrAppNotDelegated.Wrapf("application not delegated to gateway with address: %s", msg.GatewayAddress)
logger.Info(fmt.Sprintf("Application not delegated to gateway with address [%s]", msg.GetGatewayAddress()))
return nil, status.Error(
codes.FailedPrecondition,
apptypes.ErrAppNotDelegated.Wrapf(
"application with address %q not delegated to gateway with address: %q",
msg.GetAppAddress(), msg.GetGatewayAddress(),
).Error(),
)
}

// Remove the gateway from the application's delegatee gateway public keys
Expand All @@ -55,7 +68,7 @@ func (k msgServer) UndelegateFromGateway(ctx context.Context, msg *apptypes.MsgU
currentHeight := sdkCtx.BlockHeight()
sessionEndHeight := k.sharedKeeper.GetSessionEndHeight(ctx, currentHeight)

k.recordPendingUndelegation(ctx, &foundApp, msg.GatewayAddress, currentHeight)
k.recordPendingUndelegation(ctx, &foundApp, msg.GetGatewayAddress(), currentHeight)

// Update the application store with the new delegation
k.SetApplication(ctx, foundApp)
Expand All @@ -67,8 +80,9 @@ func (k msgServer) UndelegateFromGateway(ctx context.Context, msg *apptypes.MsgU
SessionEndHeight: sessionEndHeight,
}
if err := sdkCtx.EventManager().EmitTypedEvent(event); err != nil {
logger.Error(fmt.Sprintf("Failed to emit application redelegation event: %v", err))
return nil, err
err = fmt.Errorf("failed to emit application redelegation event: %w", err)
logger.Error(err.Error())
return nil, status.Error(codes.Internal, err.Error())
}
logger.Info(fmt.Sprintf("Emitted application redelegation event %v", event))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func TestMsgServer_UndelegateFromGateway_FailNotDelegated(t *testing.T) {

// Attempt to undelgate the application from the gateway
_, err = srv.UndelegateFromGateway(ctx, undelegateMsg)
require.ErrorIs(t, err, types.ErrAppNotDelegated)
require.ErrorContains(t, err, types.ErrAppNotDelegated.Error())
foundApp, isAppFound := k.GetApplication(ctx, appAddr)
require.True(t, isAppFound)
require.Equal(t, appAddr, foundApp.Address)
Expand Down Expand Up @@ -221,7 +221,7 @@ func TestMsgServer_UndelegateFromGateway_FailNotDelegated(t *testing.T) {

// Ensure the failed undelegation did not affect the application
_, err = srv.UndelegateFromGateway(ctx, undelegateMsg)
require.ErrorIs(t, err, types.ErrAppNotDelegated)
require.ErrorContains(t, err, apptypes.ErrAppNotDelegated.Error())

events = sdkCtx.EventManager().Events()
require.Equal(t, 0, len(events), "expected no events")
Expand Down
29 changes: 22 additions & 7 deletions x/application/keeper/msg_server_unstake_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,40 @@ func (k msgServer) UnstakeApplication(
// Check if the application already exists or not.
foundApp, isAppFound := k.GetApplication(ctx, msg.GetAddress())
if !isAppFound {
logger.Info(fmt.Sprintf("Application not found. Cannot unstake address (%s)", msg.GetAddress()))
return nil, apptypes.ErrAppNotFound.Wrapf("application (%s)", msg.GetAddress())
logger.Info(fmt.Sprintf("Application not found. Cannot unstake address [%s]", msg.GetAddress()))
return nil, status.Error(
codes.NotFound,
apptypes.ErrAppNotFound.Wrapf(
"application with address %q", msg.GetAddress(),
).Error(),
)
}
logger.Info(fmt.Sprintf("Application found. Unstaking application for address (%s)", msg.GetAddress()))
logger.Info(fmt.Sprintf("Application found. Unstaking application for address [%s]", msg.GetAddress()))

// Check if the application has already initiated the unstaking process.
if foundApp.IsUnbonding() {
logger.Warn(fmt.Sprintf("Application (%s) is still unbonding from previous unstaking", msg.GetAddress()))
return nil, apptypes.ErrAppIsUnstaking.Wrapf("application (%s)", msg.GetAddress())
logger.Info(fmt.Sprintf("Application with address [%s] is still unbonding from previous unstaking", msg.GetAddress()))
return nil, status.Error(
codes.FailedPrecondition,
apptypes.ErrAppIsUnstaking.Wrapf(
"application with address %q", msg.GetAddress(),
).Error(),
)
}

// Check if the application has already initiated a transfer process.
// Transferring applications CANNOT unstake.
if foundApp.HasPendingTransfer() {
logger.Warn(fmt.Sprintf(
"Application (%s) has a pending transfer to (%s)",
"Application with address [%s] has a pending transfer to [%s]",
msg.Address, foundApp.GetPendingTransfer().GetDestinationAddress()),
)
return nil, apptypes.ErrAppHasPendingTransfer.Wrapf("application (%s)", msg.GetAddress())
return nil, status.Error(
codes.FailedPrecondition,
apptypes.ErrAppHasPendingTransfer.Wrapf(
"application with address %q", msg.GetAddress(),
).Error(),
)
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
Expand Down
4 changes: 2 additions & 2 deletions x/application/keeper/msg_server_unstake_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func TestMsgServer_UnstakeApplication_FailIfNotStaked(t *testing.T) {
unstakeMsg := &apptypes.MsgUnstakeApplication{Address: appAddr}
_, err := srv.UnstakeApplication(ctx, unstakeMsg)
require.Error(t, err)
require.ErrorIs(t, err, apptypes.ErrAppNotFound)
require.ErrorContains(t, err, apptypes.ErrAppNotFound.Error())

_, isAppFound = applicationModuleKeepers.GetApplication(ctx, appAddr)
require.False(t, isAppFound)
Expand Down Expand Up @@ -253,7 +253,7 @@ func TestMsgServer_UnstakeApplication_FailIfCurrentlyUnstaking(t *testing.T) {

// Verify that the application cannot unstake if it is already unstaking.
_, err = srv.UnstakeApplication(ctx, unstakeMsg)
require.ErrorIs(t, err, apptypes.ErrAppIsUnstaking)
require.ErrorContains(t, err, apptypes.ErrAppIsUnstaking.Error())
}

func createAppStakeMsg(appAddr string, stakeAmount int64) *apptypes.MsgStakeApplication {
Expand Down
18 changes: 14 additions & 4 deletions x/application/keeper/msg_server_update_param.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ func (k msgServer) UpdateParam(ctx context.Context, msg *apptypes.MsgUpdateParam
)

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

if k.GetAuthority() != msg.Authority {
return nil, status.Error(
codes.InvalidArgument,
codes.PermissionDenied,
apptypes.ErrAppInvalidSigner.Wrapf(
"invalid authority; expected %s, got %s",
k.GetAuthority(), msg.Authority,
Expand All @@ -38,12 +38,22 @@ func (k msgServer) UpdateParam(ctx context.Context, msg *apptypes.MsgUpdateParam

params.MaxDelegatedGateways = msg.GetAsUint64()
if _, ok := msg.AsType.(*apptypes.MsgUpdateParam_AsUint64); !ok {
return nil, apptypes.ErrAppParamInvalid.Wrapf("unsupported value type for %s param: %T", msg.Name, msg.AsType)
return nil, status.Error(
codes.InvalidArgument,
apptypes.ErrAppParamInvalid.Wrapf(
"unsupported value type for %s param: %T", msg.Name, msg.AsType,
).Error(),
)
}
maxDelegatedGateways := msg.GetAsUint64()

if err := apptypes.ValidateMaxDelegatedGateways(maxDelegatedGateways); err != nil {
return nil, apptypes.ErrAppParamInvalid.Wrapf("maxdelegegated_gateways (%d): %v", maxDelegatedGateways, err)
return nil, status.Error(
codes.InvalidArgument,
apptypes.ErrAppParamInvalid.Wrapf(
"max_delegegated_gateways (%d): %s", maxDelegatedGateways, err,
).Error(),
)
}
params.MaxDelegatedGateways = maxDelegatedGateways
case apptypes.ParamMinStake:
Expand Down
20 changes: 17 additions & 3 deletions x/application/keeper/msg_update_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,34 @@ package keeper

import (
"context"
"fmt"

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

"github.com/pokt-network/poktroll/x/application/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.ErrAppInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority)
return nil, status.Error(
codes.PermissionDenied,
types.ErrAppInvalidSigner.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
Loading