Skip to content
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

fix: test and fix key assignment branch #1431

Merged
merged 7 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sainoe could you please change this to state-breaking?

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Change the states by adding a consumer key for each chain that is
not yet registered meaning for which the gov proposal has not passed.
([\#1339](https://github.com/cosmos/interchain-security/pull/1339))
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Update how consumer-assigned keys are checked when a validator is
created on the provider. ([\#1339](https://github.com/cosmos/interchain-
security/pull/1339))
34 changes: 19 additions & 15 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ type App struct { // nolint: golint
// different fee-pool from the consumer chain ConsumerKeeper
DistrKeeper distrkeeper.Keeper

GovKeeper govkeeper.Keeper
GovKeeper *govkeeper.Keeper // Gov Keeper must be a pointer in the app, so we can SetRouter on it correctly
CrisisKeeper crisiskeeper.Keeper
UpgradeKeeper upgradekeeper.Keeper
ParamsKeeper paramskeeper.Keeper
Expand Down Expand Up @@ -412,6 +412,17 @@ func New(
app.SlashingKeeper,
)

app.GovKeeper = govkeeper.NewKeeper(
appCodec,
keys[govtypes.StoreKey],
app.AccountKeeper,
app.BankKeeper,
app.StakingKeeper,
app.MsgServiceRouter(),
govtypes.DefaultConfig(),
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

app.ProviderKeeper = ibcproviderkeeper.NewKeeper(
appCodec,
keys[providertypes.StoreKey],
Expand All @@ -427,6 +438,7 @@ func New(
app.EvidenceKeeper,
app.DistrKeeper,
app.BankKeeper,
app.GovKeeper,
authtypes.FeeCollectorName,
)

Expand All @@ -441,22 +453,14 @@ func New(
AddRoute(ibchost.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)).
AddRoute(providertypes.RouterKey, ibcprovider.NewProviderProposalHandler(app.ProviderKeeper)).
AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
govConfig := govtypes.DefaultConfig()

app.GovKeeper = *govkeeper.NewKeeper(
appCodec,
keys[govtypes.StoreKey],
app.AccountKeeper,
app.BankKeeper,
app.StakingKeeper,
app.MsgServiceRouter(),
govConfig,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

// Set legacy router for backwards compatibility with gov v1beta1
app.GovKeeper.SetLegacyRouter(govRouter)

app.GovKeeper = app.GovKeeper.SetHooks(
govtypes.NewMultiGovHooks(app.ProviderKeeper.Hooks()),
)

app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
keys[ibctransfertypes.StoreKey],
Expand Down Expand Up @@ -493,7 +497,7 @@ func New(
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)),
capability.NewAppModule(appCodec, *app.CapabilityKeeper, false),
crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants, app.GetSubspace(crisistypes.ModuleName)),
gov.NewAppModule(appCodec, &app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)),
gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)),
mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, nil, app.GetSubspace(minttypes.ModuleName)),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(slashingtypes.ModuleName)),
distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(distrtypes.ModuleName)),
Expand Down Expand Up @@ -590,7 +594,7 @@ func New(
auth.NewAppModule(appCodec, app.AccountKeeper, authsims.RandomGenesisAccounts, app.GetSubspace(authtypes.ModuleName)),
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)),
capability.NewAppModule(appCodec, *app.CapabilityKeeper, false),
gov.NewAppModule(appCodec, &app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)),
gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)),
mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, nil, app.GetSubspace(minttypes.ModuleName)),
staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName)),
distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(distrtypes.ModuleName)),
Expand Down
21 changes: 21 additions & 0 deletions proto/interchain_security/ccv/provider/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ service Query {
option (google.api.http).get =
"/interchain_security/ccv/provider/registered_consumer_reward_denoms";
}

// QueryProposedConsumerChainIDs returns the chain IDs of the proposed consumer chain addition proposals
Copy link
Contributor Author

@sainoe sainoe Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// that are still in the voting period
rpc QueryProposedConsumerChainIDs(
QueryProposedChainIDsRequest)
returns (QueryProposedChainIDsResponse) {
option (google.api.http).get =
"/interchain_security/ccv/provider/proposed_consumer_chains";
}
}

message QueryConsumerGenesisRequest { string chain_id = 1; }
Expand Down Expand Up @@ -150,3 +159,15 @@ message QueryRegisteredConsumerRewardDenomsRequest {}
message QueryRegisteredConsumerRewardDenomsResponse {
repeated string denoms = 1;
}

message QueryProposedChainIDsRequest {}

message QueryProposedChainIDsResponse {
repeated ProposedChain proposedChains = 1
[ (gogoproto.nullable) = false ];
}

message ProposedChain {
string chainID = 1;
uint64 proposalID = 2;
}
3 changes: 1 addition & 2 deletions tests/e2e/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,15 +419,14 @@ func (s *TestConfig) SetRelayerConfig(useRly bool) {
}

