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/authz Module Readiness Checklist #8982

Closed
22 of 26 tasks
clevinson opened this issue Mar 24, 2021 · 4 comments
Closed
22 of 26 tasks

x/authz Module Readiness Checklist #8982

clevinson opened this issue Mar 24, 2021 · 4 comments

Comments

@clevinson
Copy link
Contributor

clevinson commented Mar 24, 2021

x/authz 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.

Reports

Prior to beta tag

  • API audit (at least 1 person) (@robert-zaremba , @anilcse)
    • spec audit: check if the spec is complete.
    • 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)
    • check the proto definition - make sure everything is in accordance to ADR-30
  • Completeness audit, fully implemented with tests (at least 1 person) (@robert-zaremba )
    • 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)
    • Assess maintainability and risks of future migrations

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.

  • State machine audit (at least 2 people) (@robert-zaremba @cyberbono3 @anilcse)
    • Read through MsgServer code and verify correctness upon visual inspection
    • Ensure all state machine code which could be confusing is properly commented
    • Make sure state machine logic matches Msg method documentation
    • Ensure that all state machine edge cases are covered with tests and that test coverage is sufficient (at least 90% coverage on module code)
    • 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. Specifically we should be paying attention to:
      • algorithmic complexity and places this could be exploited (ex. nested for loops)
      • charging gas complex computation (ex. for loops)
      • Storage is safe (we don't pollute the state).
    • Assess potential risks of any new third party dependencies and decide whether a dependency audit is needed

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):

  • Code snippets update after merging x/authz: audit updates #9042.
  • Testnet / devnet testing (2-3 people) (@robert-zaremba @cyberbono3 @anilcse )
    • 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
@robert-zaremba
Copy link
Collaborator

I've added few more entries to the checklist.
The state machines checks we will do in pairs:
Robert: keeper (Wed 20:00 CEST), client (Tue 17:00 CEST)
Anil: client, simulation
Andrei: keeper, simulation

@blushi
Copy link
Contributor

blushi commented Mar 30, 2021

@robert-zaremba more generally, should we update the module readiness checklist template to account for those new entries and have it consistent across all audited modules?

@robert-zaremba
Copy link
Collaborator

@blushi - yes, that would be good. The checks are

  • Storage is safe (we don't pollute the state).
  • make sure everything is in accordance to the respective ADR (in case of authz it's ADR-30)

@blushi
Copy link
Contributor

blushi commented Mar 31, 2021

@blushi - yes, that would be good. The checks are

  • Storage is safe (we don't pollute the state).
  • make sure everything is in accordance to the respective ADR (in case of authz it's ADR-30)

We have also added simulation as part of the Module Readiness Checklist for feegrant as it was not already explicitly stated in the checklist.
Maybe let's discuss and align on those new checks before updating the Module Readiness Checklist template but afaic, those make sense to me.

cc/ @clevinson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants