refactor: apply Cosmos SDK v0.50 conventions (#208)

## Description

The main changes are:
- Use `Walk()` instead of old iterator-based approaches where applicable
- Return an error only instead of a bit of redundant `(T, bool error)`
- Remove unused `storeService` references in keepers

This PR introduces no state-breaking changes but note that merging this
PR is not urgent.

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is
not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR
Targeting](https://github.com/milkyway-labs/milkyway/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building
modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [ ] included the necessary unit and integration
[tests](https://github.com/milkyway-labs/milkyway/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go
code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable
and please add
your handle next to the items reviewed if you only reviewed selected
items.*

I have...

- [ ] confirmed the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

.changeset/entries/861a96d7026170f055a293299b782c79785a8c6c3f7034ebe62f3ea878266583.yaml

@@ -0,0 +1,6 @@
+type: refactor
+module: none
+pull_request: 208
+description: Apply Cosmos SDK v0.50 conventions
+backward_compatible: true
+date: 2024-12-10T08:38:13.018999Z

x/assets/keeper/keeper.go

@@ -13,8 +13,7 @@ import (
 )
 
 type Keeper struct {
-	cdc          codec.Codec
-	storeService corestoretypes.KVStoreService
+	cdc codec.Codec
 
 	Schema        collections.Schema
 	Assets        collections.Map[string, types.Asset]                 // denom => types.Asset
@@ -30,8 +29,7 @@ func NewKeeper(
 ) *Keeper {
 	sb := collections.NewSchemaBuilder(storeService)
 	k := &Keeper{
-		cdc:          cdc,
-		storeService: storeService,
+		cdc: cdc,
 
 		Assets: collections.NewMap(
 			sb,

x/liquidvesting/keeper/send_restriction_test.go

@@ -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)
@@ -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)
@@ -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)

x/operators/abci_test.go

@@ -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
@@ -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

x/operators/keeper/alias_functions.go

@@ -2,9 +2,10 @@ package keeper
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"time"
 
+	"cosmossdk.io/collections"
 	storetypes "cosmossdk.io/store/types"
 	"github.com/cosmos/cosmos-sdk/runtime"
 	"github.com/cosmos/cosmos-sdk/telemetry"
@@ -23,29 +24,10 @@ func (k *Keeper) createAccountIfNotExists(ctx context.Context, address sdk.AccAd
 
 // IterateOperators iterates over the operators in the store and performs a callback function
 func (k *Keeper) IterateOperators(ctx context.Context, cb func(operator types.Operator) (stop bool, err error)) error {
-	iterator, err := k.operators.Iterate(ctx, nil)
-	if err != nil {
-		return err
-	}
-	defer iterator.Close()
-
-	for ; iterator.Valid(); iterator.Next() {
-		operator, err := iterator.Value()
-		if err != nil {
-			return err
-		}
-
-		stop, err := cb(operator)
-		if err != nil {
-			return err
-		}
-
-		if stop {
-			break
-		}
-	}
-
-	return nil
+	err := k.operators.Walk(ctx, nil, func(_ uint32, operator types.Operator) (stop bool, err error) {
+		return cb(operator)
+	})
+	return err
 }
 
 // GetOperators returns the operators stored in the KVStore
@@ -63,15 +45,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, types.ErrOperatorNotFound
+			}
 			return true, err
 		}
-
-		if !found {
-			return true, fmt.Errorf("operator %d does not exist", operatorID)
-		}
-
 		return fn(operator)
 	})
 }
@@ -126,26 +106,13 @@ func (k *Keeper) IsOperatorAddress(ctx context.Context, address string) (bool, e
 
 // GetAllOperatorParamsRecords returns all the operator params records
 func (k *Keeper) GetAllOperatorParamsRecords(ctx context.Context) ([]types.OperatorParamsRecord, error) {
-	iterator, err := k.operatorParams.Iterate(ctx, nil)
-	if err != nil {
-		return nil, err
-	}
-	defer iterator.Close()
-
 	var records []types.OperatorParamsRecord
-	for ; iterator.Valid(); iterator.Next() {
-		// Get the operator params
-		params, err := iterator.Value()
-		if err != nil {
-			return nil, err
-		}
-		// Get the operator id from the map key
-		operatorID, err := iterator.Key()
-		if err != nil {
-			return nil, err
-		}
+	err := k.operatorParams.Walk(ctx, nil, func(operatorID uint32, params types.OperatorParams) (stop bool, err error) {
 		records = append(records, types.NewOperatorParamsRecord(operatorID, params))
+		return false, nil
+	})
+	if err != nil {
+		return nil, err
 	}
-
 	return records, nil
 }

x/operators/keeper/genesis.go

@@ -1,8 +1,10 @@
 package keeper
 
 import (
-	"fmt"
+	"errors"
 
+	"cosmossdk.io/collections"
+	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/milkyway-labs/milkyway/v3/x/operators/types"
@@ -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 errorsmod.Wrapf(types.ErrOperatorNotFound, "operator %d 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

x/operators/keeper/genesis_test.go

@@ -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,
@@ -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,

x/operators/keeper/grpc_query.go

@@ -2,9 +2,9 @@ package keeper
 
 import (
 	"context"
+	"errors"
 
-	"cosmossdk.io/store/prefix"
-	"github.com/cosmos/cosmos-sdk/runtime"
+	"cosmossdk.io/collections"
 	"github.com/cosmos/cosmos-sdk/types/query"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -16,53 +16,40 @@ 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
 }
 
 // Operators implements the Query/Operators gRPC method
 func (k *Keeper) Operators(ctx context.Context, request *types.QueryOperatorsRequest) (*types.QueryOperatorsResponse, error) {
-	store := k.storeService.OpenKVStore(ctx)
-	operatorsStore := prefix.NewStore(runtime.KVStoreAdapter(store), types.OperatorPrefix)
-
-	var operators []types.Operator
-	pageRes, err := query.Paginate(operatorsStore, request.Pagination, func(key []byte, value []byte) error {
-		var operator types.Operator
-		if err := k.cdc.Unmarshal(value, &operator); err != nil {
-			return status.Error(codes.Internal, err.Error())
-		}
-
-		operators = append(operators, operator)
-		return nil
+	operators, pageRes, err := query.CollectionPaginate(ctx, k.operators, request.Pagination, func(_ uint32, operator types.Operator) (types.Operator, error) {
+		return operator, nil
 	})
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
-
 	return &types.QueryOperatorsResponse{
 		Operators:  operators,
 		Pagination: pageRes,
 	}, nil
 }
 
 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