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.

Sign up for GitHub

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

Jump to bottom

Validate Transaction::V5 Halo2 proofs #2105

Closed
dconnolly opened this issue May 4, 2021 · 3 comments · Fixed by #3039
Closed

Validate Transaction::V5 Halo2 proofs #2105

dconnolly opened this issue May 4, 2021 · 3 comments · Fixed by #3039
Assignees
@dconnolly
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU-5 Network Upgrade: NU5 specific tasks
Milestone
2021 Sprint 22

Comments

@dconnolly
Copy link
Contributor

dconnolly commented May 4, 2021 •
edited
Loading

Is your feature request related to a problem? Please describe.
As part of consensus validation, we must validate the Halo2 proofs of the Action Descriptions in Transaction::V5.

Describe the solution you'd like
When the async Halo2Verifier service is available, make async requests to validate the proofsOrchard aggregated proof(s)(?) for the Actions of the transaction.

Testing:

Generate test vectors for halo2 proofs from zcash-hackworks; if they give trouble in our full validation pipeline, punt this full integration until we get the first NU5 block with an Action in it for testnet.

Additional context
Depends on #2104
@dconnolly dconnolly added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code S-blocked Status: Blocked on other tasks NU-5 Network Upgrade: NU5 specific tasks P-Medium labels May 4, 2021
@dconnolly dconnolly self-assigned this May 4, 2021
@mpguerra mpguerra mentioned this issue May 5, 2021
Closed
53 tasks
@mpguerra mpguerra mentioned this issue May 13, 2021
Closed
@mpguerra mpguerra added this to the 2021 Sprint 12 milestone May 13, 2021
@mpguerra
Copy link
Contributor

#1912 is a potential duplicate?

@dconnolly
Copy link
Contributor Author

dconnolly commented May 13, 2021 •
edited
Loading

Yes this and #1912 describe the same work, this one has more detail

@teor2345
Copy link
Contributor

I moved this task to sprint 13 because the anchors are a higher priority - they are blocking the history chain contextual validation and testing.

@jvff jvff mentioned this issue Jul 7, 2021
Closed
@dconnolly dconnolly mentioned this issue Nov 8, 2021
Merged
3 tasks
@dconnolly dconnolly linked a pull request Nov 9, 2021 that will close this issue
Verify Halo2 proofs as part of V5 transaction verification #3039
Merged
3 tasks
@mpguerra mpguerra removed this from the 2021 Sprint 23 milestone Nov 9, 2021
@mpguerra mpguerra added this to the 2021 Sprint 22 milestone Nov 9, 2021
@mpguerra mpguerra removed the S-blocked Status: Blocked on other tasks label Nov 11, 2021
@mpguerra mpguerra mentioned this issue Dec 16, 2021
Closed
4 tasks
@arya2 arya2 mentioned this issue Mar 31, 2023
Merged
6 tasks
@mpguerra mpguerra mentioned this issue Apr 11, 2023
Closed
4 tasks
mpguerra added a commit that referenced this issue May 19, 2023
@mpguerra
#2105 was fixed by #3039 and #2379 was closed by #3069
9f8161d
mergify bot pushed a commit that referenced this issue May 23, 2023
@mpguerra @teor2345
Delete outdated TODOs refering to closed issues (#6732)
ec2e9ca 
* ZIPs were updated to remove ambiguity, this was tracked in #1267.

* #2105 was fixed by #3039 and #2379 was closed by #3069

* #2230 was a duplicate of #2231 which was closed by #2511

* #3235 was obsoleted by #2156 which was fixed by #3505

* #1850 was fixed by #2944, #1851 was fixed by #2961 and #2902 was fixed by #2969

* We migrated to Rust 2021 edition in Jan 2022 with #3332

* #1631 was closed as not needed

* #338 was fixed by #3040 and #1162 was fixed by #3067

* #2079 was fixed by #2445

* #4794 was fixed by #6122

* #1678 stopped being an issue

* #3151 was fixed by #3934

* #3204 was closed as not needed

* #1213 was fixed by #4586

* #1774 was closed as not needed

* #4633 was closed as not needed

* Clarify behaviour of difficulty spacing

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour when retrying block downloads

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify when we might want to fix

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify what we might want to change in future

Co-authored-by: teor <teor@riseup.net>

* Clarify benefits of how we do block verification

Co-authored-by: teor <teor@riseup.net>

* Fix rustfmt errors

---------

Co-authored-by: teor <teor@riseup.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dconnolly dconnolly

Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Milestone
2021 Sprint 22
Development

Successfully merging a pull request may close this issue.

Verify Halo2 proofs as part of V5 transaction verification ZcashFoundation/zebra
3 participants
@dconnolly @mpguerra @teor2345