fix: limit tokenfactory changeadmin with page requests & index it properly

CHANGELOG.md

@@ -19,6 +19,8 @@
 - Ensure feeTokenMetadata initial prices after updateFeeTokenMetadata is picked up from oracle
 - Use `DecCoins.Validate()` on `RewardPool.ValidateGenesis` to catch malformed denom formats, duplicate denoms, bad ordering
 - Enforce denom consistency in `GenesisState.Validate` with `Params.TokenDenom`
+- Limited tokenfactory queries, removing denial of service possibility
+- Indexed admins to reduce query space on tokenfactory denom queries
 
 ### Removed
 

app/app.go

@@ -68,7 +68,7 @@ import (
 	kiiante "github.com/kiichain/kiichain/v7/ante"
 	"github.com/kiichain/kiichain/v7/app/keepers"
 	"github.com/kiichain/kiichain/v7/app/upgrades"
-	v7_1_mainnet "github.com/kiichain/kiichain/v7/app/upgrades/v7_1_mainnet"
+	v7_2 "github.com/kiichain/kiichain/v7/app/upgrades/v7_2"
 	"github.com/kiichain/kiichain/v7/client/docs"
 )
 
@@ -78,7 +78,7 @@ var (
 
 	// Upgrades is a list of all the upgrades that are available for the application.
 	Upgrades = []upgrades.Upgrade{
-		v7_1_mainnet.Upgrade,
+		v7_2.Upgrade,
 	}
 )
 

app/upgrades/v7_2/constants.go

@@ -0,0 +1,17 @@
+package v720
+
+import (
+	"github.com/kiichain/kiichain/v7/app/upgrades"
+)
+
+const (
+	// UpgradeName is the name of the upgrade
+	UpgradeName = "v7.2.0"
+)
+
+// Upgrade defines the upgrade, running module migrations to backfill the
+// tokenfactory admin secondary index.
+var Upgrade = upgrades.Upgrade{
+	UpgradeName:          UpgradeName,
+	CreateUpgradeHandler: CreateUpgradeHandler,
+}

app/upgrades/v7_2/upgrade.go

@@ -0,0 +1,35 @@
+package v720
+
+import (
+	"context"
+
+	upgradetypes "cosmossdk.io/x/upgrade/types"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/module"
+
+	"github.com/kiichain/kiichain/v7/app/keepers"
+)
+
+// CreateUpgradeHandler creates the upgrade handler for the v7.2.0 upgrade.
+// It runs all pending module migrations, which includes the tokenfactory v1→v2
+// migration that backfills the secondary admin index.
+func CreateUpgradeHandler(
+	mm *module.Manager,
+	configurator module.Configurator,
+	_ *keepers.AppKeepers,
+) upgradetypes.UpgradeHandler {
+	return func(c context.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
+		ctx := sdk.UnwrapSDKContext(c)
+
+		ctx.Logger().Info("Starting module migrations for v7.2.0...")
+
+		vm, err := mm.RunMigrations(ctx, configurator, vm)
+		if err != nil {
+			return vm, err
+		}
+
+		ctx.Logger().Info("Upgrade v7.2.0 complete")
+		return vm, nil
+	}
+}

proto/kiichain/tokenfactory/v1beta1/query.proto

@@ -70,22 +70,26 @@ message QueryDenomAuthorityMetadataResponse {
 // DenomsFromCreator gRPC query.
 message QueryDenomsFromCreatorRequest {
   string creator = 1 [ (gogoproto.moretags) = "yaml:\"creator\"" ];
+  cosmos.base.query.v1beta1.PageRequest pagination = 2;
 }
 
-// QueryDenomsFromCreatorRequest defines the response structure for the
+// QueryDenomsFromCreatorResponse defines the response structure for the
 // DenomsFromCreator gRPC query.
 message QueryDenomsFromCreatorResponse {
   repeated string denoms = 1 [ (gogoproto.moretags) = "yaml:\"denoms\"" ];
+  cosmos.base.query.v1beta1.PageResponse pagination = 2;
 }
 
 // QueryDenomsFromAdminRequest defines the request structure for the
 // DenomsFromAdmin gRPC query.
 message QueryDenomsFromAdminRequest {
   string admin = 1 [ (gogoproto.moretags) = "yaml:\"admin\"" ];
+  cosmos.base.query.v1beta1.PageRequest pagination = 2;
 }
 
-// QueryDenomsFromAdminRequest defines the response structure for the
+// QueryDenomsFromAdminResponse defines the response structure for the
 // DenomsFromAdmin gRPC query.
 message QueryDenomsFromAdminResponse {
   repeated string denoms = 1 [ (gogoproto.moretags) = "yaml:\"denoms\"" ];
+  cosmos.base.query.v1beta1.PageResponse pagination = 2;
 }

wasmbinding/tokenfactory/queries.go

@@ -135,7 +135,10 @@ func (qp QueryPlugin) GetTokenfactoryDenomsByCreator(ctx context.Context, creato
 	if _, err := sdk.AccAddressFromBech32(creator); err != nil {
 		return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address %q: %v", creator, err)
 	}
-	denoms := qp.tokenFactoryKeeper.GetDenomsFromCreator(sdk.UnwrapSDKContext(ctx), creator)
+	denoms, _, err := qp.tokenFactoryKeeper.GetDenomsFromCreator(sdk.UnwrapSDKContext(ctx), creator, nil)
+	if err != nil {
+		return nil, err
+	}
 	return &tfbindingtypes.DenomsByCreatorResponse{Denoms: denoms}, nil
 }
 

x/tokenfactory/client/cli/query.go

@@ -101,8 +101,14 @@ func GetCmdDenomsFromCreator() *cobra.Command {
 			}
 			queryClient := types.NewQueryClient(clientCtx)
 
+			pageReq, err := client.ReadPageRequest(cmd.Flags())
+			if err != nil {
+				return err
+			}
+
 			res, err := queryClient.DenomsFromCreator(cmd.Context(), &types.QueryDenomsFromCreatorRequest{
-				Creator: args[0],
+				Creator:    args[0],
+				Pagination: pageReq,
 			})
 			if err != nil {
 				return err
@@ -113,6 +119,7 @@ func GetCmdDenomsFromCreator() *cobra.Command {
 	}
 
 	flags.AddQueryFlagsToCmd(cmd)
+	flags.AddPaginationFlagsToCmd(cmd, "denoms-from-creator")
 
 	return cmd
 }
@@ -129,8 +136,14 @@ func GetCmdDenomsFromAdmin() *cobra.Command {
 			}
 			queryClient := types.NewQueryClient(clientCtx)
 
+			pageReq, err := client.ReadPageRequest(cmd.Flags())
+			if err != nil {
+				return err
+			}
+
 			res, err := queryClient.DenomsFromAdmin(cmd.Context(), &types.QueryDenomsFromAdminRequest{
-				Admin: args[0],
+				Admin:      args[0],
+				Pagination: pageReq,
 			})
 			if err != nil {
 				return err
@@ -141,6 +154,7 @@ func GetCmdDenomsFromAdmin() *cobra.Command {
 	}
 
 	flags.AddQueryFlagsToCmd(cmd)
+	flags.AddPaginationFlagsToCmd(cmd, "denoms-from-admin")
 
 	return cmd
 }

x/tokenfactory/keeper/admins.go

@@ -5,7 +5,11 @@ import (
 
 	"github.com/cosmos/gogoproto/proto"
 
+	"cosmossdk.io/errors"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	query "github.com/cosmos/cosmos-sdk/types/query"
 
 	"github.com/kiichain/kiichain/v7/x/tokenfactory/types"
 )
@@ -22,45 +26,79 @@ func (k Keeper) GetAuthorityMetadata(ctx context.Context, denom string) (types.D
 	return metadata, nil
 }
 
-// setAuthorityMetadata stores authority metadata for a specific denom
+// setAuthorityMetadata stores authority metadata for a specific denom and keeps the
+// admin secondary index consistent. It reads the current admin so it can remove the
+// denom from the old admin's index before inserting it under the new admin
 func (k Keeper) setAuthorityMetadata(ctx context.Context, denom string, metadata types.DenomAuthorityMetadata) error {
 	err := metadata.Validate()
 	if err != nil {
 		return err
 	}
 
-	store := k.GetDenomPrefixStore(sdk.UnwrapSDKContext(ctx), denom)
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+
+	// Remove denom from the current admin's index (if any metadata already exists)
+	oldMetadata, err := k.GetAuthorityMetadata(sdkCtx, denom)
+	if err != nil {
+		return err
+	}
+	k.removeDenomFromAdmin(sdkCtx, oldMetadata.Admin, denom)
+
+	store := k.GetDenomPrefixStore(sdkCtx, denom)
 
 	bz, err := proto.Marshal(&metadata)
 	if err != nil {
 		return err
 	}
 
 	store.Set([]byte(types.DenomAuthorityMetadataKey), bz)
+
+	// Insert denom under the new admin's index.
+	k.addDenomFromAdmin(sdkCtx, metadata.Admin, denom)
+
 	return nil
 }
 
+// addDenomFromAdmin inserts denom into the secondary index for admin
+func (k Keeper) addDenomFromAdmin(ctx context.Context, admin, denom string) {
+	k.GetAdminPrefixStore(sdk.UnwrapSDKContext(ctx), admin).Set([]byte(denom), []byte(denom))
+}
+
+// AddDenomFromAdmin is the exported form of addDenomFromAdmin
+func (k Keeper) AddDenomFromAdmin(ctx context.Context, admin, denom string) {
+	k.addDenomFromAdmin(ctx, admin, denom)
+}
+
+// removeDenomFromAdmin removes denom from the secondary index for admin
+func (k Keeper) removeDenomFromAdmin(ctx context.Context, admin, denom string) {
+	k.GetAdminPrefixStore(sdk.UnwrapSDKContext(ctx), admin).Delete([]byte(denom))
+}
+
 func (k Keeper) setAdmin(ctx context.Context, metadata types.DenomAuthorityMetadata, denom string, admin string) error {
 	metadata.Admin = admin
 
 	return k.setAuthorityMetadata(ctx, denom, metadata)
 }
 
-// GetDenomsFromAdmin returns all denoms for which the provided address is the admin
-func (k Keeper) GetDenomsFromAdmin(ctx context.Context, admin string) ([]string, error) {
-	iterator := k.GetAllDenomsIterator(ctx)
-	defer iterator.Close()
-
-	denoms := []string{}
-	for ; iterator.Valid(); iterator.Next() {
-		denom := string(iterator.Value())
-		metadata, err := k.GetAuthorityMetadata(sdk.UnwrapSDKContext(ctx), denom)
-		if err != nil {
-			return nil, err
-		}
-		if metadata.Admin == admin {
-			denoms = append(denoms, denom)
-		}
+// GetDenomsFromAdmin returns a paginated list of denoms the given admin.
+// It has maximum of types.MaxPageSize entries per request
+func (k Keeper) GetDenomsFromAdmin(ctx context.Context, admin string, req *query.PageRequest) ([]string, *query.PageResponse, error) {
+	if req == nil {
+		req = &query.PageRequest{Limit: types.MaxPageSize}
+	} else if req.Limit > types.MaxPageSize {
+		return nil, nil, errors.Wrapf(sdkerrors.ErrInvalidRequest,
+			"page size %d exceeds maximum allowed %d", req.Limit, types.MaxPageSize)
+	}
+
+	store := k.GetAdminPrefixStore(sdk.UnwrapSDKContext(ctx), admin)
+
+	var denoms []string
+	pageRes, err := query.Paginate(store, req, func(key, _ []byte) error {
+		denoms = append(denoms, string(key))
+		return nil
+	})
+	if err != nil {
+		return nil, nil, err
 	}
-	return denoms, nil
+	return denoms, pageRes, nil
 }

x/tokenfactory/keeper/admins_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/query"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 
 	"github.com/kiichain/kiichain/v7/x/tokenfactory/types"
@@ -584,3 +585,76 @@ func (suite *KeeperTestSuite) TestSetDenomMetaData() {
 		})
 	}
 }
