feat!: provider proposal for changing reward denoms (#1280)

* new provider prop type * add methods and tests for new prop, update docs * remove old tx, fix tests * e2e handling * fix command type * boilerplate * fix e2e tests * Update CHANGELOG.md * lint * validate denoms * Update proposal.go * rm msg string * fix tests * rm chain in change denom action * lint * test for invalid denom * events for both add and remove * Update proposal_test.go (cherry picked from commit 48a2186) # Conflicts: # CHANGELOG.md # app/provider/app.go # proto/interchain_security/ccv/provider/v1/provider.proto # proto/interchain_security/ccv/provider/v1/tx.proto # tests/e2e/actions.go # tests/integration/distribution.go # x/ccv/provider/client/cli/tx.go # x/ccv/provider/client/proposal_handler.go # x/ccv/provider/keeper/distribution.go # x/ccv/provider/keeper/distribution_test.go # x/ccv/provider/proposal_handler_test.go # x/ccv/provider/types/codec.go # x/ccv/provider/types/proposal.go # x/ccv/provider/types/provider.pb.go # x/ccv/provider/types/tx.pb.go
cosmos · Sep 12, 2023 · 35b78f0 · 35b78f0
1 parent 6b0a94a
commit 35b78f0
Show file tree

Hide file tree

Showing 25 changed files with 1,133 additions and 470 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,17 @@
 
 Add an entry to the unreleased section whenever merging a PR to main that is not targeted at a specific release. These entries will eventually be included in a release.
 
+<<<<<<< HEAD
 ## v2.0.0-lsm
+=======
+* (feature!) [#1280](https://github.com/cosmos/interchain-security/pull/1280) provider proposal for changing reward denoms
+* (feature!) [#1244](https://github.com/cosmos/interchain-security/pull/1244) Update the default consumer unbonding period to 2 weeks.
+* (deps) [#1259](https://github.com/cosmos/interchain-security/pull/1259) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.47.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.5).
+* (deps!) [#1258](https://github.com/cosmos/interchain-security/pull/1258) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v7.3.0](https://github.com/cosmos/ibc-go/releases/tag/v7.3.0).
+* (deps) [#1258](https://github.com/cosmos/interchain-security/pull/1258) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.47.4](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.4).
+* (deps!) [#1196](https://github.com/cosmos/interchain-security/pull/1196) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v7.2.0](https://github.com/cosmos/ibc-go/releases/tag/v7.2.0).
+* `[x/ccv/provider]` (fix) [#1076](https://github.com/cosmos/interchain-security/pull/1076) Add `InitTimeoutTimestamps` and `ExportedVscSendTimestamps` to exported genesis.
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
 
 Date: August 18th, 2023
 

diff --git a/app/provider/app.go b/app/provider/app.go
@@ -131,6 +131,7 @@ var (
 		mint.AppModuleBasic{},
 		distr.AppModuleBasic{},
 		gov.NewAppModuleBasic(
+<<<<<<< HEAD
 			paramsclient.ProposalHandler,
 			distrclient.ProposalHandler,
 			upgradeclient.ProposalHandler,
@@ -140,6 +141,19 @@ var (
 			ibcproviderclient.ConsumerAdditionProposalHandler,
 			ibcproviderclient.ConsumerRemovalProposalHandler,
 			ibcproviderclient.EquivocationProposalHandler,
+=======
+			[]govclient.ProposalHandler{
+				paramsclient.ProposalHandler,
+				upgradeclient.LegacyProposalHandler,
+				upgradeclient.LegacyCancelProposalHandler,
+				ibcclientclient.UpdateClientProposalHandler,
+				ibcclientclient.UpgradeProposalHandler,
+				ibcproviderclient.ConsumerAdditionProposalHandler,
+				ibcproviderclient.ConsumerRemovalProposalHandler,
+				ibcproviderclient.EquivocationProposalHandler,
+				ibcproviderclient.ChangeRewardDenomsProposalHandler,
+			},
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
 		),
 		params.AppModuleBasic{},
 		crisis.AppModuleBasic{},

diff --git a/docs/docs/features/proposals.md b/docs/docs/features/proposals.md
@@ -105,6 +105,23 @@ Minimal example:
 }
 ```
 
+## ChangeRewardDenomProposal
+:::tip
+`ChangeRewardDenomProposal` will only be accepted on the provider chain if at least one of the denomsToAdd or denomsToRemove fields is populated with at least one denom. Also, a denom cannot be repeated in both sets.
+:::
+
+Proposal type used to mutate the set of denoms accepted by the provider as rewards.
+
+Minimal example:
+```js
+{
+  "title": "Add untrn as a reward denom",
+  "description": "Here is more information about the proposal",
+  "denomsToAdd": ["untrn"],
+  "denomsToRemove": []
+}
+```
+
 ### Notes
 When submitting equivocation evidence through an `EquivocationProposal` please take note that you need to use the consensus address (`valcons`) of the offending validator on the **provider chain**.
 Besides that, the `height` and the `time` fields should be mapped to the **provider chain** to avoid your evidence being rejected.

diff --git a/proto/interchain_security/ccv/provider/v1/provider.proto b/proto/interchain_security/ccv/provider/v1/provider.proto
@@ -95,8 +95,27 @@ message EquivocationProposal {
   repeated cosmos.evidence.v1beta1.Equivocation equivocations = 3;
 }
 
+<<<<<<< HEAD
 // A persisted queue entry indicating that a slash packet data instance needs to be handled.
 // This type belongs in the "global" queue, to coordinate slash packet handling times between consumers.
+=======
+// ChangeRewardDenomsProposal is a governance proposal on the provider chain to
+// mutate the set of denoms accepted by the provider as rewards.
+message ChangeRewardDenomsProposal {
+  // the title of the proposal
+  string title = 1;
+  // the description of the proposal
+  string description = 2;
+  // the list of consumer reward denoms to add
+  repeated string denoms_to_add = 3;
+  // the list of consumer reward denoms to remove
+  repeated string denoms_to_remove = 4;
+}
+
+// A persisted queue entry indicating that a slash packet data instance needs to
+// be handled. This type belongs in the "global" queue, to coordinate slash
+// packet handling times between consumers.
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
 message GlobalSlashEntry {
   // Block time that slash packet was received by provider chain.
   // This field is used for store key iteration ordering.

diff --git a/proto/interchain_security/ccv/provider/v1/tx.proto b/proto/interchain_security/ccv/provider/v1/tx.proto
@@ -10,8 +10,13 @@ import "google/protobuf/any.proto";
 
 // Msg defines the Msg service.
 service Msg {
+<<<<<<< HEAD
   rpc AssignConsumerKey(MsgAssignConsumerKey) returns (MsgAssignConsumerKeyResponse);
   rpc RegisterConsumerRewardDenom(MsgRegisterConsumerRewardDenom) returns (MsgRegisterConsumerRewardDenomResponse);
+=======
+  rpc AssignConsumerKey(MsgAssignConsumerKey)
+      returns (MsgAssignConsumerKeyResponse);
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
 }
 
 message MsgAssignConsumerKey {
@@ -29,6 +34,7 @@ message MsgAssignConsumerKey {
 }
 
 message MsgAssignConsumerKeyResponse {}
+<<<<<<< HEAD
 
 // MsgRegisterConsumerRewardDenom allows an account to register 
 // a consumer reward denom, i.e., add it to the list of denoms 
@@ -42,4 +48,6 @@ message MsgRegisterConsumerRewardDenom {
 }
 
 // MsgRegisterConsumerRewardDenomResponse defines the Msg/RegisterConsumerRewardDenom response type.
-message MsgRegisterConsumerRewardDenomResponse {}
+message MsgRegisterConsumerRewardDenomResponse {}
+=======
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
diff --git a/tests/e2e/actions.go b/tests/e2e/actions.go
@@ -1675,17 +1675,16 @@ func (tr TestRun) registerRepresentative(
 	wg.Wait()
 }
 
-type registerConsumerRewardDenomAction struct {
-	chain chainID
-	from  validatorID
-	denom string
+type submitChangeRewardDenomsProposalAction struct {
+	denom   string
+	deposit uint
+	from    validatorID
 }
 
-func (tr TestRun) registerConsumerRewardDenom(action registerConsumerRewardDenomAction, verbose bool) {
-	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
-	bz, err := exec.Command("docker", "exec", tr.containerConfig.instanceName, tr.chainConfigs[action.chain].binaryName,
-		"tx", "provider", "register-consumer-reward-denom", action.denom,
+func (tr TestRun) submitChangeRewardDenomsProposal(action submitChangeRewardDenomsProposalAction, verbose bool) {
+	providerChain := tr.chainConfigs[chainID("provi")]
 
+<<<<<<< HEAD
 		`--from`, `validator`+fmt.Sprint(action.from),
 		`--chain-id`, string(action.chain),
 		`--home`, tr.getValidatorHome(action.chain, action.from),
@@ -1698,11 +1697,59 @@ func (tr TestRun) registerConsumerRewardDenom(action registerConsumerRewardDenom
 
 	if verbose {
 		fmt.Println("redelegate cmd:", string(bz))
+=======
+	prop := client.ChangeRewardDenomsProposalJSON{
+		Summary: "Change reward denoms",
+		ChangeRewardDenomsProposal: types.ChangeRewardDenomsProposal{
+			Title:          "Change reward denoms",
+			Description:    "Change reward denoms",
+			DenomsToAdd:    []string{action.denom},
+			DenomsToRemove: []string{"stake"},
+		},
+		Deposit: fmt.Sprint(action.deposit) + `stake`,
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
+	}
+
+	bz, err := json.Marshal(prop)
+	if err != nil {
+		log.Fatal(err)
 	}
 
+	jsonStr := string(bz)
+	if strings.Contains(jsonStr, "'") {
+		log.Fatal("prop json contains single quote")
+	}
+
+	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
+	bz, err = exec.Command("docker", "exec", tr.containerConfig.instanceName,
+		"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/change-reward-denoms-proposal.json")).CombinedOutput()
+
 	if err != nil {
 		log.Fatal(err, "\n", string(bz))
 	}
+<<<<<<< HEAD
+=======
+
+	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
+	// CHANGE REWARDS DENOM PROPOSAL
+	bz, err = exec.Command("docker", "exec", tr.containerConfig.instanceName, providerChain.binaryName,
+		"tx", "gov", "submit-legacy-proposal", "change-reward-denoms", "/change-reward-denoms-proposal.json",
+		`--from`, `validator`+fmt.Sprint(action.from),
+		`--chain-id`, string(providerChain.chainId),
+		`--home`, tr.getValidatorHome(providerChain.chainId, action.from),
+		`--node`, tr.getValidatorNode(providerChain.chainId, action.from),
+		`--gas`, "9000000",
+		`--keyring-backend`, `test`,
+		`-y`,
+	).CombinedOutput()
+
+	if err != nil {
+		log.Fatal(err, "\n", string(bz))
+	}
+
+	// wait for inclusion in a block -> '--broadcast-mode block' is deprecated
+	tr.waitBlocks(chainID("provi"), 2, 30*time.Second)
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
 }
 
 // Creates an additional node on selected chain

diff --git a/tests/e2e/main.go b/tests/e2e/main.go
@@ -173,8 +173,8 @@ func (tr *TestRun) runStep(step Step, verbose bool) {
 		tr.waitForSlashThrottleDequeue(action, verbose)
 	case startRelayerAction:
 		tr.startRelayer(action, verbose)
-	case registerConsumerRewardDenomAction:
-		tr.registerConsumerRewardDenom(action, verbose)
+	case submitChangeRewardDenomsProposalAction:
+		tr.submitChangeRewardDenomsProposal(action, verbose)
 	default:
 		log.Fatalf("unknown action in testRun %s: %#v", tr.name, action)
 	}

diff --git a/tests/e2e/steps_democracy.go b/tests/e2e/steps_democracy.go
@@ -132,19 +132,29 @@ func stepsDemocracy(consumerName string) []Step {
 			},
 		},
 		{
-			action: registerConsumerRewardDenomAction{
-				chain: chainID("provi"),
-				from:  validatorID("bob"),
-				denom: consumerRewardDenom,
+			action: submitChangeRewardDenomsProposalAction{
+				denom:   consumerRewardDenom,
+				deposit: 10000001,
+				from:    validatorID("bob"),
+			},
+			state: State{
+				chainID("provi"): ChainState{
+					// Denom not yet registered, gov prop needs to pass first
+					RegisteredConsumerRewardDenoms: &[]string{},
+				},
+			},
+		},
+		{
+			action: voteGovProposalAction{
+				chain:      chainID("provi"),
+				from:       []validatorID{validatorID("alice"), validatorID("bob"), validatorID("carol")},
+				vote:       []string{"yes", "yes", "yes"},
+				propNumber: 2,
 			},
 			state: State{
 				chainID("provi"): ChainState{
 					// Check that the denom is registered on provider chain
 					RegisteredConsumerRewardDenoms: &[]string{consumerRewardDenom},
-					ValBalances: &map[validatorID]uint{
-						// make sure that bob's account was debited
-						validatorID("bob"): 9490000000,
-					},
 				},
 			},
 		},

diff --git a/tests/e2e/steps_reward_denom.go b/tests/e2e/steps_reward_denom.go
@@ -130,19 +130,29 @@ func stepsRewardDenomConsumer(consumerName string) []Step {
 			},
 		},
 		{
-			action: registerConsumerRewardDenomAction{
-				chain: chainID("provi"),
-				from:  validatorID("bob"),
-				denom: "ibc/3C3D7B3BE4ECC85A0E5B52A3AEC3B7DFC2AA9CA47C37821E57020D6807043BE9",
+			action: submitChangeRewardDenomsProposalAction{
+				denom:   "ibc/3C3D7B3BE4ECC85A0E5B52A3AEC3B7DFC2AA9CA47C37821E57020D6807043BE9",
+				deposit: 10000001,
+				from:    validatorID("bob"),
+			},
+			state: State{
+				chainID("provi"): ChainState{
+					// Denom not yet registered, gov prop needs to pass first
+					RegisteredConsumerRewardDenoms: &[]string{},
+				},
+			},
+		},
+		{
+			action: voteGovProposalAction{
+				chain:      chainID("provi"),
+				from:       []validatorID{validatorID("alice"), validatorID("bob"), validatorID("carol")},
+				vote:       []string{"yes", "yes", "yes"},
+				propNumber: 2,
 			},
 			state: State{
 				chainID("provi"): ChainState{
 					// Check that the denom is registered on provider chain
 					RegisteredConsumerRewardDenoms: &[]string{"ibc/3C3D7B3BE4ECC85A0E5B52A3AEC3B7DFC2AA9CA47C37821E57020D6807043BE9"},
-					ValBalances: &map[validatorID]uint{
-						// make sure that bob's account was debited
-						validatorID("bob"): 9490000000,
-					},
 				},
 			},
 		},

diff --git a/tests/integration/distribution.go b/tests/integration/distribution.go
@@ -5,11 +5,20 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+<<<<<<< HEAD
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 	transfertypes "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types"
 	consumertypes "github.com/cosmos/interchain-security/v2/x/ccv/consumer/types"
 	providertypes "github.com/cosmos/interchain-security/v2/x/ccv/provider/types"
 	ccv "github.com/cosmos/interchain-security/v2/x/ccv/types"
+=======
+
+	icstestingutils "github.com/cosmos/interchain-security/v3/testutil/integration"
+	consumerkeeper "github.com/cosmos/interchain-security/v3/x/ccv/consumer/keeper"
+	consumertypes "github.com/cosmos/interchain-security/v3/x/ccv/consumer/types"
+	providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
+	ccv "github.com/cosmos/interchain-security/v3/x/ccv/types"
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
 )
 
 // This test is valid for minimal viable consumer chain
@@ -92,24 +101,12 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	// Check that the coins got into the ConsumerRewardsPool
 	s.Require().True(rewardCoins[ibcCoinIndex].Amount.Equal(providerExpectedRewards[0].Amount))
 
-	// Attempt to register the consumer reward denom, but fail because the account has no coins
-
-	// Get the balance of delAddr to send it out
-	senderCoins := providerBankKeeper.GetAllBalances(s.providerCtx(), delAddr)
-
-	// Send the coins to the governance module just to have a place to send them
-	err = providerBankKeeper.SendCoinsFromAccountToModule(s.providerCtx(), delAddr, govtypes.ModuleName, senderCoins)
-	s.Require().NoError(err)
-
-	// Attempt to register the consumer reward denom, but fail because the account has no coins
-	err = s.providerApp.GetProviderKeeper().RegisterConsumerRewardDenom(s.providerCtx(), rewardCoins[ibcCoinIndex].Denom, delAddr)
-	s.Require().Error(err)
-
 	// Advance a block and check that the coins are still in the ConsumerRewardsPool
 	s.providerChain.NextBlock()
 	rewardCoins = providerBankKeeper.GetAllBalances(s.providerCtx(), rewardPool)
 	s.Require().True(rewardCoins[ibcCoinIndex].Amount.Equal(providerExpectedRewards[0].Amount))
 
+<<<<<<< HEAD
 	// Successfully register the consumer reward denom this time
 
 	// Send the coins back to the delAddr
@@ -127,6 +124,10 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	senderCoins2 := providerBankKeeper.GetAllBalances(s.providerCtx(), delAddr)
 	consumerRewardDenomRegistrationFee := s.providerApp.GetProviderKeeper().GetConsumerRewardDenomRegistrationFee(s.providerCtx())
 	s.Require().Equal(senderCoins1.Sub(senderCoins2), sdk.NewCoins(consumerRewardDenomRegistrationFee))
+=======
+	// Set the consumer reward denom. This would be done by a governance proposal in prod
+	s.providerApp.GetProviderKeeper().SetConsumerRewardDenom(s.providerCtx(), rewardCoins[ibcCoinIndex].Denom)
+>>>>>>> 48a2186 (feat!: provider proposal for changing reward denoms (#1280))
 
 	s.providerChain.NextBlock()