Concept ACK, though is the inclusion of the last commit intentional?

An alternative follow-up could be to split the function in two:

• One that doesn't return whether overflow occurred, and only promises weak normalization.
• One that fails in case overflow occurred (and returns it), and promises strong normalization.

This would get rid of the fact that this is one of the few places in the codebase where field magnitude/normalization properties depend on runtime values (they're almost always compile-time known otherwise).

Concept ACK, though is the inclusion of the last commit intentional?

Yes, but I'm happy to remove it, of course. The story behind this is that I had forgotten to remove a debug printer first when I opened this PR. I mentioned this to @jonasnick who told me that he also had to write these functions multiple times in the past, so we thought it's a good idea to have one in the code base. Then I made it a bit nicer and added a commit. And I felt it's not worth a separate PR (but maybe I was just being lazy there).

An alternative follow-up could be to split the function in two: [...]

That's a great idea.

We could still have overflow as an explicit argument of the (second) function. What do you think?

We could still have overflow as an explicit argument of the (second) function. What do you think?

It could, but I think that'd be confusing. The second function cannot handle overflow; if it was provided input that's out of range, it should return failure, and not make any promises about the parsed secp256k1_fe. The caller is supposed to return or handle failure in this case.

Ok yes, understood now.

utACK 727d50a

force-pushed to address @jonasnick's nits