ScalarMul on Bandersnatch #263
Conversation
| B := Point{} | ||
| // s1 + λ * s2 == s + k*r | ||
| // api.AssertIsEqual(api.Add(s1, api.Mul(s2, &curve.lambda)), api.Add(scalar, api.Mul(&curve.Order, sd[2]))) | ||
| api.AssertIsEqual(api.Sub(api.Mul(s2, &curve.lambda), s1), api.Add(scalar, api.Mul(&curve.Order, sd[2]))) |
There was a problem hiding this comment.
Aren't there bounds to check as well? If o is the order of bandersnatch's prime group, in Z we have
s1 + λ * s2 - s - k*o = 0
s1, s2 < sqrt(o)
o<r
Here you only check that s1 + λ * s2 = s + k*o mod r, so it means that in Z s1 + λ * s2 - s - k*o = c * r. From the scalar decomposition we know that s1, s2 < sqrt(o)<sqrt(r), but λ is anywhere between 0 and o, so to me nothing guarantees that c=0 here... I feel that something is missing, linked to the emulation of Z/oZ arithmetic in Z/rZ
There was a problem hiding this comment.
creating a ticket for this #268
|
converting the PR to draft until we make sure of the hinted scalar decomposition. |
|
@yelhousni when is is going to be mergeable? I want to fix the weird apis in twistededwards / eddsa (see #262 ) and you may get quite a lot of conflicts if I do so before this PR :) |
|
@yelhousni ⬆️ ? |
I'll take care of that tomorrow. The GLV decomposition is correct (and scalar size bound is increased) but we may need to constrain few more things. |
* build: updated to latest gnark-crypto * build: updated to latest gnark-crypto * refactor: introduce Curve interface in std/ and updated eddsa tests * feat: added std/eddsa publicKey and signature assign helpers * refactor(std): merged twistededwards and bandersnatch. IsOnCurve failing for bandersnatch * fix: closes #283. ensure test.Assert compile cache handles different object of same type * fix: use UnsafeAddr instead of UnsafePointer to be retro compatible * fix: fix previous commit * test: test all twisted ed curve operations * Fixes #283 : ensure test.Assert compile cache handles different objects of same type (#284) * fix: closes #283. ensure test.Assert compile cache handles different object of same type * fix: use UnsafeAddr instead of UnsafePointer to be retro compatible * fix: fix previous commit * fix: apply pr patch * style: make twistededwards/Point methods package private
ScalarMulon Bandersnatch now uses the D=-8 endomorphism with a hinted scalar decomposition.Groth16 bench:
ScalarMul2436 constraintsScalarMul1914 constraintsScalarMul4844 constraintsFor Jubjub, it costs:
ScalarMul3060 constraintsScalarMul2538 constraintsScalarMul4844 constraintsNote that the variable-double-base
ScalarMulalgortihm is the same for both curves. It uses an interleaved simultaneous scanning of two scalars and a window table of 4 (Lookup2). It is better constraint-wise to do so than using the endomorphism separately on twoScalarMul. Therefore, EdDSA verification cost is (almost) the same on both curve (Bandersnatch has 4C less as the cofactor clearing costs one doubling less), but theScalarMulindependently costs 624C less.