feat!: key assignment checking key in use (#1339)

* feat!: store proposed chainID before voting finishes (#1289) * feat: store chain in proposal * add govHook * delete GetChainsInProposal * check proposal type * update key * feat: add query proposed chainIDs * feat: set govhook * feat: parse key * refactor: names * feat: add list proposed consumer chains * test: add e2e test * add e2e test * update comments * update ProposeConsumerChains in e2e test * remove wait for block * docs: update changelog * fix: lint * add TestParseProposedConsumerChainKey * refactor gov hook * Update proto/interchain_security/ccv/provider/v1/query.proto Co-authored-by: MSalopek <[email protected]> * update proto * add test for set kv * refactor key to be prefix_proposalID * formatting * update e2e test * format * Update x/ccv/provider/keeper/gov_hook.go Co-authored-by: Shawn <[email protected]> * Update x/ccv/provider/keeper/keeper.go Co-authored-by: Shawn <[email protected]> * Update x/ccv/provider/keeper/keeper.go Co-authored-by: Shawn <[email protected]> * fix e2e test * fix gosec * remove type url check * test: add unit test * lint * fix lint * fix err --------- Co-authored-by: MSalopek <[email protected]> Co-authored-by: Shawn <[email protected]> * fix test * feat: update GetAllValidatorsByConsumerAddr for fast find consensus key in use (#1279) * update GetAllValidatorsByConsumerAddr * fix test * update ValidatorConsensusKeyInUse * update changelog * Update proto/interchain_security/ccv/provider/v1/query.proto * Update x/ccv/provider/keeper/gov_hook.go * Update x/ccv/provider/keeper/keeper.go * Update x/ccv/provider/keeper/keeper.go * Update x/ccv/provider/keeper/keeper.go * fix gov hooks * fix bug and add tests * finish unit testing of consu addition legacy prop getter * nit * update changelog * lint * update changelog entry * Update .changelog/unreleased/features/provider/1339-check-key-assignment-in-use.md Co-authored-by: Marius Poke <[email protected]> * fix #1282 * remove todos and update doc * tests: fix broken unit tests * tests: fix broken integration test consumer registration * tests: update names --------- Co-authored-by: MSalopek <[email protected]> Co-authored-by: Shawn <[email protected]> Co-authored-by: Simon Noetzlin <[email protected]> Co-authored-by: Marius Poke <[email protected]>
cosmos · Nov 28, 2023 · 11b5858 · 11b5858
1 parent 2a8a3d4
commit 11b5858
Show file tree

Hide file tree

Showing 27 changed files with 1,433 additions and 154 deletions.
diff --git a/.changelog/unreleased/features/provider/1339-check-key-assignment-in-use.md b/.changelog/unreleased/features/provider/1339-check-key-assignment-in-use.md
@@ -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))
diff --git a/.changelog/unreleased/state-breaking/provider/1339-check-key-assignment-in-use.md b/.changelog/unreleased/state-breaking/provider/1339-check-key-assignment-in-use.md
@@ -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))
diff --git a/app/provider/app.go b/app/provider/app.go
@@ -203,7 +203,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
@@ -403,6 +403,25 @@ func New(
 		scopedIBCKeeper,
 	)
 
+	// create evidence keeper with router
+	app.EvidenceKeeper = *evidencekeeper.NewKeeper(
+		appCodec,
+		keys[evidencetypes.StoreKey],
+		app.StakingKeeper,
+		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],
@@ -417,6 +436,7 @@ func New(
 		app.AccountKeeper,
 		app.DistrKeeper,
 		app.BankKeeper,
+		app.GovKeeper,
 		authtypes.FeeCollectorName,
 	)
 
@@ -431,22 +451,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],
@@ -493,7 +505,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)),
@@ -590,7 +602,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)),

diff --git a/go.sum b/go.sum
@@ -1056,6 +1056,7 @@ golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
 golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
 golang.org/x/mod v0.11.0 h1:bUO06HqtnRcc/7l71XBe4WcqTZ+3AH1J59zWDDwLKgU=
