It is not neccessary for the second argument in `secp256k1_fe_equal_var`
(or `secp256k1_fe_equal`) to have magnitude = 1.
Hence, removed the `secp256k1_fe_normalize_weak` call for those argument.
b7c685e74adbd83937990e90f076600fabf8ccf0 Save _normalize_weak calls in group add methods (Peter Dettman)
c83afa66e0c324e42d13adff0e4f7db9b2868788 Tighten group magnitude limits (Peter Dettman)
173e8d061a8d1526f80d9ae79dd7f0371d38f7e0 Implement current magnitude assumptions (Peter Dettman)
49afd2f5d8c323d32a21f2fe182823b6d7704eb2 Take use of _fe_verify_magnitude in field_impl.h (Sebastian Falbesoner)
4e9661fc426c6068b2472f52a772c312bc26acc9 Add _fe_verify_magnitude (no-op unless VERIFY is enabled) (Peter Dettman)
690b0fc05abd76cb7f6bd87e88bf7b8b0fd1ab70 add missing group element invariant checks (Sebastian Falbesoner)
Pull request description:
This PR picks up #1032 by peterdettman. It's essentially a rebase on master; the original first commit (09dbba561fdb9d57a2cc9842ce041d9ba29a6189) which introduced group verification methods has mostly been replaced by PR #1299 (commit f20266722ac93ca66d1beb0d2f2d2469b95aafea) and what remains now is only adding a few missing checks at some places. The remaining commits are unchanged, though some (easy-to-solve) conflicts appeared through cherry-picking. The last commit which actually removes the `normalize_weak` calls is obviously the critical one and needs the most attention for review.
ACKs for top commit:
sipa:
utACK b7c685e74adbd83937990e90f076600fabf8ccf0
real-or-random:
ACK b7c685e74adbd83937990e90f076600fabf8ccf0
jonasnick:
ACK b7c685e74adbd83937990e90f076600fabf8ccf0
Tree-SHA512: f15167eff7ef6ed971c726a4d738de9a15be95b0c947d7e38329e7b16656202b7113497d36625304e784866349f2293f6f1d8cb97df35393af9ea465a4156da3
- adjust test methods that randomize magnitudes
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
600c5adcd59240305e22918943f45dceeabb7e93 clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3) (Sebastian Falbesoner)
Pull request description:
Some of the C source files contain contain in-comment Sage code calculating secp256k1 parameters that are already defined in the file secp256k1_params.sage. Replace that by a corresponding load instruction and access the necessary variables. In ecdsa_impl.h, update the comment to use a one-line shell command calling sage to get the values.
The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated to work with a current version based on Python3 (Sage 9.0+, see https://wiki.sagemath.org/Python3-Switch).
The latter can be seen as a small follow-up to PR #849 (commit 13c88efed0005eb6745a222963ee74564054eafb).
ACKs for top commit:
sipa:
ACK 600c5adcd59240305e22918943f45dceeabb7e93
real-or-random:
ACK 600c5adcd59240305e22918943f45dceeabb7e93
Tree-SHA512: a9e52f6afbce65edd9ab14203612c3d423639f450fe8f0d269a3dda04bebefa95b607f7aa0faec864cb78b46d49f281632bb1277118749b7d8613e9f5dcc8f3d
Some of the C source files contain contain in-comment Sage code
calculating secp256k1 parameters that are already defined in the file
secp256k1_params.sage. Replace that by a corresponding load instruction
and access the necessary variables. In ecdsa_impl.h, update the comment
to use a one-line shell command calling sage to get the values.
The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated
to work with a current version based on Python3 (Sage 9.0+, see
https://wiki.sagemath.org/Python3-Switch).
The latter can be seen as a small follow-up to PR #849 (commit
13c88efed0005eb6745a222963ee74564054eafb).
There are several instances in the tests where random non-zero field
elements are generated by calling `random_fe_test` in a do/while-loop.
This commit deduplicates all these by introducing a
`random_fe_non_zero_test` helper. Note that some instances checked the
is-zero condition via `secp256k1_fe_normalizes_to_zero_var`, which is
unnecessary, as the result of `random_fe_test` is already normalized (so
strictly speaking, this is not a pure refactor).
There is a function `random_fe_test` which does exactly the
same, so use that instead. Note that it's also moved up before the
`random_group_element_test` function, in order to avoid needing a forward
declaration.
be8ff3a02aeff87c60d49883a1b2afa8b2999bbe field: Static-assert that int args affecting magnitude are constant (Tim Ruffing)
Pull request description:
See #1001.
Try to revert the lines in `tests.c` to see the error message in action.
ACKs for top commit:
sipa:
ACK be8ff3a02aeff87c60d49883a1b2afa8b2999bbe. Verified by introducing some non-constant expressions and seeing compilation fail.
theStack:
ACK be8ff3a02aeff87c60d49883a1b2afa8b2999bbe
Tree-SHA512: 8befec6ee64959cdc7f3e29b4b622410794cfaf69e9df8df17600390a93bc787dba5cf86239de6eb2e99c038b9aca5461e4b3c82f0e0c4cf066ad7c689941b19
The recently merged ellswift PR (#1129) introduced a helper
`secp256k1_ge_x_on_curve_var` to check if a given X coordinate is
valid (i.e. the expression x^3 + 7 is square, see commit
79e5b2a8b80f507e2c9936ff1c4e2fb39bc66a4e). This can be used for code
deduplication in the `ecmult_const_mult_xonly` test.
ade5b367018a624ff7ca1ecbb4a64889d47b0142 tests: add checks for scalar constants `secp256k1_scalar_{zero,one}` (Sebastian Falbesoner)
654246c63585422a184121a26d42dcae792e87c6 refactor: take use of `secp256k1_scalar_{zero,one}` constants (Sebastian Falbesoner)
Pull request description:
Rather than allocating a (non-constant) scalar variable on the stack with the sole purpose of setting it to a constant value, the global constants `secp256k1_scalar_{zero,one}` (apparently introduced in 34a67c773b0871e5797c7ab506d004e80911f120, PR #710) can be directly used instead for the values 0 or 1. There is very likely not even a difference in run-time, but it leads to simpler and less code which might be nice.
ACKs for top commit:
sipa:
utACK ade5b367018a624ff7ca1ecbb4a64889d47b0142
real-or-random:
utACK ade5b367018a624ff7ca1ecbb4a64889d47b0142
Tree-SHA512: 0ff05a449c153f7117a4a56efef04b2087c2330f4692f3390a0b1d95573785ac7ae3fe689ed0ec2ecc64b575d2489d6e341d32567e75a1a4b4d458c3ecd406a1
a575339c0282ba49a4f46c9c660a4cc3b6bfc703 Remove bits argument from secp256k1_wnaf_const (always 256) (Pieter Wuille)
Pull request description:
There is little reason for having the number of bits in the scalar as a parameter, as I don't think there are any (current) use cases for non-256-bit scalars.
ACKs for top commit:
jonasnick:
ACK a575339c0282ba49a4f46c9c660a4cc3b6bfc703
real-or-random:
utACK a575339c0282ba49a4f46c9c660a4cc3b6bfc703
Tree-SHA512: 994b1f19b4c513f6d070ed259a5d6f221a0c2450271ec824c5eba1cd0ecace276de391c170285bfeae96aaf8f1e0f7fe6260966ded0336c75c522ab6c56d182c
e5de45460953c8ae16521b1928ac14de218998a3 tests: Add Wycheproof ECDSA vectors (RandomLattice)
Pull request description:
This PR adds a test using the Wycheproof vectors as outlined in #1106. We add all 463 ECDSA test vectors. These vectors cover:
- edge cases in arithmetic operations
- signatures with special values for (r,s) that should be rejected
- special cases of public keys
The vectors are pulled from the Wycheproof project using a python script to emit C code.
All the new ECDSA Wycheproof vectors pass.
ACKs for top commit:
sipa:
ACK e5de45460953c8ae16521b1928ac14de218998a3
real-or-random:
ACK e5de45460953c8ae16521b1928ac14de218998a3
Tree-SHA512: e9684f14ff3f5225a4a4949b490e07527d559c28aa61ed03c03bc52ea64785f0b80b9e1b1628665eacf24006526271ea0fb108629c9c3c1d758e52d214a056f1
Adds a test using the Wycheproof vectors as outlined in #1106. The
vectors are taken from the Wycheproof repo. We use a python script
to convert the JSON-formatted vectors into C code.
Co-authored-by: Sean Andersen <6730974+andozw@users.noreply.github.com>
The implementation calls the secp256k1_modinvNN_jacobi_var code, falling back
to computing a square root in the (extremely rare) case it failed converge.
This introduces variants of the divsteps-based GCD algorithm used for
modular inverses to compute Jacobi symbols. Changes compared to
the normal vartime divsteps:
* Only positive matrices are used, guaranteeing that f and g remain
positive.
* An additional jac variable is updated to track sign changes during
matrix computation.
* There is (so far) no proof that this algorithm terminates within
reasonable amount of time for every input, but experimentally it
appears to almost always need less than 900 iterations. To account
for that, only a bounded number of iterations is performed (1500),
after which failure is returned. In VERIFY mode a lower iteration
count is used to make sure that callers exercise their fallback.
* The algorithm converges to f=g=gcd(f0,g0) rather than g=0. To keep
this test simple, the end condition is f=1, which won't be reached
if started with non-coprime or g=0 inputs. Because of that we only
support coprime non-zero inputs.
