This method doesn’t test anything and after looking through commits it was added in https://github.com/bitcoin-core/secp256k1/commit/59447da38d6fa394ab7b10f32aea55930da732be and got removed in the next commit https://github.com/bitcoin-core/secp256k1/commit/21f81a846957408d4a2fe30cff32370567887a93. I’m not sure what the “problem” mentioned in the first commit is but if it is checking it using the SECP256K1_FE_VERIFY
method, it is pointless to call run_sqr
without this build configuration.
fe_sqr tests could be improved #1472
issue Coding-Enthusiast openend this issue on December 24, 2023-
Coding-Enthusiast commented at 5:41 am on December 24, 2023: contributor
-
real-or-random commented at 5:23 pm on December 24, 2023: contributor
it is pointless to call
run_sqr
without this build configuration.Well, there’s nothing wrong with it, except that it adds a few milliseconds to the noverify_tests. I claim that adding
#ifdefs
isn’t worth the complexity and the development time. So this is not an issue.The test itself could perhaps be improved. We could compare with the result of another method, e.g.,
fe_mul
. We can’t usefe_mul
to computex*x
directly (see docs) but we could, e.g., check that(x+y)*(x-y) = x^2 - y^2
for some random valuesy != 0
. -
real-or-random added the label assurance on Dec 24, 2023
-
real-or-random added the label refactor/smell on Dec 24, 2023
-
real-or-random commented at 0:08 am on January 9, 2024: contributor
we could, e.g., check that
(x+y)*(x-y) = x^2 - y^2
for some random valuesy != 0
.Do you want to give it a try and work on a PR?
-
real-or-random renamed this:
run_sqr(void) in tests needs #ifdef VERIFY
fe_sqr tests could be improved
on Jan 9, 2024 -
Coding-Enthusiast commented at 2:18 pm on January 9, 2024: contributorSadly my C skills are readonly 😊
-
real-or-random commented at 5:58 pm on January 9, 2024: contributorWe all started small… ;)
-
theStack cross-referenced this on Feb 1, 2024 from issue tests: add missing fe comparison checks for inverse field test cases by theStack
-
theStack cross-referenced this on Feb 1, 2024 from issue tests: improve fe_sqr test (issue #1472) by theStack
-
jonasnick referenced this in commit 427e86b9ed on Feb 27, 2024
-
real-or-random commented at 8:19 am on March 5, 2024: contributorThis has been resolved by #1490.
-
real-or-random closed this on Mar 5, 2024
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-31 23:15 UTC
This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-31 23:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me
More mirrored repositories can be found on mirror.b10c.me