fix: backport #1949 + fix handler entry (#1960)

* feat!: added E2E test and docs for ConsumerModificationProposal (#1949) * added E2E test for the ConsumerModificationProposal * added docs * add to nightly tests * fix markdown links * Update docs/docs/features/proposals.md Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> --------- Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * fix e2e test 'ConsumerModificationProposal' * app: added missing ConsumerModificationProposalHandle * Apply suggestions from code review Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com> --------- Co-authored-by: insumity <karolos@informal.systems> Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
cosmos · Jun 13, 2024 · 1670012 · 1670012
1 parent 330cfdb
commit 1670012
Show file tree

Hide file tree

Showing 16 changed files with 753 additions and 23 deletions.
diff --git a/.github/workflows/nightly-e2e.yml b/.github/workflows/nightly-e2e.yml
@@ -277,6 +277,22 @@ jobs:
           go-version: "1.22" # The Go version to download (if necessary) and use.
       - name: E2E partial set security denylist
         run: go run ./tests/e2e/... --tc partial-set-security-validators-denylisted
+  partial-set-security-modification-proposal:
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+    steps:
+      - uses: actions/setup-go@v5
+        with:
+          go-version: "1.22"
+      - uses: actions/checkout@v4
+      - name: Checkout LFS objects
+        run: git lfs checkout
+      - name: Setup Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: "1.22" # The Go version to download (if necessary) and use.
+      - name: E2E partial set security modification proposal
+        run: go run ./tests/e2e/... --tc partial-set-security-modification-proposal
 
   nightly-test-fail:
     needs:
@@ -295,6 +311,7 @@ jobs:
       - partial-set-security-validators-power-cap-test
       - partial-set-security-validators-allowlisted-test
       - partial-set-security-validators-denylisted-test
+      - partial-set-security-modification-proposal
     if: ${{ failure() }}
     runs-on: ubuntu-latest
     steps:

diff --git a/app/provider/app.go b/app/provider/app.go
@@ -145,6 +145,7 @@ var (
 				paramsclient.ProposalHandler,
 				ibcproviderclient.ConsumerAdditionProposalHandler,
 				ibcproviderclient.ConsumerRemovalProposalHandler,
+				ibcproviderclient.ConsumerModificationProposalHandler,
 				ibcproviderclient.ChangeRewardDenomsProposalHandler,
 			},
 		),
@@ -572,6 +573,7 @@ func New(
 					paramsclient.ProposalHandler,
 					ibcproviderclient.ConsumerAdditionProposalHandler,
 					ibcproviderclient.ConsumerRemovalProposalHandler,
+					ibcproviderclient.ConsumerModificationProposalHandler,
 					ibcproviderclient.ChangeRewardDenomsProposalHandler,
 				},
 			),

diff --git a/docs/docs/features/partial-set-security.md b/docs/docs/features/partial-set-security.md
@@ -26,3 +26,8 @@ For Top N chains, this is also the long term vision for how they are launched.
 
 For Opt In chains, this is a temporary measure to prevent issues around chain ID squatting, i.e. someone could spuriously register many desirable chain IDs of upcoming consumer chain and simply deny legitimate consumer chains from using them. Eventually, the plan is to allow launching Opt In chains permissionlessly without going through governance, with quality control being handled by the market of validators deciding which chains they would like to validate on.
 :::
