-
Notifications
You must be signed in to change notification settings - Fork 85
[Draft] wNAF multiplication for BN256 curve #36
[Draft] wNAF multiplication for BN256 curve #36
Conversation
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.
Everything looks great - as discussed, let's move to the cheaper mul circuit and we should be done here :)
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.
As discussed, this can be deleted
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.
Can also be deleted I believe
src/gadgets/curves/bn256/mul.rs
Outdated
|
||
/// Converts the specified `BN256ScalarNNField` element to a | ||
/// WNAF representation using an array of `UInt8<F>` elements. | ||
fn convert_to_wnaf<F: SmallField, CS: ConstraintSystem<F>>( |
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.
All wNAF functionality can be removed. We keep the GLV decomposition but instead use Alex's fixed-width 4-bit mul circuit found here. Note that the code here deals with potential 129 bit decomposed scalars, whereas bn254 will only occupy max 127 bits. Thus, there should be some extra constraints we can drop as the scalars fit in a smaller size.
Hey @jules, enormous thanks for the review! Updated the wnaf-based scalar multiplication to a fixed-width 4-bit multiplication, as requested, according to implementation here. After your comment, I am now aware of the extra circuit complexity due to dealing with the potential 129-bit decomposed scalar. Still, I suggest staying with the current implementation since it is well-tested. Then, after we successfully integrate the |
BN254-specific functions will be transferred to |
What ❔
This pull request implements multiplication by a scalar for the
BN256
curve. This PR adds:.sage
files for finding:Why ❔
Currently, multiplication by a scalar is not implemented for BN256 and therefore one needs to add support for such an operation.
Checklist
zk fmt
andzk lint
.