feat!: Removed legacy-submit-proposal support for consumer addition/m…

…odification/removal (#2130) * Remove legacy-submit-proposal consumer-addition * Remove legacy-submit-proposal consumer-removal * Remove legacy-submit-proposal consumer-modification * docs * fix e2e * addressed review comments * fix compatibility tests * addressed some more review comments * addressed PR comments * fix e2e when run on current ws (invalid sem version) * Apply suggestions from code review Co-authored-by: insumity <[email protected]> --------- Co-authored-by: insumity <[email protected]>
cosmos · Aug 14, 2024 · cca6887 · cca6887
1 parent 9a303a0
commit cca6887
Show file tree

Hide file tree

Showing 14 changed files with 539 additions and 374 deletions.
diff --git a/.changelog/unreleased/api-breaking/provider/2130-remove-legacy-proposal.md b/.changelog/unreleased/api-breaking/provider/2130-remove-legacy-proposal.md
@@ -0,0 +1,25 @@
+- Remove support for legacy-proposal to add/modify/remove consumer proposals and change reward denoms
+  ([\#2130](https://github.com/cosmos/interchain-security/pull/2130))
+  To submit a proposal to add/modify/remove a consumer use the following command
+  ```shell
+  interchain-security-pd tx gov submit-proposal [proposal-file]
+  ```
+
+  Run `interchain-security-pd tx gov draft-proposal` command and select in `other` one of the following
+  message types to generate a draft proposal json file:
+  - `/interchain_security.ccv.provider.v1.MsgConsumerAddition`
+
+  - `/interchain_security.ccv.provider.v1.MsgConsumerModification`
+
+  - `/interchain_security.ccv.provider.v1.MsgConsumerRemoval`
+
+  - `/interchain_security.ccv.provider.v1.MsgChangeRewardDenoms`
+
+  This replaces the following command which are not supported anymore:
+
+  ```shell
+  interchain-security-pd tx gov submit-legacy-proposal consumer-addition [proposal-file]
+  interchain-security-pd tx gov submit-legacy-proposal consumer-modification [proposal-file]
+  interchain-security-pd tx gov submit-legacy-proposal consumer-removal [proposal-file]
+  interchain-security-pd tx gov submit-legacy-proposal change-reward-denoms [proposal-file]
+  ```
diff --git a/.github/workflows/nightly-e2e.yml b/.github/workflows/nightly-e2e.yml
@@ -36,7 +36,7 @@ jobs:
         # Run compatibility tests for different consumer (-cv) and provider (-pv) versions.
         # Combination of all provider versions with consumer versions are tested.
         # For new versions to be tested add/modify -pc/-cv parameters.
-        run: go run ./tests/e2e/... --tc compatibility -pv latest -pv v4.3.0-lsm -pv v3.3.3-lsm -cv latest -cv v4.3.0 -cv v3.3.0
+        run: go run ./tests/e2e/... --tc compatibility -pv latest -pv v4.3.1-lsm -pv v3.3.3-lsm -cv latest -cv v4.4.0 -cv v3.3.0
   happy-path-test:
     runs-on: ubuntu-latest
     timeout-minutes: 20

diff --git a/UPGRADING.md b/UPGRADING.md
@@ -27,6 +27,74 @@ func InitializeMaxProviderConsensusParam(ctx sdk.Context, providerKeeper provide
 }
 ```
 
+### Governance Proposals
+
+Legacy proposals are not supported anymore by the current version of the provider. Legacy proposals which are still active (voting period or deposit period) and contain one of the following messages that need to be migrated as described below:
+- `ConsumerAdditionProposal` needs to be converted to `MsgConsumerAddition`
+- `ConsumerModificationProposal` needs to be converted to `MsgConsumerModification`
+- `ConsumerRemovalProposal` needs to be converted to `MsgConsumerRemoval`
+- `ChangeRewardDenomsProposal` needs to be converted to `MsgChangeRewardDenoms`
+
+The following shows an example on how to migrate a proposal containing a legacy consumer addition proposal message.
+Migration for the other messages aobve follows the same pattern. The resulting migration code has to be added to the upgrade handler of the provider chain.
+
+#### Migrate Legacy Proposal Content
+
+```go
+
+// MigrateLegacyConsumerAddition converts a ConsumerAdditionProposal to a MsgConsumerAdditionProposal
+// and returns it as `Any` suitable to replace the legacy message.
+// `authority` contains the signer address
+func MigrateLegacyConsumerAddition(msg providertypes.ConsumerAdditionProposal, authority string) (*codec.Any, error) {
+  sdkMsg := providertypes.MsgConsumerAddition{
+		ChainId:                           msg.ChainId,
+		InitialHeight:                     msg.InitialHeight,
+		GenesisHash:                       msg.GenesisHash,
+		BinaryHash:                        msg.BinaryHash,
+		SpawnTime:                         msg.SpawnTime,
+		UnbondingPeriod:                   msg.UnbondingPeriod,
+		CcvTimeoutPeriod:                  msg.CcvTimeoutPeriod,
+		TransferTimeoutPeriod:             msg.TransferTimeoutPeriod,
+		ConsumerRedistributionFraction:    msg.ConsumerRedistributionFraction,
+		BlocksPerDistributionTransmission: msg.BlocksPerDistributionTransmission,
+		HistoricalEntries:                 msg.HistoricalEntries,
+		DistributionTransmissionChannel:   msg.DistributionTransmissionChannel,
+		Top_N:                             msg.Top_N,
+		ValidatorsPowerCap:                msg.ValidatorsPowerCap,
+		ValidatorSetCap:                   msg.ValidatorSetCap,
+		Allowlist:                         msg.Allowlist,
+		Denylist:                          msg.Denylist,
+		Authority:                         authority,
+		MinStake:                          msg.MinStake,
+		AllowInactiveVals:                 msg.AllowInactiveVals,
+	}
+	return codec.NewAnyWithValue(&sdkMsg)
+}
+
+func MigrateProposal(proposal proposal govtypes.Proposal) err {
+	for idx, msg := range proposal.GetMessages() {
+		sdkLegacyMsg, isLegacyProposal := msg.GetCachedValue().(*govtypes.MsgExecLegacyContent)
+		if !isLegacyProposal {
+			continue
+		}
+		content, err := govtypes.LegacyContentFromMessage(sdkLegacyMsg)
+		if err != nil {
+			continue
+		}
+
+		msgAdd, ok := content.(*providertypes.ConsumerAdditionProposal)
+		if ok {
+			anyMsg, err := migrateLegacyConsumerAddition(*msgAdd, govKeeper.GetAuthority())
+			if err != nil {
+				return err
+			}
+			proposal.Messages[idx] = anyMsg
+		}
+	}
+	return govKeeper.SetProposal(ctx, proposal)
+}
+```
+
 ## [v5.1.x](https://github.com/cosmos/interchain-security/releases/tag/v5.1.0)
 
 ### Provider
@@ -41,7 +109,7 @@ Upgrade code will be executed automatically during the upgrade procedure.
 
 ### Consumer
 
-Upgrading the consumer from `v5.0.0` to `v5.1.0` will not require state migration. 
+Upgrading the consumer from `v5.0.0` to `v5.1.0` will not require state migration.
 
 This guide provides instructions for upgrading to specific versions of Replicated Security.
 
@@ -55,7 +123,7 @@ v5.0.0 was a **consumer only release**.
 
 ### Consumer
 
-Upgrading the consumer from `v4.x` to `v5.0.0` will require state migrations. 
+Upgrading the consumer from `v4.x` to `v5.0.0` will require state migrations.
 
 Consumer versions `v4.0.x`, `v4.1.x`, `v4.2.x`, `v4.3.x` and `v4.4.x` can cleanly be upgraded to `v5.0.0`.
 
@@ -69,15 +137,15 @@ Upgrade code will be executed automatically during the upgrade procedure.
 
 ### Consumer
 
-Upgrading the consumer from `v4.0.0` to `v4.4.0` will not require state migration. 
+Upgrading the consumer from `v4.0.0` to `v4.4.0` will not require state migration.
 
 This guide provides instructions for upgrading to specific versions of Replicated Security.
 
 ## [v4.3.x](https://github.com/cosmos/interchain-security/releases/tag/v4.3.0)
 
 ### Provider
 
-Upgrading a provider from `v4.2.0` to `v4.3.0` requires state migrations that will be done automatically via the upgrade module. 
+Upgrading a provider from `v4.2.0` to `v4.3.0` requires state migrations that will be done automatically via the upgrade module.
 
 ### Consumer
 
@@ -132,17 +200,17 @@ func InitICSEpochs(ctx sdk.Context, pk providerkeeper.Keeper, sk stakingkeeper.K
 
 ## [v4.0.x](https://github.com/cosmos/interchain-security/tree/release/v4.0.x)
 
-`v4.0.x` sets the minimum required version of Go to `1.21`, see https://github.com/cosmos/interchain-security/blob/release/v4.0.x/go.mod#L3. 
+`v4.0.x` sets the minimum required version of Go to `1.21`, see https://github.com/cosmos/interchain-security/blob/release/v4.0.x/go.mod#L3.
 
-### Provider 
+### Provider
 
-Upgrading a provider from `v3.3.0` to `v4.0.0` will require state migrations, see https://github.com/cosmos/interchain-security/blob/release/v4.0.x/x/ccv/provider/migrations/migrator.go#L31. 
+Upgrading a provider from `v3.3.0` to `v4.0.0` will require state migrations, see https://github.com/cosmos/interchain-security/blob/release/v4.0.x/x/ccv/provider/migrations/migrator.go#L31.
 
-### Consumer 
+### Consumer
 
-***Note that consumer chains can upgrade directly from `v3.1.0` to `v4.0.0`.*** 
+***Note that consumer chains can upgrade directly from `v3.1.0` to `v4.0.0`.***
 
-Upgrading a consumer from `v3.2.0` to `v4.0.0` will not require state migration, however, upgrading directly from `v3.1.0` to `v4.0.0` will require state migrations, see https://github.com/cosmos/interchain-security/blob/release/v4.0.x/x/ccv/consumer/keeper/migrations.go#L22. 
+Upgrading a consumer from `v3.2.0` to `v4.0.0` will not require state migration, however, upgrading directly from `v3.1.0` to `v4.0.0` will require state migrations, see https://github.com/cosmos/interchain-security/blob/release/v4.0.x/x/ccv/consumer/keeper/migrations.go#L22.
 
 In addition, the following migration needs to be added to the upgrade handler of the consumer chain:
 ```golang
@@ -166,15 +234,15 @@ func migrateICSOutstandingDowntime(ctx sdk.Context, keepers *upgrades.UpgradeKee
 
 ## [v3.3.x](https://github.com/cosmos/interchain-security/tree/release/v3.2.x)
 
-### Provider 
+### Provider
 
-Upgrading the provider from `v2.x.y` to `v3.3.0` will not require state migration. 
+Upgrading the provider from `v2.x.y` to `v3.3.0` will not require state migration.
 
 ## [v3.2.x](https://github.com/cosmos/interchain-security/tree/release/v3.2.x)
 
 `v3.2.0` bumps IBC to `v7.3`. As a result, `legacy_ibc_testing` is not longer required and was removed, see https://github.com/cosmos/interchain-security/pull/1185. This means that when upgrading to `v3.2.0`, any customized tests relying on `legacy_ibc_testing` need to be updated.
 
-### Consumer 
+### Consumer
 
 Upgrading the consumer from either `v3.0.0` or `v3.1.0` to `v3.2.0` will require state migrations, see https://github.com/cosmos/interchain-security/blob/release/v3.2.x/x/ccv/consumer/keeper/migration.go#L25.
 
@@ -184,13 +252,13 @@ Upgrading the consumer from either `v3.0.0` or `v3.1.0` to `v3.2.0` will require
 
 The following should be considered as complementary to [Cosmos SDK v0.47 UPGRADING.md](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc2/UPGRADING.md).
 
-#### Protobuf 
+#### Protobuf
 
 Protobuf code generation, linting and formatting have been updated to leverage the `ghcr.io/cosmos/proto-builder:0.11.5` docker container. Replicated Security protobuf definitions are now packaged and published to [buf.build/cosmos/interchain-security](https://buf.build/cosmos/interchain-security) via CI workflows. The `third_party/proto` directory has been removed in favour of dependency management using [buf.build](https://docs.buf.build/introduction).
 
 #### App modules
 
-Legacy APIs of the `AppModule` interface have been removed from ccv modules. For example, for 
+Legacy APIs of the `AppModule` interface have been removed from ccv modules. For example, for
 
 ```diff
 - // Route implements the AppModule interface
@@ -240,10 +308,10 @@ import (
 
 ## [v2.0.x](https://github.com/cosmos/interchain-security/releases/tag/v2.0.0)
 
-### Provider 
+### Provider
 
 Upgrading a provider from `v1.1.0-multiden` to `v2.0.0` will require state migrations. See [migration.go](https://github.com/cosmos/interchain-security/blob/v2.0.0/x/ccv/provider/keeper/migration.go).
 
 ### Consumer
 
-Upgrading a consumer from `v1.2.0-multiden` to `v2.0.0` will NOT require state migrations. 
+Upgrading a consumer from `v1.2.0-multiden` to `v2.0.0` will NOT require state migrations.
diff --git a/app/provider/app.go b/app/provider/app.go
@@ -114,7 +114,6 @@ import (
 	no_valupdates_genutil "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_genutil"
 	no_valupdates_staking "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_staking"
 	ibcprovider "github.com/cosmos/interchain-security/v5/x/ccv/provider"
-	ibcproviderclient "github.com/cosmos/interchain-security/v5/x/ccv/provider/client"
 	ibcproviderkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
 	providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
 )
@@ -144,10 +143,6 @@ var (
 		gov.NewAppModuleBasic(
 			[]govclient.ProposalHandler{
 				paramsclient.ProposalHandler,
-				ibcproviderclient.ConsumerAdditionProposalHandler,
-				ibcproviderclient.ConsumerRemovalProposalHandler,
-				ibcproviderclient.ConsumerModificationProposalHandler,
-				ibcproviderclient.ChangeRewardDenomsProposalHandler,
 			},
 		),
 		mint.AppModuleBasic{},
@@ -579,10 +574,6 @@ func New(
 			govtypes.ModuleName: gov.NewAppModuleBasic(
 				[]govclient.ProposalHandler{
 					paramsclient.ProposalHandler,
-					ibcproviderclient.ConsumerAdditionProposalHandler,
-					ibcproviderclient.ConsumerRemovalProposalHandler,
-					ibcproviderclient.ConsumerModificationProposalHandler,
-					ibcproviderclient.ChangeRewardDenomsProposalHandler,
 				},
 			),
 		})

diff --git a/tests/e2e/action_rapid_test.go b/tests/e2e/action_rapid_test.go
@@ -99,6 +99,7 @@ func GetActionGen() *rapid.Generator[any] {
 func CreateSubmitChangeRewardDenomsProposalActionGen() *rapid.Generator[SubmitChangeRewardDenomsProposalAction] {
 	return rapid.Custom(func(t *rapid.T) SubmitChangeRewardDenomsProposalAction {
 		return SubmitChangeRewardDenomsProposalAction{
+			Chain:   GetChainIDGen().Draw(t, "Chain"),
 			From:    GetValidatorIDGen().Draw(t, "From"),
 			Deposit: rapid.Uint().Draw(t, "Deposit"),
 			Denom:   rapid.String().Draw(t, "Denom"),