-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: extend consumer chains query #2172
Changes from all commits
05fd5f3
6a1df54
8a1e849
4d72cf4
d13ebff
b9e2600
0dbfd70
ddae30a
7133fb0
a0794fe
ef4a788
3b4feba
f2e9868
3e13821
bfd8070
0e391fb
585bbfa
f4c2070
3d95aea
c218d6f
2c8b701
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,6 +2,7 @@ package cli | |||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||
"fmt" | ||||||||||||||||||||||||||||||
"strconv" | ||||||||||||||||||||||||||||||
"strings" | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
"github.com/spf13/cobra" | ||||||||||||||||||||||||||||||
|
@@ -75,9 +76,12 @@ func CmdConsumerGenesis() *cobra.Command { | |||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
func CmdConsumerChains() *cobra.Command { | ||||||||||||||||||||||||||||||
cmd := &cobra.Command{ | ||||||||||||||||||||||||||||||
Use: "list-consumer-chains", | ||||||||||||||||||||||||||||||
Short: "Query active consumer chains for provider chain.", | ||||||||||||||||||||||||||||||
Args: cobra.ExactArgs(0), | ||||||||||||||||||||||||||||||
Use: "list-consumer-chains [phase] [limit]", | ||||||||||||||||||||||||||||||
Short: "Query consumer chains for provider chain.", | ||||||||||||||||||||||||||||||
Long: `Query consumer chains for provider chain. An optional | ||||||||||||||||||||||||||||||
integer parameter can be passed for phase filtering of consumer chains, | ||||||||||||||||||||||||||||||
(Registered=1|Initialized=2|Launched=3|Stopped=4).`, | ||||||||||||||||||||||||||||||
Args: cobra.MaximumNArgs(2), | ||||||||||||||||||||||||||||||
RunE: func(cmd *cobra.Command, args []string) (err error) { | ||||||||||||||||||||||||||||||
clientCtx, err := client.GetClientQueryContext(cmd) | ||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||
|
@@ -86,6 +90,23 @@ func CmdConsumerChains() *cobra.Command { | |||||||||||||||||||||||||||||
queryClient := types.NewQueryClient(clientCtx) | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
req := &types.QueryConsumerChainsRequest{} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if args[0] != "" { | ||||||||||||||||||||||||||||||
phase, err := strconv.ParseInt(args[0], 10, 32) | ||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
req.Phase = types.ConsumerPhase(phase) | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
Comment on lines
+94
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix potential index out of range error. Accessing - if args[0] != "" {
+ if len(args) > 0 && args[0] != "" { Apply this diff to fix the issue. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if args[1] != "" { | ||||||||||||||||||||||||||||||
limit, err := strconv.ParseInt(args[1], 10, 32) | ||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
req.Limit = int32(limit) | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
Comment on lines
+102
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix potential index out of range error. Accessing - if args[1] != "" {
+ if len(args) > 1 && args[1] != "" { Apply this diff to fix the issue. Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
res, err := queryClient.QueryConsumerChains(cmd.Context(), req) | ||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,9 @@ import ( | |
"bytes" | ||
"fmt" | ||
"sort" | ||
"strconv" | ||
"testing" | ||
"time" | ||
|
||
"github.com/golang/mock/gomock" | ||
"github.com/stretchr/testify/require" | ||
|
@@ -18,6 +20,7 @@ import ( | |
|
||
cryptotestutil "github.com/cosmos/interchain-security/v5/testutil/crypto" | ||
testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" | ||
"github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" | ||
"github.com/cosmos/interchain-security/v5/x/ccv/provider/types" | ||
ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" | ||
) | ||
|
@@ -427,6 +430,8 @@ func TestGetConsumerChain(t *testing.T) { | |
math.NewInt(300), | ||
} | ||
|
||
metadataLists := []types.ConsumerMetadata{} | ||
|
||
expectedGetAllOrder := []types.Chain{} | ||
for i, consumerID := range consumerIDs { | ||
pk.SetConsumerChainId(ctx, consumerID, chainIDs[i]) | ||
|
@@ -437,6 +442,8 @@ func TestGetConsumerChain(t *testing.T) { | |
Top_N: topN, | ||
ValidatorSetCap: validatorSetCaps[i], | ||
ValidatorsPowerCap: validatorPowerCaps[i], | ||
MinStake: minStakes[i].Uint64(), | ||
AllowInactiveVals: allowInactiveVals[i], | ||
}) | ||
pk.SetMinimumPowerInTopN(ctx, consumerID, expectedMinPowerInTopNs[i]) | ||
for _, addr := range allowlists[i] { | ||
|
@@ -455,6 +462,12 @@ func TestGetConsumerChain(t *testing.T) { | |
strDenylist[j] = addr.String() | ||
} | ||
|
||
metadataLists = append(metadataLists, types.ConsumerMetadata{Name: chainIDs[i]}) | ||
pk.SetConsumerMetadata(ctx, consumerID, metadataLists[i]) | ||
|
||
phase := types.ConsumerPhase(int32(i + 1)) | ||
pk.SetConsumerPhase(ctx, consumerID, phase) | ||
|
||
expectedGetAllOrder = append(expectedGetAllOrder, | ||
types.Chain{ | ||
ChainId: chainIDs[i], | ||
|
@@ -465,13 +478,15 @@ func TestGetConsumerChain(t *testing.T) { | |
ValidatorsPowerCap: validatorPowerCaps[i], | ||
Allowlist: strAllowlist, | ||
Denylist: strDenylist, | ||
Phase: phase, | ||
Metadata: metadataLists[i], | ||
AllowInactiveVals: allowInactiveVals[i], | ||
MinStake: minStakes[i].Uint64(), | ||
}) | ||
} | ||
|
||
for i, chainID := range pk.GetAllActiveConsumerIds(ctx) { | ||
c, err := pk.GetConsumerChain(ctx, chainID) | ||
for i, cId := range consumerIDs { | ||
c, err := pk.GetConsumerChain(ctx, cId) | ||
require.NoError(t, err) | ||
require.Equal(t, expectedGetAllOrder[i], c) | ||
} | ||
|
@@ -562,3 +577,115 @@ func TestQueryConsumerIdFromClientId(t *testing.T) { | |
require.NoError(t, err) | ||
require.Equal(t, expectedConsumerId, res.ConsumerId) | ||
} | ||
|
||
func TestQueryConsumerChains(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to Consider renaming the function to |
||
pk, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) | ||
defer ctrl.Finish() | ||
|
||
consumerNum := 4 | ||
consumerIds := make([]string, consumerNum) | ||
consumers := make([]*types.Chain, consumerNum) | ||
|
||
// expect no error and returned chains | ||
res, err := pk.QueryConsumerChains(ctx, &types.QueryConsumerChainsRequest{}) | ||
require.NoError(t, err) | ||
require.Len(t, res.Chains, 0) | ||
|
||
// create four consumer chains in different phase | ||
for i := 0; i < consumerNum; i++ { | ||
cID := pk.FetchAndIncrementConsumerId(ctx) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarify the reason for skipping the first consumer ID. The reason for skipping the first consumer ID is not clear. Consider adding a comment to explain this logic. |
||
chainID := "consumer-" + strconv.Itoa(i) | ||
c := types.Chain{ | ||
ChainId: chainID, | ||
MinPowerInTop_N: -1, | ||
ValidatorsPowerCap: 0, | ||
ValidatorSetCap: 0, | ||
Allowlist: []string{}, | ||
Denylist: []string{}, | ||
Phase: types.ConsumerPhase(i + 1), | ||
Metadata: types.ConsumerMetadata{Name: chainID}, | ||
} | ||
pk.SetConsumerPhase(ctx, cID, c.Phase) | ||
pk.SetConsumerMetadata(ctx, cID, c.Metadata) | ||
pk.SetConsumerChainId(ctx, cID, chainID) | ||
|
||
consumerIds[i] = cID | ||
consumers[i] = &c | ||
} | ||
|
||
testCases := []struct { | ||
name string | ||
setup func(ctx sdk.Context, pk keeper.Keeper) | ||
phase_filter types.ConsumerPhase | ||
limit int32 | ||
expConsumers []*types.Chain | ||
}{ | ||
{ | ||
name: "expect all consumers when phase filter isn't set", | ||
setup: func(ctx sdk.Context, pk keeper.Keeper) {}, | ||
expConsumers: consumers, | ||
}, | ||
{ | ||
name: "expect an amount of consumer equal to the limit", | ||
setup: func(ctx sdk.Context, pk keeper.Keeper) {}, | ||
expConsumers: consumers[:3], | ||
limit: int32(3), | ||
}, | ||
{ | ||
name: "expect registered consumers when phase filter is set to Registered", | ||
setup: func(ctx sdk.Context, pk keeper.Keeper) { | ||
consumers[0].Phase = types.ConsumerPhase_CONSUMER_PHASE_REGISTERED | ||
pk.SetConsumerPhase(ctx, consumerIds[0], types.ConsumerPhase_CONSUMER_PHASE_REGISTERED) | ||
}, | ||
phase_filter: types.ConsumerPhase_CONSUMER_PHASE_REGISTERED, | ||
expConsumers: consumers[0:1], | ||
}, | ||
{ | ||
name: "expect initialized consumers when phase is set to Initialized", | ||
setup: func(ctx sdk.Context, pk keeper.Keeper) { | ||
consumers[1].Phase = types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED | ||
err := pk.AppendConsumerToBeLaunchedOnSpawnTime(ctx, consumerIds[1], time.Now()) | ||
require.NoError(t, err) | ||
pk.SetConsumerPhase(ctx, consumerIds[1], types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED) | ||
}, | ||
phase_filter: types.ConsumerPhase_CONSUMER_PHASE_INITIALIZED, | ||
expConsumers: consumers[1:2], | ||
}, | ||
{ | ||
name: "expect launched consumers when phase is set to Launched", | ||
setup: func(ctx sdk.Context, pk keeper.Keeper) { | ||
consumers[2].Phase = types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED | ||
consumers[2].ClientId = "ClientID" | ||
pk.SetConsumerClientId(ctx, consumerIds[2], consumers[2].ClientId) | ||
pk.SetConsumerPhase(ctx, consumerIds[2], types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED) | ||
}, | ||
phase_filter: types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED, | ||
expConsumers: consumers[2:3], | ||
}, | ||
{ | ||
name: "expect stopped consumers when phase is set to Stopped", | ||
setup: func(ctx sdk.Context, pk keeper.Keeper) { | ||
consumers[3].Phase = types.ConsumerPhase_CONSUMER_PHASE_STOPPED | ||
pk.SetConsumerPhase(ctx, consumerIds[3], types.ConsumerPhase_CONSUMER_PHASE_STOPPED) | ||
}, | ||
phase_filter: types.ConsumerPhase_CONSUMER_PHASE_STOPPED, | ||
expConsumers: consumers[3:], | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
tc.setup(ctx, pk) | ||
req := types.QueryConsumerChainsRequest{ | ||
Phase: tc.phase_filter, | ||
Limit: tc.limit, | ||
} | ||
expectedResponse := types.QueryConsumerChainsResponse{ | ||
Chains: tc.expConsumers, | ||
} | ||
res, err := pk.QueryConsumerChains(ctx, &req) | ||
require.NoError(t, err) | ||
require.Equal(t, &expectedResponse, res) | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.