-
Notifications
You must be signed in to change notification settings - Fork 120
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
Implement Scott's method for fast cofactor multiplication. #102
Conversation
2819326
to
0fe5d6f
Compare
Co-authored-by: "Anita Durr <anita.durr@psl.edu>"
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.
Thanks for submitting this. Changes requested.
0x190937e76bc3e447, | ||
0x08ab05f8bdd54cde, | ||
]); | ||
let wx = wx.inverse().unwrap(); |
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.
wx
and wy
could be precomputed. (I'd suggest still computing them this way, just in advance.)
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.
Please add a comment explaining where the exponents come from.
0xcd91de4547085aba, | ||
0x91d50792876a202, | ||
0x5d543a95414e7f1, | ||
|
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.
Add a comment referencing the paper this comes from, and briefly explaining what's going on.
self.y.mul_assign(&wy); | ||
} | ||
|
||
fn scale_by_cofactor(&self) -> G2 { |
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.
This looks like a less efficient method than Budroni and Pintore (see zcash/zcash#3425 (comment) ). Their method requires only two multiplications by the parameter x, roughly speaking.
assert!(got.is_on_curve()); | ||
assert!(got.is_in_correct_subgroup_assuming_on_curve()); | ||
} | ||
} |
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 we should also test that the new scale_by_cofactor
matches the old one for random curve points (not necessarily in the subgroup).
Hello @daira! Sorry for the very late reply on this pull request. Thanks for this preliminary review. |
I've recently been looking at faster way for multiplying by the cofactor over G2. It seems to me that this problem is generally solved in the literature by writing the cofactor (or a multiple of it) modulo q, and using an endomorphism to speed up the computation for scalar multiplication modulo q.
There's two viable methods for BLS12-381: one from Scott et al., one from Budroni et al.. Together with @AnitaDurr we took a chance at implementing Scott's method. There are some trivial optimizations that can still be put in place, but the bottom line is that we can multiply by the cofactor over G2 with 3 scalar multiplications (< r) and 2 point additions.
It seems that relic has also done something similar, however they keep the constant
x
around and use Alessandro's method, making this slightly slower.