-
Notifications
You must be signed in to change notification settings - Fork 18
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
355 mnt4 mnt6 pairing component #357
base: master
Are you sure you want to change the base?
Conversation
1ca01f0
to
1321b8f
Compare
include/nil/blueprint/components/algebra/fields/plonk/non_native/mnt4_fp4_fixed_power.hpp
Outdated
Show resolved
Hide resolved
gate_list.push_back(bp.add_gate(mult_constrs)); | ||
} | ||
|
||
if (std::count(exp_precompute.begin(),exp_precompute.end(),3)) { |
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.
Cubing and square gates are used only to build cube/square in the beginning and are unused everywhere else. More efficient to just build cube/square through multiplications, without adding more gates.
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 is obviously inspired by my BLS12 pairing code, which also used cube and square only once. Back then I just didn't realize how important gate economy was. Right now I would also suggest replacing squaring and cubing by multiplication at the expense of some extra rows.
That is, unless we need this component really quick.
include/nil/blueprint/components/algebra/pairing/weierstrass/plonk/mnt4_exponentiation.hpp
Outdated
Show resolved
Hide resolved
* Ensures x_next = x_prev/x : x_next * x - x_prev = 0 | ||
*/ | ||
{ | ||
R = Xn*X - Xp; |
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.
We need to forbid passing 0 into this circuit: if x_prev
= x
= 0, we can set x_next
arbitrarily.
Unsure if this is going to actually come up in practice.
But given that it can be solved with one-row constraint in the beginning to the tune of x * y = 1
, we probably still should? Open to discussion on this one.
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.
While the issue of division by zero should be, of course, analyzed, I would question the necessity of a division gate in general. It is used only once and can be replaced by a multiplication gate using some copy constraints. In view of the gate (i.e. selector column) economy this achieves, I would seriously consider that.
include/nil/blueprint/components/algebra/pairing/weierstrass/plonk/mnt4_miller_loop.hpp
Show resolved
Hide resolved
* Tnext.y = (3*T.x^2+a)/2T.y *(T.x-Tnext.x)-T.y | ||
* Rewrite: | ||
* (Tnext.x + 2*Tx) * (2*T.y)^2 - (3*T.x^2+a)^2 = 0 | ||
* (Tnext.y + T.y) * (2*T.y) - (3*T.x^2+a)*(T.x-Tnext.x) = 0 |
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.
While T cannot start as zero point during the calculation, I am unsure what guarantees that it cannot become a zero point during it.
Is the above guaranteed by some property?
Otherwise, if both T.x and T.y are zero, there is no possible assignment which would satisfy the above constraints.
(there is also a similar assumption made in calculation of lf
just above this)
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.
See previous comment.
T starts with Q, then it is 2Q, 3Q, 6Q and so on, up to ate_loop_count
*Q, point Q (all points on E') has order q
, T will never be zero.
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.
While this is certainly a clever and important observation, I believe it should be placed as a comment to the code, not just mentionned in an answer in a github discussion.
include/nil/blueprint/components/algebra/pairing/weierstrass/plonk/mnt4_miller_loop.hpp
Outdated
Show resolved
Hide resolved
include/nil/blueprint/components/algebra/pairing/weierstrass/plonk/mnt4_miller_loop.hpp
Show resolved
Hide resolved
f61f69e
to
0de7980
Compare
// SOFTWARE. | ||
//---------------------------------------------------------------------------// | ||
// @file Declaration of F_p^3 elements over an abstract entity (to be used with constraints) | ||
// with F_p^3 = F_p[u]/(u^3 + non_residue). |
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, it should be "minus non-residue", not "plus", i.e. F_p[u]/(u^3 - non_residue) instead of F_p[u]/(u^3 + non_residue)
|
||
constexpr abstract_fp4_element operator*(abstract_fp4_element const& y) { | ||
// Devegili et al - Multiplication and squaring in pairing-friendly fields | ||
// https://eprint.iacr.org/2006/471.pdf |
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'd say the reference is misleading because we don't use any optimisation here. Just the "schoolbook" formulas.
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.
It is most important that the reasons for T != O in the Miller loop make their way into the code comments and are preserved for future reference.
No description provided.