Skip to content

Commit

Permalink
addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bermuell committed Aug 7, 2024
1 parent 35978c7 commit 6c92999
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 139 deletions.
3 changes: 0 additions & 3 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ import (
no_valupdates_genutil "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_genutil"
no_valupdates_staking "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_staking"
ibcprovider "github.com/cosmos/interchain-security/v5/x/ccv/provider"
ibcproviderclient "github.com/cosmos/interchain-security/v5/x/ccv/provider/client"
ibcproviderkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
)
Expand Down Expand Up @@ -144,7 +143,6 @@ var (
gov.NewAppModuleBasic(
[]govclient.ProposalHandler{
paramsclient.ProposalHandler,
ibcproviderclient.ChangeRewardDenomsProposalHandler,
},
),
mint.AppModuleBasic{},
Expand Down Expand Up @@ -576,7 +574,6 @@ func New(
govtypes.ModuleName: gov.NewAppModuleBasic(
[]govclient.ProposalHandler{
paramsclient.ProposalHandler,
ibcproviderclient.ChangeRewardDenomsProposalHandler,
},
),
})
Expand Down
34 changes: 13 additions & 21 deletions docs/docs/upgrading/migrate_v4_v5.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,36 +159,28 @@ interchain-security-pd q provider params -o json

### Governance proposals

Submitting the following legacy proposals is still supported:

# Consumer addition proposal

```shell
interchain-security-pd tx gov submit-proposal <proposal_file.json>
interchain-security-pd tx gov submit-legacy-proposal consumer-addition <proposal_file.json>
```

# Consumer removal proposal

```shell
interchain-security-pd tx gov submit-proposal <proposal_file.json>
interchain-security-pd tx gov submit-legacy-proposal consumer-removal <proposal_file.json>
```

# Consumer modification proposal
# Consumer addition proposal
```shell
interchain-security-pd tx gov submit-proposal <proposal_file.json>
interchain-security-pd tx gov submit-legacy-proposal change-reward-denoms <proposal_file.json>
```

Run `interchain-security-pd tx gov draft-proposal` command and select in `other` one of the following
message types to generate a draft example:
- `/interchain_security.ccv.provider.v1.MsgConsumerAddition`
- `/interchain_security.ccv.provider.v1.MsgConsumerModification`
- `/interchain_security.ccv.provider.v1.MsgConsumerRemoval`
You may also submit proposal messages above using `submit-proposal`.


# Change reward denoms

```shell
interchain-security-pd tx gov submit-legacy-proposal change-reward-denoms <proposal_file.json>
```

## Consumer

### Keeper initialization
Expand Down Expand Up @@ -234,22 +226,22 @@ app.ConsumerKeeper = ibcconsumerkeeper.NewKeeper(
// for x/ccv/democracy using the x/gov module address is correct
// if you don't have a way of updating consumer params you may still use the line below as it will have no affect
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),

