Skip to content

Commit

Permalink
refactor: return an error instead of (bool, error)
Browse files Browse the repository at this point in the history
this is the common convention and the caller is still able to check if
the error was collections.ErrNotFound
  • Loading branch information
hallazzang committed Nov 29, 2024
1 parent 32d810a commit 83a96ff
Show file tree
Hide file tree
Showing 40 changed files with 410 additions and 552 deletions.
9 changes: 3 additions & 6 deletions x/liquidvesting/keeper/send_restriction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,8 @@ func (suite *KeeperTestSuite) TestKeeper_BankSend() {
lockedDenom, err := types.GetLockedRepresentationDenom("stake")
suite.Require().NoError(err)

pool, found, err := suite.pk.GetPool(ctx, 1)
pool, err := suite.pk.GetPool(ctx, 1)
suite.Require().NoError(err)
suite.Require().True(found)

poolCoins := suite.bk.GetAllBalances(ctx, sdk.MustAccAddressFromBech32(pool.GetAddress()))
suite.Require().Equal(sdk.NewCoins(sdk.NewInt64Coin(lockedDenom, 1000)), poolCoins)
Expand Down Expand Up @@ -126,9 +125,8 @@ func (suite *KeeperTestSuite) TestKeeper_BankSend() {
lockedDenom, err := types.GetLockedRepresentationDenom("stake")
suite.Require().NoError(err)

operator, found, err := suite.ok.GetOperator(ctx, 1)
operator, err := suite.ok.GetOperator(ctx, 1)
suite.Require().NoError(err)
suite.Require().True(found)

poolCoins := suite.bk.GetAllBalances(ctx, sdk.MustAccAddressFromBech32(operator.GetAddress()))
suite.Require().Equal(sdk.NewCoins(sdk.NewInt64Coin(lockedDenom, 1000)), poolCoins)
Expand Down Expand Up @@ -173,9 +171,8 @@ func (suite *KeeperTestSuite) TestKeeper_BankSend() {
lockedDenom, err := types.GetLockedRepresentationDenom("stake")
suite.Require().NoError(err)

service, found, err := suite.sk.GetService(ctx, 1)
service, err := suite.sk.GetService(ctx, 1)
suite.Require().NoError(err)
suite.Require().True(found)

poolCoins := suite.bk.GetAllBalances(ctx, sdk.MustAccAddressFromBech32(service.GetAddress()))
suite.Require().Equal(sdk.NewCoins(sdk.NewInt64Coin(lockedDenom, 1000)), poolCoins)
Expand Down
6 changes: 2 additions & 4 deletions x/operators/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ func TestBeginBlocker(t *testing.T) {
},
check: func(ctx sdk.Context) {
// Make sure the operator is still inactivating
operator, found, err := operatorsKeeper.GetOperator(ctx, 1)
operator, err := operatorsKeeper.GetOperator(ctx, 1)
require.NoError(t, err)
require.True(t, found)
require.Equal(t, types.OPERATOR_STATUS_INACTIVATING, operator.Status)

// Make sure the operator is still in the inactivating queue
Expand Down Expand Up @@ -116,9 +115,8 @@ func TestBeginBlocker(t *testing.T) {
},
check: func(ctx sdk.Context) {
// Make sure the operator is inactive
operator, found, err := operatorsKeeper.GetOperator(ctx, 1)
operator, err := operatorsKeeper.GetOperator(ctx, 1)
require.NoError(t, err)
require.True(t, found)
require.Equal(t, types.OPERATOR_STATUS_INACTIVE, operator.Status)

// Make sure the operator is not in the inactivating queue
Expand Down
12 changes: 6 additions & 6 deletions x/operators/keeper/alias_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package keeper

import (
"context"
"errors"
"fmt"
"time"

"cosmossdk.io/collections"
storetypes "cosmossdk.io/store/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/telemetry"
Expand Down Expand Up @@ -44,15 +46,13 @@ func (k *Keeper) GetOperators(ctx context.Context) ([]types.Operator, error) {
func (k *Keeper) IterateInactivatingOperatorQueue(ctx context.Context, endTime time.Time, fn func(operator types.Operator) (stop bool, err error)) error {
return k.iterateInactivatingOperatorsKeys(ctx, endTime, func(key, value []byte) (stop bool, err error) {
operatorID, _ := types.SplitInactivatingOperatorQueueKey(key)
operator, found, err := k.GetOperator(ctx, operatorID)
operator, err := k.GetOperator(ctx, operatorID)
if err != nil {
if errors.Is(err, collections.ErrNotFound) {
return true, fmt.Errorf("operator %d does not exist", operatorID)
}
return true, err
}

if !found {
return true, fmt.Errorf("operator %d does not exist", operatorID)
}

return fn(operator)
})
}
Expand Down
11 changes: 6 additions & 5 deletions x/operators/keeper/genesis.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package keeper

import (
"errors"
"fmt"

"cosmossdk.io/collections"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/milkyway-labs/milkyway/v2/x/operators/types"
Expand Down Expand Up @@ -65,15 +67,14 @@ func (k *Keeper) InitGenesis(ctx sdk.Context, state *types.GenesisState) error {
// Store the operator params
for _, operatorParams := range state.OperatorsParams {
// Ensure that the operator is present
_, found, err := k.GetOperator(ctx, operatorParams.OperatorID)
_, err := k.GetOperator(ctx, operatorParams.OperatorID)
if err != nil {
if errors.Is(err, collections.ErrNotFound) {
return fmt.Errorf("can't set operator params for %d, operator not found", operatorParams.OperatorID)
}
return err
}

if !found {
return fmt.Errorf("can't set operator params for %d, operator not found", operatorParams.OperatorID)
}

err = k.SaveOperatorParams(ctx, operatorParams.OperatorID, operatorParams.Params)
if err != nil {
return err
Expand Down
6 changes: 2 additions & 4 deletions x/operators/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,8 @@ func (suite *KeeperTestSuite) TestKeeper_InitGenesis() {
Params: types.DefaultParams(),
},
check: func(ctx sdk.Context) {
operator, found, err := suite.k.GetOperator(ctx, 1)
operator, err := suite.k.GetOperator(ctx, 1)
suite.Require().NoError(err)
suite.Require().True(found)
suite.Require().Equal(types.NewOperator(
1,
types.OPERATOR_STATUS_ACTIVE,
Expand All @@ -305,9 +304,8 @@ func (suite *KeeperTestSuite) TestKeeper_InitGenesis() {
params, err := suite.k.GetOperatorParams(ctx, 1)
suite.Require().Equal(types.DefaultOperatorParams(), params)

operator, found, err = suite.k.GetOperator(ctx, 2)
operator, err = suite.k.GetOperator(ctx, 2)
suite.Require().NoError(err)
suite.Require().True(found)
suite.Require().Equal(types.NewOperator(
2,
types.OPERATOR_STATUS_INACTIVATING,
Expand Down
20 changes: 10 additions & 10 deletions x/operators/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package keeper

import (
"context"
"errors"

"cosmossdk.io/collections"
"github.com/cosmos/cosmos-sdk/types/query"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand All @@ -14,15 +16,14 @@ var _ types.QueryServer = &Keeper{}

// Operator implements the Query/Operator gRPC method
func (k *Keeper) Operator(ctx context.Context, request *types.QueryOperatorRequest) (*types.QueryOperatorResponse, error) {
operator, found, err := k.GetOperator(ctx, request.OperatorId)
operator, err := k.GetOperator(ctx, request.OperatorId)
if err != nil {
if errors.Is(err, collections.ErrNotFound) {
return nil, status.Error(codes.NotFound, "operator not found")
}
return nil, status.Error(codes.Internal, err.Error())
}

if !found {
return nil, status.Error(codes.NotFound, "operator not found")
}

return &types.QueryOperatorResponse{Operator: operator}, nil
}

Expand All @@ -41,15 +42,14 @@ func (k *Keeper) Operators(ctx context.Context, request *types.QueryOperatorsReq
}

func (k *Keeper) OperatorParams(ctx context.Context, request *types.QueryOperatorParamsRequest) (*types.QueryOperatorParamsResponse, error) {
_, found, err := k.GetOperator(ctx, request.OperatorId)
_, err := k.GetOperator(ctx, request.OperatorId)
if err != nil {
if errors.Is(err, collections.ErrNotFound) {
return nil, status.Error(codes.NotFound, "operator not found")
}
return nil, status.Error(codes.Internal, err.Error())
}

if !found {
return nil, types.ErrOperatorNotFound
}

params, err := k.GetOperatorParams(ctx, request.OperatorId)
if err != nil {
return nil, err
Expand Down
55 changes: 25 additions & 30 deletions x/operators/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"cosmossdk.io/collections"
"cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -98,15 +99,14 @@ func (k msgServer) RegisterOperator(ctx context.Context, msg *types.MsgRegisterO
// UpdateOperator defines the rpc method for Msg/UpdateOperator
func (k msgServer) UpdateOperator(ctx context.Context, msg *types.MsgUpdateOperator) (*types.MsgUpdateOperatorResponse, error) {
// Check if the operator exists
operator, found, err := k.GetOperator(ctx, msg.OperatorID)
operator, err := k.GetOperator(ctx, msg.OperatorID)
if err != nil {
if errors.IsOf(err, collections.ErrNotFound) {
return nil, types.ErrOperatorNotFound
}
return nil, err
}

if !found {
return nil, types.ErrOperatorNotFound
}

// Make sure only the admin can update the operator
if operator.Admin != msg.Sender {
return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "only the admin can update the operator")
Expand Down Expand Up @@ -141,15 +141,14 @@ func (k msgServer) UpdateOperator(ctx context.Context, msg *types.MsgUpdateOpera
// DeactivateOperator defines the rpc method for Msg/DeactivateOperator
func (k msgServer) DeactivateOperator(ctx context.Context, msg *types.MsgDeactivateOperator) (*types.MsgDeactivateOperatorResponse, error) {
// Check if the operator exists
operator, found, err := k.GetOperator(ctx, msg.OperatorID)
operator, err := k.GetOperator(ctx, msg.OperatorID)
if err != nil {
if errors.IsOf(err, collections.ErrNotFound) {
return nil, types.ErrOperatorNotFound
}
return nil, err
}

if !found {
return nil, types.ErrOperatorNotFound
}

// Make sure only the admin can deactivate the operator
if operator.Admin != msg.Sender {
return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "only the admin can deactivate the operator")
Expand All @@ -175,15 +174,14 @@ func (k msgServer) DeactivateOperator(ctx context.Context, msg *types.MsgDeactiv
// ReactivateOperator defines the rpc method for Msg/ReactivateOperator
func (k msgServer) ReactivateOperator(ctx context.Context, msg *types.MsgReactivateOperator) (*types.MsgReactivateOperatorResponse, error) {
// Check if the operator exists
operator, found, err := k.GetOperator(ctx, msg.OperatorID)
operator, err := k.GetOperator(ctx, msg.OperatorID)
if err != nil {
if errors.IsOf(err, collections.ErrNotFound) {
return nil, types.ErrOperatorNotFound
}
return nil, err
}

if !found {
return nil, types.ErrOperatorNotFound
}

// Make sure only the admin can reactivate the operator
if operator.Admin != msg.Sender {
return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "only the admin can deactivate the operator")
Expand All @@ -209,15 +207,14 @@ func (k msgServer) ReactivateOperator(ctx context.Context, msg *types.MsgReactiv
// DeleteOperator defines the rpc method for Msg/DeleteOperator
func (k msgServer) DeleteOperator(ctx context.Context, msg *types.MsgDeleteOperator) (*types.MsgDeleteOperatorResponse, error) {
// Check if the operator exists
operator, found, err := k.GetOperator(ctx, msg.OperatorID)
operator, err := k.GetOperator(ctx, msg.OperatorID)
if err != nil {
if errors.IsOf(err, collections.ErrNotFound) {
return nil, types.ErrOperatorNotFound
}
return nil, err
}

if !found {
return nil, types.ErrOperatorNotFound
}

// Make sure only the admin can delete the operator
if operator.Admin != msg.Sender {
return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "only the admin can delete the operator")
Expand All @@ -243,15 +240,14 @@ func (k msgServer) DeleteOperator(ctx context.Context, msg *types.MsgDeleteOpera
// TransferOperatorOwnership defines the rpc method for Msg/TransferOperatorOwnership
func (k msgServer) TransferOperatorOwnership(ctx context.Context, msg *types.MsgTransferOperatorOwnership) (*types.MsgTransferOperatorOwnershipResponse, error) {
// Check if the operator exists
operator, found, err := k.GetOperator(ctx, msg.OperatorID)
operator, err := k.GetOperator(ctx, msg.OperatorID)
if err != nil {
if errors.IsOf(err, collections.ErrNotFound) {
return nil, types.ErrOperatorNotFound
}
return nil, err
}

if !found {
return nil, types.ErrOperatorNotFound
}

// Make sure only the admin can transfer the operator ownership
if operator.Admin != msg.Sender {
return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "only the admin can transfer the operator ownership")
Expand All @@ -278,15 +274,14 @@ func (k msgServer) TransferOperatorOwnership(ctx context.Context, msg *types.Msg

// SetOperatorParams defines the rpc method for Msg/SetOperatorParams
func (k msgServer) SetOperatorParams(ctx context.Context, msg *types.MsgSetOperatorParams) (*types.MsgSetOperatorParamsResponse, error) {
operator, found, err := k.GetOperator(ctx, msg.OperatorID)
operator, err := k.GetOperator(ctx, msg.OperatorID)
if err != nil {
if errors.IsOf(err, collections.ErrNotFound) {
return nil, types.ErrOperatorNotFound
}
return nil, err
}

if !found {
return nil, types.ErrOperatorNotFound
}

// Make sure only the admin can update the operator
if operator.Admin != msg.Sender {
return nil, errors.Wrapf(sdkerrors.ErrUnauthorized, "only the admin can update the operator params")
Expand Down
Loading

0 comments on commit 83a96ff

Please sign in to comment.