It is/(looks like) an unnecessary test for an overflow flag which is always 0 #697

https://github.com/bitcoin-core/secp256k1/blob/e541a90ef6461007d9c6a74b9f9a7fb8aa34aaa8/src/secp256k1.c#L592 tests for an overflow flag, which is, according to https://github.com/bitcoin-core/secp256k1/blob/137d304a6bf419bbbafd70d1dbdb2162354d71b7/src/scalar_low_impl.h#L58, always negative

You're looking at a mock implementation of scalar*, for exhaustive testing purposes. Real 256-bit scalar implementations do set the flag.

Right, sorry, I found it. But why the overflow is important for the key tweaking? If we add the point to itself n times (where n is the field order) we will get the same original point. Is the overflow flag tells about this? If yes, why it prevents from doing the tweak?

For instance, knowing the original public key, and knowing the curve parameters, I can compose a tweaking factor that is equal to n so the tweak procedure always results the same original public key. However, I would not be able to compose a message, that results in the digest equial to n, so at least for commitment procedures this should be not a problem.

On the other hand, coming back to the issue https://github.com/sipa/bips/issues/152 it seems that usage of SHA256 instead of HMAC can allow the attacker (very theoretically) to compose a message which hash will result in n, opening possibility for instance to claim that any public key is committed for the equivalent message, which bytecode can be re-interpreted under different protocols opening a way for a hack.

Right, sorry, I found it. But why the overflow is important for the key tweaking?

Scalars are numbers in Z_p, which are typically represented by the integers 0..p-1 (where p is the group order of the elliptic curve group secp256k1, not the field order). An overflow is basically a number s >= p.

Now there are two ways to look at it:

• You can say that integers s >= p are simply invalid representations, and we don't want invalid scalars.
• You can interpret a 256-bit s >= p by taking it mod p. Then the resulting integer is in range again but then there are a few scalars now which have two 256-bit representations instead of just one. That means that taking an uniform 256-bit integer is not equivalent to taking an uniform scalar in Z_p. We don't want that either.

However, overflow happens with negligible probability for a random tweak, so it's not an issue that the function fails here.

Thank you for the explanation, I think we can close the issue.