arkworks-rs / r1cs-std

R1CS constraints for bits, fields, and elliptic curves
https://www.arkworks.rs
Apache License 2.0
137 stars 59 forks source link

Handle zero-case in group scalar multiplication #124

Closed slumber closed 1 year ago

slumber commented 1 year ago

Description

closes: #94

See doc-comments for more details.

This part of code could as well be moved into to_affine, but this exact behavior might not be expected outside of scalar_mul


Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why.

Pratyush commented 1 year ago

Thanks for the PR! Could you fix the errors in CI and then I'll merge this.

slumber commented 1 year ago

Thanks for the PR! Could you fix the errors in CI and then I'll merge this.

I don't think these errors are coming from my PR, perhaps from the CI setup. I don't mind fixing it, but then it would bring some unrelated lines here.

error: call to `.borrow()` on a reference in this situation does nothing
   --> src/groups/curves/twisted_edwards/mod.rs:340:44
    |
340 |                 let base_power = base_power.borrow();
    |                                            ^^^^^^^^^ help: remove this redundant call
    |
    = note: the type `ark_ec::twisted_edwards::Projective<P>` does not implement `Borrow`, so calling `borrow` on `&ark_ec::twisted_edwards::Projective<P>` copies the reference, which does not do anything and can be removed
slumber commented 1 year ago

@Pratyush should be fixed now

UPD: or not

slumber commented 1 year ago

Requires https://github.com/arkworks-rs/curves/pull/167

Pratyush commented 1 year ago

@slumber would you like to add a changelog entry for this?

slumber commented 1 year ago

@slumber would you like to add a changelog entry for this?

Yes, will do