ECDSA: add parse and tryParse

[Amxx]

Following discussion with @frangio, we believe that this could be usefull for someone that wants to re-implement isValidSignatureNow in a way taht supports both 65 and 64 bytes signatures.

We should add this support nativelly in v6.0, but for the same reason as the one mentioned in this GHSA this is not a change that should happen in a minor release.

function isValidSignatureNow64or65(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) {
        if (signer.code.length == 0) {
            (uint8 v, bytes32 r, bytes32 s) = ECDSA.parse(signature);
            (address recovered, ECDSA.RecoverError err, ) = ECDSA.tryRecover(hash, v, r, s);
            return err == ECDSA.RecoverError.NoError && recovered == signer;
        } else {
            return isValidERC1271SignatureNow(signer, hash, signature);
        }
    }

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

🦋 Changeset detected

Latest commit: 23c393f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
openzeppelin-solidity	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[frangio]

For discussion (I'm undecided): should this have a special name given that, unlike the rest of this library, it allows 64-65 byte malleability? E.g., parseMalleable.

[Amxx]

Would that note be good enough ?

IMO not using signatures as unique identifier is basic good practices, and I'm not sure we should complexify the name of the functions (creating possible confusion?) as a way to "help enforce" these basic good practices.

[frangio]

not using signatures as unique identifier is basic good practices

Yeah I agree and we should move towards this assumption. My point is only about how this breaks from the existing library design.

How about deprecating malleability protection in all of the ECDSA library?

[Amxx]

How about deprecating malleability protection in all of the ECDSA library?

YES !

[Amxx]

@frangio how does that look ?
144b0e2

[arr00]

Why is v a signed integer?

[Amxx]

Why is v a signed integer?

Because I made a mistake. Thanks for spotting it !