+
+:::tip
+A running Top N consumer chain might want to become an Opt-In chain or vice versa. This can be achieved by issuing
+a [`ConsumerModificationProposal`](./proposals.md#consumermodificationproposal).
+:::
diff --git a/docs/docs/features/power-shaping.md b/docs/docs/features/power-shaping.md
@@ -59,4 +59,8 @@ an allowlist that is too short can very quickly become outdated and leave too fe
 the power distribution on the provider shifts and the denylisted validators gain more power.
 
 In general, when setting these parameters, consider that the voting power distribution in the future might be very different from the one right now,
-and that the chain should be secure even if the power distribution changes significantly.
+and that the chain should be secure even if the power distribution changes significantly.
+
+:::tip
+The power shaping parameters of a running consumer chain can be changed through a [`ConsumerModificationProposal`](./proposals.md#consumermodificationproposal).
+:::
diff --git a/docs/docs/features/proposals.md b/docs/docs/features/proposals.md
@@ -5,7 +5,7 @@ sidebar_position: 3
 
 # ICS Provider Proposals
 
-Interchain security module introduces 3 new proposal types to the provider.
+Interchain security module introduces new proposal types to the provider.
 
 The proposals are used to propose upcoming interchain security events through governance.
 
@@ -84,6 +84,38 @@ After the introduction of Partial Set Security, the use of the soft opt-out mech
 encouraged to use the topN parameter to not force validators with little stake to validate the chain.
 :::
 
+
+## `ConsumerModificationProposal`
+Proposal type used to change the power shaping parameters of a running consumer chain, as well as to change a Top N running
+consumer chain to an Opt-In chain and vice versa.
+
+When a `ConsumerModificationProposal` passes for a running consumer chain, the consumer chain would change all its
+parameters to the ones passed in the `ConsumerModificationProposal`.
+
+Assume, a `chain-1` is a Top N chain. If the following `ConsumerModificationProposal` passes, then `chain-1` would become
+an Opt-In chain with a 40% validators power cap, a maximum number of 30 validators, and one denylisted validator.
+```js
+{
+    "title": "Modify consumer chain",
+    "description": ".md description of your chain and all other relevant information",
+    "chain_id": "chain-1",
+    "top_N": 0,
+    "validators_power_cap": 40,
+    "validator_set_cap": 30,
+    "allowlist": [],
+    "denylist": ["cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq"]
+}
+```
+
+:::warning
+If `top_N`, `validators_power_cap`, or some other argument is not included in the proposal, then it is considered
+that the default value is set for this argument. For example, if a Top 50% chain wants to only modify `validators_power_cap`
+from 35 to 40, then the `ConsumerModificationProposal` would still need to include that `top_N` is 50. Otherwise
+`top_N` would be set to its default value of 0, and the chain would become an Opt-In chain.
+
+To be **safe**, always include `top_N` and all the power shaping parameters in your `ConsumerModificationProposal`.
+:::
+
 ## ChangeRewardDenomProposal
 
 Proposal type used to mutate the set of denoms accepted by the provider as rewards.

diff --git a/docs/docs/frequently-asked-questions.md b/docs/docs/frequently-asked-questions.md
@@ -129,3 +129,9 @@ Yes, the consumer chain will halt with an ERR CONSENSUS FAILURE error after the
 ## Can validators set a commission rate for chains they have not opted in to?
 Yes, and this is useful for validators that are not in the top N% of the provider chain, but might move into the top N% in the future.
 By setting the commission rate ahead of time, they can make sure that they immediately have a commission rate of their choosing as soon as they are in the top N%.
+
+## Can a consumer chain modify its power shaping parameters?
+Yes, by issuing a [`ConsumerModificationProposal`](./features/proposals.md#consumermodificationproposal).
+
+## Can a Top N consumer chain become Opt-In or vice versa? 
+Yes, by issuing a [`ConsumerModificationProposal`](./features/proposals.md#consumermodificationproposal).
diff --git a/docs/docs/validators/joining-testnet.md b/docs/docs/validators/joining-testnet.md
@@ -12,7 +12,7 @@ The experience gained in the testnet will prepare you for validating interchain
 :::tip
 Provider and consumer chain represent distinct networks and infrastructures operated by the same validator set.
 
-For general information about running cosmos-sdk based chains check out the [validator basics](https://hub.cosmos.network/validators/validator-setup) and [Running a Node section](https://docs.cosmos.network/main/run-node/run-node) of Cosmos SDK docs
+For general information about running cosmos-sdk based chains check out the [validator basics](https://hub.cosmos.network/main/validators/validator-setup) and [Running a Node section](https://docs.cosmos.network/main/run-node/run-node) of Cosmos SDK docs.
 :::
 
 ## Joining the provider chain
@@ -79,7 +79,7 @@ gaiad tx staking create-validator \
 ```
 
 :::tip
-Check this [guide](https://hub.cosmos.network/validators/validator-setup#edit-validator-description) to edit your validator.
+Check this [guide](https://hub.cosmos.network/main/validators/validator-setup#edit-validator-description) to edit your validator.
 :::
 
 After this step, your validator is created and you can start your node and catch up to the rest of the network. It is recommended that you use `statesync` to catch up to the rest of the network.

diff --git a/docs/docs/validators/overview.md b/docs/docs/validators/overview.md
@@ -85,7 +85,7 @@ At present, the consumer chain can report evidence about downtime infractions to
 :::info
 Causing a downtime infraction on any consumer chain will not incur a slash penalty. Instead, the offending validator will be jailed on the provider chain and consequently on all consumer chains.
 
-To unjail, the validator must wait for the jailing period to elapse on the provider chain and [submit an unjail transaction](https://hub.cosmos.network/validators/validator-setup#unjail-validator) on the provider chain. After unjailing on the provider, the validator will be unjailed on all consumer chains.
+To unjail, the validator must wait for the jailing period to elapse on the provider chain and [submit an unjail transaction](https://hub.cosmos.network/main/validators/validator-setup#unjail-validator) on the provider chain. After unjailing on the provider, the validator will be unjailed on all consumer chains.
 
 More information is available in [Downtime Slashing documentation](../features/slashing.md#downtime-infractions)
 :::
@@ -99,7 +99,7 @@ Validators can use different consensus keys on the provider and each of the cons
 For more information check out the [Key assignment overview and guide](../features/key-assignment.md)
 
 ## References:
-- [Cosmos Hub Validators FAQ](https://hub.cosmos.network/validators/validator-faq)
-- [Cosmos Hub Running a validator](https://hub.cosmos.network/validators/validator-setup)
+- [Cosmos Hub Validators FAQ](https://hub.cosmos.network/main/validators/validator-faq)
+- [Cosmos Hub Running a validator](https://hub.cosmos.network/main/validators/validator-setup)
 - [Startup Sequence](https://github.com/cosmos/testnets/blob/master/interchain-security/CONSUMER_LAUNCH_GUIDE.md#chain-launch)
-- [Submit Unjailing Transaction](https://hub.cosmos.network/validators/validator-setup#unjail-validator)
+- [Submit Unjailing Transaction](https://hub.cosmos.network/main/validators/validator-setup#unjail-validator)
diff --git a/tests/e2e/actions.go b/tests/e2e/actions.go
@@ -404,6 +404,83 @@ func (tr Chain) submitConsumerRemovalProposal(
 	tr.waitBlocks(ChainID("provi"), 2, 20*time.Second)
 }
 
+type SubmitConsumerModificationProposalAction struct {
+	Chain              ChainID
+	From               ValidatorID
+	Deposit            uint
+	ConsumerChain      ChainID
+	TopN               uint32
+	ValidatorsPowerCap uint32
+	ValidatorSetCap    uint32
+	Allowlist          []string
+	Denylist           []string
+}
+
+func (tr Chain) submitConsumerModificationProposal(
+	action SubmitConsumerModificationProposalAction,
+	verbose bool,
+) {
+	prop := client.ConsumerModificationProposalJSON{
+		Title:              "Propose the modification of the PSS parameters of a chain",
+		Summary:            "summary of a modification proposal",
+		ChainId:            string(tr.testConfig.chainConfigs[action.ConsumerChain].ChainId),
+		Deposit:            fmt.Sprint(action.Deposit) + `stake`,
+		TopN:               action.TopN,
+		ValidatorsPowerCap: action.ValidatorsPowerCap,
+		ValidatorSetCap:    action.ValidatorSetCap,
+		Allowlist:          action.Allowlist,
+		Denylist:           action.Denylist,
+	}
+
+	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 unsafe quoting warning (no production code)
+	bz, err = tr.target.ExecCommand(
+		"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/temp-proposal.json"),
+	).CombinedOutput()
+	if err != nil {
+		log.Fatal(err, "\n", string(bz))
+	}
+
+	// CONSUMER MODIFICATION PROPOSAL
+	cmd := tr.target.ExecCommand(
+		tr.testConfig.chainConfigs[action.Chain].BinaryName,
+		"tx", "gov", "submit-legacy-proposal", "consumer-modification", "/temp-proposal.json",
+		`--from`, `validator`+fmt.Sprint(action.From),
+		`--chain-id`, string(tr.testConfig.chainConfigs[action.Chain].ChainId),
+		`--home`, tr.getValidatorHome(action.Chain, action.From),
+		`--gas`, `900000`,
+		`--node`, tr.getValidatorNode(action.Chain, action.From),
+		`--keyring-backend`, `test`,
+		`-y`,
+	)
+	if verbose {
+		log.Println("submitConsumerModificationProposal cmd: ", cmd.String())
+		log.Println("submitConsumerModificationProposal json: ", jsonStr)
+	}
+
+	bz, err = cmd.CombinedOutput()
+
+	if err != nil {
+		log.Fatal(err, "\n", string(bz))
+	}
+
+	if verbose {
+		log.Println("submitConsumerModificationProposal output: ", string(bz))
+	}
+
+	// wait for inclusion in a block -> '--broadcast-mode block' is deprecated
+	tr.waitBlocks(ChainID("provi"), 2, 10*time.Second)
+}
+
 type SubmitEnableTransfersProposalAction struct {
 	Chain   ChainID
 	From    ValidatorID
@@ -415,7 +492,7 @@ func (tr Chain) submitEnableTransfersProposalAction(
 	action SubmitEnableTransfersProposalAction,
 	verbose bool,
 ) {
-	// gov signed addres got by checking the gov module acc address in the test container
+	// gov signed address got by checking the gov module acc address in the test container
 	// interchain-security-cdd q auth module-account gov --node tcp://7.7.9.253:26658
 	template := `
 	{

diff --git a/tests/e2e/main.go b/tests/e2e/main.go
@@ -192,6 +192,12 @@ var stepChoices = map[string]StepChoice{
 		description: "test partial set security for an Opt-In chain that has a validator denylisted",
 		testConfig:  DefaultTestCfg,
 	},
+	"partial-set-security-modification-proposal": {
+		name:        "partial-set-security-modification-proposal",
+		steps:       stepsModifyChain(),
+		description: "test partial set security parameters can be changed through a modification proposal",
+		testConfig:  DefaultTestCfg,
+	},
 }
 
 func getTestCaseUsageString() string {
@@ -280,6 +286,7 @@ func getTestCases(selectedPredefinedTests, selectedTestFiles TestSet, providerVe
 			"consumer-double-downtime", "partial-set-security-opt-in", "partial-set-security-top-n",
 			"partial-set-security-validator-set-cap", "partial-set-security-validators-power-cap",
 			"partial-set-security-validators-allowlisted", "partial-set-security-validators-denylisted",
+			"partial-set-security-modification-proposal",
 		}
 		if includeMultiConsumer != nil && *includeMultiConsumer {
 			selectedPredefinedTests = append(selectedPredefinedTests, "multiconsumer")

diff --git a/tests/e2e/state.go b/tests/e2e/state.go
@@ -19,18 +19,19 @@ import (
 
 // type aliases
 type (
-	ChainState                = e2e.ChainState
-	Proposal                  = e2e.Proposal
-	Rewards                   = e2e.Rewards
-	TextProposal              = e2e.TextProposal
-	UpgradeProposal           = e2e.UpgradeProposal
-	ConsumerAdditionProposal  = e2e.ConsumerAdditionProposal
-	ConsumerRemovalProposal   = e2e.ConsumerRemovalProposal
-	IBCTransferParams         = e2e.IBCTransferParams
-	IBCTransferParamsProposal = e2e.IBCTransferParamsProposal
-	Param                     = e2e.Param
-	ParamsProposal            = e2e.ParamsProposal
-	TargetDriver              = e2e.TargetDriver
+	ChainState                   = e2e.ChainState
+	Proposal                     = e2e.Proposal
+	Rewards                      = e2e.Rewards
+	TextProposal                 = e2e.TextProposal
+	UpgradeProposal              = e2e.UpgradeProposal
+	ConsumerAdditionProposal     = e2e.ConsumerAdditionProposal
+	ConsumerRemovalProposal      = e2e.ConsumerRemovalProposal
+	ConsumerModificationProposal = e2e.ConsumerModificationProposal
+	IBCTransferParams            = e2e.IBCTransferParams
+	IBCTransferParamsProposal    = e2e.IBCTransferParamsProposal
+	Param                        = e2e.Param
+	ParamsProposal               = e2e.ParamsProposal
+	TargetDriver                 = e2e.TargetDriver
 )
 
 type State map[ChainID]ChainState
@@ -41,7 +42,6 @@ type Chain struct {
 }
 
 func (tr Chain) GetChainState(chain ChainID, modelState ChainState) ChainState {
-
 	chainState := ChainState{}
 
 	if modelState.ValBalances != nil {
@@ -487,6 +487,31 @@ func (tr Commands) GetProposal(chain ChainID, proposal uint) Proposal {
 			Title:   title,
 			Params:  params,
 		}
+
+	case "/interchain_security.ccv.provider.v1.ConsumerModificationProposal":
+		chainId := rawContent.Get("chain_id").String()
+
+		var chain ChainID
+		for i, conf := range tr.chainConfigs {
+			if string(conf.ChainId) == chainId {
+				chain = i
+				break
+			}
+		}
+
+		return ConsumerModificationProposal{
+			Deposit: uint(deposit),
+			Status:  status,
+			Chain:   chain,
+		}
+	case "/cosmos.params.v1beta1.ParameterChangeProposal":
+		return ParamsProposal{
+			Deposit:  uint(deposit),
+			Status:   status,
+			Subspace: gjson.Get(string(bz), `messages.0.content.changes.0.subspace`).String(),
+			Key:      gjson.Get(string(bz), `messages.0.content.changes.0.key`).String(),
+			Value:    gjson.Get(string(bz), `messages.0.content.changes.0.value`).String(),
+		}
 	}
 
 	log.Fatal("received unknown proposal type: ", propType, "proposal JSON:", string(bz))