feat!: Add DistributionTransmissionChannel to ConsumerAdditionProposal

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [Unreleased]
 
 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.
+* feat!: Add DistributionTransmissionChannel to ConsumerAdditionProposal [#965](https://github.com/cosmos/interchain-security/pull/965)
 
 ## v.2.0.0
 

app/consumer-democracy/app.go

@@ -645,12 +645,6 @@ func New(
 			// upgrade handler code is application specific. However, as an example, standalone to consumer
 			// changeover chains should utilize customized upgrade handler code similar to below.
 
-			// Setting the standalone transfer channel ID is only needed for standalone to consumer changeover chains
-			// who wish to preserve existing IBC transfer denoms. Here's an example.
-			//
-			// Note: This setter needs to execute before the ccv channel handshake is initiated.
-			app.ConsumerKeeper.SetStandaloneTransferChannelID(ctx, "hardcoded-existing-channel-id")
-
 			// TODO: should have a way to read from current node home
 			userHomeDir, err := os.UserHomeDir()
 			if err != nil {

docs/docs/consumer-development/onboarding.md

@@ -91,6 +91,13 @@ Example of a consumer chain addition proposal.
     // This param is a part of the cosmos sdk staking module. In the case of
     // a ccv enabled consumer chain, the ccv module acts as the staking module.
     "historical_entries": 10000,
+    // The ID of a token transfer channel used for the Reward Distribution
+	// sub-protocol. If DistributionTransmissionChannel == "", a new transfer
+	// channel is created on top of the same connection as the CCV channel.
+	// Note that transfer_channel_id is the ID of the channel end on the consumer chain.
+    // it is most relevant for chains performing a sovereign to consumer changeover
+    // in order to maintan the existing ibc transfer channel
+    "distribution_transmission_channel": "channel-123"
 }
 ```
 

docs/docs/features/proposals.md

@@ -43,6 +43,9 @@ Minimal example:
     "historical_entries": 10000,
     "genesis_hash": "d86d756e10118e66e6805e9cc476949da2e750098fcc7634fd0cc77f57a0b2b0",
     "binary_hash": "376cdbd3a222a3d5c730c9637454cd4dd925e2f9e2e0d0f3702fc922928583f1"
+    // relevant for chains performing a sovereign to consumer changeover
+    // in order to maintan the existing ibc transfer channel
+    "distribution_transmission_channel": "channel-123"
 }
 ```
 More examples can be found in the replicated security testnet repository [here](https://github.com/cosmos/testnets/blob/master/replicated-security/baryon-1/proposal-baryon-1.json) and [here](https://github.com/cosmos/testnets/blob/master/replicated-security/noble-1/start-proposal-noble-1.json).

proto/interchain_security/ccv/consumer/v1/genesis.proto

@@ -34,12 +34,12 @@ message GenesisState {
   // OutstandingDowntimes nil on new chain, filled  in on restart.
   repeated OutstandingDowntime outstanding_downtime_slashing = 10
       [ (gogoproto.nullable) = false ];
-   // PendingConsumerPackets nil on new chain, filled in on restart.
-   interchain_security.ccv.v1.ConsumerPacketDataList pending_consumer_packets = 11
-   [ (gogoproto.nullable) = false ];
+  // PendingConsumerPackets nil on new chain, filled in on restart.
+  interchain_security.ccv.v1.ConsumerPacketDataList pending_consumer_packets = 11
+      [ (gogoproto.nullable) = false ];
   // LastTransmissionBlockHeight nil on new chain, filled in on restart.
   interchain_security.ccv.consumer.v1.LastTransmissionBlockHeight last_transmission_block_height = 12
-  [ (gogoproto.nullable) = false ];
+      [ (gogoproto.nullable) = false ];
   bool preCCV = 13; // flag indicating whether the consumer CCV module starts in pre-CCV state
 }
 

proto/interchain_security/ccv/provider/v1/provider.proto

@@ -62,6 +62,13 @@ message ConsumerAdditionProposal {
     // This param is a part of the cosmos sdk staking module. In the case of 
     // a ccv enabled consumer chain, the ccv module acts as the staking module.
     int64 historical_entries = 13;
+    // The ID of a token transfer channel used for the Reward Distribution
+    // sub-protocol. If DistributionTransmissionChannel == "", a new transfer
+    // channel is created on top of the same connection as the CCV channel.
+    // Note that transfer_channel_id is the ID of the channel end on the consumer chain.
+    // it is most relevant for chains performing a sovereign to consumer changeover
+    // in order to maintan the existing ibc transfer channel
+    string distribution_transmission_channel = 14;
 }
 
 // ConsumerRemovalProposal is a governance proposal on the provider chain to remove (and stop) a consumer chain.

testutil/integration/debug_test.go

@@ -248,11 +248,3 @@ func TestPacketRoundtrip(t *testing.T) {
 func TestQueueAndSendVSCMaturedPackets(t *testing.T) {
 	runCCVTestByName(t, "TestQueueAndSendVSCMaturedPackets")
 }
-
-//
-// Changeover tests
-//
-
-func TestRecycleTransferChannel(t *testing.T) {
-	runCCVTestByName(t, "TestRecycleTransferChannel")
-}

testutil/keeper/unit_test_helpers.go

@@ -237,6 +237,7 @@ func GetTestConsumerAdditionProp() *providertypes.ConsumerAdditionProposal {
 		time.Now(),
 		consumertypes.DefaultConsumerRedistributeFrac,
 		consumertypes.DefaultBlocksPerDistributionTransmission,
+		"",
 		consumertypes.DefaultHistoricalEntries,
 		types.DefaultCCVTimeoutPeriod,
 		consumertypes.DefaultTransferTimeoutPeriod,

x/ccv/consumer/ibc_module.go

@@ -140,15 +140,10 @@ func (am AppModule) OnChanOpenAck(
 	///////////////////////////////////////////////////
 	// Initialize distribution token transfer channel
 
-	// First check if an existing transfer channel exists, if this consumer was a previously standalone chain.
-	if am.keeper.IsPrevStandaloneChain(ctx) {
-		transChannelID := am.keeper.GetStandaloneTransferChannelID(ctx)
-		found := am.keeper.TransferChannelExists(ctx, transChannelID)
-		if found {
-			// If existing transfer channel is found, persist that channel ID and return
-			am.keeper.SetDistributionTransmissionChannel(ctx, transChannelID)
-			return nil
-		}
+	// First check if an existing transfer channel already exists.
+	transChannelID := am.keeper.GetDistributionTransmissionChannel(ctx)
+	if found := am.keeper.TransferChannelExists(ctx, transChannelID); found {
+		return nil
 	}
 
 	// NOTE The handshake for this channel is handled by the ibc-go/transfer

x/ccv/consumer/ibc_module_test.go

@@ -214,7 +214,7 @@ func TestOnChanOpenAck(t *testing.T) {
 		expPass bool
 	}{
 		{
-			"success",
+			"success - empty transferChannelID",
 			func(keeper *consumerkeeper.Keeper, params *params, mocks testkeeper.MockedKeepers) {
 				// Expected msg
 				distrTransferMsg := channeltypes.NewMsgChannelOpenInit(
@@ -226,8 +226,13 @@ func TestOnChanOpenAck(t *testing.T) {
 					"", // signer unused
 				)
 
+				transferChannelID := ""
+				keeper.SetDistributionTransmissionChannel(params.ctx, transferChannelID)
+
 				// Expected mock calls
 				gomock.InOrder(
+					mocks.MockChannelKeeper.EXPECT().GetChannel(
+						params.ctx, transfertypes.PortID, transferChannelID).Return(channeltypes.Channel{}, false).Times(1),
 					mocks.MockChannelKeeper.EXPECT().GetChannel(
 						params.ctx, params.portID, params.channelID).Return(channeltypes.Channel{
 						ConnectionHops: []string{"connectionID"},

x/ccv/consumer/keeper/keeper.go

@@ -618,17 +618,3 @@ func (k Keeper) IsPrevStandaloneChain(ctx sdk.Context) bool {
 	store := ctx.KVStore(k.storeKey)
 	return store.Has(types.PrevStandaloneChainKey())
 }
-
-// SetStandaloneTransferChannelID sets the channelID of an existing transfer channel,
-// for a chain which used to be a standalone chain.
-func (k Keeper) SetStandaloneTransferChannelID(ctx sdk.Context, channelID string) {
-	store := ctx.KVStore(k.storeKey)
-	store.Set(types.StandaloneTransferChannelIDKey(), []byte(channelID))
-}
-
-// GetStandaloneTransferChannelID returns the channelID of an existing transfer channel,
-// for a chain which used to be a standalone chain.
-func (k Keeper) GetStandaloneTransferChannelID(ctx sdk.Context) string {
-	store := ctx.KVStore(k.storeKey)
-	return string(store.Get(types.StandaloneTransferChannelIDKey()))
-}

x/ccv/consumer/keeper/keeper_test.go

@@ -541,19 +541,6 @@ func TestGetAllOutstandingDowntimes(t *testing.T) {
 	require.Equal(t, result, expectedGetAllOrder)
 }
 
-// TestStandaloneTransferChannelID tests the getter and setter for the existing transfer channel id
-func TestStandaloneTransferChannelID(t *testing.T) {
-	ck, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
-	defer ctrl.Finish()
-
-	// Test that the default value is empty
-	require.Equal(t, "", ck.GetStandaloneTransferChannelID(ctx))
-
-	// Test that the value can be set and retrieved
-	ck.SetStandaloneTransferChannelID(ctx, "channelID1234")
-	require.Equal(t, "channelID1234", ck.GetStandaloneTransferChannelID(ctx))
-}
-
 func TestPrevStandaloneChainFlag(t *testing.T) {
 	ck, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 	defer ctrl.Finish()

x/ccv/consumer/keeper/migration_test.go

@@ -116,7 +116,7 @@ func (p *v1Params) ParamSetPairs() paramtypes.ParamSetPairs {
 		paramtypes.NewParamSetPair(consumertypes.KeyBlocksPerDistributionTransmission,
 			p.BlocksPerDistributionTransmission, ccvtypes.ValidatePositiveInt64),
 		paramtypes.NewParamSetPair(consumertypes.KeyDistributionTransmissionChannel,
-			p.DistributionTransmissionChannel, consumertypes.ValidateDistributionTransmissionChannel),
+			p.DistributionTransmissionChannel, ccvtypes.ValidateDistributionTransmissionChannel),
 		paramtypes.NewParamSetPair(consumertypes.KeyProviderFeePoolAddrStr,
 			p.ProviderFeePoolAddrStr, consumertypes.ValidateProviderFeePoolAddrStr),
 		paramtypes.NewParamSetPair(ccvtypes.KeyCCVTimeoutPeriod,

x/ccv/consumer/types/genesis_test.go

@@ -210,7 +210,7 @@ func TestValidateInitialGenesisState(t *testing.T) {
 			true,
 		},
 		{
-			"invalid new consumer genesis state: invalid params",
+			"invalid new consumer genesis state: invalid params - ccvTimeoutPeriod",
 			types.NewInitialGenesisState(cs, consensusState, valUpdates,
 				types.NewParams(
 					true,
@@ -228,6 +228,25 @@ func TestValidateInitialGenesisState(t *testing.T) {
 				)),
 			true,
 		},
+		{
+			"invalid new consumer genesis state: invalid params - distributionTransmissionChannel",
+			types.NewInitialGenesisState(cs, consensusState, valUpdates,
+				types.NewParams(
+					true,
+					types.DefaultBlocksPerDistributionTransmission,
+					"badchannel/",
+					"",
+					ccv.DefaultCCVTimeoutPeriod,
+					types.DefaultTransferTimeoutPeriod,
+					types.DefaultConsumerRedistributeFrac,
+					types.DefaultHistoricalEntries,
+					types.DefaultConsumerUnbondingPeriod,
+					types.DefaultSoftOptOutThreshold,
+					[]string{},
+					[]string{},
+				)),
+			true,
+		},
 	}
 
 	for _, c := range cases {

x/ccv/consumer/types/params.go

@@ -113,7 +113,7 @@ func (p Params) Validate() error {
 	if err := ccvtypes.ValidatePositiveInt64(p.BlocksPerDistributionTransmission); err != nil {
 		return err
 	}
-	if err := ValidateDistributionTransmissionChannel(p.DistributionTransmissionChannel); err != nil {
+	if err := ccvtypes.ValidateDistributionTransmissionChannel(p.DistributionTransmissionChannel); err != nil {
 		return err
 	}
 	if err := ValidateProviderFeePoolAddrStr(p.ProviderFeePoolAddrStr); err != nil {
@@ -153,7 +153,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 		paramtypes.NewParamSetPair(KeyBlocksPerDistributionTransmission,
 			p.BlocksPerDistributionTransmission, ccvtypes.ValidatePositiveInt64),
 		paramtypes.NewParamSetPair(KeyDistributionTransmissionChannel,
-			p.DistributionTransmissionChannel, ValidateDistributionTransmissionChannel),
+			p.DistributionTransmissionChannel, ccvtypes.ValidateDistributionTransmissionChannel),
 		paramtypes.NewParamSetPair(KeyProviderFeePoolAddrStr,
 			p.ProviderFeePoolAddrStr, ValidateProviderFeePoolAddrStr),
 		paramtypes.NewParamSetPair(ccvtypes.KeyCCVTimeoutPeriod,
@@ -175,15 +175,6 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	}
 }
 
-func ValidateDistributionTransmissionChannel(i interface{}) error {
-	// Accept empty string as valid, since this will be the default value on genesis
-	if i == "" {
-		return nil
-	}
-	// Otherwise validate as usual for a channelID
-	return ccvtypes.ValidateChannelIdentifier(i)
-}
-
 func ValidateProviderFeePoolAddrStr(i interface{}) error {
 	// Accept empty string as valid, since this will be the default value on genesis
 	if i == "" {

x/ccv/provider/client/proposal_handler.go

@@ -58,6 +58,7 @@ Where proposal.json contains:
     "spawn_time": "2022-01-27T15:59:50.121607-08:00",
     "blocks_per_distribution_transmission": 1000,
     "consumer_redistribution_fraction": "0.75",
+	"distribution_transmission_channel": "",
     "historical_entries": 10000,
     "transfer_timeout_period": 3600000000000,
     "ccv_timeout_period": 2419200000000000,
@@ -82,7 +83,8 @@ Where proposal.json contains:
 			content := types.NewConsumerAdditionProposal(
 				proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight,
 				proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime,
-				proposal.ConsumerRedistributionFraction, proposal.BlocksPerDistributionTransmission, proposal.HistoricalEntries,
+				proposal.ConsumerRedistributionFraction, proposal.BlocksPerDistributionTransmission,
+				proposal.DistributionTransmissionChannel, proposal.HistoricalEntries,
 				proposal.CcvTimeoutPeriod, proposal.TransferTimeoutPeriod, proposal.UnbondingPeriod)
 
 			from := clientCtx.GetFromAddress()
@@ -224,6 +226,7 @@ type ConsumerAdditionProposalJSON struct {
 
 	ConsumerRedistributionFraction    string        `json:"consumer_redistribution_fraction"`
 	BlocksPerDistributionTransmission int64         `json:"blocks_per_distribution_transmission"`
+	DistributionTransmissionChannel   string        `json:"distribution_transmission_channel"`
 	HistoricalEntries                 int64         `json:"historical_entries"`
 	CcvTimeoutPeriod                  time.Duration `json:"ccv_timeout_period"`
 	TransferTimeoutPeriod             time.Duration `json:"transfer_timeout_period"`
@@ -246,6 +249,7 @@ type ConsumerAdditionProposalReq struct {
 
 	ConsumerRedistributionFraction    string        `json:"consumer_redistribution_fraction"`
 	BlocksPerDistributionTransmission int64         `json:"blocks_per_distribution_transmission"`
+	DistributionTransmissionChannel   string        `json:"distribution_transmission_channel"`
 	HistoricalEntries                 int64         `json:"historical_entries"`
 	CcvTimeoutPeriod                  time.Duration `json:"ccv_timeout_period"`
 	TransferTimeoutPeriod             time.Duration `json:"transfer_timeout_period"`
@@ -375,7 +379,8 @@ func postConsumerAdditionProposalHandlerFn(clientCtx client.Context) http.Handle
 		content := types.NewConsumerAdditionProposal(
 			req.Title, req.Description, req.ChainId, req.InitialHeight,
 			req.GenesisHash, req.BinaryHash, req.SpawnTime,
-			req.ConsumerRedistributionFraction, req.BlocksPerDistributionTransmission, req.HistoricalEntries,
+			req.ConsumerRedistributionFraction, req.BlocksPerDistributionTransmission,
+			req.DistributionTransmissionChannel, req.HistoricalEntries,
 			req.CcvTimeoutPeriod, req.TransferTimeoutPeriod, req.UnbondingPeriod)
 
 		msg, err := govtypes.NewMsgSubmitProposal(content, req.Deposit, req.Proposer)

x/ccv/provider/keeper/proposal.go

@@ -303,7 +303,7 @@ func (k Keeper) MakeConsumerGenesis(
 	consumerGenesisParams := consumertypes.NewParams(
 		true,
 		prop.BlocksPerDistributionTransmission,
-		"", // distributionTransmissionChannel
+		prop.DistributionTransmissionChannel,
 		"", // providerFeePoolAddrStr,
 		prop.CcvTimeoutPeriod,
 		prop.TransferTimeoutPeriod,