call `fe_verfiy` for not normalized field element in `fe_set_b32`

[siv2r]

https://github.com/bitcoin-core/secp256k1/blob/a1102b12196ea27f44d6201de4d25926a2ae9640/src/field_5x52_impl.h#L331-L339

Shouldn't the call to secp256k1_fe_verify come after the else block?

[real-or-random]

This was changed in https://github.com/bitcoin-core/secp256k1/commit/34a67c773b0871e5797c7ab506d004e80911f120. I believe this is this intentionally to optimize the tests. Can you check if the tests take noticeably longer if the fe_verify is moved after the else block?

[siv2r]

build commands: ./autogen.sh && ./configure && make -j13 Execution time (in seconds) of tests.c for three iterations (on 64-bit, i7-8750H). I used gettime_i64 function to measure the execution time. You can find my complete code here.

branch	min	avg	max
master (fe_verify inside if block)	145.94	146.26	146.72
bench-tests (fe_verify after else block)	145.74	146.04	146.27

There isn't much difference in the execution time.

[real-or-random]

Oh I think the time command would have been fine-grained enough for the tests. :)

Yeah, I mean if the difference is below a second, I'd say it's reasonable to always do the verify.

[siv2r]

Makes sense. The change required here is relatively small. I will check other parts of the code for similar issues and bundle them up in a PR.

[siv2r]

Oh I think the time command would have been fine-grained enough for the tests. :)

I didn't know such a command existed. I initially tried to use clock_gettime() for nanosecond precision..xD. It didn't work since c89 does not seem to support it.

[real-or-random]

Maybe this could be solved in https://github.com/bitcoin-core/secp256k1/pull/1062 ? I think it will be similar to some changes proposed there.