Inconsistent usage of V values in signatures

[stevenlcf]

Hi, I found some inconsistent usage (code and comments) of V values (namely using 0/1 or 27/28) in signature related functions (sign and recover), which might lead to misunderstandings and misoperations for developers.

For example, this PR, OpenZeppelin/openzeppelin-contracts#1622, would make the contracts using this popular library deny signatures with 0/1 V values, which are generated by go-ethereum's secp256k1.Sign() function.

Here are some of the inconsistent code and comments:

code using 27/28 for V values:

• ethapi.EcRecover(): in its comment, it says

Note, the signature must conform to the secp256k1 curve R, S and V values, where the V value must be 27 or 28 for legacy reasons.

code using 0/1 for V values:

• secp256k1.Sign(), which is used by crypto.Sign() and further used by types.SignTx():

The produced signature is in the 65-byte [R || S || V] format where V is 0 or 1.

• types.SignatureValues():

SignatureValues returns signature values. This signature needs to be in the [R || S || V] format where V is 0 or 1.

[karalabe]

Originally Ethereum used 27 / 28 (which internally is just 0 / 1, just some weird bitcoin legacy to add 27). Later when we needed to support chain IDs in the signatures, the V as changed to ID*2 + 35 / ID*2 + 35. However, both V's are still supported on mainnet (Homestead vs. EIP155). The code was messy to pass V's around from low level crypto primitives in 27/28 notation, and then later for EIP155 to subtract 27, then do the whole x2+35 magic.

The current logic is that the low level crypto operations returns 0/1 (because that is the canonical V value), and the higher level signers (Frontier, Homestead, EIP155) convert that V to whatever Ethereum specs on top of secp256k1.

TL;DR: Use the high level signers, don't use the secp256k1 library directly. If you use the low level crypto library directly, you need to be aware of how generic ECC relates to Ethereum signatures.