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

Fix Redjubjub to support low-order Jubjub points, move small-order exclusion checks to Sapling SpendAuthorizationKey and ValueCommit #2549

Closed
dconnolly opened this issue Jul 30, 2021 · 1 comment · Fixed by #3154
Closed

Fix Redjubjub to support low-order Jubjub points, move small-order exclusion checks to Sapling SpendAuthorizationKey and ValueCommit #2549

dconnolly opened this issue Jul 30, 2021 · 1 comment · Fixed by #3154
Assignees
@conradoplg @dconnolly
Labels
A-rust Area: Updates to Rust code NU-1 Sapling Network Upgrade: Sapling specific tasks
Milestone
2021 Sprint 25 - ...

Comments

@dconnolly
Copy link
Contributor

Motivation

There is a bug in redjubjub where it rejects Jubjub small order points and the identity when they should be valid VerificationKeys to conform to the Zcash Spec. While those points are rejected when the VerificationKey is being used as a SpendAuthorizationKey, they should not be rejected to conform to the pure RedJubjub spec in other cases. See ZcashFoundation/redjubjub#127 for more.

If redjubjub changes to strictly conform and resolve ZcashFoundation/redjubjub#127, we need to expliticly make our derived key types and Sapling value commit to check and reject small order values:

image

Related Work

redjubjub ZcashFoundation/redjubjub#127
@dconnolly dconnolly added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Jul 30, 2021
@dconnolly dconnolly assigned dconnolly and unassigned dconnolly Jul 30, 2021
@dconnolly dconnolly added A-rust Area: Updates to Rust code NU-1 Sapling Network Upgrade: Sapling specific tasks P-Medium and removed C-enhancement Category: This is an improvement labels Jul 30, 2021
@dconnolly dconnolly added this to the 2021 Sprint 21 milestone Jul 30, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Aug 16, 2021
@mpguerra mpguerra added the S-blocked Status: Blocked on other tasks label Oct 1, 2021
@teor2345 teor2345 added S-needs-investigation Status: Needs further investigation and removed S-needs-investigation Status: Needs further investigation labels Oct 13, 2021
@mpguerra mpguerra removed this from the 2021 Sprint 22 milestone Oct 13, 2021
This was referenced Nov 16, 2021
Closed
Merged
@dconnolly dconnolly added this to the 2021 Sprint 24 milestone Nov 16, 2021
@dconnolly dconnolly self-assigned this Nov 29, 2021
@dconnolly dconnolly removed the S-blocked Status: Blocked on other tasks label Nov 29, 2021
@dconnolly
Copy link
Contributor Author

@conradoplg is going to shepherd this to completion 🙏

@conradoplg conradoplg mentioned this issue Dec 22, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@conradoplg conradoplg

@dconnolly dconnolly

Labels
A-rust Area: Updates to Rust code NU-1 Sapling Network Upgrade: Sapling specific tasks
Projects
None yet
Milestone
2021 Sprint 25 - Last Sprint of 2021
Development

Successfully merging a pull request may close this issue.

Check jubjub key correctness independent of redjubjub / jubjub ZcashFoundation/zebra
4 participants
@conradoplg @dconnolly @mpguerra @teor2345