From 09702d6688ad1ec34f668700690092e20018c0f6 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 10:14:48 +0100 Subject: [PATCH 01/10] [Code Health] fix: gateway module gRPC status error returns (#955) ## Summary Ensure all gateway message and query handlers return gRPC status errors. ## Issue - #954 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- x/gateway/keeper/msg_server_stake_gateway.go | 2 +- x/gateway/keeper/msg_server_unstake_gateway.go | 18 ++++++++++++------ .../keeper/msg_server_unstake_gateway_test.go | 2 +- x/gateway/keeper/msg_update_params.go | 18 +++++++++++++++--- x/gateway/keeper/query_gateway.go | 5 ++++- 5 files changed, 33 insertions(+), 12 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..d8c757713 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,22 +38,28 @@ 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 with address %s", msg.Address, + ).Error(), + ) } logger.Info(fmt.Sprintf("Gateway found. Unstaking gateway for address %s", msg.Address)) // 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)) - return nil, err + 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)) - return nil, 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()) } // Update the Gateway in the store 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 d8039b896..9b13b4d17 100644 --- a/x/gateway/keeper/msg_update_params.go +++ b/x/gateway/keeper/msg_update_params.go @@ -2,8 +2,11 @@ package keeper import ( "context" + "fmt" 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" ) @@ -12,17 +15,26 @@ 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, 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 + 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/gateway/keeper/query_gateway.go b/x/gateway/keeper/query_gateway.go index 0b6d2ab2a..8420bde17 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: %+v", key, err)) + return status.Error(codes.Internal, err.Error()) } gateways = append(gateways, gateway) From be817c2eabd5ca1d44629c45e474d71d935e9db9 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 10:15:14 +0100 Subject: [PATCH 02/10] [Code Health] fix: proof module gRPC status error returns (#956) ## Summary Ensure all proof module message and query handlers return gRPC status errors. ## Issue - #954 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- x/proof/keeper/msg_server_create_claim.go | 2 +- x/proof/keeper/msg_update_params.go | 19 ++++++++++++++++--- x/proof/keeper/query_claim.go | 7 ++++++- x/proof/keeper/query_proof.go | 6 +++++- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/x/proof/keeper/msg_server_create_claim.go b/x/proof/keeper/msg_server_create_claim.go index d9c4a32d3..f9e06055c 100644 --- a/x/proof/keeper/msg_server_create_claim.go +++ b/x/proof/keeper/msg_server_create_claim.go @@ -33,7 +33,7 @@ func (k msgServer) CreateClaim( // Basic validation of the CreateClaim message. if err = msg.ValidateBasic(); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } logger.Info("validated the createClaim message") diff --git a/x/proof/keeper/msg_update_params.go b/x/proof/keeper/msg_update_params.go index 392f9333f..8b95781b2 100644 --- a/x/proof/keeper/msg_update_params.go +++ b/x/proof/keeper/msg_update_params.go @@ -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/proof/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.ErrProofInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority) + return nil, status.Error( + codes.PermissionDenied, + types.ErrProofInvalidSigner.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/proof/keeper/query_claim.go b/x/proof/keeper/query_claim.go index 520aa4b0c..399cf3f27 100644 --- a/x/proof/keeper/query_claim.go +++ b/x/proof/keeper/query_claim.go @@ -3,6 +3,7 @@ package keeper import ( "context" "encoding/binary" + "fmt" "cosmossdk.io/store/prefix" "github.com/cosmos/cosmos-sdk/runtime" @@ -14,6 +15,8 @@ import ( ) func (k Keeper) AllClaims(ctx context.Context, req *types.QueryAllClaimsRequest) (*types.QueryAllClaimsResponse, error) { + logger := k.Logger().With("method", "AllClaims") + if req == nil { return nil, status.Error(codes.InvalidArgument, "invalid request") } @@ -65,7 +68,9 @@ func (k Keeper) AllClaims(ctx context.Context, req *types.QueryAllClaimsRequest) // The value is the encoded claim. var claim types.Claim if err := k.cdc.Unmarshal(value, &claim); err != nil { - return err + err = fmt.Errorf("unable to unmarshal claim with key (hex): %x: %+v", key, err) + logger.Error(err.Error()) + return status.Error(codes.Internal, err.Error()) } claims = append(claims, claim) } diff --git a/x/proof/keeper/query_proof.go b/x/proof/keeper/query_proof.go index 12eae432c..f2a84ba42 100644 --- a/x/proof/keeper/query_proof.go +++ b/x/proof/keeper/query_proof.go @@ -14,6 +14,8 @@ import ( ) func (k Keeper) AllProofs(ctx context.Context, req *types.QueryAllProofsRequest) (*types.QueryAllProofsResponse, error) { + logger := k.Logger().With("method", "AllProofs") + if req == nil { return nil, status.Error(codes.InvalidArgument, "invalid request") } @@ -63,7 +65,9 @@ func (k Keeper) AllProofs(ctx context.Context, req *types.QueryAllProofsRequest) // The value is the encoded proof. var proof types.Proof if err := k.cdc.Unmarshal(value, &proof); err != nil { - return err + err = fmt.Errorf("unable to unmarshal proof with key (hex): %x: %+v", key, err) + logger.Error(err.Error()) + return status.Error(codes.Internal, err.Error()) } proofs = append(proofs, proof) From 1c490ce2577991b21188b6a8625d8421d7c8ba16 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 10:15:31 +0100 Subject: [PATCH 03/10] [Code Health] fix: service module gRPC status error returns (#959) ## Summary Ensure all service module message and query handlers return gRPC status errors. ## Issue - #860 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- x/service/keeper/msg_server_add_service.go | 58 ++++++++++++------- x/service/keeper/msg_update_params.go | 19 +++++- .../keeper/query_relay_mining_difficulty.go | 7 ++- x/service/keeper/query_service.go | 7 ++- 4 files changed, 65 insertions(+), 26 deletions(-) diff --git a/x/service/keeper/msg_server_add_service.go b/x/service/keeper/msg_server_add_service.go index 52fe8365e..2d3dd69c0 100644 --- a/x/service/keeper/msg_server_add_service.go +++ b/x/service/keeper/msg_server_add_service.go @@ -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" @@ -33,39 +35,48 @@ 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(), ) } @@ -73,10 +84,12 @@ func (k msgServer) AddService( 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(), ) } @@ -84,10 +97,13 @@ func (k msgServer) AddService( 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(), ) } diff --git a/x/service/keeper/msg_update_params.go b/x/service/keeper/msg_update_params.go index 964d1db26..d8317c864 100644 --- a/x/service/keeper/msg_update_params.go +++ b/x/service/keeper/msg_update_params.go @@ -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 diff --git a/x/service/keeper/query_relay_mining_difficulty.go b/x/service/keeper/query_relay_mining_difficulty.go index 17cef487a..82403fd0a 100644 --- a/x/service/keeper/query_relay_mining_difficulty.go +++ b/x/service/keeper/query_relay_mining_difficulty.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "cosmossdk.io/store/prefix" "github.com/cosmos/cosmos-sdk/runtime" @@ -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") } @@ -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) diff --git a/x/service/keeper/query_service.go b/x/service/keeper/query_service.go index 0792fa688..73f420edc 100644 --- a/x/service/keeper/query_service.go +++ b/x/service/keeper/query_service.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "cosmossdk.io/store/prefix" "github.com/cosmos/cosmos-sdk/runtime" @@ -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") } @@ -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) From 41a373bdce36982f90452eb833e8072aeb8124ff Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 10:15:47 +0100 Subject: [PATCH 04/10] [Code Health] fix: session module gRPC status return errors (#960) ## Summary Ensure all session message and query handlers return gRPC status errors. ## Issue - #860 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- x/proof/keeper/session.go | 4 ++++ x/session/keeper/msg_update_params.go | 19 ++++++++++++++++--- x/session/keeper/query_get_session.go | 7 +++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/x/proof/keeper/session.go b/x/proof/keeper/session.go index 9d33d76c6..68a72cf10 100644 --- a/x/proof/keeper/session.go +++ b/x/proof/keeper/session.go @@ -2,8 +2,10 @@ package keeper import ( "context" + "fmt" cosmostypes "github.com/cosmos/cosmos-sdk/types" + "google.golang.org/grpc/status" "github.com/pokt-network/poktroll/x/proof/types" sessiontypes "github.com/pokt-network/poktroll/x/session/types" @@ -30,6 +32,8 @@ func (k Keeper) queryAndValidateSessionHeader( // session header is to be validated. sessionRes, err := k.sessionKeeper.GetSession(ctx, sessionReq) if err != nil { + // NB: Strip internal error status from error. An appropriate status will be associated by the caller. + err = fmt.Errorf("%s", status.Convert(err).Message()) return nil, err } onChainSession := sessionRes.GetSession() diff --git a/x/session/keeper/msg_update_params.go b/x/session/keeper/msg_update_params.go index 84a5debfa..57564b921 100644 --- a/x/session/keeper/msg_update_params.go +++ b/x/session/keeper/msg_update_params.go @@ -2,21 +2,34 @@ package keeper import ( "context" + "fmt" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/pokt-network/poktroll/x/session/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.ErrSessionInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority) + return nil, status.Error( + codes.PermissionDenied, + types.ErrSessionInvalidSigner.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/session/keeper/query_get_session.go b/x/session/keeper/query_get_session.go index bc25fff0e..412d77c45 100644 --- a/x/session/keeper/query_get_session.go +++ b/x/session/keeper/query_get_session.go @@ -14,6 +14,8 @@ import ( // GetSession should be deterministic and always return the same session for // the same block height. func (k Keeper) GetSession(ctx context.Context, req *types.QueryGetSessionRequest) (*types.QueryGetSessionResponse, error) { + logger := k.Logger().With("method", "GetSession") + if req == nil { return nil, status.Error(codes.InvalidArgument, "invalid request") } @@ -36,12 +38,13 @@ func (k Keeper) GetSession(ctx context.Context, req *types.QueryGetSessionReques blockHeight = req.BlockHeight } - k.Logger().Debug(fmt.Sprintf("Getting session for height: %d", blockHeight)) + logger.Debug(fmt.Sprintf("Getting session for height: %d", blockHeight)) sessionHydrator := NewSessionHydrator(req.ApplicationAddress, req.ServiceId, blockHeight) session, err := k.HydrateSession(ctx, sessionHydrator) if err != nil { - return nil, err + logger.Error(err.Error()) + return nil, status.Error(codes.Internal, err.Error()) } res := &types.QueryGetSessionResponse{ From 9678997e62c6a8cd73ab4f0c155ad0592f7e6f36 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 10:16:05 +0100 Subject: [PATCH 05/10] [Code Health] fix: shared module gRPC return errors (#961) ## Summary Ensure all shared message and query handlers return gRPC status errors. ## Issue - #860 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- x/shared/keeper/msg_update_params.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/x/shared/keeper/msg_update_params.go b/x/shared/keeper/msg_update_params.go index 9df191361..c313387f6 100644 --- a/x/shared/keeper/msg_update_params.go +++ b/x/shared/keeper/msg_update_params.go @@ -3,23 +3,34 @@ package keeper import ( "context" - sdkerrors "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/pokt-network/poktroll/x/shared/types" ) 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, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if k.GetAuthority() != req.Authority { - return nil, sdkerrors.Wrapf(types.ErrSharedInvalidSigner, "invalid authority; expected %s, got %s", k.GetAuthority(), req.Authority) + return nil, status.Error( + codes.PermissionDenied, + types.ErrSharedInvalidSigner.Wrapf( + "invalid authority; expected %s, got %s", + k.GetAuthority(), req.Authority, + ).Error(), + ) } ctx := sdk.UnwrapSDKContext(goCtx) if err := k.SetParams(ctx, req.Params); err != nil { - return nil, err + err = types.ErrSharedParamInvalid.Wrapf("unable to set params: %v", err) + logger.Error(err.Error()) + return nil, status.Error(codes.Internal, err.Error()) } return &types.MsgUpdateParamsResponse{}, nil From f35e87943124ddc2965fc1ffa4c2bba4ce4139ee Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 10:16:18 +0100 Subject: [PATCH 06/10] [Code Health] fix: application module gRPC status error returns (#954) ## Summary Ensure all application message and query handlers return gRPC status errors. ## Issue - #860 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- .../keeper/msg_server_delegate_to_gateway.go | 55 ++++++++++++++----- .../msg_server_delegate_to_gateway_test.go | 6 +- .../msg_server_undelegate_from_gateway.go | 40 +++++++++----- ...msg_server_undelegate_from_gateway_test.go | 4 +- .../keeper/msg_server_unstake_application.go | 29 +++++++--- .../msg_server_unstake_application_test.go | 4 +- .../keeper/msg_server_update_param.go | 18 ++++-- x/application/keeper/msg_update_params.go | 20 ++++++- x/application/keeper/query_application.go | 6 +- 9 files changed, 132 insertions(+), 50 deletions(-) diff --git a/x/application/keeper/msg_server_delegate_to_gateway.go b/x/application/keeper/msg_server_delegate_to_gateway.go index f109d0ce3..395346d88 100644 --- a/x/application/keeper/msg_server_delegate_to_gateway.go +++ b/x/application/keeper/msg_server_delegate_to_gateway.go @@ -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" @@ -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 @@ -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 diff --git a/x/application/keeper/msg_server_delegate_to_gateway_test.go b/x/application/keeper/msg_server_delegate_to_gateway_test.go index 18525fa4f..873817d8b 100644 --- a/x/application/keeper/msg_server_delegate_to_gateway_test.go +++ b/x/application/keeper/msg_server_delegate_to_gateway_test.go @@ -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)) @@ -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)) @@ -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) diff --git a/x/application/keeper/msg_server_undelegate_from_gateway.go b/x/application/keeper/msg_server_undelegate_from_gateway.go index 8af02752d..9fd7b1258 100644 --- a/x/application/keeper/msg_server_undelegate_from_gateway.go +++ b/x/application/keeper/msg_server_undelegate_from_gateway.go @@ -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" @@ -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 @@ -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) @@ -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)) diff --git a/x/application/keeper/msg_server_undelegate_from_gateway_test.go b/x/application/keeper/msg_server_undelegate_from_gateway_test.go index bc562a81a..4a2f7663b 100644 --- a/x/application/keeper/msg_server_undelegate_from_gateway_test.go +++ b/x/application/keeper/msg_server_undelegate_from_gateway_test.go @@ -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) @@ -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") diff --git a/x/application/keeper/msg_server_unstake_application.go b/x/application/keeper/msg_server_unstake_application.go index 337df4382..bdd3a6bb5 100644 --- a/x/application/keeper/msg_server_unstake_application.go +++ b/x/application/keeper/msg_server_unstake_application.go @@ -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) diff --git a/x/application/keeper/msg_server_unstake_application_test.go b/x/application/keeper/msg_server_unstake_application_test.go index fa1a59002..68fb1db13 100644 --- a/x/application/keeper/msg_server_unstake_application_test.go +++ b/x/application/keeper/msg_server_unstake_application_test.go @@ -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) @@ -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 { diff --git a/x/application/keeper/msg_server_update_param.go b/x/application/keeper/msg_server_update_param.go index a06925e75..0912fcb14 100644 --- a/x/application/keeper/msg_server_update_param.go +++ b/x/application/keeper/msg_server_update_param.go @@ -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, @@ -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: diff --git a/x/application/keeper/msg_update_params.go b/x/application/keeper/msg_update_params.go index 52335b954..1e101297e 100644 --- a/x/application/keeper/msg_update_params.go +++ b/x/application/keeper/msg_update_params.go @@ -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 diff --git a/x/application/keeper/query_application.go b/x/application/keeper/query_application.go index 446449d29..4ec5e8ac6 100644 --- a/x/application/keeper/query_application.go +++ b/x/application/keeper/query_application.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "cosmossdk.io/store/prefix" "github.com/cosmos/cosmos-sdk/runtime" @@ -13,6 +14,8 @@ import ( ) func (k Keeper) AllApplications(ctx context.Context, req *types.QueryAllApplicationsRequest) (*types.QueryAllApplicationsResponse, error) { + logger := k.Logger().With("method", "AllApplications") + if req == nil { return nil, status.Error(codes.InvalidArgument, "invalid request") } @@ -25,7 +28,8 @@ func (k Keeper) AllApplications(ctx context.Context, req *types.QueryAllApplicat pageRes, err := query.Paginate(applicationStore, req.Pagination, func(key []byte, value []byte) error { var application types.Application if err := k.cdc.Unmarshal(value, &application); err != nil { - return err + logger.Error(fmt.Sprintf("unmarshaling application with key (hex): %x: %+v", key, err)) + return status.Error(codes.Internal, err.Error()) } // Ensure that the PendingUndelegations is an empty map and not nil when From acd7fc8a60f44a05a117050c441609361dd608d3 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 14:14:57 +0100 Subject: [PATCH 07/10] [Supplier] feat: add `staking_fee` param to supplier module (#944) ## Summary Add `staking_fee` param to the tokenomics module. ## Issue - `TODO_BETA` ## Type of change Select one or more from the following: - [x] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: red-0ne --- api/poktroll/supplier/params.pulsar.go | 141 +++++++++++++++--- config.yml | 4 + makefiles/params.mk | 4 + proto/poktroll/supplier/params.proto | 3 + testutil/integration/suites/param_configs.go | 7 +- tools/scripts/params/supplier_all.json | 4 + .../scripts/params/supplier_staking_fee.json | 15 ++ x/supplier/keeper/msg_server_update_param.go | 3 + x/supplier/keeper/msg_update_params_test.go | 7 +- x/supplier/keeper/params_test.go | 61 +++++++- x/supplier/types/message_update_param.go | 19 ++- x/supplier/types/params.go | 72 +++++++-- x/supplier/types/params.pb.go | 92 ++++++++++-- 13 files changed, 369 insertions(+), 63 deletions(-) create mode 100644 tools/scripts/params/supplier_staking_fee.json diff --git a/api/poktroll/supplier/params.pulsar.go b/api/poktroll/supplier/params.pulsar.go index bb3c96973..9048f64d1 100644 --- a/api/poktroll/supplier/params.pulsar.go +++ b/api/poktroll/supplier/params.pulsar.go @@ -16,14 +16,16 @@ import ( ) var ( - md_Params protoreflect.MessageDescriptor - fd_Params_min_stake protoreflect.FieldDescriptor + md_Params protoreflect.MessageDescriptor + fd_Params_min_stake protoreflect.FieldDescriptor + fd_Params_staking_fee protoreflect.FieldDescriptor ) func init() { file_poktroll_supplier_params_proto_init() md_Params = File_poktroll_supplier_params_proto.Messages().ByName("Params") fd_Params_min_stake = md_Params.Fields().ByName("min_stake") + fd_Params_staking_fee = md_Params.Fields().ByName("staking_fee") } var _ protoreflect.Message = (*fastReflection_Params)(nil) @@ -97,6 +99,12 @@ func (x *fastReflection_Params) Range(f func(protoreflect.FieldDescriptor, proto return } } + if x.StakingFee != nil { + value := protoreflect.ValueOfMessage(x.StakingFee.ProtoReflect()) + if !f(fd_Params_staking_fee, value) { + return + } + } } // Has reports whether a field is populated. @@ -114,6 +122,8 @@ func (x *fastReflection_Params) Has(fd protoreflect.FieldDescriptor) bool { switch fd.FullName() { case "poktroll.supplier.Params.min_stake": return x.MinStake != nil + case "poktroll.supplier.Params.staking_fee": + return x.StakingFee != nil default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: poktroll.supplier.Params")) @@ -132,6 +142,8 @@ func (x *fastReflection_Params) Clear(fd protoreflect.FieldDescriptor) { switch fd.FullName() { case "poktroll.supplier.Params.min_stake": x.MinStake = nil + case "poktroll.supplier.Params.staking_fee": + x.StakingFee = nil default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: poktroll.supplier.Params")) @@ -151,6 +163,9 @@ func (x *fastReflection_Params) Get(descriptor protoreflect.FieldDescriptor) pro case "poktroll.supplier.Params.min_stake": value := x.MinStake return protoreflect.ValueOfMessage(value.ProtoReflect()) + case "poktroll.supplier.Params.staking_fee": + value := x.StakingFee + return protoreflect.ValueOfMessage(value.ProtoReflect()) default: if descriptor.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: poktroll.supplier.Params")) @@ -173,6 +188,8 @@ func (x *fastReflection_Params) Set(fd protoreflect.FieldDescriptor, value proto switch fd.FullName() { case "poktroll.supplier.Params.min_stake": x.MinStake = value.Message().Interface().(*v1beta1.Coin) + case "poktroll.supplier.Params.staking_fee": + x.StakingFee = value.Message().Interface().(*v1beta1.Coin) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: poktroll.supplier.Params")) @@ -198,6 +215,11 @@ func (x *fastReflection_Params) Mutable(fd protoreflect.FieldDescriptor) protore x.MinStake = new(v1beta1.Coin) } return protoreflect.ValueOfMessage(x.MinStake.ProtoReflect()) + case "poktroll.supplier.Params.staking_fee": + if x.StakingFee == nil { + x.StakingFee = new(v1beta1.Coin) + } + return protoreflect.ValueOfMessage(x.StakingFee.ProtoReflect()) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: poktroll.supplier.Params")) @@ -214,6 +236,9 @@ func (x *fastReflection_Params) NewField(fd protoreflect.FieldDescriptor) protor case "poktroll.supplier.Params.min_stake": m := new(v1beta1.Coin) return protoreflect.ValueOfMessage(m.ProtoReflect()) + case "poktroll.supplier.Params.staking_fee": + m := new(v1beta1.Coin) + return protoreflect.ValueOfMessage(m.ProtoReflect()) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: poktroll.supplier.Params")) @@ -287,6 +312,10 @@ func (x *fastReflection_Params) ProtoMethods() *protoiface.Methods { l = options.Size(x.MinStake) n += 1 + l + runtime.Sov(uint64(l)) } + if x.StakingFee != nil { + l = options.Size(x.StakingFee) + n += 1 + l + runtime.Sov(uint64(l)) + } if x.unknownFields != nil { n += len(x.unknownFields) } @@ -316,6 +345,20 @@ func (x *fastReflection_Params) ProtoMethods() *protoiface.Methods { i -= len(x.unknownFields) copy(dAtA[i:], x.unknownFields) } + if x.StakingFee != nil { + encoded, err := options.Marshal(x.StakingFee) + if err != nil { + return protoiface.MarshalOutput{ + NoUnkeyedLiterals: input.NoUnkeyedLiterals, + Buf: input.Buf, + }, err + } + i -= len(encoded) + copy(dAtA[i:], encoded) + i = runtime.EncodeVarint(dAtA, i, uint64(len(encoded))) + i-- + dAtA[i] = 0x12 + } if x.MinStake != nil { encoded, err := options.Marshal(x.MinStake) if err != nil { @@ -415,6 +458,42 @@ func (x *fastReflection_Params) ProtoMethods() *protoiface.Methods { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, err } iNdEx = postIndex + case 2: + if wireType != 2 { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: wrong wireType = %d for field StakingFee", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, runtime.ErrIntOverflow + } + if iNdEx >= l { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, runtime.ErrInvalidLength + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, runtime.ErrInvalidLength + } + if postIndex > l { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, io.ErrUnexpectedEOF + } + if x.StakingFee == nil { + x.StakingFee = &v1beta1.Coin{} + } + if err := options.Unmarshal(dAtA[iNdEx:postIndex], x.StakingFee); err != nil { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, err + } + iNdEx = postIndex default: iNdEx = preIndex skippy, err := runtime.Skip(dAtA[iNdEx:]) @@ -472,6 +551,8 @@ type Params struct { // min_stake is the minimum amount of uPOKT that a supplier must stake to be // included in network sessions and remain staked. MinStake *v1beta1.Coin `protobuf:"bytes,1,opt,name=min_stake,json=minStake,proto3" json:"min_stake,omitempty"` + // staking_fee is the fee charged by the protocol for staking a supplier. + StakingFee *v1beta1.Coin `protobuf:"bytes,2,opt,name=staking_fee,json=stakingFee,proto3" json:"staking_fee,omitempty"` } func (x *Params) Reset() { @@ -501,6 +582,13 @@ func (x *Params) GetMinStake() *v1beta1.Coin { return nil } +func (x *Params) GetStakingFee() *v1beta1.Coin { + if x != nil { + return x.StakingFee + } + return nil +} + var File_poktroll_supplier_params_proto protoreflect.FileDescriptor var file_poktroll_supplier_params_proto_rawDesc = []byte{ @@ -511,28 +599,34 @@ var file_poktroll_supplier_params_proto_rawDesc = []byte{ 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x14, 0x67, 0x6f, 0x67, 0x6f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x67, 0x6f, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x1e, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, 0x62, 0x61, 0x73, 0x65, 0x2f, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, - 0x31, 0x2f, 0x63, 0x6f, 0x69, 0x6e, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0x88, 0x01, 0x0a, + 0x31, 0x2f, 0x63, 0x6f, 0x69, 0x6e, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xeb, 0x01, 0x0a, 0x06, 0x50, 0x61, 0x72, 0x61, 0x6d, 0x73, 0x12, 0x59, 0x0a, 0x09, 0x6d, 0x69, 0x6e, 0x5f, 0x73, 0x74, 0x61, 0x6b, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x19, 0x2e, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x62, 0x61, 0x73, 0x65, 0x2e, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0x2e, 0x43, 0x6f, 0x69, 0x6e, 0x42, 0x21, 0xea, 0xde, 0x1f, 0x09, 0x6d, 0x69, 0x6e, 0x5f, 0x73, 0x74, 0x61, 0x6b, 0x65, 0xf2, 0xde, 0x1f, 0x10, 0x79, 0x61, 0x6d, 0x6c, 0x3a, 0x22, 0x6d, 0x69, 0x6e, 0x5f, 0x73, 0x74, 0x61, 0x6b, 0x65, 0x22, 0x52, 0x08, 0x6d, 0x69, 0x6e, 0x53, 0x74, 0x61, - 0x6b, 0x65, 0x3a, 0x23, 0xe8, 0xa0, 0x1f, 0x01, 0x8a, 0xe7, 0xb0, 0x2a, 0x1a, 0x70, 0x6f, 0x6b, - 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x2f, 0x78, 0x2f, 0x73, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, - 0x2f, 0x50, 0x61, 0x72, 0x61, 0x6d, 0x73, 0x42, 0xb1, 0x01, 0xd8, 0xe2, 0x1e, 0x01, 0x0a, 0x15, - 0x63, 0x6f, 0x6d, 0x2e, 0x70, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x2e, 0x73, 0x75, 0x70, - 0x70, 0x6c, 0x69, 0x65, 0x72, 0x42, 0x0b, 0x50, 0x61, 0x72, 0x61, 0x6d, 0x73, 0x50, 0x72, 0x6f, - 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x22, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x73, 0x64, 0x6b, 0x2e, - 0x69, 0x6f, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x70, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x2f, - 0x73, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0xa2, 0x02, 0x03, 0x50, 0x53, 0x58, 0xaa, 0x02, - 0x11, 0x50, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x2e, 0x53, 0x75, 0x70, 0x70, 0x6c, 0x69, - 0x65, 0x72, 0xca, 0x02, 0x11, 0x50, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x5c, 0x53, 0x75, - 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0xe2, 0x02, 0x1d, 0x50, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, - 0x6c, 0x5c, 0x53, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0x5c, 0x47, 0x50, 0x42, 0x4d, 0x65, - 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xea, 0x02, 0x12, 0x50, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, - 0x6c, 0x3a, 0x3a, 0x53, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0x62, 0x06, 0x70, 0x72, 0x6f, - 0x74, 0x6f, 0x33, + 0x6b, 0x65, 0x12, 0x61, 0x0a, 0x0b, 0x73, 0x74, 0x61, 0x6b, 0x69, 0x6e, 0x67, 0x5f, 0x66, 0x65, + 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x19, 0x2e, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, + 0x2e, 0x62, 0x61, 0x73, 0x65, 0x2e, 0x76, 0x31, 0x62, 0x65, 0x74, 0x61, 0x31, 0x2e, 0x43, 0x6f, + 0x69, 0x6e, 0x42, 0x25, 0xea, 0xde, 0x1f, 0x0b, 0x73, 0x74, 0x61, 0x6b, 0x69, 0x6e, 0x67, 0x5f, + 0x66, 0x65, 0x65, 0xf2, 0xde, 0x1f, 0x12, 0x79, 0x61, 0x6d, 0x6c, 0x3a, 0x22, 0x73, 0x74, 0x61, + 0x6b, 0x69, 0x6e, 0x67, 0x5f, 0x66, 0x65, 0x65, 0x22, 0x52, 0x0a, 0x73, 0x74, 0x61, 0x6b, 0x69, + 0x6e, 0x67, 0x46, 0x65, 0x65, 0x3a, 0x23, 0xe8, 0xa0, 0x1f, 0x01, 0x8a, 0xe7, 0xb0, 0x2a, 0x1a, + 0x70, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x2f, 0x78, 0x2f, 0x73, 0x75, 0x70, 0x70, 0x6c, + 0x69, 0x65, 0x72, 0x2f, 0x50, 0x61, 0x72, 0x61, 0x6d, 0x73, 0x42, 0xb1, 0x01, 0xd8, 0xe2, 0x1e, + 0x01, 0x0a, 0x15, 0x63, 0x6f, 0x6d, 0x2e, 0x70, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x2e, + 0x73, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0x42, 0x0b, 0x50, 0x61, 0x72, 0x61, 0x6d, 0x73, + 0x50, 0x72, 0x6f, 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x22, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x73, + 0x64, 0x6b, 0x2e, 0x69, 0x6f, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x70, 0x6f, 0x6b, 0x74, 0x72, 0x6f, + 0x6c, 0x6c, 0x2f, 0x73, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0xa2, 0x02, 0x03, 0x50, 0x53, + 0x58, 0xaa, 0x02, 0x11, 0x50, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x2e, 0x53, 0x75, 0x70, + 0x70, 0x6c, 0x69, 0x65, 0x72, 0xca, 0x02, 0x11, 0x50, 0x6f, 0x6b, 0x74, 0x72, 0x6f, 0x6c, 0x6c, + 0x5c, 0x53, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0xe2, 0x02, 0x1d, 0x50, 0x6f, 0x6b, 0x74, + 0x72, 0x6f, 0x6c, 0x6c, 0x5c, 0x53, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0x5c, 0x47, 0x50, + 0x42, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xea, 0x02, 0x12, 0x50, 0x6f, 0x6b, 0x74, + 0x72, 0x6f, 0x6c, 0x6c, 0x3a, 0x3a, 0x53, 0x75, 0x70, 0x70, 0x6c, 0x69, 0x65, 0x72, 0x62, 0x06, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -554,11 +648,12 @@ var file_poktroll_supplier_params_proto_goTypes = []interface{}{ } var file_poktroll_supplier_params_proto_depIdxs = []int32{ 1, // 0: poktroll.supplier.Params.min_stake:type_name -> cosmos.base.v1beta1.Coin - 1, // [1:1] is the sub-list for method output_type - 1, // [1:1] is the sub-list for method input_type - 1, // [1:1] is the sub-list for extension type_name - 1, // [1:1] is the sub-list for extension extendee - 0, // [0:1] is the sub-list for field type_name + 1, // 1: poktroll.supplier.Params.staking_fee:type_name -> cosmos.base.v1beta1.Coin + 2, // [2:2] is the sub-list for method output_type + 2, // [2:2] is the sub-list for method input_type + 2, // [2:2] is the sub-list for extension type_name + 2, // [2:2] is the sub-list for extension extendee + 0, // [0:2] is the sub-list for field type_name } func init() { file_poktroll_supplier_params_proto_init() } diff --git a/config.yml b/config.yml index f6bec72d1..ccde339a4 100644 --- a/config.yml +++ b/config.yml @@ -200,6 +200,10 @@ genesis: min_stake: amount: "1000000" # 1 POKT denom: upokt + # TODO_BETA(@bryanchriswhite): Determine realistic amount for supplier staking fee. + staking_fee: + amount: "1" # 1 uPOKT + denom: upokt supplierList: - owner_address: pokt19a3t4yunp0dlpfjrp7qwnzwlrzd5fzs2gjaaaj operator_address: pokt19a3t4yunp0dlpfjrp7qwnzwlrzd5fzs2gjaaaj diff --git a/makefiles/params.mk b/makefiles/params.mk index bbe99698a..a2968c102 100644 --- a/makefiles/params.mk +++ b/makefiles/params.mk @@ -148,6 +148,10 @@ params_update_supplier_all: ## Update the supplier module params params_update_supplier_min_stake: ## Update the supplier module min_stake param poktrolld tx authz exec ./tools/scripts/params/supplier_min_stake.json $(PARAM_FLAGS) +.PHONY: params_update_supplier_staking_fee +params_update_supplier_staking_fee: ## Update the supplier module staking_fee param + poktrolld tx authz exec ./tools/scripts/params/supplier_staking_fee.json $(PARAM_FLAGS) + ### Session Module Params ### .PHONY: params_get_session params_get_session: ## Get the session module params diff --git a/proto/poktroll/supplier/params.proto b/proto/poktroll/supplier/params.proto index d4e8c77ec..06fa9b7bc 100644 --- a/proto/poktroll/supplier/params.proto +++ b/proto/poktroll/supplier/params.proto @@ -16,4 +16,7 @@ message Params { // min_stake is the minimum amount of uPOKT that a supplier must stake to be // included in network sessions and remain staked. cosmos.base.v1beta1.Coin min_stake = 1 [(gogoproto.jsontag) = "min_stake", (gogoproto.moretags) = "yaml:\"min_stake\""]; + + // staking_fee is the fee charged by the protocol for staking a supplier. + cosmos.base.v1beta1.Coin staking_fee = 2 [(gogoproto.jsontag) = "staking_fee", (gogoproto.moretags) = "yaml:\"staking_fee\""]; } diff --git a/testutil/integration/suites/param_configs.go b/testutil/integration/suites/param_configs.go index 1e3a52e9e..d95a3050e 100644 --- a/testutil/integration/suites/param_configs.go +++ b/testutil/integration/suites/param_configs.go @@ -1,8 +1,6 @@ package suites import ( - "encoding/hex" - cosmostypes "github.com/cosmos/cosmos-sdk/types" "github.com/pokt-network/poktroll/app/volatile" @@ -70,8 +68,8 @@ var ( ValidProofMissingPenaltyCoin = cosmostypes.NewInt64Coin(volatile.DenomuPOKT, 500) ValidProofSubmissionFeeCoin = cosmostypes.NewInt64Coin(volatile.DenomuPOKT, 5000000) ValidProofRequirementThresholdCoin = cosmostypes.NewInt64Coin(volatile.DenomuPOKT, 100) - ValidRelayDifficultyTargetHash, _ = hex.DecodeString("00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff") ValidActorMinStake = cosmostypes.NewInt64Coin(volatile.DenomuPOKT, 100) + ValidStakingFee = cosmostypes.NewInt64Coin(volatile.DenomuPOKT, 1) SharedModuleParamConfig = ModuleParamConfig{ ParamsMsgs: ModuleParamsMessages{ @@ -192,7 +190,8 @@ var ( QueryParamsResponse: suppliertypes.QueryParamsResponse{}, }, ValidParams: suppliertypes.Params{ - MinStake: &ValidActorMinStake, + MinStake: &ValidActorMinStake, + StakingFee: &ValidStakingFee, }, ParamTypes: map[ParamType]any{ ParamTypeCoin: suppliertypes.MsgUpdateParam_AsCoin{}, diff --git a/tools/scripts/params/supplier_all.json b/tools/scripts/params/supplier_all.json index cded3172f..01d632f4c 100644 --- a/tools/scripts/params/supplier_all.json +++ b/tools/scripts/params/supplier_all.json @@ -8,6 +8,10 @@ "min_stake": { "amount": "1000000", "denom": "upokt" + }, + "staking_fee": { + "amount": "1", + "denom": "upokt" } } } diff --git a/tools/scripts/params/supplier_staking_fee.json b/tools/scripts/params/supplier_staking_fee.json new file mode 100644 index 000000000..baad22561 --- /dev/null +++ b/tools/scripts/params/supplier_staking_fee.json @@ -0,0 +1,15 @@ +{ + "body": { + "messages": [ + { + "@type": "/poktroll.supplier.MsgUpdateParam", + "authority": "pokt10d07y265gmmuvt4z0w9aw880jnsr700j8yv32t", + "name": "staking_fee", + "as_coin": { + "amount": "1", + "denom": "upokt" + } + } + ] + } +} diff --git a/x/supplier/keeper/msg_server_update_param.go b/x/supplier/keeper/msg_server_update_param.go index 47e96f528..6d9a160ce 100644 --- a/x/supplier/keeper/msg_server_update_param.go +++ b/x/supplier/keeper/msg_server_update_param.go @@ -38,6 +38,9 @@ func (k msgServer) UpdateParam(ctx context.Context, msg *suppliertypes.MsgUpdate case suppliertypes.ParamMinStake: logger = logger.With("min_stake", msg.GetAsCoin()) params.MinStake = msg.GetAsCoin() + case suppliertypes.ParamStakingFee: + logger = logger.With("staking_fee", msg.GetAsCoin()) + params.StakingFee = msg.GetAsCoin() default: return nil, status.Error( codes.InvalidArgument, diff --git a/x/supplier/keeper/msg_update_params_test.go b/x/supplier/keeper/msg_update_params_test.go index 5a0019a64..5b2c45f1d 100644 --- a/x/supplier/keeper/msg_update_params_test.go +++ b/x/supplier/keeper/msg_update_params_test.go @@ -3,8 +3,10 @@ package keeper_test import ( "testing" + cosmostypes "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + "github.com/pokt-network/poktroll/app/volatile" suppliertypes "github.com/pokt-network/poktroll/x/supplier/types" ) @@ -13,6 +15,8 @@ func TestMsgUpdateParams(t *testing.T) { params := suppliertypes.DefaultParams() require.NoError(t, k.SetParams(ctx, params)) + zerouPokt := cosmostypes.NewInt64Coin(volatile.DenomuPOKT, 0) + // default params tests := []struct { desc string @@ -42,7 +46,8 @@ func TestMsgUpdateParams(t *testing.T) { params: &suppliertypes.MsgUpdateParams{ Authority: k.GetAuthority(), Params: suppliertypes.Params{ - MinStake: &suppliertypes.DefaultMinStake, + MinStake: &suppliertypes.DefaultMinStake, + StakingFee: &zerouPokt, }, }, shouldError: false, diff --git a/x/supplier/keeper/params_test.go b/x/supplier/keeper/params_test.go index 3d74b7813..8ac48c13f 100644 --- a/x/supplier/keeper/params_test.go +++ b/x/supplier/keeper/params_test.go @@ -38,7 +38,18 @@ func TestParams_ValidateMinStake(t *testing.T) { Amount: math.NewInt(-1), }, expectedErr: suppliertypes.ErrSupplierParamInvalid.Wrapf( - "min_stake amount must be greater than 0: got -1%s", + "min_stake amount must be positive: got -1%s", + volatile.DenomuPOKT, + ), + }, + { + desc: "MinStake equal to zero", + minStake: &cosmostypes.Coin{ + Denom: volatile.DenomuPOKT, + Amount: math.NewInt(0), + }, + expectedErr: suppliertypes.ErrSupplierParamInvalid.Wrapf( + "min_stake amount must be greater than 0: got 0%s", volatile.DenomuPOKT, ), }, @@ -60,3 +71,51 @@ func TestParams_ValidateMinStake(t *testing.T) { }) } } + +func TestParams_ValidateStakingFee(t *testing.T) { + tests := []struct { + desc string + minStake any + expectedErr error + }{ + { + desc: "invalid type", + minStake: "420", + expectedErr: suppliertypes.ErrSupplierParamInvalid.Wrapf("invalid parameter type: string"), + }, + { + desc: "SupplierStakingFee less than zero", + minStake: &cosmostypes.Coin{ + Denom: volatile.DenomuPOKT, + Amount: math.NewInt(-1), + }, + expectedErr: suppliertypes.ErrSupplierParamInvalid.Wrapf( + "staking_fee amount must be positive: got -1%s", + volatile.DenomuPOKT, + ), + }, + { + desc: "zero SupplierStakingFee", + minStake: &cosmostypes.Coin{ + Denom: volatile.DenomuPOKT, + Amount: math.NewInt(0), + }, + }, + { + desc: "valid SupplierStakingFee", + minStake: &suppliertypes.DefaultStakingFee, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + err := suppliertypes.ValidateStakingFee(test.minStake) + if test.expectedErr != nil { + require.Error(t, err) + require.Contains(t, err.Error(), test.expectedErr.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/x/supplier/types/message_update_param.go b/x/supplier/types/message_update_param.go index 0e6a1cc03..e1ce28298 100644 --- a/x/supplier/types/message_update_param.go +++ b/x/supplier/types/message_update_param.go @@ -45,23 +45,28 @@ func (msg *MsgUpdateParam) ValidateBasic() error { // Parameter name MUST be supported by this module. switch msg.Name { case ParamMinStake: - if err := msg.paramTypeIsCoin(); err != nil { + if err := genericParamTypeIs[*MsgUpdateParam_AsCoin](msg); err != nil { return err } return ValidateMinStake(msg.GetAsCoin()) + case ParamStakingFee: + if err := genericParamTypeIs[*MsgUpdateParam_AsCoin](msg); err != nil { + return err + } + return ValidateStakingFee(msg.GetAsCoin()) default: return ErrSupplierParamInvalid.Wrapf("unsupported param %q", msg.Name) } } -// paramTypeIsCoin checks if the parameter type is a Coin, returning an error if not. -func (msg *MsgUpdateParam) paramTypeIsCoin() error { - if _, ok := msg.AsType.(*MsgUpdateParam_AsCoin); !ok { +// genericParamTypeIs checks if the parameter type is T, returning an error if not. +func genericParamTypeIs[T any](msg *MsgUpdateParam) error { + if _, ok := msg.AsType.(T); !ok { return ErrSupplierParamInvalid.Wrapf( - "invalid type for param %q expected %T, got %T", - msg.Name, &MsgUpdateParam_AsCoin{}, - msg.AsType, + "invalid type for param %q; expected %T, got %T", + msg.Name, *new(T), msg.AsType, ) } + return nil } diff --git a/x/supplier/types/params.go b/x/supplier/types/params.go index 48a90f97d..7fe2e081a 100644 --- a/x/supplier/types/params.go +++ b/x/supplier/types/params.go @@ -14,6 +14,10 @@ var ( ParamMinStake = "min_stake" // TODO_MAINNET: Determine the default value. DefaultMinStake = cosmostypes.NewInt64Coin("upokt", 1000000) // 1 POKT + KeyStakingFee = []byte("StakingFee") + ParamStakingFee = "staking_fee" + // TODO_MAINNET: Determine the default value. + DefaultStakingFee = cosmostypes.NewInt64Coin("upokt", 1) // 1 uPOKT ) // ParamKeyTable the param key table for launch module @@ -22,15 +26,22 @@ func ParamKeyTable() paramtypes.KeyTable { } // NewParams creates a new Params instance -func NewParams(minStake *cosmostypes.Coin) Params { +func NewParams( + minStake *cosmostypes.Coin, + stakingFee *cosmostypes.Coin, +) Params { return Params{ - MinStake: minStake, + MinStake: minStake, + StakingFee: stakingFee, } } // DefaultParams returns a default set of parameters func DefaultParams() Params { - return NewParams(&DefaultMinStake) + return NewParams( + &DefaultMinStake, + &DefaultStakingFee, + ) } // ParamSetPairs get the params.ParamSet @@ -41,6 +52,11 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { &p.MinStake, ValidateMinStake, ), + paramtypes.NewParamSetPair( + KeyStakingFee, + &p.StakingFee, + ValidateStakingFee, + ), } } @@ -50,30 +66,58 @@ func (p Params) Validate() error { return err } + if err := ValidateStakingFee(p.StakingFee); err != nil { + return err + } + return nil } // ValidateMinStake validates the MinStake param. func ValidateMinStake(minStakeAny any) error { - minStakeCoin, ok := minStakeAny.(*cosmostypes.Coin) + minStakeCoin, err := paramAsPositiveuPOKT(minStakeAny, "min_stake") + if err != nil { + return err + } + + if minStakeCoin.IsZero() { + return ErrSupplierParamInvalid.Wrapf("min_stake amount must be greater than 0: got %s", minStakeCoin) + } + + return nil +} + +// ValidateStakingFee validates the StakingFee param. +func ValidateStakingFee(stakingFeeAny any) error { + if _, err := paramAsPositiveuPOKT(stakingFeeAny, "staking_fee"); err != nil { + return err + } + + return nil +} + +// paramAsPositiveuPOKT checks that paramAny is a *cosmostypes.Coin and that its +// amount is positive, returning an error if either is not the case. +func paramAsPositiveuPOKT(paramAny any, paramName string) (*cosmostypes.Coin, error) { + paramCoin, ok := paramAny.(*cosmostypes.Coin) if !ok { - return ErrSupplierParamInvalid.Wrapf("invalid parameter type: %T", minStakeAny) + return nil, ErrSupplierParamInvalid.Wrapf("invalid parameter type: %T", paramAny) } - if minStakeCoin == nil { - return ErrSupplierParamInvalid.Wrapf("missing min_stake") + if paramCoin == nil { + return nil, ErrSupplierParamInvalid.Wrapf("missing param") } - if minStakeCoin.Denom != volatile.DenomuPOKT { - return ErrSupplierParamInvalid.Wrapf( - "invalid min_stake denom %q; expected %q", - minStakeCoin.Denom, volatile.DenomuPOKT, + if paramCoin.Denom != volatile.DenomuPOKT { + return nil, ErrSupplierParamInvalid.Wrapf( + "invalid %s denom %q; expected %q", + paramName, paramCoin.Denom, volatile.DenomuPOKT, ) } - if minStakeCoin.IsZero() || minStakeCoin.IsNegative() { - return ErrSupplierParamInvalid.Wrapf("min_stake amount must be greater than 0: got %s", minStakeCoin) + if paramCoin.IsNegative() { + return nil, ErrSupplierParamInvalid.Wrapf("%s amount must be positive: got %s", paramName, paramCoin) } - return nil + return paramCoin, nil } diff --git a/x/supplier/types/params.pb.go b/x/supplier/types/params.pb.go index 8a79db314..06b031634 100644 --- a/x/supplier/types/params.pb.go +++ b/x/supplier/types/params.pb.go @@ -30,6 +30,8 @@ type Params struct { // min_stake is the minimum amount of uPOKT that a supplier must stake to be // included in network sessions and remain staked. MinStake *types.Coin `protobuf:"bytes,1,opt,name=min_stake,json=minStake,proto3" json:"min_stake" yaml:"min_stake"` + // staking_fee is the fee charged by the protocol for staking a supplier. + StakingFee *types.Coin `protobuf:"bytes,2,opt,name=staking_fee,json=stakingFee,proto3" json:"staking_fee" yaml:"staking_fee"` } func (m *Params) Reset() { *m = Params{} } @@ -68,6 +70,13 @@ func (m *Params) GetMinStake() *types.Coin { return nil } +func (m *Params) GetStakingFee() *types.Coin { + if m != nil { + return m.StakingFee + } + return nil +} + func init() { proto.RegisterType((*Params)(nil), "poktroll.supplier.Params") } @@ -75,25 +84,27 @@ func init() { func init() { proto.RegisterFile("poktroll/supplier/params.proto", fileDescriptor_60f7a8031a8c22d5) } var fileDescriptor_60f7a8031a8c22d5 = []byte{ - // 273 bytes of a gzipped FileDescriptorProto + // 313 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x92, 0x2b, 0xc8, 0xcf, 0x2e, 0x29, 0xca, 0xcf, 0xc9, 0xd1, 0x2f, 0x2e, 0x2d, 0x28, 0xc8, 0xc9, 0x4c, 0x2d, 0xd2, 0x2f, 0x48, 0x2c, 0x4a, 0xcc, 0x2d, 0xd6, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0x12, 0x84, 0xc9, 0xeb, 0xc1, 0xe4, 0xa5, 0x04, 0x13, 0x73, 0x33, 0xf3, 0xf2, 0xf5, 0xc1, 0x24, 0x44, 0x95, 0x94, 0x48, 0x7a, 0x7e, 0x7a, 0x3e, 0x98, 0xa9, 0x0f, 0x62, 0x41, 0x45, 0xe5, 0x92, 0xf3, 0x8b, 0x73, 0xf3, 0x8b, 0xf5, 0x93, 0x12, 0x8b, 0x53, 0xf5, 0xcb, 0x0c, 0x93, 0x52, 0x4b, 0x12, 0x0d, 0xf5, 0x93, 0xf3, - 0x33, 0xf3, 0x20, 0xf2, 0x4a, 0x1d, 0x8c, 0x5c, 0x6c, 0x01, 0x60, 0xcb, 0x84, 0x22, 0xb9, 0x38, - 0x73, 0x33, 0xf3, 0xe2, 0x8b, 0x4b, 0x12, 0xb3, 0x53, 0x25, 0x18, 0x15, 0x18, 0x35, 0xb8, 0x8d, - 0x24, 0xf5, 0x20, 0xda, 0xf5, 0x40, 0xda, 0xf5, 0xa0, 0xda, 0xf5, 0x9c, 0xf3, 0x33, 0xf3, 0x9c, - 0x14, 0x5f, 0xdd, 0x93, 0x47, 0xa8, 0xff, 0x74, 0x4f, 0x5e, 0xa0, 0x32, 0x31, 0x37, 0xc7, 0x4a, - 0x09, 0x2e, 0xa4, 0x14, 0xc4, 0x91, 0x9b, 0x99, 0x17, 0x0c, 0x62, 0x5a, 0x29, 0xbf, 0x58, 0x20, - 0xcf, 0xd8, 0xf5, 0x7c, 0x83, 0x96, 0x14, 0xdc, 0xab, 0x15, 0x08, 0xcf, 0x42, 0xec, 0x77, 0xf2, - 0x3f, 0xf1, 0x48, 0x8e, 0xf1, 0xc2, 0x23, 0x39, 0xc6, 0x1b, 0x8f, 0xe4, 0x18, 0x1f, 0x3c, 0x92, - 0x63, 0x9c, 0xf0, 0x58, 0x8e, 0xe1, 0xc2, 0x63, 0x39, 0x86, 0x1b, 0x8f, 0xe5, 0x18, 0xa2, 0x0c, - 0xd3, 0x33, 0x4b, 0x32, 0x4a, 0x93, 0xf4, 0x92, 0xf3, 0x73, 0xf5, 0x41, 0x86, 0xe8, 0xe6, 0xa5, - 0x96, 0x94, 0xe7, 0x17, 0x65, 0xeb, 0x63, 0x33, 0xb1, 0xa4, 0xb2, 0x20, 0xb5, 0x38, 0x89, 0x0d, - 0xec, 0x45, 0x63, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0xc3, 0x4d, 0x0e, 0x37, 0x60, 0x01, 0x00, - 0x00, + 0x33, 0xf3, 0x20, 0xf2, 0x4a, 0xaf, 0x19, 0xb9, 0xd8, 0x02, 0xc0, 0x96, 0x09, 0x45, 0x72, 0x71, + 0xe6, 0x66, 0xe6, 0xc5, 0x17, 0x97, 0x24, 0x66, 0xa7, 0x4a, 0x30, 0x2a, 0x30, 0x6a, 0x70, 0x1b, + 0x49, 0xea, 0x41, 0xb4, 0xeb, 0x81, 0xb4, 0xeb, 0x41, 0xb5, 0xeb, 0x39, 0xe7, 0x67, 0xe6, 0x39, + 0x29, 0xbe, 0xba, 0x27, 0x8f, 0x50, 0xff, 0xe9, 0x9e, 0xbc, 0x40, 0x65, 0x62, 0x6e, 0x8e, 0x95, + 0x12, 0x5c, 0x48, 0x29, 0x88, 0x23, 0x37, 0x33, 0x2f, 0x18, 0xc4, 0x14, 0x4a, 0xe4, 0xe2, 0x06, + 0x89, 0x65, 0xe6, 0xa5, 0xc7, 0xa7, 0xa5, 0xa6, 0x4a, 0x30, 0x11, 0x32, 0x5c, 0xf5, 0xd5, 0x3d, + 0x79, 0x64, 0x1d, 0x9f, 0xee, 0xc9, 0x0b, 0x41, 0x8c, 0x47, 0x12, 0x54, 0x0a, 0xe2, 0x82, 0xf2, + 0xdc, 0x52, 0x53, 0xad, 0x94, 0x5f, 0x2c, 0x90, 0x67, 0xec, 0x7a, 0xbe, 0x41, 0x4b, 0x0a, 0x1e, + 0x9a, 0x15, 0x88, 0xf0, 0x84, 0x78, 0xd1, 0xc9, 0xff, 0xc4, 0x23, 0x39, 0xc6, 0x0b, 0x8f, 0xe4, + 0x18, 0x6f, 0x3c, 0x92, 0x63, 0x7c, 0xf0, 0x48, 0x8e, 0x71, 0xc2, 0x63, 0x39, 0x86, 0x0b, 0x8f, + 0xe5, 0x18, 0x6e, 0x3c, 0x96, 0x63, 0x88, 0x32, 0x4c, 0xcf, 0x2c, 0xc9, 0x28, 0x4d, 0xd2, 0x4b, + 0xce, 0xcf, 0xd5, 0x07, 0x19, 0xa2, 0x9b, 0x97, 0x5a, 0x52, 0x9e, 0x5f, 0x94, 0xad, 0x8f, 0xcd, + 0xc4, 0x92, 0xca, 0x82, 0xd4, 0xe2, 0x24, 0x36, 0x70, 0x28, 0x1a, 0x03, 0x02, 0x00, 0x00, 0xff, + 0xff, 0xe0, 0x47, 0xa0, 0x65, 0xc3, 0x01, 0x00, 0x00, } func (this *Params) Equal(that interface{}) bool { @@ -118,6 +129,9 @@ func (this *Params) Equal(that interface{}) bool { if !this.MinStake.Equal(that1.MinStake) { return false } + if !this.StakingFee.Equal(that1.StakingFee) { + return false + } return true } func (m *Params) Marshal() (dAtA []byte, err error) { @@ -140,6 +154,18 @@ func (m *Params) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if m.StakingFee != nil { + { + size, err := m.StakingFee.MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintParams(dAtA, i, uint64(size)) + } + i-- + dAtA[i] = 0x12 + } if m.MinStake != nil { { size, err := m.MinStake.MarshalToSizedBuffer(dAtA[:i]) @@ -176,6 +202,10 @@ func (m *Params) Size() (n int) { l = m.MinStake.Size() n += 1 + l + sovParams(uint64(l)) } + if m.StakingFee != nil { + l = m.StakingFee.Size() + n += 1 + l + sovParams(uint64(l)) + } return n } @@ -250,6 +280,42 @@ func (m *Params) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex + case 2: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field StakingFee", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowParams + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthParams + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return ErrInvalidLengthParams + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.StakingFee == nil { + m.StakingFee = &types.Coin{} + } + if err := m.StakingFee.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipParams(dAtA[iNdEx:]) From 1a6cabd63b41096c1222a60be035839ca3b29741 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 14:15:38 +0100 Subject: [PATCH 08/10] [Code Health] fix: supplier module gRPC return errors (#962) ## Summary Ensure all supplier message and query handlers return gRPC status errors. ## Issue - #860 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- .../keeper/msg_server_unstake_supplier.go | 39 ++++++++++++------- .../msg_server_unstake_supplier_test.go | 4 +- x/supplier/keeper/msg_update_params.go | 19 +++++++-- x/supplier/keeper/query_supplier.go | 6 ++- 4 files changed, 49 insertions(+), 19 deletions(-) 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) From 74a23ebb7f4a003763270e162989220176312ec1 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 14:15:53 +0100 Subject: [PATCH 09/10] [Code Health] fix: tokenomics module gRPC return errors (#963) ## Summary Ensure all supplier message and query handlers return gRPC status errors. ## Issue - #860 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [x] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: red-0ne --- .../keeper/msg_server_update_param.go | 10 ++++++++-- x/tokenomics/keeper/msg_update_params.go | 20 +++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/x/tokenomics/keeper/msg_server_update_param.go b/x/tokenomics/keeper/msg_server_update_param.go index 92af35e30..643ee17a8 100644 --- a/x/tokenomics/keeper/msg_server_update_param.go +++ b/x/tokenomics/keeper/msg_server_update_param.go @@ -22,11 +22,17 @@ func (k msgServer) UpdateParam( ) if err := msg.ValidateBasic(); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if k.GetAuthority() != msg.Authority { - return nil, tokenomicstypes.ErrTokenomicsInvalidSigner.Wrapf("invalid authority; expected %s, got %s", k.GetAuthority(), msg.Authority) + return nil, status.Error( + codes.PermissionDenied, + tokenomicstypes.ErrTokenomicsInvalidSigner.Wrapf( + "invalid authority; expected %s, got %s", + k.GetAuthority(), msg.Authority, + ).Error(), + ) } params := k.GetParams(ctx) diff --git a/x/tokenomics/keeper/msg_update_params.go b/x/tokenomics/keeper/msg_update_params.go index cefb8e7d6..16e1bfe95 100644 --- a/x/tokenomics/keeper/msg_update_params.go +++ b/x/tokenomics/keeper/msg_update_params.go @@ -4,6 +4,9 @@ import ( "context" "fmt" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "github.com/pokt-network/poktroll/x/tokenomics/types" ) @@ -11,21 +14,26 @@ func (k msgServer) UpdateParams(ctx context.Context, msg *types.MsgUpdateParams) logger := k.Logger() if err := msg.ValidateBasic(); err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } if msg.Authority != k.GetAuthority() { - return nil, types.ErrTokenomicsInvalidSigner.Wrapf( - "invalid authority; expected %s, got %s", - k.GetAuthority(), - msg.Authority, + return nil, status.Error( + codes.PermissionDenied, + types.ErrTokenomicsInvalidSigner.Wrapf( + "invalid authority; expected %s, got %s", + k.GetAuthority(), + msg.Authority, + ).Error(), ) } logger.Info(fmt.Sprintf("About to update params from [%v] to [%v]", k.GetParams(ctx), msg.Params)) if err := k.SetParams(ctx, msg.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()) } logger.Info("Done updating params") From ac4c77986cc52f76b53c54da5ecc2b952cc672d6 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Mon, 2 Dec 2024 14:16:05 +0100 Subject: [PATCH 10/10] chore: add localnet_config option to disable hot-reloading (#979) ## Summary Add a top-level `hot-reloading` boolean field to `localnet_config.yaml` in order to support easier disabling hot-reload. ## Issue There are times where hot-reloading is not desireable. In these instances, in the absence of such a config option, it takes 6 button clicks in the UI to accomplish the same result. - #N/A ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --- Tiltfile | 48 +++++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/Tiltfile b/Tiltfile index bc0d78b92..03cf76bb4 100644 --- a/Tiltfile +++ b/Tiltfile @@ -26,6 +26,7 @@ def merge_dicts(base, updates): # environment is not broken for future engineers. localnet_config_path = "localnet_config.yaml" localnet_config_defaults = { + "hot-reloading": True, "validator": { "cleanupBeforeEachStart": True, "logs": { @@ -182,29 +183,30 @@ secret_create_generic( # Import configuration files into Kubernetes ConfigMap configmap_create("poktrolld-configs", from_file=listdir("localnet/poktrolld/config/"), watch=True) -# Hot reload protobuf changes -local_resource( - "hot-reload: generate protobufs", - "make proto_regen", - deps=["proto"], - labels=["hot-reloading"], -) -# Hot reload the poktrolld binary used by the k8s cluster -local_resource( - "hot-reload: poktrolld", - "GOOS=linux ignite chain build --skip-proto --output=./bin --debug -v", - deps=hot_reload_dirs, - labels=["hot-reloading"], - resource_deps=["hot-reload: generate protobufs"], -) -# Hot reload the local poktrolld binary used by the CLI -local_resource( - "hot-reload: poktrolld - local cli", - "ignite chain build --skip-proto --debug -v -o $(go env GOPATH)/bin", - deps=hot_reload_dirs, - labels=["hot-reloading"], - resource_deps=["hot-reload: generate protobufs"], -) +if localnet_config["hot-reloading"]: + # Hot reload protobuf changes + local_resource( + "hot-reload: generate protobufs", + "make proto_regen", + deps=["proto"], + labels=["hot-reloading"], + ) + # Hot reload the poktrolld binary used by the k8s cluster + local_resource( + "hot-reload: poktrolld", + "GOOS=linux ignite chain build --skip-proto --output=./bin --debug -v", + deps=hot_reload_dirs, + labels=["hot-reloading"], + resource_deps=["hot-reload: generate protobufs"], + ) + # Hot reload the local poktrolld binary used by the CLI + local_resource( + "hot-reload: poktrolld - local cli", + "ignite chain build --skip-proto --debug -v -o $(go env GOPATH)/bin", + deps=hot_reload_dirs, + labels=["hot-reloading"], + resource_deps=["hot-reload: generate protobufs"], + ) # Build an image with a poktrolld binary docker_build_with_restart(