call fe_verfiy for not normalized field element in fe_set_b32

[siv2r]

secp256k1/src/field_5x52_impl.h

Lines 331 to 339 in a1102b1

	#ifdef VERIFY
	r->magnitude = 1;
	if (ret) {
	r->normalized = 1;
	secp256k1_fe_verify(r);
	} else {
	r->normalized = 0;
	}
	#endif

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

[real-or-random]

This was changed in 34a67c7. 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 #1062 ? I think it will be similar to some changes proposed there.

[real-or-random]

This has been resolved by #1062.