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

docs: Add ticket templates for RPC methods and state format upgrades #7738

Open
2 of 4 tasks
teor2345 opened this issue Oct 12, 2023 · 2 comments
Open
2 of 4 tasks

docs: Add ticket templates for RPC methods and state format upgrades #7738

teor2345 opened this issue Oct 12, 2023 · 2 comments
Labels
A-docs Area: Documentation A-rpc Area: Remote Procedure Call interfaces A-state Area: State / database changes C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG extra-reviews This PR needs at least 2 reviews to merge S-needs-triage Status: A bug report needs triage

Comments

@teor2345
Copy link
Collaborator

teor2345 commented Oct 12, 2023

Motivation

Motivation

We have learnt a lot about reliably implementing RPCs and in-place state format upgrades over the past few years. We have a well-defined process for RPCs, and we need a well-defined process for state upgrades.

It might also help to upgrade the PR template to add a checklist for the PR author, because we often forget tests or doc updates in PRs.

Rough Plan

Checklist:

Author:

  • add or update tests
  • update function documentation

Reviewer:

  • edit tests to say: "do the tests cover the goals of the ticket?"
  • does it match the ticket scope?
  • what is needed to merge this PR?
  • can the PR be split? Can we open other PRs for cleanup?

Review

We should get multiple developers to read and review these templates, to make sure we can all understand and use them.

Related Work

The state upgrade template might reference the docs in #7737.

@teor2345 teor2345 added A-docs Area: Documentation C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Low ❄️ A-rpc Area: Remote Procedure Call interfaces A-state Area: State / database changes extra-reviews This PR needs at least 2 reviews to merge labels Oct 12, 2023
@teor2345 teor2345 self-assigned this Oct 12, 2023
@teor2345 teor2345 added C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG and removed C-enhancement Category: This is an improvement labels Oct 12, 2023
@teor2345
Copy link
Collaborator Author

Here is an updated checklist which was taken out of the state docs:

Implementation Steps

  • update the database format in the Zebra docs
  • increment the state minor version
  • create a new module for the upgrade and its format checks
  • refactor the write methods so they can be used for the upgrade and block writes
    • write the new format in the block write task
    • update older formats in the format upgrade task
  • write detailed format checks for the new format (quick checks are optional)
  • test that the new format works when fully syncing a new state, and updating an older state

See the upgrade design docs for more details.

@teor2345
Copy link
Collaborator Author

teor2345 commented Oct 22, 2023

For the state format upgrade template:

Required Tests

  • Randomised property tests on any new format serializations
  • Unit tests for any calculations or data processing
  • A state validity check that checks the entire format, or if that would take hours:
    • A quick check that makes sure a few blocks of the format are correct, and
    • A detailed check that covers all the alternative code paths
  • A manual full sync
  • An upgrade from the latest supported Zebra version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation A-rpc Area: Remote Procedure Call interfaces A-state Area: State / database changes C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG extra-reviews This PR needs at least 2 reviews to merge S-needs-triage Status: A bug report needs triage
Projects
Status: New
Development

No branches or pull requests

2 participants