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

Draft RFC: Treestate management #983

Merged
merged 18 commits into from
Apr 15, 2021
Merged

Draft RFC: Treestate management #983

merged 18 commits into from
Apr 15, 2021

Conversation

dconnolly
Copy link
Contributor

@dconnolly dconnolly commented Sep 1, 2020

TODO

  • decide if we want to merge this RFC as a draft, or make further edits

RFC

Rendered.

Follow up work

Todo's in #958

@dconnolly dconnolly added A-docs Area: Documentation C-design Category: Software design work C-research Category: Engineering notes in support of design choices labels Sep 1, 2020
@dconnolly dconnolly added this to the Validate transactions. milestone Sep 1, 2020
@dconnolly dconnolly changed the title RFC: Treestate [WIP] RFC: Treestate Sep 1, 2020
@dconnolly dconnolly force-pushed the treestate-design branch 2 times, most recently from 043b9e8 to 517a7ec Compare September 1, 2020 21:55
@dconnolly dconnolly linked an issue Sep 3, 2020 that may be closed by this pull request
1 task
Comment on lines 86 to 89
nullifier sets are updated in memory as note nullifiers are revealed. If the
rest of the block is validated according to consensus rules, that root is
committed to its own datastructure via our state service (Sprout anchors,
Sapling anchors). Sapling block validation includes comparing the specified
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/ZcashFoundation/zebra/pull/902/files#r482674070 for a suggestion of how to store these anchors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, maybe we should cross-reference here?

@dconnolly dconnolly mentioned this pull request Sep 3, 2020
27 tasks
@teor2345
Copy link
Contributor

teor2345 commented Sep 4, 2020

This RFC has the same number as the state RFC in #902, let's use 0005 for the higher-level RFC ?

Also stolon in #1006 has taken 0006 🤔

@dconnolly dconnolly marked this pull request as ready for review September 7, 2020 05:26
@dconnolly
Copy link
Contributor Author

I'm marking this 'ready for review' because I want to start getting eyes on the Guide-level section

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and it makes sense to me, even though this isn't an area I've spent a lot of time on.

I had a few questions and style notes.

I wonder if some of this detail belongs in the reference-level section. Or maybe the guide-level explanation needs a summary, and some headings to split up the text, and help people navigate.

book/src/dev/rfcs/0005-treestate.md Outdated Show resolved Hide resolved
book/src/dev/rfcs/0005-treestate.md Outdated Show resolved Hide resolved
book/src/dev/rfcs/0005-treestate.md Outdated Show resolved Hide resolved
book/src/dev/rfcs/0005-treestate.md Outdated Show resolved Hide resolved
book/src/dev/rfcs/0005-treestate.md Outdated Show resolved Hide resolved
Comment on lines 86 to 89
nullifier sets are updated in memory as note nullifiers are revealed. If the
rest of the block is validated according to consensus rules, that root is
committed to its own datastructure via our state service (Sprout anchors,
Sapling anchors). Sapling block validation includes comparing the specified
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, maybe we should cross-reference here?

book/src/dev/rfcs/0005-treestate.md Outdated Show resolved Hide resolved
book/src/dev/rfcs/0005-treestate.md Outdated Show resolved Hide resolved
@hdevalence hdevalence mentioned this pull request Sep 8, 2020
@dconnolly dconnolly changed the title [WIP] RFC: Treestate RFC: Treestate management Sep 9, 2020
@yaahc
Copy link
Contributor

yaahc commented Sep 30, 2020

suddenly I feel so much more prepared to review this rfc, ty @str4d

@dconnolly dconnolly changed the title RFC: Treestate management [WIP] RFC: Treestate management Oct 1, 2020
@mpguerra mpguerra removed this from the Transaction Validation milestone Jan 5, 2021
@dconnolly dconnolly added this to the 2021 Sprint 3 milestone Feb 8, 2021
@mpguerra mpguerra modified the milestones: 2021 Sprint 3, 2021 Sprint 4 Feb 22, 2021
@dconnolly dconnolly changed the title [WIP] RFC: Treestate management Draft RFC: Treestate management Apr 6, 2021
@dconnolly
Copy link
Contributor Author

I would be tempted to merge this as a draft, but I don't want to lose the review comments

I think I either addressed or added as TODO's in the document all of these! 💃

@dconnolly dconnolly enabled auto-merge (squash) April 15, 2021 17:34
@dconnolly
Copy link
Contributor Author

Approved and merging as a draft RFC

@dconnolly dconnolly disabled auto-merge April 15, 2021 17:39
@dconnolly dconnolly merged commit 370d048 into main Apr 15, 2021
@dconnolly dconnolly deleted the treestate-design branch April 15, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation C-design Category: Software design work C-research Category: Engineering notes in support of design choices
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Design: treestate (note commitment trees and nullifier sets)
6 participants