feat!: add memo to IBC transfers of ICS rewards (#2290)

* add consumer ID to consumer genesis

* add RewardMemo to token transfer

* handle memo on provider side

* fix democ tests

* wip

* add e2e-test

* refactor consumer reward tests

* add consumer migration info

* add changelog

* fix changelog filename

---------

Co-authored-by: Simon Noetzlin <[email protected]>

Author: mpoke

.changelog/unreleased/features/2290-add-memo-to-ICS-rewards.md

@@ -0,0 +1,4 @@
+- `[x/consumer]` Populate the memo on the IBC transfer packets used to send ICS rewards.
+with the required consumer chain Id to identify the consumer to the provider.
+- `[x/provider]` Identify the source of ICS rewards from the IBC transfer packet memo.
+  ([\#2290](https://github.com/cosmos/interchain-security/pull/2290))

.changelog/unreleased/state-breaking/2290-add-memo-to-ICS-rewards.md

@@ -0,0 +1,4 @@
+- `[x/consumer]` Populate the memo on the IBC transfer packets used to send ICS rewards.
+with the required consumer chain Id to identify the consumer to the provider.
+- `[x/provider]` Identify the source of ICS rewards from the IBC transfer packet memo.
+  ([\#2290](https://github.com/cosmos/interchain-security/pull/2290))

UPGRADING.md

@@ -2,6 +2,26 @@
 
 ## Unreleased
 
+### Consumer
+
+Upgrading a consumer from v4.4.x to v4.5.x and from v5.x or v6.1.x to v6.2.x requires state migrations. The following migrators should be added to the upgrade handler of the consumer chain:
+
+
+```go
+// InitializeConsumerId sets the consumer Id parameter in the consumer module,
+// to the consumer id for which the consumer is registered on the provider chain.
+// The consumer id can be obtained in by querying the provider, e.g. by using the
+// QueryConsumerIdFromClientId query.
+func InitializeConsumerId(ctx sdk.Context, consumerKeeper consumerkeeper.Keeper) error {
+	params, err := consumerKeeper.GetParams(ctx)
+	if err != nil {
+		return err
+	}
+	params.ConsumerId = ConsumerId
+	return consumerKeeper.SetParams(ctx, params)
+}
+```
+
 ### Provider
 
 Upgrading a provider from v5.1.x requires state migrations. The following migrators should be added to the upgrade handler of the provider chain:

proto/interchain_security/ccv/v1/shared_consumer.proto

@@ -76,6 +76,10 @@ message ConsumerParams {
     // The period after which a consumer can retry sending a throttled packet.
     google.protobuf.Duration retry_delay_period = 13
         [ (gogoproto.nullable) = false, (gogoproto.stdduration) = true ];
+
+    // The consumer ID of this consumer chain. Used by the consumer module to send 
+    // ICS rewards. 
+    string consumer_id = 14;
 }
 
 // ConsumerGenesisState defines shared genesis information between provider and

tests/e2e/action_rapid_test.go

@@ -102,7 +102,7 @@ func CreateSubmitChangeRewardDenomsProposalActionGen() *rapid.Generator[SubmitCh
 			Chain:   GetChainIDGen().Draw(t, "Chain"),
 			From:    GetValidatorIDGen().Draw(t, "From"),
 			Deposit: rapid.Uint().Draw(t, "Deposit"),
-			Denom:   rapid.String().Draw(t, "Denom"),
+			Denoms:  rapid.SliceOf(rapid.String()).Draw(t, "Denoms"),
 		}
 	})
 }

tests/e2e/actions.go

@@ -495,7 +495,7 @@ func (tr Chain) UpdateConsumer(providerChain ChainID, validator ValidatorID, upd
 	bz, err = cmd.CombinedOutput()
 	if err != nil {
 		fmt.Println("command failed: ", cmd)
-		log.Fatal("update consumer failed error: %w, output: %s", err, string(bz))
+		log.Fatalf("update consumer failed error: %s, output: %s", err, string(bz))
 	}
 
 	// Check transaction
@@ -2536,14 +2536,14 @@ func (tr Chain) registerRepresentative(
 
 type SubmitChangeRewardDenomsProposalAction struct {
 	Chain   ChainID
-	Denom   string
+	Denoms  []string
 	Deposit uint
 	From    ValidatorID
 }
 
 func (tr Chain) submitChangeRewardDenomsProposal(action SubmitChangeRewardDenomsProposalAction, verbose bool) {
 	changeRewMsg := types.MsgChangeRewardDenoms{
-		DenomsToAdd:    []string{action.Denom},
+		DenomsToAdd:    action.Denoms,
 		DenomsToRemove: []string{"stake"},
 		Authority:      "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn",
 	}
@@ -2604,7 +2604,7 @@ func (tr Chain) submitChangeRewardDenomsLegacyProposal(action SubmitChangeReward
 		ChangeRewardDenomsProposal: types.ChangeRewardDenomsProposal{
 			Title:          "Change reward denoms",
 			Description:    "Change reward denoms",
-			DenomsToAdd:    []string{action.Denom},
+			DenomsToAdd:    action.Denoms,
 			DenomsToRemove: []string{"stake"},
 		},
 		Deposit: fmt.Sprint(action.Deposit) + `stake`,
@@ -3309,3 +3309,95 @@ func (tr Commands) AssignConsumerPubKey(action e2e.AssignConsumerPubKeyAction, g
 
 	return cmd.CombinedOutput()
 }
+
+type CreateIbcClientAction struct {
+	ChainA ChainID
+	ChainB ChainID
+}
+
+func (tr Chain) createIbcClientHermes(
+	action CreateIbcClientAction,
+	verbose bool,
+) {
+	cmd := tr.target.ExecCommand("hermes",
+		"create", "client",
+		"--host-chain", string(tr.testConfig.chainConfigs[action.ChainA].ChainId),
+		"--reference-chain", string(tr.testConfig.chainConfigs[action.ChainB].ChainId),
+		"--trusting-period", "1200000s",
+	)
+
+	cmdReader, err := cmd.StdoutPipe()
+	if err != nil {
+		log.Fatal(err)
+	}
+	cmd.Stderr = cmd.Stdout
+
+	if err := cmd.Start(); err != nil {
+		log.Fatal(err)
+	}
+
+	scanner := bufio.NewScanner(cmdReader)
+
+	for scanner.Scan() {
+		out := scanner.Text()
+		if verbose {
+			fmt.Println("createIbcClientHermes: " + out)
+		}
+		if out == done {
+			break
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		log.Fatal(err)
+	}
+}
+
+type TransferIbcTokenAction struct {
+	Chain   ChainID
+	DstAddr string
+	From    ValidatorID
+	Amount  uint
+	Channel uint
+	Memo    string
+}
+
+func (tr Chain) transferIbcToken(
+	action TransferIbcTokenAction,
+	verbose bool,
+) {
+	// Note: to get error response reported back from this command '--gas auto' needs to be set.
+	gas := "auto"
+
+	transferCmd := fmt.Sprintf(
+		`%s tx ibc-transfer transfer transfer \
+%s %s %s --memo %q --from validator%s --chain-id %s \
+--home %s --node %s --gas %s --keyring-backend test -y -o json`,
+		tr.testConfig.chainConfigs[action.Chain].BinaryName,
+		"channel-"+fmt.Sprint(action.Channel),
+		action.DstAddr,
+		fmt.Sprint(action.Amount)+`stake`,
+		action.Memo,
+		action.From,
+		string(tr.testConfig.chainConfigs[action.Chain].ChainId),
+		tr.getValidatorHome(action.Chain, action.From),
+		tr.getValidatorNode(action.Chain, action.From),
+		gas,
+	)
+
+	cmd := tr.target.ExecCommand(
+		"/bin/bash", "-c",
+		transferCmd,
+	)
+
+	if verbose {
+		fmt.Println("transferIbcToken cmd:", cmd.String())
+	}
+
+	bz, err := cmd.CombinedOutput()
+	if err != nil {
+		log.Fatalf("unexpected error during IBC token transfer: %s: %s", string(bz), err)
+	}
+
+	// wait for inclusion in a block -> '--broadcast-mode block' is deprecated
+	tr.waitBlocks(action.Chain, 2, 30*time.Second)
+}

tests/e2e/state.go

@@ -9,6 +9,7 @@ import (
 	"regexp"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
@@ -154,10 +155,10 @@ func (tr Chain) GetRewards(chain ChainID, modelState Rewards) Rewards {
 		currentBlock = 1
 	}
 	for k := range modelState.IsRewarded {
-		receivedRewards[k] = tr.target.GetReward(chain, k, nextBlock, modelState.IsNativeDenom) > tr.target.GetReward(chain, k, currentBlock, modelState.IsNativeDenom)
+		receivedRewards[k] = tr.target.GetReward(chain, k, nextBlock, modelState.Denom) > tr.target.GetReward(chain, k, currentBlock, modelState.Denom)
 	}
 
-	return Rewards{IsRewarded: receivedRewards, IsIncrementalReward: modelState.IsIncrementalReward, IsNativeDenom: modelState.IsNativeDenom}
+	return Rewards{IsRewarded: receivedRewards, IsIncrementalReward: modelState.IsIncrementalReward, Denom: modelState.Denom}
 }
 
 func (tr Chain) GetConsumerAddresses(chain ChainID, modelState map[ValidatorID]string) map[ValidatorID]string {
@@ -252,7 +253,7 @@ func (tr Commands) GetBlockHeight(chain ChainID) uint {
 	return uint(blockHeight)
 }
 
-func (tr Commands) GetReward(chain ChainID, validator ValidatorID, blockHeight uint, isNativeDenom bool) float64 {
+func (tr Commands) GetReward(chain ChainID, validator ValidatorID, blockHeight uint, denom string) float64 {
 	valCfg := tr.validatorConfigs[validator]
 	delAddresss := valCfg.DelAddress
 	if chain != ChainID("provi") {
@@ -285,12 +286,23 @@ func (tr Commands) GetReward(chain ChainID, validator ValidatorID, blockHeight u
 		log.Fatal("failed getting rewards: ", err, "\n", string(bz))
 	}
 
-	denomCondition := `total.#(denom!="stake").amount`
-	if isNativeDenom {
-		denomCondition = `total.#(denom=="stake").amount`
+	denomCondition := fmt.Sprintf(`total.#(%%"*%s*")`, denom)
+	amount := strings.Split(gjson.Get(string(bz), denomCondition).String(), denom)[0]
+
+	fmt.Println("denomCondition:", denomCondition)
+	fmt.Println("json:", gjson.Parse(string(bz)))
+
+	res := float64(0)
+	if amount != "" {
+		res, err = strconv.ParseFloat(amount, 64)
+		if err != nil {
+			log.Fatal("failed parsing consumer reward:", err)
+		}
 	}
 
-	return gjson.Get(string(bz), denomCondition).Float()
+	fmt.Println("res", res)
+
+	return res
 }
 
 // interchain-securityd query gov proposals

tests/e2e/state_rapid_test.go

@@ -104,8 +104,8 @@ func GetRewardsGen() *rapid.Generator[Rewards] {
 	return rapid.Custom(func(t *rapid.T) Rewards {
 		return Rewards{
 			IsIncrementalReward: rapid.Bool().Draw(t, "IsIncrementalReward"),
-			IsNativeDenom:       rapid.Bool().Draw(t, "IsNativeDenom"),
-			IsRewarded:          rapid.MapOf(GetValidatorIDGen(), rapid.Bool()).Draw(t, "IsRewarded"),
+			// Denom:       rapid.Str,
+			IsRewarded: rapid.MapOf(GetValidatorIDGen(), rapid.Bool()).Draw(t, "IsRewarded"),
 		}
 	})
 }