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

Update the README goals and scope for our first Beta release #2857

Closed
Tracked by #2867
mpguerra opened this issue Oct 11, 2021 · 11 comments · Fixed by #2967
Closed
Tracked by #2867

Update the README goals and scope for our first Beta release #2857

mpguerra opened this issue Oct 11, 2021 · 11 comments · Fixed by #2967
Assignees

Comments

@mpguerra
Copy link
Contributor

mpguerra commented Oct 11, 2021

Similar to #2372, when we finish the mempool work and are ready to tag our first beta release, we should update the README.

In order to do this, we will need to explicitly outline any full node functionality which is not yet implemented in Zebra, in particular:

  • Any zcash consensus rules not yet implemented in zebra
    • Any sections in the zcash spec that we have not yet implemented
    • Any ZIPs which are not implemented
    • Any bitcoin rules that are not implemented
  • Anything else that zcashd does that zebra does not yet do?
    • RPCs
    • wallet interfaces and wallet functionality
    • mining
    • network protocol bloom filter messages
    • Tor integration
@mpguerra mpguerra added this to the 2021 Sprint 21 milestone Oct 11, 2021
@mpguerra
Copy link
Contributor Author

@mpguerra
Copy link
Contributor Author

mpguerra commented Oct 15, 2021

This is an initial list that I've put together for:

  • Any zcash consensus rules not yet implemented in zebra
    - Any sections in the zcash spec that we have not yet implemented
    - Any ZIPs which are not implemented
    - Any bitcoin rules that are not implemented

Known Issues

What is the beta not implementing

  • Validation of transaction lock times
  • Verification of lock scripts on transparent coinbase outputs.
  • Validation of shielded (orchard, sapling and sprout) note commitment trees

NU5

NU4 - Canopy

  • Calculation of Block Subsidy and Funding streams
  • Validation of coinbase miner subsidy and miner fees
  • Validation of shielded outputs for coinbase transactions (ZIP-212/ZIP-213)

NU1 - Sapling

  • Validation of Sapling anchors

NU0 - Overwinter

  • ZIP-203: Transaction Expiry consensus rules

Sprout

  • Validation of Sprout anchors
  • Validation of JoinSplit proofs using Groth16 verifier

@mpguerra
Copy link
Contributor Author

I have the suspicion that some of the above items can be merged together... :) e.g "Halo2 verification" and "Validation of transaction V5 Halo2 proofs" ?

@teor2345

This comment has been minimized.

@teor2345
Copy link
Contributor

I have the suspicion that some of the above items can be merged together... :) e.g "Halo2 verification" and "Validation of transaction V5 Halo2 proofs" ?

Let's make a full list, and then get some engineers to merge similar items?

@dconnolly or @conradoplg could merge the cryptographic ones, and I can do the rest?

@conradoplg
Copy link
Collaborator

I think that's the only thing to merge, we can also move everything related to Orchard to subitems:

NU5

@teor2345
Copy link
Contributor

Here's the draft list from #2372:

Zebra is a fully validating Canopy and NU5 node, except for:

  • mempool transactions,
  • block subsidies,
  • transaction fees,
  • some undocumented rules derived from Bitcoin, and
  • some consensus rules removed before Canopy activation.
    (Zebra checkpoints on Canopy activation.)

Zebra's network stack is interoperable with zcashd.
Zebra implements all the features required to reach Zcash network consensus.

@mpguerra
Copy link
Contributor Author

I have the suspicion that some of the above items can be merged together... :) e.g "Halo2 verification" and "Validation of transaction V5 Halo2 proofs" ?

Let's make a full list, and then get some engineers to merge similar items?

I believe my list from #2857 (comment) is the full list based on the issues that we still need to solve before NU5 activation. I think we need to be very explicit on exactly which consensus rules and ZIPs we are not implementing for the beta release.

I think the list in #2372 is good for NU5 activation.

@teor2345
Copy link
Contributor

teor2345 commented Oct 25, 2021

I believe my list from #2857 (comment) is the full list based on the issues that we still need to solve before NU5 activation. I think we need to be very explicit on exactly which consensus rules and ZIPs we are not implementing for the beta release.

I'm surprised we don't have more things on the list.
We have multiple sprints worth of tickets, are we sure we haven't missed anything important?

It might be easier to edit this list in a pad or interactive document.

I'd like us to make the following changes to the list:

  • Move "Validation of transaction lock times" to Sprout
  • Delete "Verification of lock scripts on transparent coinbase outputs", because we're not sure if we actually need to validate them (it might not be required until they're spent, it's undocumented, we don't know what zcashd does, but we have a ticket to find out)
  • Split "note commitment trees" into each network upgrade, for example: "Validation of Sprout anchors and note commitment trees"
  • Move "Validation of JoinSplit proofs using Groth16 verifier" to the network upgrade where Sprout moved from BCTV14 to Groth16. (I don't know which one it is, but Deirdre should.)

I don't think we're going to validate Sprout on BCTV14 proofs, they're covered by "Zebra does not implement some consensus rules removed before Canopy activation."

Deletions:

I think we might also be missing:

Known issues:

Zebra does not implement:

  • some undocumented rules derived from Bitcoin, and
  • some consensus rules removed before Canopy activation. (Zebra checkpoints on Canopy activation.)

@mpguerra
Copy link
Contributor Author

I think we might also be missing:

Known issues:

* [Security: Stop disconnecting from nodes that send unexpected messages, to prevent disconnection attacks, Credit: Equilibrium #2107](https://github.com/ZcashFoundation/zebra/issues/2107)

* [Zebra should limit the number of addresses it sends to peers, to avoid address book discovery #1889](https://github.com/ZcashFoundation/zebra/issues/1889)

* [Security: Zebra's address book can use all available memory #1873](https://github.com/ZcashFoundation/zebra/issues/1873)

* [Zebra should limit the number of addresses it uses from a single Addrs response, to avoid address book takeover #1869](https://github.com/ZcashFoundation/zebra/issues/1869)

I did not have these on my radar. I have added them to sprints between now and the end of the year

@mpguerra
Copy link
Contributor Author

Deletions:

* [Validate nSpendsSapling, nOutputsSapling, and nActionsOrchard limits in V5 transactions #2379](https://github.com/ZcashFoundation/zebra/issues/2379) is optional and I think we should delete it from the list. (We already do stricter checks, we just need to test and document them.)

I moved this to Sprint 24 for one final check before we decide we really don't need to do it

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

Successfully merging a pull request may close this issue.

3 participants