derive-eip712: initial implementation of eip712 derive macro

[Ryanmtate]

This commit provides an initial implementation for a derive macro
to encode typed data according to EIP-712, https://eips.ethereum.org/EIPS/eip-712

Additionally, this commit introduces a new signer trait method:

    async fn sign_typed_data<T: Eip712 + Send + Sync>(
        &self,
        payload: &T,
    ) -> Result<Signature, Self::Error>;

And implements the new method for each of the signers (wallet, ledger,
aws).

At the moment, derive does not recurse the primary type to find nested
Eip712 structs. In the future, a field helper attribute #[eip712] could be
used to denote a nested field and handle the proper encoding for the field.

Motivation

#16

Solution

This implementation is based on the proposed solution in #16 .

PR Checklist

• Added Tests
• Added Documentation
• Updated the changelog

[mattsse]

some comments/questions/recommendations

[Ryanmtate]

@gakonst @mattsse Looking to finish up on the example and tests today and open for a review.

One last issue I am battling is with the example/eip712.rs final encoding.

Able to confirm domain separator, type hash and struct hash are all matching their solidity equivalents, but for some reason the final encoding does not match between the contract and the Eip712 trait derived macro implementing.

examples/DeriveEip712Test.sol and examples/derive_eip712_abi.json contain the full example, but the problem method is, encodeEip712:

function encodeEip712(FooBar memory fooBar) public pure returns (bytes32) {
        return
            keccak256(
                abi.encodePacked(
                    "\\x19\\x01",
                    domainSeparator(),
                    structHash(fooBar)
                )
            );
    }

examples/eip712.rs file contains full text, but the test seems to fail on encodeEip712 comparison:

let domain_separator = contract.domain_separator().call().await?;
    let type_hash = contract.type_hash().call().await?;
    let struct_hash = contract.struct_hash(derived_foo_bar.clone()).call().await?;
    let encoded = contract
        .encode_eip_712(derived_foo_bar.clone())
        .call()
        .await?;
    let verify = contract
        .verify_foo_bar(wallet.address(), derived_foo_bar, r, s, v)
        .call()
        .await?;

    assert_eq!(
        domain_separator,
        FooBar::domain_separator()?,
        "domain separator does not match contract domain separator!"
    );

    assert_eq!(
        type_hash,
        FooBar::type_hash()?,
        "type hash does not match contract struct type hash!"
    );

    assert_eq!(
        struct_hash,
        foo_bar.clone().struct_hash()?,
        "struct hash does not match contract struct struct hash!"
    );

    assert_eq!(
        encoded,
        foo_bar.encode_eip712()?,
        "Encoded value does not match!"
    );

[mattsse]

hmm,
just to make sure I understand that snippet,

• domain seprator matches
• struct hash matches

but encode_eip712 fails?

this would mean that

                abi.encodePacked(
                    "\\x19\\x01",
                    domainSeparator(),
                    structHash(fooBar)
                )

is not identical to

               let digest_input = [
                    &[0x19, 0x01],
                    &Self::domain_separator()?[..],
                    &self.struct_hash()?[..]
                ].concat();

and since the values (domainSeparator, structHash) are identical (ensured by the test), this would indicate encodePacked is not identical to concat?
could you check that?

[gakonst]

I think abi.encode is what's equivalent to concat. encodePacked is a non-standard encoding. It may be exposed via ethabi though I haven't investigated.

[Ryanmtate]

Great news! The problem was much simpler. Let it be known that \\x19\\x01 does not encode to &[x19, x01], but \x19\x01 does.

Going to open this up for review after resolving merge conflicts.

[mattsse]

Great news! The problem was much simpler. Let it be known that \\x19\\x01 does not encode to &[x19, x01], but \x19\x01 does.

Going to open this up for review after resolving merge conflicts.

does &[x19, x01][..] do?

[Ryanmtate]

Finding some issues in the signer methods, so let's hold off from merging until can confirm correctness.

[Ryanmtate]

Was able to manually test 909c491 is working for sign typed data and recoverable inside solidity with ecrecover for ledger signer! 🎉

I have not tested AWS signer, and would love if someone who has a setup can check it out.

[Ryanmtate]

Also still open to comments for possibly relocating files to enable re-exportation of the derive macro. I agree current user experience is not great with having to add another dependency. But doing will require some re-working of the file struct so we are not hitting a circular dependency.

[Ryanmtate]

Thanks to @sebastinez for helping test ledger signer!

[gakonst]

@mattsse @Ryanmtate I think the best way to resolve the cyclical dependency is for us to make an ethers-macros crate where we hold all useful derives that are low-level type related. We'd still keep abigen in ethers-contract.

I'm OK with merging as-is now, and we can resolve the deps in a follow-up PR

[mattsse]

@mattsse @Ryanmtate I think the best way to resolve the cyclical dependency is for us to make an ethers-macros crate where we hold all useful derives that are low-level type related. We'd still keep abigen in ethers-contract.

I'm OK with merging as-is now, and we can resolve the deps in a follow-up PR

second that,
amazing work @Ryanmtate

[gakonst]

---- eth_tests::test_derive_eip712 stdout ----
thread 'eth_tests::test_derive_eip712' panicked at 'called `Result::unwrap()` on an `Err` value: SolcError("Error: Source file requires different compiler version (current compiler is 0.6.6+commit.6c089d02.Linux.g++ - note that nightly builds are considered to be strictly less than the released version\n --> tests/solidity-contracts/DeriveEip712Test.sol:2:1:\n  |\n2 | pragma solidity ^0.8.0;\n  | ^^^^^^^^^^^^^^^^^^^^^^^\n\n")', ethers-contract/tests/common/mod.rs:35:10
error: test failed, to rerun pass '-p ethers-contract --test contract'
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Just need to fix the solc version to be the same one as the one installed in CI.

The WASM test is also seemingly failing, is this a flake?

[mattsse]

---- eth_tests::test_derive_eip712 stdout ----
thread 'eth_tests::test_derive_eip712' panicked at 'called `Result::unwrap()` on an `Err` value: SolcError("Error: Source file requires different compiler version (current compiler is 0.6.6+commit.6c089d02.Linux.g++ - note that nightly builds are considered to be strictly less than the released version\n --> tests/solidity-contracts/DeriveEip712Test.sol:2:1:\n  |\n2 | pragma solidity ^0.8.0;\n  | ^^^^^^^^^^^^^^^^^^^^^^^\n\n")', ethers-contract/tests/common/mod.rs:35:10
error: test failed, to rerun pass '-p ethers-contract --test contract'
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Just need to fix the solc version to be the same one as the one installed in CI.

The WASM test is also seemingly failing, is this a flake?

weird, only the http provider wasm test fails due to a 400 MiddlewareError(MiddlewareError(JsonRpcClientError(SerdeJson { err: Error("invalid type: integer400, expected struct Response", line: 1, column: 3), text: "400 Bad Request" })))
still works locally -.-

[Ryanmtate]

Thanks @gakonst @mattsse for the help and pointers!

[gakonst]

@Ryanmtate need to enable abi encoder v2 i think

[Ryanmtate]

🎉

[sebastinez]

kudos @Ryanmtate! 🥳