test: refactor: simplify tests by using _ecmult_gen_ge helper, add test#1864
Conversation
If the generator point multiplication result in Jacobian coordinates is immediately converted to affine coordinates after and is not needed for anything else, we can deduplicate by using the helper introduced in bitcoin-core#1861. Note that in a very strict sense this is not a refactor, as the Jacobian object is now cleared out which was not done on master, but for the logic in the tests this shouldn't matter at all.
real-or-random
left a comment
real-or-random
left a comment
There was a problem hiding this comment.
Concept ACK
Maybe secp256k1_ecmult_gen_ge should itself get a dedicated test
Makes sense, added one. I decided to run the loop |
theStack
changed the title
_ecmult_gen_ge helper_ecmult_gen_ge helper, add test
There was a problem hiding this comment.
Pull request overview
This PR updates the test suite to use the new secp256k1_ecmult_gen_ge helper where generator scalar multiplication results are immediately needed in affine coordinates, and adds a unit test to confirm secp256k1_ecmult_gen_ge matches the gej-plus-conversion path.
Changes:
- Replace
secp256k1_ecmult_gen_gej+secp256k1_ge_set_gejsequences in tests withsecp256k1_ecmult_gen_ge. - Add a new
run_ecmult_gen_getest that comparessecp256k1_ecmult_gen_geoutput to the manualgej -> geconversion over random scalars. - Update exhaustive tests to use the affine helper directly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/tests.c |
Refactors generator-multiply test callsites to secp256k1_ecmult_gen_ge and adds a new unit test for the helper. |
src/tests_exhaustive.c |
Simplifies the exhaustive ecmult_gen verification by using secp256k1_ecmult_gen_ge directly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
theStack
force-pushed
the
test-dedup-using-ecmult_gen_ge-helper
branch
from
bb51480 to
2ee79e7
Compare
This PR is a small follow-up to #1861. If the generator point multiplication result in Jacobian coordinates is immediately converted to affine coordinates after and is not needed for anything else, we can deduplicate by using the new
secp256k1_ecmult_gen_gehelper. The second commit adds a simple unit tests, verifying for random scalars that the result ofsecp256k1_ecmult_gen_gematches the two expected steps (secp256k1_ecmult_gen_gejplus Jacobian->affine conersion viasecp256k1_ge_set_gej).Note that in a very strict sense the first commit is not a refactor, as the Jacobian object is now cleared out which was not done on master, but for the logic in the tests this shouldn't matter at all.