// add address codecs
// add address codecs
+ authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ValidatorAddrPrefix()),
+ authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ConsensusAddrPrefix()),
)
```


* `authority` was added - requirement for executing `MsgUpdateParams`
* make sure the authority address makes sense for your chain
* make sure the authority address makes sense for your chain
* the exact module account may differ depending on your setup (`x/gov`, `x/admin` or custom module)
* for `x/ccv/democracy` using the `x/gov` module address is correct
* if you don't have a way of updating consumer params you may use `authtypes.NewModuleAddress(govtypes.ModuleName).String()` (has no effect on functionality)

* `validatorAddressCodec` & `consensusAddressCodec` were added - they must match the bech32 address codec used by `x/auth`, `x/bank`, `x/staking`


### Additions

Expand All @@ -258,7 +250,7 @@ app.ConsumerKeeper = ibcconsumerkeeper.NewKeeper(
**This functionality is not supported on `x/ccv/consumer` without additional configuration.**
* if you are using `x/ccv/democracy` the feature is supported out of the box
* if you are using custom logic for changing consumer params, please update your code by providing the appropriate `authority` module account during `ConsumerKeeper` initialization in `app.go`.

**You must add `"/interchain_security.ccv.consumer.v1.MsgUpdateParams"` to your parameters whitelist to be able to change `ccvconsumer` parameters via governance.**

It is available when using `gov` CLI commands:
Expand Down Expand Up @@ -384,7 +376,7 @@ The consumer implements the `StakingKeeper` interface shown above.

## Democracy

Changes in `Consumer` also apply to `Democracy`.
Changes in `Consumer` also apply to `Democracy`.

Democracy `x/staking`, `x/distribution` and `x/gov` were updated to reflect changes in `cosmos-sdk v0.50.x`.

Expand Down
42 changes: 42 additions & 0 deletions docs/docs/upgrading/migrate_v5_v6.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
sidebar_position: 1
---

# Upgrading to ICS v6.x from v5.x

ICS specific changes are outlined below.

Pre-requisite version for this upgrade: any from the `v5.x` release line.

## Note

## Provider

### Migration (v5 -> v6)

ConensusVersion was bumped to `6`.

### Governance proposals

#### Consumer addition proposal

```shell
interchain-security-pd tx gov submit-proposal <proposal_file.json>
```

#### Consumer removal proposal

```shell
interchain-security-pd tx gov submit-proposal <proposal_file.json>
```

#### Consumer modification proposal
```shell
interchain-security-pd tx gov submit-proposal <proposal_file.json>
```

Run `interchain-security-pd tx gov draft-proposal` command and select in `other` one of the following
message types to generate a draft example:
- `/interchain_security.ccv.provider.v1.MsgConsumerAddition`
- `/interchain_security.ccv.provider.v1.MsgConsumerModification`
- `/interchain_security.ccv.provider.v1.MsgConsumerRemoval`
1 change: 1 addition & 0 deletions tests/e2e/action_rapid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func GetActionGen() *rapid.Generator[any] {
func CreateSubmitChangeRewardDenomsProposalActionGen() *rapid.Generator[SubmitChangeRewardDenomsProposalAction] {
return rapid.Custom(func(t *rapid.T) SubmitChangeRewardDenomsProposalAction {
return SubmitChangeRewardDenomsProposalAction{
Chain: GetChainIDGen().Draw(t, "Chain"),
From: GetValidatorIDGen().Draw(t, "From"),
Deposit: rapid.Uint().Draw(t, "Deposit"),
Denom: rapid.String().Draw(t, "Denom"),
Expand Down
80 changes: 47 additions & 33 deletions tests/e2e/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"golang.org/x/mod/semver"

e2e "github.com/cosmos/interchain-security/v5/tests/e2e/testlib"
"github.com/cosmos/interchain-security/v5/x/ccv/provider/client"
"github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types"
)

Expand Down Expand Up @@ -2019,54 +2017,70 @@ func (tr Chain) registerRepresentative(
}

type SubmitChangeRewardDenomsProposalAction struct {
Chain ChainID
Denom string
Deposit uint
From ValidatorID
}

func (tr Chain) submitChangeRewardDenomsProposal(action SubmitChangeRewardDenomsProposalAction, verbose bool) {
providerChain := tr.testConfig.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)
}
template := `
{
"messages": [
{
"@type": "/interchain_security.ccv.provider.v1.MsgChangeRewardDenoms",
"denoms_to_add": %s,
"denoms_to_remove": %s,
"authority": "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn"
}
],
"metadata": "ipfs://CID",
"deposit": "%dstake",
"title": "change reward denoms",
"summary": "Proposal to change reward denoms",
"expedited": false
}`

jsonStr := string(bz)
if strings.Contains(jsonStr, "'") {
log.Fatal("prop json contains single quote")
}
denomsToAdd := []string{action.Denom}
denomsToRemove := []string{"stake"}
jsonStr := fmt.Sprintf(template,
denomsToAdd,
denomsToRemove,
action.Deposit)

bz, err = tr.target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, "/change-reward-denoms-proposal.json")).CombinedOutput()
//#nosec G204 -- bypass unsafe quoting warning (no production code)
proposalFile := "/consumer-addition.proposal"
bz, err := tr.target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, jsonStr, proposalFile),
).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

// CHANGE REWARDS DENOM PROPOSAL
bz, err = tr.target.ExecCommand(providerChain.BinaryName,
"tx", "gov", "submit-legacy-proposal", "change-reward-denoms", "/change-reward-denoms-proposal.json",
cmd := tr.target.ExecCommand(
tr.testConfig.chainConfigs[action.Chain].BinaryName,
"tx", "gov", "submit-proposal", proposalFile,
`--from`, `validator`+fmt.Sprint(action.From),
`--chain-id`, string(providerChain.ChainId),
`--home`, tr.getValidatorHome(providerChain.ChainId, action.From),
`--node`, tr.getValidatorNode(providerChain.ChainId, action.From),
`--gas`, "9000000",
`--chain-id`, string(tr.testConfig.chainConfigs[action.Chain].ChainId),
`--home`, tr.getValidatorHome(action.Chain, action.From),
`--gas`, `900000`,
`--node`, tr.getValidatorNode(action.Chain, action.From),
`--keyring-backend`, `test`,
`-y`,
).CombinedOutput()
)

if verbose {
fmt.Println("change rewards denom props cmd:", cmd.String())
fmt.Println("change rewards denom props json:", jsonStr)
}
bz, err = cmd.CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
log.Fatal("submit-proposal failed:", err, "\n", string(bz))
}

if verbose {
fmt.Println("change rewards denom props output:", string(bz))
}

// wait for inclusion in a block -> '--broadcast-mode block' is deprecated
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/steps_democracy.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func stepsDemocracy(consumerName string, expectRegisteredRewardDistribution bool
},
{
Action: SubmitChangeRewardDenomsProposalAction{
Chain: ChainID("provi"),
Denom: consumerRewardDenom,
Deposit: 10000001,
From: ValidatorID("bob"),
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/provider_gov_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (s *CCVTestSuite) TestGetConsumerAdditionFromProp() {
s.Require().NoError(err)

// create a valid consumer addition proposal
addConsumerProp := testkeeper.GetTestMsgConsumerAddition()
msgConsumerAddition := testkeeper.GetTestMsgConsumerAddition()

// create a legacy consumer addition proposal content
// (not supported anymore)
Expand Down Expand Up @@ -107,8 +107,8 @@ func (s *CCVTestSuite) TestGetConsumerAdditionFromProp() {
expPanic: false,
},
{
name: "msg contains a prop of legacy ConsumerAdditionProposal type - hook should create a new proposed chain",
propMsg: &addConsumerProp,
name: "msg contains a prop of MsgConsumerAddition type - hook should create a new proposed chain",
propMsg: &msgConsumerAddition,
expectConsumerPropFound: true,
expPanic: false,
},
Expand Down
76 changes: 0 additions & 76 deletions x/ccv/provider/client/legacy_proposal_handler.go

This file was deleted.

4 changes: 1 addition & 3 deletions x/ccv/provider/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterImplementations(
(*sdk.Msg)(nil),
&MsgAssignConsumerKey{},
&MsgConsumerAddition{}, //TOOD @permissionless: replace with below
//&MsgInitializeConsumer{}, //TOOD @permissionless: uncomment once implemented
//&MsgUpdateConsumer{}, //TOOD @permissionless: uncomment once implemented
&MsgConsumerAddition{},
&MsgConsumerRemoval{},
&MsgChangeRewardDenoms{},
&MsgUpdateParams{},
Expand Down

0 comments on commit 6c92999

Please sign in to comment.