+golang.org/x/mod v0.11.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -1350,6 +1351,7 @@ golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.8/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
 golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM=
+golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

diff --git a/proto/interchain_security/ccv/provider/v1/query.proto b/proto/interchain_security/ccv/provider/v1/query.proto
@@ -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
+  // 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; }
@@ -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;
+}
diff --git a/tests/e2e/config.go b/tests/e2e/config.go
@@ -505,15 +505,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")
 		}

diff --git a/tests/e2e/state.go b/tests/e2e/state.go
@@ -20,6 +20,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
@@ -123,6 +124,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)
@@ -836,6 +842,28 @@ func (tc TestConfig) getTrustedHeight(
 	return clienttypes.Height{RevisionHeight: uint64(revHeight), RevisionNumber: uint64(revNumber)}
 }
 
+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
 }
diff --git a/tests/e2e/steps_start_chains.go b/tests/e2e/steps_start_chains.go
@@ -54,6 +54,7 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint
 							Status:        "PROPOSAL_STATUS_VOTING_PERIOD",
 						},
 					},
+					ProposedConsumerChains: &[]string{consumerName},
 				},
 			},
 		},
@@ -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{

diff --git a/tests/integration/setup.go b/tests/integration/setup.go
@@ -24,6 +24,7 @@ import (
 	testutil "github.com/cosmos/interchain-security/v3/testutil/integration"
 	"github.com/cosmos/interchain-security/v3/testutil/simibc"
 	consumertypes "github.com/cosmos/interchain-security/v3/x/ccv/consumer/types"
+	"github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
 	ccv "github.com/cosmos/interchain-security/v3/x/ccv/types"
 )
 
@@ -129,6 +130,9 @@ func (suite *CCVTestSuite) SetupTest() {
 	providerKeeper := suite.providerApp.GetProviderKeeper()
 
 	// re-assign all validator keys for the first consumer chain
+	providerKeeper.SetPendingConsumerAdditionProp(suite.providerCtx(), &types.ConsumerAdditionProposal{
+		ChainId: icstestingutils.FirstConsumerChainID,
+	})
 	preProposalKeyAssignment(suite, icstestingutils.FirstConsumerChainID)
 
 	// start consumer chains

diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go
diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go
@@ -88,6 +88,7 @@ type MockedKeepers struct {
 	*MockIBCTransferKeeper
 	*MockIBCCoreKeeper
 	*MockDistributionKeeper
+	*MockGovKeeper
 }
 
 // NewMockedKeepers instantiates a struct with pointers to properly instantiated mocked keepers.
@@ -105,6 +106,7 @@ func NewMockedKeepers(ctrl *gomock.Controller) MockedKeepers {
 		MockIBCTransferKeeper:  NewMockIBCTransferKeeper(ctrl),
 		MockIBCCoreKeeper:      NewMockIBCCoreKeeper(ctrl),
 		MockDistributionKeeper: NewMockDistributionKeeper(ctrl),
+		MockGovKeeper:          NewMockGovKeeper(ctrl),
 	}
 }
 
@@ -124,6 +126,7 @@ func NewInMemProviderKeeper(params InMemKeeperParams, mocks MockedKeepers) provi
 		mocks.MockAccountKeeper,
 		mocks.MockDistributionKeeper,
 		mocks.MockBankKeeper,
+		mocks.MockGovKeeper,
 		authtypes.FeeCollectorName,
 	)
 }

diff --git a/x/ccv/provider/client/cli/query.go b/x/ccv/provider/client/cli/query.go
@@ -32,6 +32,7 @@ func NewQueryCmd() *cobra.Command {
 	cmd.AddCommand(CmdProviderValidatorKey())
 	cmd.AddCommand(CmdThrottleState())
 	cmd.AddCommand(CmdRegisteredConsumerRewardDenoms())
+	cmd.AddCommand(CmdProposedConsumerChains())
 
 	return cmd
 }
@@ -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",