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

Tracking: Document Zcash consensus rules in Zebra codebase #3125

Closed
33 of 50 tasks
mpguerra opened this issue Dec 1, 2021 · 4 comments
Closed
33 of 50 tasks

Tracking: Document Zcash consensus rules in Zebra codebase #3125

mpguerra opened this issue Dec 1, 2021 · 4 comments
Assignees
Labels
A-docs Area: Documentation C-enhancement Category: This is an improvement C-research Category: Engineering notes in support of design choices

Comments

@mpguerra
Copy link
Contributor

mpguerra commented Dec 1, 2021

Motivation

We want to ensure that all of the Zcash consensus rules are implemented in Zebra. In order to help us do this, we should document where the Zcash consensus rules are implemented in the Zebra codebase.

This will also help when auditing Zebra.

Tasks for each consensus rule:

  • check that the rule is implemented in Zebra
  • make sure the latest version of the rule is quoted in the docs for the code that implements it
  • make sure the code implements the latest version of the rule

Consensus Rules found in the Zcash Protocol Specification

Consensus ZIPs

Related Work

@mpguerra mpguerra added A-docs Area: Documentation C-research Category: Engineering notes in support of design choices C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Medium labels Dec 1, 2021
@teor2345 teor2345 added the Epic Zenhub Label. Denotes a theme of work under which related issues will be grouped label Dec 7, 2021
@mpguerra mpguerra changed the title Document Zcash consensus rules in Zebra Document Zcash consensus rules in Zebra codebase Dec 10, 2021
@conradoplg
Copy link
Collaborator

conradoplg commented Dec 10, 2021

Thank you @mpguerra for listing those!

How should we carry out this work?

I think the hardest part is identifying each rule. My suggestion is coming up with a way to create an identifier for each one, and them mention each of them in the code where they are implemented. Then we could maybe create a script or just grep the code to list them all, and compare the list to the full list of rules to make sure we didn't forget anything. Eventually we could reach out to ECC to maybe include the identifier in the spec. As long as we have an identifier for each one of them, it's easy to rename them later if needed.

Does this make sense, or is it too much work? Without this it will be really hard to find where each rule is implemented, and to determine if we missed any rule.

My suggestion would look like:

  • Decide on a standard for the rule identifiers (e.g. something like rule-7.3-anchor-sapling-less-qj)
  • Go over the spec and fill an e.g. spreadsheet with the (sub)section, the identifier, and the text of the rule (this can probably be automated somehow, since consensus rules are marked with a macro in the latex source)
  • Decide on how to mention the identifier in the code to make it easy to find
  • For each rule, find where it's implemented and add the mention to the rule
  • List all identifiers in the code, compare to the rule list, make sure we didn't forget anything

@dconnolly @teor2345 @upbqdn what do you think?
I feel like @daira and @str4d probably have thought about this in the past and may already have something in mind 😆 In fact I just saw zcash/zips#468 and zcash/zcash#3957. Anything that we should keep in mind when considering this?

@upbqdn
Copy link
Member

upbqdn commented Dec 10, 2021

I like the general idea of having the consensus rules mapped in a one-to-one fashion between the code and the spec by leveraging unique identifiers. I think it might be helpful even if one consensus rule is scattered around multiple places in the codebase.

To me, the consensus rules resemble math definitions, so having a naming system that would allow referring to each rule by its name would be convenient.

@teor2345
Copy link
Contributor

I think the hardest part is identifying each rule. My suggestion is coming up with a way to create an identifier for each one

I don't think we should create our own identifiers, because we would have to maintain them separately with every spec change. But if the zcashd developers can commit to maintaining the identifiers in the spec, in the timeframe we need, we could use those.

Otherwise linking to the sub-section is good enough for now.

@mpguerra mpguerra removed the Epic Zenhub Label. Denotes a theme of work under which related issues will be grouped label Jan 26, 2022
@mpguerra mpguerra changed the title Document Zcash consensus rules in Zebra codebase Tracking: Document Zcash consensus rules in Zebra codebase Jan 27, 2022
@teor2345
Copy link
Contributor

This is done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation C-enhancement Category: This is an improvement C-research Category: Engineering notes in support of design choices
Projects
None yet
Development

No branches or pull requests

7 participants