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

Transaction Tips & Middlewares Readiness Checklist #11087

Closed
21 of 24 tasks
Tracked by #11096
amaury1093 opened this issue Feb 1, 2022 · 1 comment · Fixed by #11632
Closed
21 of 24 tasks
Tracked by #11096

Transaction Tips & Middlewares Readiness Checklist #11087

amaury1093 opened this issue Feb 1, 2022 · 1 comment · Fixed by #11632
Assignees
@blushi @atheeshp
Labels
module-readiness-checklist

Comments

@amaury1093
Copy link
Contributor

amaury1093 commented Feb 1, 2022 •
edited by likhita-809

"Transaction Tips & Middlewares" Readiness Checklist

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

See Epic: #9557

Release Candidate Checklist

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

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

The main folders to look at are:

  • proto/cosmos/tx (for the new sign mode and for the Tip definition),
  • baseapp (for middleware setup, as well as baseapp logic ported to middlewares),
  • x/auth/middleware (for middlewares),
  • x/auth/tx (for the new sign mode),
  • client/tx (for the new sign mode).

Audit:

  • API audit (at least 1 person) (@blushi ) chore: Tx Tips API audit #11641
    • Are middlewares well-named and well-documented?
    • Does the new AuxSignerData have well-defined fields?
    • Does the new SignDocDirectAux have well-defined fields?
    • Does the StdSignDoc have well-defined fields for tip and fee payer?
    • Does the new AuxTxBuilder have a good API for the tipper?
    • Does the existing TxBuilder have a good API for the fee payer dealing with tips?
    • Is everything well documented (inline godoc, docs, ADRs) ?
  • State machine audit (at least 2 people) (@blushi, @likhita-809)
    • Read through the baseapp code that has been ported to middlewares, and verify correctness upon visual inspection. A git diff baseapp/ can be helpful: 5326280..v0.46.0-beta2. @blushi
    • Read through all existing antehandler code and verify it has been correctly ported to middlewares (@likhita-809 )
    • Verify correctness of the TipMiddleware (@likhita-809 )
    • Verify correctness of DIRECT_AUX sign mode handler (@likhita-809 )
    • Ensure all state machine code which could be confusing is properly commented (@likhita-809 )
    • Make sure state machine logic matches middlewares documentation (@likhita-809 )
    • Ensure that all state machine edge cases are covered with tests and that test coverage is sufficient (at least 80% coverage on middleware code) (@atheeshp ) chore: add more tests in middleware for code cov #11694
    • Assess potential threats for new middlewares (Tip, baseapp-ported middlewares), including spam attacks @likhita-809
    • Assess potential threats for new sign modes including spam attacks and ensure that threats have been addressed sufficiently. One important threat to double-check is that SIGN_MODE_DIRECT_AUX and SIGN_MODE_LEGACY_AMINO_JSON both don't introduce malleability issues. (@blushi)
    • 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) @atheeshp ) chore: tips cli audit changes #11632
    • CLI methods for tipper and fee-payer

Published Release Checklist

After the above checks have been audited and the tips & middleware refactor 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)
  • Nice to have (and needed in some cases if threats could be high): Official 3rd party audit
@tac0turtle tac0turtle added this to the v0.46 milestone Feb 2, 2022
This was referenced Mar 9, 2022
Closed
Closed
@atheeshp atheeshp mentioned this issue Apr 13, 2022
Merged
19 tasks
@mergify mergify bot closed this as completed in #11632 Apr 14, 2022
mergify bot pushed a commit that referenced this issue Apr 14, 2022
@atheeshp
chore: tips cli audit changes (#11632)
958d63b 
## Description

Closes: #11087 



---

### 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)
@amaury1093 amaury1093 reopened this Apr 14, 2022
@blushi blushi mentioned this issue Apr 14, 2022
Merged
19 tasks
mergify bot pushed a commit that referenced this issue Apr 14, 2022
@blushi
chore: Tx Tips API audit (#11641)
0dbf7dc 
## Description

ref: #11087



---

### 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)
@atheeshp atheeshp mentioned this issue Apr 20, 2022
Merged
19 tasks
@amaury1093
Copy link
Contributor Author

Marking tips as beta for v0.46, #12089 . The code is audited, but still untested yet. I'll mark this issue as closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@blushi blushi

@atheeshp atheeshp

Labels
module-readiness-checklist
Projects
Cosmos-SDK
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: tips cli audit changes cosmos/cosmos-sdk
6 participants
@amaury1093 @blushi @JeancarloBarrios @tac0turtle @cmwaters @atheeshp