+
+// TestDenomsFromAdminPagination verifies that DenomsFromAdmin pagination works
+// correctly and that requests exceeding MaxPageSize are rejected.
+func (suite *KeeperTestSuite) TestDenomsFromAdminPagination() {
+	suite.SetupTest()
+
+	// Create 3 denoms for TestAccs[0] to use as paging targets.
+	subdenoms := []string{"aaa", "bbb", "ccc"}
+	for _, sub := range subdenoms {
+		_, err := suite.msgServer.CreateDenom(suite.Ctx, types.NewMsgCreateDenom(suite.TestAccs[0].String(), sub))
+		suite.Require().NoError(err)
+	}
+
+	// Nil pagination defaults to MaxPageSize limit.
+	res, err := suite.queryClient.DenomsFromAdmin(suite.Ctx.Context(), &types.QueryDenomsFromAdminRequest{
+		Admin: suite.TestAccs[0].String(),
+	})
+	suite.Require().NoError(err)
+	suite.Require().Len(res.Denoms, 3)
+
+	// Limit=2 returns exactly 2 denoms with a non-nil next key.
+	res, err = suite.queryClient.DenomsFromAdmin(suite.Ctx.Context(), &types.QueryDenomsFromAdminRequest{
+		Admin:      suite.TestAccs[0].String(),
+		Pagination: &query.PageRequest{Limit: 2},
+	})
+	suite.Require().NoError(err)
+	suite.Require().Len(res.Denoms, 2)
+	suite.Require().NotNil(res.Pagination)
+	suite.Require().NotEmpty(res.Pagination.NextKey)
+
+	// A request with Limit > MaxPageSize must be rejected.
+	_, err = suite.queryClient.DenomsFromAdmin(suite.Ctx.Context(), &types.QueryDenomsFromAdminRequest{
+		Admin:      suite.TestAccs[0].String(),
+		Pagination: &query.PageRequest{Limit: types.MaxPageSize + 1},
+	})
+	suite.Require().Error(err)
+}
+
+// TestDenomsFromCreatorPagination verifies that DenomsFromCreator pagination works
+// correctly and that requests exceeding MaxPageSize are rejected.
+func (suite *KeeperTestSuite) TestDenomsFromCreatorPagination() {
+	suite.SetupTest()
+
+	subdenoms := []string{"aaa", "bbb", "ccc"}
+	for _, sub := range subdenoms {
+		_, err := suite.msgServer.CreateDenom(suite.Ctx, types.NewMsgCreateDenom(suite.TestAccs[0].String(), sub))
+		suite.Require().NoError(err)
+	}
+
+	// Nil pagination defaults to MaxPageSize limit.
+	res, err := suite.queryClient.DenomsFromCreator(suite.Ctx.Context(), &types.QueryDenomsFromCreatorRequest{
+		Creator: suite.TestAccs[0].String(),
+	})
+	suite.Require().NoError(err)
+	suite.Require().Len(res.Denoms, 3)
+
+	// Limit=1 returns exactly 1 denom.
+	res, err = suite.queryClient.DenomsFromCreator(suite.Ctx.Context(), &types.QueryDenomsFromCreatorRequest{
+		Creator:    suite.TestAccs[0].String(),
+		Pagination: &query.PageRequest{Limit: 1},
+	})
+	suite.Require().NoError(err)
+	suite.Require().Len(res.Denoms, 1)
+	suite.Require().NotNil(res.Pagination)
+	suite.Require().NotEmpty(res.Pagination.NextKey)
+
+	// A request with Limit > MaxPageSize must be rejected.
+	_, err = suite.queryClient.DenomsFromCreator(suite.Ctx.Context(), &types.QueryDenomsFromCreatorRequest{
+		Creator:    suite.TestAccs[0].String(),
+		Pagination: &query.PageRequest{Limit: types.MaxPageSize + 1},
+	})
+	suite.Require().Error(err)
+}