// validateStringLiterals enforces that configs follow the constraints
// necessary to to execute the tests
// necessary to execute the tests
//
// Note: Network interfaces (name of virtual ethernet interfaces for ip link)
// within the container will be named as "$CHAIN_ID-$VAL_ID-out" etc.
// where this name is constrained to 15 bytes or less. Therefore each string literal
// used as a validatorID or chainID needs to be 5 char or less.
func (s *TestConfig) validateStringLiterals() {
for valID, valConfig := range s.validatorConfigs {

if len(valID) > 5 {
panic("validator id string literal must be 5 char or less")
}
Expand Down
28 changes: 28 additions & 0 deletions tests/e2e/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type State map[ChainID]ChainState
type ChainState struct {
ValBalances *map[ValidatorID]uint
Proposals *map[uint]Proposal
ProposedConsumerChains *[]string
ValPowers *map[ValidatorID]uint
StakedTokens *map[ValidatorID]uint
Params *[]Param
Expand Down Expand Up @@ -131,6 +132,11 @@ func (tr TestConfig) getChainState(chain ChainID, modelState ChainState) ChainSt
chainState.Proposals = &proposals
}

if modelState.ProposedConsumerChains != nil {
proposedConsumerChains := tr.getProposedConsumerChains(chain)
chainState.ProposedConsumerChains = &proposedConsumerChains
}

if modelState.ValPowers != nil {
tr.waitBlocks(chain, 1, 10*time.Second)
powers := tr.getValPowers(chain, *modelState.ValPowers)
Expand Down Expand Up @@ -768,6 +774,28 @@ func (tr TestConfig) curlJsonRPCRequest(method, params, address string) {
executeCommandWithVerbosity(cmd, "curlJsonRPCRequest", verbosity)
}

func (tr TestConfig) getProposedConsumerChains(chain ChainID) []string {
tr.waitBlocks(chain, 1, 10*time.Second)
//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
bz, err := exec.Command("docker", "exec", tr.containerConfig.InstanceName, tr.chainConfigs[chain].BinaryName,
"query", "provider", "list-proposed-consumer-chains",
`--node`, tr.getQueryNode(chain),
`-o`, `json`,
).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

arr := gjson.Get(string(bz), "proposedChains").Array()
chains := []string{}
for _, c := range arr {
cid := c.Get("chainID").String()
chains = append(chains, cid)
}

return chains
}

func uintPtr(i uint) *uint {
return &i
}
2 changes: 2 additions & 0 deletions tests/e2e/steps_start_chains.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint
Status: "PROPOSAL_STATUS_VOTING_PERIOD",
},
},
ProposedConsumerChains: &[]string{consumerName},
},
},
},
Expand Down Expand Up @@ -163,6 +164,7 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint
ValidatorID("bob"): 9500000000,
ValidatorID("carol"): 9500000000,
},
ProposedConsumerChains: &[]string{},
},
ChainID(consumerName): ChainState{
ValBalances: &map[ValidatorID]uint{
Expand Down
57 changes: 51 additions & 6 deletions testutil/keeper/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions testutil/keeper/unit_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type MockedKeepers struct {
*MockIBCCoreKeeper
*MockEvidenceKeeper
*MockDistributionKeeper
*MockGovKeeper
}

// NewMockedKeepers instantiates a struct with pointers to properly instantiated mocked keepers.
Expand All @@ -107,6 +108,7 @@ func NewMockedKeepers(ctrl *gomock.Controller) MockedKeepers {
MockIBCCoreKeeper: NewMockIBCCoreKeeper(ctrl),
MockEvidenceKeeper: NewMockEvidenceKeeper(ctrl),
MockDistributionKeeper: NewMockDistributionKeeper(ctrl),
MockGovKeeper: NewMockGovKeeper(ctrl),
}
}

Expand All @@ -127,6 +129,7 @@ func NewInMemProviderKeeper(params InMemKeeperParams, mocks MockedKeepers) provi
mocks.MockEvidenceKeeper,
mocks.MockDistributionKeeper,
mocks.MockBankKeeper,
mocks.MockGovKeeper,
authtypes.FeeCollectorName,
)
}
Expand Down
28 changes: 28 additions & 0 deletions x/ccv/provider/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func NewQueryCmd() *cobra.Command {
cmd.AddCommand(CmdProviderValidatorKey())
cmd.AddCommand(CmdThrottleState())
cmd.AddCommand(CmdRegisteredConsumerRewardDenoms())
cmd.AddCommand(CmdProposedConsumerChains())

return cmd
}
Expand Down Expand Up @@ -92,6 +93,33 @@ func CmdConsumerChains() *cobra.Command {
return cmd
}

func CmdProposedConsumerChains() *cobra.Command {
cmd := &cobra.Command{
Use: "list-proposed-consumer-chains",
Short: "Query chainIDs in consumer addition proposal before voting finishes",
Args: cobra.ExactArgs(0),
RunE: func(cmd *cobra.Command, args []string) (err error) {
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
queryClient := types.NewQueryClient(clientCtx)

req := &types.QueryProposedChainIDsRequest{}
res, err := queryClient.QueryProposedConsumerChainIDs(cmd.Context(), req)
if err != nil {
return err
}

return clientCtx.PrintProto(res)
},
}

flags.AddQueryFlagsToCmd(cmd)

return cmd
}

func CmdConsumerStartProposals() *cobra.Command {
cmd := &cobra.Command{
Use: "list-start-proposals",
Expand Down
14 changes: 14 additions & 0 deletions x/ccv/provider/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,17 @@ func (k Keeper) QueryRegisteredConsumerRewardDenoms(goCtx context.Context, req *
Denoms: denoms,
}, nil
}

func (k Keeper) QueryProposedConsumerChainIDs(goCtx context.Context, req *types.QueryProposedChainIDsRequest) (*types.QueryProposedChainIDsResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}

ctx := sdk.UnwrapSDKContext(goCtx)

chains := k.GetAllProposedConsumerChainIDs(ctx)

return &types.QueryProposedChainIDsResponse{
ProposedChains: chains,
}, nil
}
Loading