Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: provider proposal for changing reward denoms #1280

Merged
merged 18 commits into from
Sep 12, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

Add an entry to the unreleased provider section whenever merging a PR to main that is not targeted at a specific release. These entries will eventually be included in a provider release.

* (feature!) [#1280](https://github.com/cosmos/interchain-security/pull/1280) provider proposal for changing reward denoms
* (feature!) [#1244](https://github.com/cosmos/interchain-security/pull/1244) Update the default consumer unbonding period to 2 weeks.
* (deps) [#1259](https://github.com/cosmos/interchain-security/pull/1259) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.47.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.5).
* (deps!) [#1258](https://github.com/cosmos/interchain-security/pull/1258) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v7.3.0](https://github.com/cosmos/ibc-go/releases/tag/v7.3.0).
Expand Down
1 change: 1 addition & 0 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ var (
ibcproviderclient.ConsumerAdditionProposalHandler,
ibcproviderclient.ConsumerRemovalProposalHandler,
ibcproviderclient.EquivocationProposalHandler,
ibcproviderclient.ChangeRewardDenomsProposalHandler,
},
),
params.AppModuleBasic{},
Expand Down
17 changes: 17 additions & 0 deletions docs/docs/features/proposals.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,23 @@ Minimal example:
}
```

## ChangeRewardDenomProposal
:::tip
`ChangeRewardDenomProposal` will only be accepted on the provider chain if at least one of the denomsToAdd or denomsToRemove fields is populated with at least one denom. Also, a denom cannot be repeated in both sets.
:::

Proposal type used to mutate the set of denoms accepted by the provider as rewards.

Minimal example:
```js
{
"title": "Add untrn as a reward denom",
"description": "Here is more information about the proposal",
"denomsToAdd": ["untrn"],
"denomsToRemove": []
}
```

### Notes
When submitting equivocation evidence through an `EquivocationProposal` please take note that you need to use the consensus address (`valcons`) of the offending validator on the **provider chain**.
Besides that, the `height` and the `time` fields should be mapped to the **provider chain** to avoid your evidence being rejected.
Expand Down
13 changes: 13 additions & 0 deletions proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,19 @@ message EquivocationProposal {
repeated cosmos.evidence.v1beta1.Equivocation equivocations = 3;
}

// ChangeRewardDenomsProposal is a governance proposal on the provider chain to
// mutate the set of denoms accepted by the provider as rewards.
message ChangeRewardDenomsProposal {
// the title of the proposal
string title = 1;
// the description of the proposal
string description = 2;
// the list of consumer reward denoms to add
repeated string denoms_to_add = 3;
// the list of consumer reward denoms to remove
repeated string denoms_to_remove = 4;
}

// A persisted queue entry indicating that a slash packet data instance needs to
// be handled. This type belongs in the "global" queue, to coordinate slash
// packet handling times between consumers.
Expand Down
17 changes: 0 additions & 17 deletions proto/interchain_security/ccv/provider/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import "google/protobuf/any.proto";
service Msg {
rpc AssignConsumerKey(MsgAssignConsumerKey)
returns (MsgAssignConsumerKeyResponse);
rpc RegisterConsumerRewardDenom(MsgRegisterConsumerRewardDenom)
returns (MsgRegisterConsumerRewardDenomResponse);
}

message MsgAssignConsumerKey {
Expand All @@ -30,18 +28,3 @@ message MsgAssignConsumerKey {
}

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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Remember to tag the protos on https://buf.build/cosmos/interchain-security with the specific version this ends up in.

option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string denom = 1;
string depositor = 2;
}

// MsgRegisterConsumerRewardDenomResponse defines the
// Msg/RegisterConsumerRewardDenom response type.
message MsgRegisterConsumerRewardDenomResponse {}
58 changes: 43 additions & 15 deletions tests/e2e/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1751,35 +1751,63 @@ func (tr TestRun) registerRepresentative(
wg.Wait()
}

type registerConsumerRewardDenomAction struct {
chain chainID
from validatorID
denom string
type submitChangeRewardDenomsProposalAction struct {
denom string
deposit uint
from validatorID
}

func (tr TestRun) registerConsumerRewardDenom(action registerConsumerRewardDenomAction, verbose bool) {
func (tr TestRun) submitChangeRewardDenomsProposal(action submitChangeRewardDenomsProposalAction, verbose bool) {
providerChain := tr.chainConfigs[chainID("provi")]

prop := client.ChangeRewardDenomsProposalJSON{
Summary: "Change reward denoms",
ChangeRewardDenomsProposal: types.ChangeRewardDenomsProposal{
Title: "Change reward denoms",
Description: "Change reward denoms",
DenomsToAdd: []string{action.denom},
DenomsToRemove: []string{"stake"},
},
Deposit: fmt.Sprint(action.deposit) + `stake`,
}

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 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,
bz, err = exec.Command("docker", "exec", tr.containerConfig.instanceName,
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/change-reward-denoms-proposal.json")).CombinedOutput()

Check failure

Code scanning / CodeQL

Potentially unsafe quoting

If this [JSON value](1) contains a single quote, it could break out of the enclosing quotes.

if err != nil {
log.Fatal(err, "\n", string(bz))
}

//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
// CHANGE REWARDS DENOM PROPOSAL
bz, err = exec.Command("docker", "exec", tr.containerConfig.instanceName, providerChain.binaryName,
"tx", "gov", "submit-legacy-proposal", "change-reward-denoms", "/change-reward-denoms-proposal.json",
`--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),
`--chain-id`, string(providerChain.chainId),
`--home`, tr.getValidatorHome(providerChain.chainId, action.from),
`--node`, tr.getValidatorNode(providerChain.chainId, action.from),
`--gas`, "9000000",
`--keyring-backend`, `test`,
`-y`,
).CombinedOutput()

