fe_sqr tests could be improved #1472

1. 
   Coding-Enthusiast commented at 5:41 am on December 24, 2023: contributor

   https://github.com/bitcoin-core/secp256k1/blob/efe85c70a2e357e3605a8901a9662295bae1001f/src/tests.c#L3289-L3303

   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.

2. 
   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 use fe_mul to compute x*x directly (see docs) but we could, e.g., check that (x+y)*(x-y) = x^2 - y^2 for some random values y != 0.

3. real-or-random added the label assurance on Dec 24, 2023
4. real-or-random added the label refactor/smell on Dec 24, 2023
5. 
   real-or-random commented at 0:08 am on January 9, 2024: contributor

   @Coding-Enthusiast

   we could, e.g., check that (x+y)*(x-y) = x^2 - y^2 for some random values y != 0.

   Do you want to give it a try and work on a PR?

6. real-or-random renamed this:
   run_sqr(void) in tests needs #ifdef VERIFY
   fe_sqr tests could be improved
   on Jan 9, 2024
7. 
   Coding-Enthusiast commented at 2:18 pm on January 9, 2024: contributor

   Sadly my C skills are readonly 😊
8. 
   real-or-random commented at 5:58 pm on January 9, 2024: contributor

   We all started small… ;)
9. theStack cross-referenced this on Feb 1, 2024 from issue tests: add missing fe comparison checks for inverse field test cases by theStack
10. theStack cross-referenced this on Feb 1, 2024 from issue tests: improve fe_sqr test (issue #1472) by theStack
11. jonasnick referenced this in commit 427e86b9ed on Feb 27, 2024
12. 
    real-or-random commented at 8:19 am on March 5, 2024: contributor

    This has been resolved by #1490.
13. real-or-random closed this on Mar 5, 2024

    ---

Contributors
Coding-Enthusiast real-or-random

Labels
assurance refactor/smell