Merge pull request from GHSA-chqw-ff63-95r8

* squash commit of multisig fix + everything involving denom fix

* fixes

* proto gen

* remove param boilerplate

---------

Co-authored-by: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com>

app/provider/app.go

@@ -154,13 +154,14 @@ var (
 
 	// module account permissions
 	maccPerms = map[string][]string{
-		authtypes.FeeCollectorName:     nil,
-		distrtypes.ModuleName:          nil,
-		minttypes.ModuleName:           {authtypes.Minter},
-		stakingtypes.BondedPoolName:    {authtypes.Burner, authtypes.Staking},
-		stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking},
-		govtypes.ModuleName:            {authtypes.Burner},
-		ibctransfertypes.ModuleName:    {authtypes.Minter, authtypes.Burner},
+		authtypes.FeeCollectorName:        nil,
+		distrtypes.ModuleName:             nil,
+		minttypes.ModuleName:              {authtypes.Minter},
+		stakingtypes.BondedPoolName:       {authtypes.Burner, authtypes.Staking},
+		stakingtypes.NotBondedPoolName:    {authtypes.Burner, authtypes.Staking},
+		govtypes.ModuleName:               {authtypes.Burner},
+		ibctransfertypes.ModuleName:       {authtypes.Minter, authtypes.Burner},
+		providertypes.ConsumerRewardsPool: nil,
 	}
 )
 
@@ -309,12 +310,14 @@ func New(
 		maccPerms,
 	)
 
