Incorrect V for EIP712 signature

[lambertkevin]

As described by #134 and #151, the V parsed by Metamask for a signTypedData is incorrect. The signature is not expected to be composed with the parity/recovery but with the classic 27 or 28.

This PR fixes this and updates the CI tests.

Thanks 🙏

[FrederikBolding]

@digiwand @lambertkevin Could we test this implementation against https://github.com/MetaMask/eth-sig-util to prevent issues like this in the future?

It may be out of scope for this PR.

[lambertkevin]

@digiwand @lambertkevin Could we test this implementation against https://github.com/MetaMask/eth-sig-util to prevent issues like this in the future?

It may be out of scope for this PR.

Hi @FrederikBolding !

Well the problem is that to my knowledge, @metamask/eth-sig-utils recoverPersonalSignature & recoverTypedSignature and ethers.js recoverAddress are all ignoring the value of v accepting 0 and 1 as a valid v because of EIP-1559, which means it doesn't matter what lib you're going to test the implementation against, you won't catch a v issue like this one. 🤔

And that's actually the reason why the issue is noticed only now that people start using ECDSA verification onchain with EIP-712 messages, because contrary to those libs, ecrecover is returning 0x0 with a v = 0 | 1. And since most of EIP-191 messages were only tested in frontends, no one had really catched the issue before 🤷‍♂️

[digiwand]

Tested the code with macOS x Chrome x Nano X using the test-dapp. LGTM! Thanks @lambertkevin

will hold on merging for @FrederikBolding, or another dev, to review

[cryptoKevinL]

I am seeing this issue as well, where signatures are different by decimal 27 between Ledger Live and Metamask+Ledger