refactor: use enum to distinguish transaction signature types

[Wodann]

This PR introduces a way to statically distinguish between a fake signature and an ECDSA signature with recoverable address.

The introduced signature::Fakeable<SignatureT> also handles caching of the address to ensure that it only will be recovered once.

These changes allow the future removal of the ExecutableTransaction, as the caller address will now always be part of transaction::Signed

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 7749b220ae81b9cabe4f58c12d52641cbb8d0ff3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[Wodann]

EcdsaWithYParity struct

The EcdsaWithYParity struct is a bit weird from a conceptual perspective, because the y parity can be always recovered from the v value which the Ecdsa struct has. As I understand the distinction comes from the fact that EIP-2930 and later transactions only include the r and s values and an odd_y_parity flag, as opposed to r, s and v values. For the internal library of an Ethereum specific tool I think it's ok if this leaks into the signature types, but ideally we'd only have a signature::Ecdsa struct where we set the v value based on the odd_y_parity and chain_id from the transaction.

I agree that the odd_y_parity can be calculated from the v value. However, the RLP-encoding and -decoding of EIP-2930 and later transactions is different as well, given that the odd_y_parity is only 1 byte, as opposed to the 8-bytes that legacy and EIP-155 transactions RLP-encode/-decode. They should also be mutually exclusive; i.e.

• Legacy/EIP155: Must have Ecdsa signature
• EIP2930/EIP1559/EIP4844: Must have EcdsaWithYParity signature

As this distinction needs to happen at compile-time, I expressed that with two different types: Ecdsa and EcdsaWithYParity. Previously this was achieved by having the r, s,, and odd_y_parity fields listed separately in the Eip2930 and later transactions, whereas the Legacy and Eip155 transactions included the Signature.

Signature trait

My understanding is that the purpose of the Signature trait is to provide a sum type over certain types in the edr_eth crate. I tend to prefer designs where sum types are represented with enums as this brings more conceptual clarity and prevents type erasure and dynamic dispatch.

I'm not trying to achieve a sum type, as the signature type that a transaction needs to receive is mutually exclusive (unless you consider polymorphism a form of sum type). Given that there are two compile-time types Ecdsa and EcdsaWithYParity (see above argumentation) that contain the r, s, and v or odd_y_parity values, to achieve both compile-time and runtime polymorphism, I introduced the trait that returns those values.

It's hard to say whether dynamic dispatch would be faster or slower than having a sum type as the compiler will potentially apply optimisations in both cases.

Given that there is only one location at which I'm using the trait for a dynamic object, I can easily remove that instance by exposing the r, s, v, and odd_y_parity values directly on the transaction::Signed. I'm happy to do that to simplify the code.

[agostbiro]

As this distinction needs to happen at compile-time, I expressed that with two different types: Ecdsa and EcdsaWithYParity. Previously this was achieved by having the r, s,, and odd_y_parity fields listed separately in the Eip2930 and later transactions, whereas the Legacy and Eip155 transactions included the Signature.

Ok makes sense!

I'm not trying to achieve a sum type, as the signature type that a transaction needs to receive is mutually exclusive (unless you consider polymorphism a form of sum type). Given that there are two compile-time types Ecdsa and EcdsaWithYParity (see above argumentation) that contain the r, s, and v or odd_y_parity values, to achieve both compile-time and runtime polymorphism, I introduced the trait that returns those values.

Ok so we want to abstract over different (data) types here. This is often done with polymorphism in object-oriented languages, but in languages that support sum types I prefer to do that as I find that it's easier to work with. In any case, I think it's ok to merge as is, so not a change request.