-	// Remove the fee-pool from the group of blocked recipient addresses in bank
+	// Remove the ConsumerRewardsPool from the group of blocked recipient addresses in bank
 	// this is required for the provider chain to be able to receive tokens from
 	// the consumer chain
 	bankBlockedAddrs := app.ModuleAccountAddrs()
 	delete(bankBlockedAddrs, authtypes.NewModuleAddress(
-		authtypes.FeeCollectorName).String())
+		providertypes.ConsumerRewardsPool).String())
+
+	// bankBlockedAddrs := make(map[string]bool)
 
 	app.BankKeeper = bankkeeper.NewBaseKeeper(
 		appCodec,
@@ -410,6 +413,8 @@ func New(
 		app.SlashingKeeper,
 		app.AccountKeeper,
 		app.EvidenceKeeper,
+		app.DistrKeeper,
+		app.BankKeeper,
 		authtypes.FeeCollectorName,
 	)
 
@@ -778,6 +783,11 @@ func (app *App) GetE2eDistributionKeeper() e2e.E2eDistributionKeeper {
 	return app.DistrKeeper
 }
 
+// GetTestAccountKeeper implements the ProviderApp interface.
+func (app *App) GetE2eAccountKeeper() e2e.E2eAccountKeeper {
+	return app.AccountKeeper
+}
+
 // TestingApp functions
 
 // GetBaseApp implements the TestingApp interface.

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

@@ -52,6 +52,13 @@ message Params {
   // which should be smaller than that of the provider in general.
   google.protobuf.Duration unbonding_period = 9
       [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
+
+  // Reward denoms. These are the denominations which are allowed to be sent to the provider as rewards.
+  repeated string reward_denoms = 10;
+
+  // Provider-originated reward denoms. These are denoms coming from the provider 
+  // which are allowed to be used as rewards. e.g. "uatom"
+  repeated string provider_reward_denoms = 11;
 }
 
 // LastTransmissionBlockHeight is the last time validator holding

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

@@ -11,6 +11,7 @@ import "ibc/core/client/v1/client.proto";
 import "ibc/lightclients/tendermint/v1/tendermint.proto";
 import "tendermint/crypto/keys.proto";
 import "cosmos/evidence/v1beta1/evidence.proto";
+import "cosmos/base/v1beta1/coin.proto";
 
 // ConsumerAdditionProposal is a governance proposal on the provider chain to spawn a new consumer chain.
 // If it passes, then all validators on the provider chain are expected to validate the consumer chain at spawn time

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

@@ -68,6 +68,12 @@ service Query {
       returns (QueryThrottledConsumerPacketDataResponse) {
     option (google.api.http).get = "/interchain_security/ccv/provider/pending_consumer_packets";
   }
+
+  // QueryRegisteredConsumerRewardDenoms returns a list of consumer reward denoms that are registered
+  rpc QueryRegisteredConsumerRewardDenoms(QueryRegisteredConsumerRewardDenomsRequest)
+      returns (QueryRegisteredConsumerRewardDenomsResponse) {
+    option (google.api.http).get = "/interchain_security/ccv/provider/registered_consumer_reward_denoms";
+  }
 }
 
 message QueryConsumerGenesisRequest { string chain_id = 1; }
@@ -169,3 +175,9 @@ message ThrottledPacketDataWrapper {
     interchain_security.ccv.v1.VSCMaturedPacketData vsc_matured_packet = 2;
   }
 }
+
+message QueryRegisteredConsumerRewardDenomsRequest {}
+
+message QueryRegisteredConsumerRewardDenomsResponse {
+  repeated string denoms = 1;
+}

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

@@ -11,6 +11,7 @@ import "google/protobuf/any.proto";
 // Msg defines the Msg service.
 service Msg {
   rpc AssignConsumerKey(MsgAssignConsumerKey) returns (MsgAssignConsumerKeyResponse);
+  rpc RegisterConsumerRewardDenom(MsgRegisterConsumerRewardDenom) returns (MsgRegisterConsumerRewardDenomResponse);
 }
 
 message MsgAssignConsumerKey {
@@ -21,9 +22,24 @@ message MsgAssignConsumerKey {
   // The validator address on the provider
   string provider_addr = 2
       [ (gogoproto.moretags) = "yaml:\"address\"" ];
-  // The consensus public key to use on the consumer
-  google.protobuf.Any consumer_key = 3
-      [ (cosmos_proto.accepts_interface) = "cosmos.crypto.PubKey" ];
+  // The consensus public key to use on the consumer.
+  // in json string format corresponding to proto-any, ex: 
+  // `{"@type":"/cosmos.crypto.ed25519.PubKey","key":"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is="}`
+  string consumer_key = 3;
 }
 
-message MsgAssignConsumerKeyResponse {}
+message MsgAssignConsumerKeyResponse {}
+
+// MsgRegisterConsumerRewardDenom allows an account to register 
+// a consumer reward denom, i.e., add it to the list of denoms 
+// accepted by the provider as rewards.
+message MsgRegisterConsumerRewardDenom {
+  option (gogoproto.equal)           = false;
+  option (gogoproto.goproto_getters) = false;
+
+  string denom = 1;
+  string depositor = 2;
+}
+
+// MsgRegisterConsumerRewardDenomResponse defines the Msg/RegisterConsumerRewardDenom response type.
+message MsgRegisterConsumerRewardDenomResponse {}

tests/difference/core/driver/setup.go

@@ -520,6 +520,8 @@ func (b *Builder) createConsumerGenesis(client *ibctmtypes.ClientState) *consume
 		consumertypes.DefaultConsumerRedistributeFrac,
 		consumertypes.DefaultHistoricalEntries,
 		b.initState.UnbondingC,
+		[]string{},
+		[]string{},
 	)
 	return consumertypes.NewInitialGenesisState(client, providerConsState, valUpdates, params)
 }

tests/e2e/distribution.go

@@ -5,8 +5,10 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	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/x/ccv/consumer/types"
+	providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types"
 	ccv "github.com/cosmos/interchain-security/x/ccv/types"
 )
 
@@ -21,6 +23,11 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	delegate(s, delAddr, bondAmt)
 	s.providerChain.NextBlock()
 
+	// register a consumer reward denom
+	params := s.consumerApp.GetConsumerKeeper().GetParams(s.consumerCtx())
+	params.RewardDenoms = []string{sdk.DefaultBondDenom}
+	s.consumerApp.GetConsumerKeeper().SetParams(s.consumerCtx(), params)
+
 	// relay VSC packets from provider to consumer
 	relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, 1)
 
