Fix bug in signature parsing for EIP712TypedData

[eelanagaraj]

Description

When parsing EIP712 signed typed data in SignatureUtils.recoverEIP712TypedDataSigner, we break and return a value as soon as a signature can be parsed. The issue is that 1) we do not serialize signatures consistently (examples from the monorepo: SignatureUtils.serializeSignature serializes as VRS, but LocalSigner.signTypedData returns an RSV-serialized sig) 2) some small percentage of signatures can be parsed as both RSV & VRS signatures, in which case the order of the parsing matters as long as we do not check against the expected signer.

Given that the usage in offchain-data-wrapper is relatively new (and breaking changes shouldn't be too disruptive), the proposed fix splits up recoverEIP712TypedDataSigner into two (VRS & RSV-specific, so the caller can choose to explicitly call these if it's necessary to "guess" an address), while modifying the verify function to be in line with how the existing message parsing verifySignature function works -- i.e. boolean that suppresses errors but works regardless of RSV/VRS serialization.

This is a breaking change in that recoverEIP712TypedDataSigner has been deprecated in favor of two functions.

Other changes

• wraps SignatureUtils.verifyEIP712TypedDataSigner in a try/catch to ensure that it always returns a boolean (and no longer propagates the error from the recovery step). This is in line with the other signature parsing/verification functions.
• adds unit tests for recover & verify: these are pretty dumb and based on hardcoded examples in order to not add the CK/LocalWallet dependencies to the utils package -- open to thoughts on this though, if you think it's worthwhile to randomize PKs but incorporate these dependencies)

Testing

• unit tests for recover & verify against a hard-coded edge case that was failing
• existing unit tests for offchain-data-wrapper pass
• programmatically tested that previous issue with certain AS codes causing this is resolved for large ranges of codes
• TODO deploy to alfajores AS + make sure no changes to existing flow (also tried with contrived code examples that would have thrown the Invalid signature error, but now throw "Invalid code" as we want)

Closes #8812

Edit: further notes for future documentation:

Notes on implementation choices in offchain-data-wrapper and future TODOs here:

Initially @nambrot and I thought it would make sense to double down on the usage of just VRS-serialization. This PR diverges from that initial thought due to the following:

• in PublicSimpleAccessor.sign we are actually using the wallet.signTypedData method which returns an RSV-signature (we can switch this to using the CK.signTypedData + SignatureUtils.serializeSignature, but this increases the scope of this PR)
• we could just use the RSV recover function, but this moves away from compatibility with serializeSignature (VRS)
• (even within the wrapper code, we use both wallet.signTypedData and serializeSignature (in the tests), so standardizing should take a more rigorous comb through of everything here IMO)

The current implementation is slightly less efficient than before, but should not suffer from the same edge case that this PR fixes (except in the case that the guessedSignature must still be used, which seems unavoidable). Notes on this:

• we repeat signature recoveries here (i.e. use the verifyEIP712TypedDataSigner) which is less efficient (on the order of a few milliseconds in total)
• if we don’t have a single standard signature, then moving the signature-guessing logic up will either 1) reintroduce the parsing edge case or 2) make it less readable & duplicates logic IMO (try to parse both, check equality against up to 2 parsed sigs)

I ultimately opted for minimizing the amount of signature standardization across the module for now and eliminating the possible edge case, and would be slightly in favor of splitting out signature standardization across the offchain-data-wrapper into a separate ticket for the future.

TODOs

• [immediate] create a ticket for the below and link here -> created issue Standardize signing serialization offchain-data-wrapper #8862 (assigned to Applications initially)

[codyborn]

Very thorough per usual! 🚢