-
Notifications
You must be signed in to change notification settings - Fork 117
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
Check jubjub key correctness independent of redjubjub / jubjub #3154
Conversation
…the prime order group
04203f8
to
ab38d70
Compare
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
…check-jubjub-key-correctness
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is done, so I'm moving out of draft, but I want to review it myself to make sure I didn't forget anything.
0cd7c8a
to
b8760ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the issue which was caused by some ValueCommitments
being the identity point in the middle of computations (e.g. binding_verification_key()
) which caused a panic. The non-small order rule only applies to the ValueCommitments
that are inside Spends and Outputs. So I created a NotSmallOrderValueCommiment
for those. (Feel free to suggest a better name 😛 )
@dconnolly could you please take a look on these recent changes? b8760ce and 0531b5f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I can't approve a PR I opened, so @conradoplg I think can approve this 😁
Codecov Report
@@ Coverage Diff @@
## main #3154 +/- ##
==========================================
+ Coverage 77.03% 77.71% +0.68%
==========================================
Files 265 264 -1
Lines 31267 31060 -207
==========================================
+ Hits 24086 24139 +53
+ Misses 7181 6921 -260 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as requested
Motivation
In the spec there are a bunch of small-order, identity or prime-order checks. Some of those were being validated in
redjubjub
crate but that was not appropriate since it's a Zcash-specific rule. See #2549Additionally, some checks were missing.
Specifications
Designs
Solution
cv
(value commitments): add new type to specify a not-small-order value commitment and use those in the Spend and Output structsrk
validating key: create type for it, enforce not small orderepk
(EphemeralPublicKey
): enforce not small orderpkd
(TransmisisonKey
): enforce prime-orderak
: enforce prime order (not a consensus rule, used by wallets)epk
: enforce not identityOrchard Action
cv
andrk
can be the identity, so no change was needed.Closes #2549
Review
Reviewer Checklist
Follow Up Work