fix!: update consumer chain queries (#2246)

* fix!: update consumer chain queries

* apply review comments

* fix err handling and gosec reports

Author: MSalopek

Diff for: proto/interchain_security/ccv/provider/v1/query.proto

@@ -12,6 +12,7 @@ import "interchain_security/ccv/v1/wire.proto";
 import "tendermint/crypto/keys.proto";
 import "cosmos_proto/cosmos.proto";
 import "cosmos/staking/v1beta1/staking.proto";
+import "cosmos/base/query/v1beta1/pagination.proto";
 
 service Query {
   // ConsumerGenesis queries the genesis state needed to start a consumer chain
@@ -28,7 +29,7 @@ service Query {
   rpc QueryConsumerChains(QueryConsumerChainsRequest)
       returns (QueryConsumerChainsResponse) {
     option (google.api.http).get =
-        "/interchain_security/ccv/provider/consumer_chains/{phase}/{limit}";
+        "/interchain_security/ccv/provider/consumer_chains/{phase}";
   }
 
   // QueryValidatorConsumerAddr queries the address
@@ -158,12 +159,14 @@ message QueryConsumerChainsRequest {
   // The phase of the consumer chains returned (optional)
   // Registered=1|Initialized=2|Launched=3|Stopped=4|Deleted=5
   ConsumerPhase phase = 1;
-  // The limit of consumer chains returned (optional)
-  // default is 100
-  int32 limit = 2;
+
+  cosmos.base.query.v1beta1.PageRequest pagination = 2;
 }
 
-message QueryConsumerChainsResponse { repeated Chain chains = 1; }
+message QueryConsumerChainsResponse {
+  repeated Chain chains = 1;
+  cosmos.base.query.v1beta1.PageResponse pagination = 2;
+}
 
 message Chain {
   string chain_id = 1;
@@ -372,10 +375,11 @@ message QueryConsumerChainRequest {
 }
 
 message QueryConsumerChainResponse {
-  string chain_id = 1;
-  string owner_address = 2;
-  string phase = 3;
-  ConsumerMetadata metadata = 4 [ (gogoproto.nullable) = false ];
-  ConsumerInitializationParameters init_params = 5;
-  PowerShapingParameters power_shaping_params = 6;  
+  string consumer_id = 1;
+  string chain_id = 2;
+  string owner_address = 3;
+  string phase = 4;
+  ConsumerMetadata metadata = 5 [ (gogoproto.nullable) = false ];
+  ConsumerInitializationParameters init_params = 6;
+  PowerShapingParameters power_shaping_params = 7;
 }

Diff for: x/ccv/provider/client/cli/query.go

@@ -102,7 +102,7 @@ func CmdConsumerChains() *cobra.Command {
 				if err != nil {
 					return err
 				}
-				req.Limit = int32(limit)
+				req.Pagination.Limit = uint64(limit)
 			}
 
 			res, err := queryClient.QueryConsumerChains(cmd.Context(), req)

Diff for: x/ccv/provider/keeper/grpc_query.go

@@ -3,15 +3,18 @@ package keeper
 import (
 	"bytes"
 	"context"
+	"encoding/binary"
 	"fmt"
 	"sort"
 
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
 	errorsmod "cosmossdk.io/errors"
+	"cosmossdk.io/store/prefix"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/query"
 
 	"github.com/cosmos/interchain-security/v6/x/ccv/provider/types"
 	ccvtypes "github.com/cosmos/interchain-security/v6/x/ccv/types"
@@ -48,37 +51,35 @@ func (k Keeper) QueryConsumerChains(goCtx context.Context, req *types.QueryConsu
 	}
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
+	var chains []*types.Chain
 
-	consumerIds := []string{}
-	for _, consumerID := range k.GetAllConsumerIds(ctx) {
-		phase := k.GetConsumerPhase(ctx, consumerID)
-		if req.Phase != types.CONSUMER_PHASE_UNSPECIFIED && req.Phase != phase {
-			// ignore consumer chain
-			continue
+	store := ctx.KVStore(k.storeKey)
+	storePrefix := types.ConsumerIdToPhaseKeyPrefix()
+	consumerPhaseStore := prefix.NewStore(store, []byte{storePrefix})
+	pageRes, err := query.Paginate(consumerPhaseStore, req.Pagination, func(key, value []byte) error {
+		consumerId, err := types.ParseStringIdWithLenKey(storePrefix, append([]byte{storePrefix}, key...))
+		if err != nil {
+			return status.Error(codes.Internal, err.Error())
 		}
-		consumerIds = append(consumerIds, consumerID)
-	}
 
-	// set limit to default value
-	limit := 100
-	if req.Limit != 0 {
-		// update limit if specified
-		limit = int(req.Limit)
-	}
-	if len(consumerIds) > limit {
-		consumerIds = consumerIds[:limit]
-	}
+		phase := types.ConsumerPhase(binary.BigEndian.Uint32(value))
+		if req.Phase != types.CONSUMER_PHASE_UNSPECIFIED && req.Phase != phase {
+			return nil
+		}
 
-	chains := make([]*types.Chain, len(consumerIds))
-	for i, cID := range consumerIds {
-		c, err := k.GetConsumerChain(ctx, cID)
+		c, err := k.GetConsumerChain(ctx, consumerId)
 		if err != nil {
-			return nil, status.Error(codes.Internal, err.Error())
+			return status.Error(codes.Internal, err.Error())
 		}
-		chains[i] = &c
+		chains = append(chains, &c)
+		return nil
+	})
+
+	if err != nil {
+		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	return &types.QueryConsumerChainsResponse{Chains: chains}, nil
+	return &types.QueryConsumerChainsResponse{Chains: chains, Pagination: pageRes}, nil
 }
 
 // GetConsumerChain returns a Chain data structure with all the necessary fields
@@ -594,6 +595,7 @@ func (k Keeper) QueryConsumerChain(goCtx context.Context, req *types.QueryConsum
 
 	return &types.QueryConsumerChainResponse{
 		ChainId:            chainId,
+		ConsumerId:         consumerId,
 		OwnerAddress:       ownerAddress,
 		Phase:              phase.String(),
 		Metadata:           metadata,

Diff for: x/ccv/provider/keeper/grpc_query_test.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/cometbft/cometbft/proto/tendermint/crypto"
 
+	sdkquery "github.com/cosmos/cosmos-sdk/types/query"
 	cryptotestutil "github.com/cosmos/interchain-security/v6/testutil/crypto"
 	testkeeper "github.com/cosmos/interchain-security/v6/testutil/keeper"
 	"github.com/cosmos/interchain-security/v6/x/ccv/provider/keeper"
@@ -558,6 +559,7 @@ func TestQueryConsumerChain(t *testing.T) {
 
 	expRes := types.QueryConsumerChainResponse{
 		ChainId:            chainId,
+		ConsumerId:         consumerId,
 		OwnerAddress:       providerKeeper.GetAuthority(),
 		Metadata:           types.ConsumerMetadata{Name: chainId},
 		Phase:              types.CONSUMER_PHASE_REGISTERED.String(),
@@ -663,7 +665,7 @@ func TestQueryConsumerChains(t *testing.T) {
 		name         string
 		setup        func(ctx sdk.Context, pk keeper.Keeper)
 		phase_filter types.ConsumerPhase
-		limit        int32
+		limit        uint64
 		expConsumers []*types.Chain
 	}{
 		{
@@ -675,7 +677,7 @@ func TestQueryConsumerChains(t *testing.T) {
 			name:         "expect an amount of consumer equal to the limit",
 			setup:        func(ctx sdk.Context, pk keeper.Keeper) {},
 			expConsumers: consumers[:3],
-			limit:        int32(3),
+			limit:        3,
 		},
 		{
 			name: "expect registered consumers when phase filter is set to Registered",
@@ -720,14 +722,19 @@ func TestQueryConsumerChains(t *testing.T) {
 			tc.setup(ctx, pk)
 			req := types.QueryConsumerChainsRequest{
 				Phase: tc.phase_filter,
-				Limit: tc.limit,
+				Pagination: &sdkquery.PageRequest{
+					Limit: tc.limit,
+				},
 			}
 			expectedResponse := types.QueryConsumerChainsResponse{
 				Chains: tc.expConsumers,
 			}
 			res, err := pk.QueryConsumerChains(ctx, &req)
 			require.NoError(t, err)
-			require.Equal(t, &expectedResponse, res, tc.name)
+			require.Equal(t, expectedResponse.GetChains(), res.GetChains(), tc.name)
+			if tc.limit != 0 {
+				require.Len(t, res.GetChains(), int(tc.limit), tc.name)
+			}
 		})
 	}
 }

Diff for: x/ccv/provider/types/keys.go

@@ -674,9 +674,14 @@ func ConsumerIdToPowerShapingParametersKey(consumerId string) []byte {
 	return StringIdWithLenKey(mustGetKeyPrefix(ConsumerIdToPowerShapingParameters), consumerId)
 }
 
+// ConsumerIdToPhaseKeyPrefix returns the key prefix used to iterate over all the consumer ids and their phases.
+func ConsumerIdToPhaseKeyPrefix() byte {
+	return mustGetKeyPrefix(ConsumerIdToPhaseKeyName)
+}
+
 // ConsumerIdToPhaseKey returns the key used to store the phase that corresponds to this consumer id
 func ConsumerIdToPhaseKey(consumerId string) []byte {
-	return StringIdWithLenKey(mustGetKeyPrefix(ConsumerIdToPhaseKeyName), consumerId)
+	return StringIdWithLenKey(ConsumerIdToPhaseKeyPrefix(), consumerId)
 }
 
 // ConsumerIdToRemovalTimeKeyPrefix returns the key prefix for storing the removal times of consumer chains
@@ -801,8 +806,8 @@ func StringIdWithLenKey(prefix byte, stringId string) []byte {
 func ParseStringIdWithLenKey(prefix byte, bz []byte) (string, error) {
 	expectedPrefix := []byte{prefix}
 	prefixL := len(expectedPrefix)
-	if prefix := bz[:prefixL]; !bytes.Equal(prefix, expectedPrefix) {
-		return "", fmt.Errorf("invalid prefix; expected: %X, got: %X", expectedPrefix, prefix)
+	if prefixBz := bz[:prefixL]; !bytes.Equal(prefixBz, expectedPrefix) {
+		return "", fmt.Errorf("invalid prefix; expected: %X, got: %X, input %X -- len %d", expectedPrefix, prefixBz, prefix, prefixL)
 	}
 	stringIdL := sdk.BigEndianToUint64(bz[prefixL : prefixL+8])
 	stringId := string(bz[prefixL+8 : prefixL+8+int(stringIdL)])