By requiring that the input group element's X coordinate (`a->x`) has a
magnitude of <= 31, the normalize_weak call and also the field element
variable `r2` are not needed anymore and hence can be dropped.
After calculating the right-hand side of the elliptic curve equation
(x^3 + 7), the field element `x3` has a magnitude of 2 (1 as result of
`secp256k1_fe_mul`, then increased by 1 due to `secp256k1_fe_add_int`).
This is fine for `secp256k1_fe_equal_var`, as the second parameter only
requires the magnitude to not exceed 31, and the normalize_weak call can
hence be dropped.
e089eecc1e54551287b12539d2211da631a6ec5c group: Further simply gej_add_ge (Tim Ruffing)
ac71020ebe052901000e5efa7a59aad77ecfc1a0 group: Save a normalize_to_zero in gej_add_ge (Tim Ruffing)
Pull request description:
As discovered by sipa in #1033.
See commit message for reasoning but note that the infinity handling will be replaced in the second commit again.
ACKs for top commit:
sipa:
ACK e089eecc1e54551287b12539d2211da631a6ec5c
apoelstra:
ACK e089eecc1e54551287b12539d2211da631a6ec5c
Tree-SHA512: fb1b5742e73dd8b2172b4d3e2852490cfd626e8673b72274d281fa34b04e9368a186895fb9cd232429c22b14011df136f4c09bdc7332beef2b3657f7f2798d66
The code currently switches to the alternative formula for lambda only if (R,M)
= (0,0) but the alternative formula works whenever M = 0: Specifically, M = 0
implies y1 = -y2. If x1 = x2, then a = -b this is the r = infinity case that we
handle separately. If x1 != x2, then the denominator in the alternative formula
is non-zero, so this formula is well-defined.
One needs to carefully check that the infinity assignment is still correct
because now the definition of m_alt at this point in the code has changed. But
this is true:
Case y1 = -y2:
Then degenerate = true and infinity = ((x1 - x2)Z == 0) & ~a->infinity .
a->infinity is handled separately.
And if ~a->infinity, then Z = Z1 != 0,
so infinity = (x1 - x2 == 0) = (a == -b) by case condition.
Case y1 != -y2:
Then degenerate = false and infinity = ((y1 + y2)Z == 0) & ~a->infinity .
a->infinity is handled separately.
And if ~a->infinity, then Z = Z1 != 0,
so infinity = (y1 + y2 == 0) = false by case condition.
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
This header contains a static array that replaces the ecmult_context pre_g and pre_g_128 tables.
The gen_ecmult_static_pre_g program generates this header file.
Previous behaviour would not initialize r->y values in the case where infinity is passed in.
Furthermore, the previous behaviour wouldn't initialize anything in the case where all inputs were infinity.
This enables testing overflow is correctly encoded in the recid, and
likely triggers more edge cases.
Also introduce a Sage script to generate the parameters.
47a7b8382fd6f1458d859b315cf3bcd3b9790b68 Clear field elements when writing infinity (Elichai Turkel)
61d1ecb02847be9d65ffe9df2d2408d85f3a0711 Added test with additions resulting in infinity (Elichai Turkel)
Pull request description:
Currently if `secp256k1_gej_add_var` / `secp256k1_gej_add_ge_var` /` secp256k1_gej_add_zinv_var` receive `P + (-P)` it will set `gej->infinity = 1` but doesn't call initialize the field elements.
Notice that this is the only branch in the function that results in an uninitialized output.
By using `secp256k1_gej_set_infinity()` it will set the field elements to zero while also setting the infinity flag.
I also added a test that fails with valgrind on current master but passes with the fix.
EDIT: This isn't a bug or something necessary, I just personally found this helpful.
ACKs for top commit:
real-or-random:
ACK 47a7b8382fd6f1458d859b315cf3bcd3b9790b68
Tree-SHA512: cdc2efc242a1b04b4f081183c07d4b2602cdba705e6b30b548df4e115e54fb97691f4b1a28f142f02d5e523c020721337a297b17d732acde147b910f5c53bd0a
ECMULT_CONST_TABLE_GET_GE was branching on its secret input.
Also makes secp256k1_gej_double_var implemented as a wrapper
on secp256k1_gej_double_nonzero instead of the other way
around. This wasn't a constant time bug but it was fragile
and could easily become one in the future if the double_var
algorithm is changed.
9bd89c8 Optimize secp256k1_fe_normalize_weak calls. Move secp256k1_fe_normalize_weak calls out of ECMULT_TABLE_GET_GE and ECMULT_TABLE_GET_GE_STORAGE and into secp256k1_ge_globalz_set_table_gej instead. (Russell O'Connor)
Pull request description:
Move secp256k1_fe_normalize_weak calls out of ECMULT_TABLE_GET_GE and ECMULT_TABLE_GET_GE_STORAGE and into secp256k1_ge_globalz_set_table_gej instead.
Tree-SHA512: 7bbb1aca8e37a268a26d7061bd1f390db129e697792f1d5ddd10ea34927616edc26ef118b500c3e5e14d1d463196033ef64e4d34b765380325c24835458b7a9b