@@ -30,7 +37,9 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	s.consumerChain.NextBlock()
 
 	consumerAccountKeeper := s.consumerApp.GetE2eAccountKeeper()
+	providerAccountKeeper := s.providerApp.GetE2eAccountKeeper()
 	consumerBankKeeper := s.consumerApp.GetE2eBankKeeper()
+	providerBankKeeper := s.providerApp.GetE2eBankKeeper()
 
 	//send coins to the fee pool which is used for reward distribution
 	consumerFeePoolAddr := consumerAccountKeeper.GetModuleAccount(s.consumerCtx(), authtypes.FeeCollectorName).GetAddress()
@@ -58,22 +67,73 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	providerTokens := consumerBankKeeper.GetAllBalances(s.consumerCtx(), providerRedistributeAddr)
 	s.Require().Equal(providerExpectedRewards.AmountOf(sdk.DefaultBondDenom), providerTokens.AmountOf(sdk.DefaultBondDenom))
 
-	//send the reward to provider chain after 2 blocks
-
+	// send the reward to provider chain after 2 blocks
 	s.consumerChain.NextBlock()
 	providerTokens = consumerBankKeeper.GetAllBalances(s.consumerCtx(), providerRedistributeAddr)
 	s.Require().Equal(0, len(providerTokens))
 
 	relayAllCommittedPackets(s, s.consumerChain, s.transferPath, transfertypes.PortID, s.transferPath.EndpointA.ChannelID, 1)
 	s.providerChain.NextBlock()
-	communityCoins := s.providerApp.GetE2eDistributionKeeper().GetFeePoolCommunityCoins(s.providerCtx())
+
+	// Since consumer reward denom is not yet registered, the coins never get into the fee pool, staying in the ConsumerRewardsPool
+	rewardPool := providerAccountKeeper.GetModuleAccount(s.providerCtx(), providertypes.ConsumerRewardsPool).GetAddress()
+	rewardCoins := providerBankKeeper.GetAllBalances(s.providerCtx(), rewardPool)
+
 	ibcCoinIndex := -1
-	for i, coin := range communityCoins {
+	for i, coin := range rewardCoins {
 		if strings.HasPrefix(coin.Denom, "ibc") {
 			ibcCoinIndex = i
 		}
 	}
+
+	// Check that we found an ibc denom in the reward pool
 	s.Require().Greater(ibcCoinIndex, -1)
+
+	// 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
+	providerBankKeeper.SendCoinsFromAccountToModule(s.providerCtx(), delAddr, govtypes.ModuleName, senderCoins)
+
+	// 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))
+
+	// Successfully register the consumer reward denom this time
+
+	// Send the coins back to the delAddr
+	providerBankKeeper.SendCoinsFromModuleToAccount(s.providerCtx(), govtypes.ModuleName, delAddr, senderCoins)
+
+	// log the sender's coins
+	senderCoins1 := providerBankKeeper.GetAllBalances(s.providerCtx(), delAddr)
+
+	// Register the consumer reward denom
+	err = s.providerApp.GetProviderKeeper().RegisterConsumerRewardDenom(s.providerCtx(), rewardCoins[ibcCoinIndex].Denom, delAddr)
+	s.Require().NoError(err)
+
+	// Check that the delAddr has the right amount less money in it after paying the fee
+	senderCoins2 := providerBankKeeper.GetAllBalances(s.providerCtx(), delAddr)
+	consumerRewardDenomRegistrationFee := s.providerApp.GetProviderKeeper().GetConsumerRewardDenomRegistrationFee(s.providerCtx())
+	s.Require().Equal(senderCoins1.Sub(senderCoins2), sdk.NewCoins(consumerRewardDenomRegistrationFee))
+
+	s.providerChain.NextBlock()
+
+	// Check that the reward pool has no more coins because they were transferred to the fee pool
+	rewardCoins = providerBankKeeper.GetAllBalances(s.providerCtx(), rewardPool)
+	s.Require().Equal(0, len(rewardCoins))
+
+	// check that the fee pool has the expected amount of coins
+	communityCoins := s.providerApp.GetE2eDistributionKeeper().GetFeePoolCommunityCoins(s.providerCtx())
 	s.Require().True(communityCoins[ibcCoinIndex].Amount.Equal(sdk.NewDecCoinFromCoin(providerExpectedRewards[0]).Amount))
 }
 
