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

x/gov v1 Module Readiness Checklist #11086

Closed
18 of 19 tasks
Tracked by #11096 ...
amaury1093 opened this issue Feb 1, 2022 · 6 comments · Fixed by #11567
Closed
18 of 19 tasks
Tracked by #11096 ...

x/gov v1 Module Readiness Checklist #11086

amaury1093 opened this issue Feb 1, 2022 · 6 comments · Fixed by #11567

Comments

@amaury1093
Copy link
Contributor

amaury1093 commented Feb 1, 2022

x/gov v1 Module Readiness Checklist

This checklist is to be used for tracking the final internal audit of new Cosmos SDK modules prior to inclusion in a published release.

Release Candidate Checklist

The following checklist should be gone through once the module has been fully implemented. This audit should be performed directly on master, or preferably on a alpha or beta release tag that includes the module.

The module should not be included in any Release Candidate tag until it has passed this checklist.

The main change in v0.46's gov module is the Msg-based proposal submission and execution path, so everything related to proposals MUST be audited.

Since the gov module has never been internally audited, we propose the reviewers to run through the whole Module Readiness Checklist, and check on existing APIs, logic and docs, making sure they are either left untouched or improved upon.

  • API audit (at least 1 person) (@likhita-809) (tracked in chore: x/gov v1 API audit changes #11474)
    • Are Msg and Query methods and types well-named and organized?
    • Is everything well documented (inline godoc as well as /spec/ folder in module directory)
  • State machine audit (at least 2 people) (@atheeshp, @likhita-809).
    • Read through MsgServer code and verify correctness upon visual inspection (@atheeshp )
    • Ensure all state machine code which could be confusing is properly commented (@likhita-809 ) (tracked in docs: x/gov state machine inline comments changes #11498)
    • Make sure state machine logic matches Msg method documentation (@atheeshp )
    • Ensure that all state machine edge cases are covered with tests and that test coverage is sufficient (at least 90% coverage on module code) (@atheeshp ) (chore: improve code cov #11502)
    • Assess potential threats for each method including spam attacks and ensure that threats have been addressed sufficiently. This should be done by writing up threat assessment for each method (@likhita-809 )
    • Assess potential risks of any new third party dependencies and decide whether a dependency audit is needed (@atheeshp )
  • Completeness audit, fully implemented with tests (at least 1 person) (@blushi ) chore: x/gov v1 Completeness audit #11567
    • Genesis import and export of all state
    • Query services
    • CLI methods
    • All necessary migration scripts are present (if this is an upgrade of existing module)

Published Release Checklist

After the above checks have been audited and the module is included in a tagged Release Candidate, the following additional checklist should be undertaken for live testing, and potentially a 3rd party audit (if deemed necessary):

  • Testnet / devnet testing (2-3 people) (@assignee1, @assignee2, @assignee3) Manual Testing for v0.46 #11880
    • All Msg methods have been tested especially in light of any potential threats identified
    • Genesis import and export has been tested
  • Nice to have (and needed in some cases if threats could be high): Official 3rd party audit
@faddat
Copy link
Contributor

faddat commented Feb 21, 2022

cosmos/ibc-go#949

I think that this might be something that we can use to work on the migration docs, but:

  1. it is very not done yet
  2. Uncertain if we're taking the right path by making all of the ibc gov tx's v1beta2.

@amaury1093
Copy link
Contributor Author

amaury1093 commented Feb 21, 2022

Uncertain if we're taking the right path by making all of the ibc gov tx's v1beta2.

It's definitely the right path. The reason we kept v1beta1 is for apps to have a smoother transition. Whether it should be done now in your PR, or later, is your decision.

@catShaark
Copy link
Contributor

The title and description from the old v1beta1.Content should then be marshal and put into v1beta2.Proposal Metadata right ?

@amaury1093
Copy link
Contributor Author

@catShaark Correct. Another way is to put an URL to IPFS cid in metadata which links to the title/description. See #10490 too.

@amaury1093 amaury1093 mentioned this issue Mar 9, 2022
56 tasks
@amaury1093 amaury1093 changed the title x/gov v1beta2 Module Readiness Checklist x/gov v1 Module Readiness Checklist Mar 22, 2022
@blushi blushi moved this from Ready to In Progress in Cosmos SDK: Gov & Group WG Mar 25, 2022
@atheeshp atheeshp mentioned this issue Mar 30, 2022
19 tasks
mergify bot pushed a commit that referenced this issue Mar 31, 2022
## Description

ref: #11086 

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
mergify bot pushed a commit that referenced this issue Apr 5, 2022
## Description

ref: #11086 

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
mergify bot pushed a commit that referenced this issue Apr 5, 2022
## Description

ref: #11086



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
@blushi blushi moved this from In Progress to In Review in Cosmos SDK: Gov & Group WG Apr 7, 2022
@mergify mergify bot closed this as completed in #11567 Apr 8, 2022
mergify bot pushed a commit that referenced this issue Apr 8, 2022
## Description

Closes: #11086



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
Repository owner moved this from In Review to Done in Cosmos SDK: Gov & Group WG Apr 8, 2022
@amaury1093
Copy link
Contributor Author

Re-opening, there's still some manual testing, in the RC phase

@amaury1093 amaury1093 reopened this Apr 8, 2022
@amaury1093 amaury1093 moved this from Done to Ready in Cosmos SDK: Gov & Group WG Apr 8, 2022
@amaury1093
Copy link
Contributor Author

All done!

Repository owner moved this from Ready to Done in Cosmos SDK: Gov & Group WG Jun 1, 2022
Repository owner moved this from 💪 In Progress to 👏 Done in Cosmos-SDK Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants