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.

Sign up for GitHub

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

Jump to bottom

feat!: Removed legacy-submit-proposal support for consumer addition/modification/removal #2130

Merged
merged 11 commits into from
Aug 14, 2024
Merged

feat!: Removed legacy-submit-proposal support for consumer addition/modification/removal #2130

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
80ad0a7
Remove legacy-submit-proposal consumer-addition
bermuell Aug 6, 2024
7577eb6
Remove legacy-submit-proposal consumer-removal
bermuell Aug 6, 2024
2833350
Remove legacy-submit-proposal consumer-modification
bermuell Aug 6, 2024
6257f41
docs
bermuell Aug 6, 2024
35978c7
fix e2e
bermuell Aug 6, 2024
6c92999
addressed review comments
bermuell Aug 7, 2024
942910f
fix compatibility tests
bermuell Aug 7, 2024
f499cd9
addressed some more review comments
bermuell Aug 12, 2024
a4630a5
addressed PR comments
bermuell Aug 13, 2024
55835d1
fix e2e when run on current ws (invalid sem version)
bermuell Aug 13, 2024
bd26ec6
Apply suggestions from code review
bermuell Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changelog/unreleased/api-breaking/provider/2130-remove-legacy-proposal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Remove support for legacy-proposal to add/modify/remove consumer proposals
([\#2130](https://github.com/cosmos/interchain-security/pull/2130))
2 changes: 1 addition & 1 deletion .github/workflows/nightly-e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
# Run compatibility tests for different consumer (-cv) and provider (-pv) versions.
# Combination of all provider versions with consumer versions are tested.
# For new versions to be tested add/modify -pc/-cv parameters.
run: go run ./tests/e2e/... --tc compatibility -pv latest -pv v4.3.0-lsm -pv v3.3.3-lsm -cv latest -cv v4.3.0 -cv v3.3.0
run: go run ./tests/e2e/... --tc compatibility -pv latest -pv v4.3.1-lsm -pv v3.3.3-lsm -cv latest -cv v4.4.0 -cv v3.3.0
happy-path-test:
runs-on: ubuntu-latest
timeout-minutes: 20
Expand Down
9 changes: 0 additions & 9 deletions app/provider/app.go
View file
Open in desktop
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,10 +143,6 @@ var (
gov.NewAppModuleBasic(
[]govclient.ProposalHandler{
paramsclient.ProposalHandler,
ibcproviderclient.ConsumerAdditionProposalHandler,
ibcproviderclient.ConsumerRemovalProposalHandler,
ibcproviderclient.ConsumerModificationProposalHandler,
ibcproviderclient.ChangeRewardDenomsProposalHandler,
},
),
mint.AppModuleBasic{},
Expand Down Expand Up @@ -579,10 +574,6 @@ func New(
govtypes.ModuleName: gov.NewAppModuleBasic(
[]govclient.ProposalHandler{
paramsclient.ProposalHandler,
ibcproviderclient.ConsumerAdditionProposalHandler,
ibcproviderclient.ConsumerRemovalProposalHandler,
ibcproviderclient.ConsumerModificationProposalHandler,
ibcproviderclient.ChangeRewardDenomsProposalHandler,
},
),
})
Expand Down
42 changes: 42 additions & 0 deletions docs/docs/upgrading/migrate_v5_v6.md
View file
Open in desktop
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
bermuell marked this conversation as resolved.
Show resolved Hide resolved

ICS specific changes are outlined below.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hyphenate "ICS specific".

When 'ICS-specific' is used as a modifier, it should be spelled with a hyphen.

- ICS specific changes are outlined below.
+ ICS-specific changes are outlined below.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Upgrading to ICS v6.x from v5.x
ICS specific changes are outlined below.
# Upgrading to ICS v6.x from v5.x
ICS-specific changes are outlined below.
Tools
LanguageTool

[uncategorized] ~6-~6: When ‘ICS-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...--- # Upgrading to ICS v6.x from v5.x ICS specific changes are outlined below. Pre-requis...

(SPECIFIC_HYPHEN)


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

## Note

## Provider

### Migration (v5 -> v6)

ConensusVersion was bumped to `6`.
bermuell marked this conversation as resolved.
Show resolved Hide resolved

### Governance proposals

#### Consumer addition proposal

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

#### Consumer removal proposal
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove multiple spaces after hash in headings.

There are multiple spaces after the hash in the atx-style heading.

- ####  Consumer removal proposal
+ #### Consumer removal proposal
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### Consumer removal proposal
#### Consumer removal proposal
Tools
Markdownlint

27-27: null
Multiple spaces after hash on atx style heading

(MD019, no-multiple-space-atx)

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove multiple spaces after hash in headings.

There are multiple spaces after the hash in the atx-style headings.

- ####  Consumer removal proposal
+ #### Consumer removal proposal

Also applies to:

- ####  Consumer modification proposal
+ #### Consumer modification proposal

Also applies to: 33-33

Tools
Markdownlint

27-27: null
Multiple spaces after hash on atx style heading

(MD019, no-multiple-space-atx)

bermuell marked this conversation as resolved.
Show resolved Hide resolved

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

#### Consumer modification proposal
```shell
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix formatting around heading and code block.

Headings and fenced code blocks should be surrounded by blank lines, and there should be no multiple spaces after the hash in the heading.

- ####  Consumer modification proposal
- ```shell
+ #### Consumer modification proposal

+ ```shell
Tools
Markdownlint

33-33: null
Multiple spaces after hash on atx style heading

(MD019, no-multiple-space-atx)

33-33: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)

34-34: null
Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

Copy link
Contributor

Choose a reason for hiding this comment

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

Add blank lines around headings and code blocks.

Headings and fenced code blocks should be surrounded by blank lines.

- #### Consumer modification proposal
- ```shell
+ #### Consumer modification proposal

+ ```shell
Tools
Markdownlint

33-33: null
Multiple spaces after hash on atx style heading

(MD019, no-multiple-space-atx)

33-33: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)

34-34: null
Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend adding a tiny bit of info here, along the lines of:
This was the command before: ```command```, now use this new command instead:

btw, why is this in the migration? I think this should be in the changelog instead, because this doesn't actually have anything to do with migrating the state (which I think the upgrading file is usually)
Maybe this should be in an "integrators guide" like I did for the inactive vals here https://github.com/cosmos/interchain-security/pull/2133/files#diff-d3b7b6fffa7d0deffa451f04bcf224da77c989b9ada8c27598cf32817661cc8f

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was described that way for migrate_v4_v6.md when the new commands were added, so I followed the same parttern.
Will remove this file and add more content to the changelog. Added also migration instructions in UPGRADE file.
Not picking up 'integration guide' idea for now as with permissionless we might provide something like that which would touch the same area (consumer proposals).

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`
bermuell marked this conversation as resolved.
Show resolved Hide resolved
bermuell marked this conversation as resolved.
Show resolved Hide resolved
- `/interchain_security.ccv.provider.v1.MsgConsumerModification`
- `/interchain_security.ccv.provider.v1.MsgConsumerRemoval`
bermuell marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions tests/e2e/action_rapid_test.go
View file
Open in desktop
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
Loading
Loading