@@ -90,6 +150,11 @@ func (s *CCVTestSuite) TestSendRewardsRetries() {
 	delegate(s, delAddr, bondAmt)
 	s.providerChain.NextBlock()
 
+	// Register denom on consumer chain
+	params := s.consumerApp.GetConsumerKeeper().GetParams(s.consumerCtx())
+	params.RewardDenoms = []string{sdk.DefaultBondDenom}
+	s.consumerApp.GetConsumerKeeper().SetParams(s.consumerCtx(), params)
+
 	// relay VSC packets from provider to consumer
 	relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, 1)
 
@@ -159,28 +224,48 @@ func (s *CCVTestSuite) TestEndBlockRD() {
 		corruptTransChannel     bool
 		expLBThUpdated          bool
 		expEscrowBalanceChanged bool
+		denomRegistered         bool
 	}{
 		{
 			name:                    "should not update LBTH before blocks per dist trans block are passed",
 			prepareRewardDist:       false,
 			corruptTransChannel:     false,
 			expLBThUpdated:          false,
+			denomRegistered:         true,
 			expEscrowBalanceChanged: false,
 		},
 		{
 			name:                    "should update LBTH when blocks per dist trans or more block are passed",
 			prepareRewardDist:       true,
 			corruptTransChannel:     false,
 			expLBThUpdated:          true,
+			denomRegistered:         true,
 			expEscrowBalanceChanged: true,
 		},
 		{
 			name:                    "should update LBTH and discard the IBC transfer states when sending rewards to provider fails",
 			prepareRewardDist:       true,
 			corruptTransChannel:     true,
 			expLBThUpdated:          true,
+			denomRegistered:         true,
 			expEscrowBalanceChanged: false,
 		},
+		{
+			name:                    "should not change escrow balance when denom is not registered",
+			prepareRewardDist:       true,
+			corruptTransChannel:     false,
+			expLBThUpdated:          true,
+			denomRegistered:         false,
+			expEscrowBalanceChanged: false,
+		},
+		{
+			name:                    "should change escrow balance when denom is registered",
+			prepareRewardDist:       true,
+			corruptTransChannel:     false,
+			expLBThUpdated:          true,
+			denomRegistered:         true,
+			expEscrowBalanceChanged: true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -195,6 +280,12 @@ func (s *CCVTestSuite) TestEndBlockRD() {
 		delegate(s, delAddr, bondAmt)
 		s.providerChain.NextBlock()
 
+		if tc.denomRegistered {
+			params := s.consumerApp.GetConsumerKeeper().GetParams(s.consumerCtx())
+			params.RewardDenoms = []string{sdk.DefaultBondDenom}
+			s.consumerApp.GetConsumerKeeper().SetParams(s.consumerCtx(), params)
+		}
+
 		// relay VSC packets from provider to consumer
 		relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, 1)
 

tests/integration/actions.go

@@ -1166,6 +1166,36 @@ func (tr TestRun) registerRepresentative(
 	wg.Wait()
 }
 
+type registerConsumerRewardDenomAction struct {
+	chain chainID
+	from  validatorID
+	denom string
+}
+
+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,
+
+		`--from`, `validator`+fmt.Sprint(action.from),
+		`--chain-id`, string(action.chain),
+		`--home`, tr.getValidatorHome(action.chain, action.from),
+		`--node`, tr.getValidatorNode(action.chain, action.from),
+		`--gas`, "9000000",
+		`--keyring-backend`, `test`,
+		`-b`, `block`,
+		`-y`,
+	).CombinedOutput()
+
+	if verbose {
+		fmt.Println("redelegate cmd:", string(bz))
+	}
+
+	if err != nil {
+		log.Fatal(err, "\n", string(bz))
+	}
+}
+
 // Creates an additional node on selected chain
 // by copying an existing validator's home folder
 //