Skip to content

Commit

Permalink
feat!: provider proposal for changing reward denoms (backport #1280) (#…
Browse files Browse the repository at this point in the history 
…1291)

* feat!: provider proposal for changing reward denoms (#1280)

* new provider prop type

* add methods and tests for new prop, update docs

* remove old tx, fix tests

* e2e handling

* fix command type

* boilerplate

* fix e2e tests

* Update CHANGELOG.md

* lint

* validate denoms

* Update proposal.go

* rm msg string

* fix tests

* rm chain in change denom action

* lint

* test for invalid denom

* events for both add and remove

* Update proposal_test.go

(cherry picked from commit 48a2186)

# Conflicts:
#	CHANGELOG.md
#	app/provider/app.go
#	proto/interchain_security/ccv/provider/v1/provider.proto
#	proto/interchain_security/ccv/provider/v1/tx.proto
#	tests/e2e/actions.go
#	tests/integration/distribution.go
#	x/ccv/provider/client/cli/tx.go
#	x/ccv/provider/client/proposal_handler.go
#	x/ccv/provider/keeper/distribution.go
#	x/ccv/provider/keeper/distribution_test.go
#	x/ccv/provider/proposal_handler_test.go
#	x/ccv/provider/types/codec.go
#	x/ccv/provider/types/proposal.go
#	x/ccv/provider/types/provider.pb.go
#	x/ccv/provider/types/tx.pb.go

* fix conflicts

* fix rest handler

* Update CHANGELOG.md

* rm uneeded tx proto

---------

Co-authored-by: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com>
  • Loading branch information
@mergify @shaspitz
mergify[bot] and shaspitz authored Sep 12, 2023
1 parent 6b0a94a commit 494c4d2
Show file tree
Hide file tree
Showing 26 changed files with 903 additions and 797 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

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.

## v2.1.0-lsm-provider

* (feature!) [#1280](https://github.com/cosmos/interchain-security/pull/1280) provider proposal for changing reward denoms

## v2.0.0-lsm

Date: August 18th, 2023
Expand Down
1 change: 1 addition & 0 deletions app/provider/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ var (
ibcproviderclient.ConsumerAdditionProposalHandler,
ibcproviderclient.ConsumerRemovalProposalHandler,
ibcproviderclient.EquivocationProposalHandler,
ibcproviderclient.ChangeRewardDenomsProposalHandler,
),
params.AppModuleBasic{},
crisis.AppModuleBasic{},
Expand Down
17 changes: 17 additions & 0 deletions docs/docs/features/proposals.md
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,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.
message GlobalSlashEntry {
Expand Down
15 changes: 0 additions & 15 deletions proto/interchain_security/ccv/provider/v1/tx.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import "google/protobuf/any.proto";
// Msg defines the Msg service.
service Msg {
rpc AssignConsumerKey(MsgAssignConsumerKey) returns (MsgAssignConsumerKeyResponse);
rpc RegisterConsumerRewardDenom(MsgRegisterConsumerRewardDenom) returns (MsgRegisterConsumerRewardDenomResponse);
}

message MsgAssignConsumerKey {
Expand All @@ -29,17 +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 {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string denom = 1;
string depositor = 2;
}

// MsgRegisterConsumerRewardDenomResponse defines the Msg/RegisterConsumerRewardDenom response type.
message MsgRegisterConsumerRewardDenomResponse {}
59 changes: 44 additions & 15 deletions tests/e2e/actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1675,34 +1675,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()

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`,
`-b`, `block`,
`-y`,
).CombinedOutput()

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

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

// wait for inclusion in a block -> '--broadcast-mode block' is deprecated
tr.waitBlocks(chainID("provi"), 2, 30*time.Second)
}

// Creates an additional node on selected chain
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,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
34 changes: 3 additions & 31 deletions tests/integration/distribution.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ 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/v2/x/ccv/consumer/types"
providertypes "github.com/cosmos/interchain-security/v2/x/ccv/provider/types"
ccv "github.com/cosmos/interchain-security/v2/x/ccv/types"
Expand Down Expand Up @@ -92,41 +92,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
38 changes: 0 additions & 38 deletions x/ccv/provider/client/cli/tx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@ 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"
"github.com/cosmos/cosmos-sdk/version"

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

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

return cmd
}
Expand Down Expand Up @@ -65,37 +61,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

0 comments on commit 494c4d2

Please sign in to comment.