✨ ECDSA Wrapper

[Vectorized]

Description

For #244.

Using this can save about 700 gas over OpenZeppelin's implementation.

Tests are ported over from OpenZeppelin.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/test/utils/cryptography/ECDSA.test.js

Although I recommend that toEthSignedMessageHash(bytes32 hash) be used for efficiency reasons, I've included the toEthSignedMessageHash(bytes memory s) function for completeness. If you want to remove the latter, let me know.

Due to the use of staticcall and gas() to call ecrecover in assembly,
the recover function has to be marked as view instead of pure.

If you know how to force the compiler to accept the function as pure, let me know.

Checklist

Ensure you completed all of the steps below before submitting your pull request:

• Ran forge snapshot?
• Ran npm run lint?
• Ran forge test?

Pull requests with an incomplete checklist will be thrown out.

[Vectorized]

Ok, let it marinate for a day or few.

[pcaversaccio]

@Vectorized I wanted you make you aware of a recent finding (h/t to @axic).

All client implementations of the precompile ecrecover (address 0x01) apparently check if the value v is 27 or 28. The references for the different client implementations can be found in the already merged Yellow Paper PR #860. There are however as well good reasons to keep the checks: OpenZeppelin/openzeppelin-contracts#3457 (comment). Nonetheless, checking a condition twice (library & pre-compile implementation) is still not right IMHO.

[Vectorized]

@pcaversaccio I have already removed the check and rearranged some opcodes for lesser stack operations. ;)

e8aaa05

I also knew that the precompile had the checks all along lol 😂 But kept the check back then cuz it made the invalid signature case much cheaper (by 3k gas).

Glad that the provocative PR inspired a yellowpaper clarification.

But the tweets made me rethink it’s better to optimize for the happy case for this PR (since we aren’t reverting here).

[pcaversaccio]

@Vectorized I just saw your recent commit. I looked at it yesterday and quickly chatted with @transmissions11 first and then came back today to raise it without checking for new commits. Yeah agreed on the happy case :-D - I mean who doesn't want to optimise for the happy case lol

[Vectorized]

@1kresh If you can't wait use this first: https://github.com/Vectorized/solady

[pcaversaccio]

Is it still planned to get this merged?

[Czar102]

Any reason this PR doesn't seem to be getting merged?