if verbose {
fmt.Println("redelegate cmd:", string(bz))
}

if err != nil {
log.Fatal(err, "\n", string(bz))
}

tr.waitBlocks(action.chain, 2, 10*time.Second)
// wait for inclusion in a block -> '--broadcast-mode block' is deprecated
tr.waitBlocks(chainID("provi"), 2, 30*time.Second)
p-offtermatt marked this conversation as resolved.
Show resolved Hide resolved
}

// Creates an additional node on selected chain
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ func (tr *TestRun) runStep(step Step, verbose bool) {
tr.waitForSlashThrottleDequeue(action, verbose)
case startRelayerAction:
tr.startRelayer(action, verbose)
case registerConsumerRewardDenomAction:
tr.registerConsumerRewardDenom(action, verbose)
case submitChangeRewardDenomsProposalAction:
tr.submitChangeRewardDenomsProposal(action, verbose)
default:
log.Fatalf("unknown action in testRun %s: %#v", tr.name, action)
}
Expand Down
26 changes: 18 additions & 8 deletions tests/e2e/steps_democracy.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,29 @@ func stepsDemocracy(consumerName string) []Step {
},
},
{
action: registerConsumerRewardDenomAction{
chain: chainID("provi"),
from: validatorID("bob"),
denom: consumerRewardDenom,
action: submitChangeRewardDenomsProposalAction{
denom: consumerRewardDenom,
deposit: 10000001,
from: validatorID("bob"),
},
state: State{
chainID("provi"): ChainState{
// Denom not yet registered, gov prop needs to pass first
RegisteredConsumerRewardDenoms: &[]string{},
},
},
},
{
action: voteGovProposalAction{
chain: chainID("provi"),
from: []validatorID{validatorID("alice"), validatorID("bob"), validatorID("carol")},
vote: []string{"yes", "yes", "yes"},
propNumber: 2,
},
state: State{
chainID("provi"): ChainState{
// Check that the denom is registered on provider chain
RegisteredConsumerRewardDenoms: &[]string{consumerRewardDenom},
ValBalances: &map[validatorID]uint{
// make sure that bob's account was debited
validatorID("bob"): 9490000000,
},
},
},
},
Expand Down
26 changes: 18 additions & 8 deletions tests/e2e/steps_reward_denom.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,29 @@ func stepsRewardDenomConsumer(consumerName string) []Step {
},
},
{
action: registerConsumerRewardDenomAction{
chain: chainID("provi"),
from: validatorID("bob"),
denom: "ibc/3C3D7B3BE4ECC85A0E5B52A3AEC3B7DFC2AA9CA47C37821E57020D6807043BE9",
action: submitChangeRewardDenomsProposalAction{
denom: "ibc/3C3D7B3BE4ECC85A0E5B52A3AEC3B7DFC2AA9CA47C37821E57020D6807043BE9",
deposit: 10000001,
from: validatorID("bob"),
},
state: State{
chainID("provi"): ChainState{
// Denom not yet registered, gov prop needs to pass first
RegisteredConsumerRewardDenoms: &[]string{},
},
},
},
{
action: voteGovProposalAction{
chain: chainID("provi"),
from: []validatorID{validatorID("alice"), validatorID("bob"), validatorID("carol")},
vote: []string{"yes", "yes", "yes"},
propNumber: 2,
},
state: State{
chainID("provi"): ChainState{
// Check that the denom is registered on provider chain
RegisteredConsumerRewardDenoms: &[]string{"ibc/3C3D7B3BE4ECC85A0E5B52A3AEC3B7DFC2AA9CA47C37821E57020D6807043BE9"},
ValBalances: &map[validatorID]uint{
// make sure that bob's account was debited
validatorID("bob"): 9490000000,
},
},
},
},
Expand Down
33 changes: 2 additions & 31 deletions tests/integration/distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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"

icstestingutils "github.com/cosmos/interchain-security/v3/testutil/integration"
consumerkeeper "github.com/cosmos/interchain-security/v3/x/ccv/consumer/keeper"
Expand Down Expand Up @@ -96,41 +95,13 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
// 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
err = providerBankKeeper.SendCoinsFromAccountToModule(s.providerCtx(), delAddr, govtypes.ModuleName, senderCoins)
s.Require().NoError(err)

// 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
err = providerBankKeeper.SendCoinsFromModuleToAccount(s.providerCtx(), govtypes.ModuleName, delAddr, senderCoins)
s.Require().NoError(err)

// 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))
// Set the consumer reward denom. This would be done by a governance proposal in prod
s.providerApp.GetProviderKeeper().SetConsumerRewardDenom(s.providerCtx(), rewardCoins[ibcCoinIndex].Denom)

s.providerChain.NextBlock()

Expand Down
37 changes: 0 additions & 37 deletions x/ccv/provider/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ package cli

import (
"fmt"
"strings"

"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/version"

"github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
)
Expand All @@ -26,7 +24,6 @@ func GetTxCmd() *cobra.Command {
}

cmd.AddCommand(NewAssignConsumerKeyCmd())
cmd.AddCommand(NewRegisterConsumerRewardDenomCmd())

return cmd
}
Expand Down Expand Up @@ -68,37 +65,3 @@ func NewAssignConsumerKeyCmd() *cobra.Command {

return cmd
}

func NewRegisterConsumerRewardDenomCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "register-consumer-reward-denom [denom]",
Args: cobra.ExactArgs(1),
Short: "Registers a denom that can be sent from consumer chains to all validators and delegators as a reward",
Long: strings.TrimSpace(
fmt.Sprintf(`Registers a denom that can be sent from consumer chains to all validators and delegators as a reward.
Costs a fee, which is specified in genesis.json under the "consumer_reward_denom_fee" key. Will fail if the sending account has an insufficient balance.
Example:
$ %s tx provider register-consumer-reward-denom untrn --from mykey
`,
version.AppName,
),
),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
return err
}
depositorAddr := clientCtx.GetFromAddress()

msg := types.NewMsgRegisterConsumerRewardDenom(args[0], depositorAddr)

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
}

flags.AddTxFlagsToCmd(cmd)

return cmd
}
Loading