Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

EIP712 Support #16

Closed
gakonst opened this issue Jun 16, 2020 · 11 comments
Closed

EIP712 Support #16

gakonst opened this issue Jun 16, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@gakonst
Copy link
Owner

gakonst commented Jun 16, 2020

Is your feature request related to a problem? Please describe.

EIP712 support is missing.

Describe the solution you'd like

We should be able to sign messages using EIP712

Additional context

There's an open PR for it in Web3js web3/web3.js#3449
0xProject/0x-protocol-specification#11

@gakonst gakonst added the enhancement New feature or request label Jun 16, 2020
@fubuloubu
Copy link

Relevant:
tomusdrw/rust-web3#169
https://crates.io/crates/eip-712

@roynalnaruto
Copy link
Collaborator

Hello @gakonst if you have not worked on this issue, I would like to take it up. The plan is to use the above mentioned crate.

One more point to note is that, the Signer trait currently supports a sign_message which eventually hashes the message. I would need to add:

/// Signs the provided message
fn sign_hashed_message<S: AsRef<[u8]>>(&self, message: S) -> Signature;

where the message argument is already hashed, and hence won't be hashed before signing. This requirement is because the hash_structured_data function already hashes the encoded data.

@gakonst
Copy link
Owner Author

gakonst commented Jun 18, 2020

I believe that this wouldn't be compatible with the MIT / Apache-2 license. That said, I think we could re-implement that functionality using a custom Serde visitor instead of the parser they use?

@roynalnaruto
Copy link
Collaborator

I believe that this wouldn't be compatible with the MIT / Apache-2 license. That said, I think we could re-implement that functionality using a custom Serde visitor instead of the parser they use?

Okay, I will go through Serde's docs and try to implement it :)

@mattsse
Copy link
Collaborator

mattsse commented Dec 18, 2020

Any progress on this?
There is now graphprotocol/eip-712-derive.
The repo contains no license yet, however author hinted that is MIT

@gakonst
Copy link
Owner Author

gakonst commented Dec 18, 2020

Nobody that I know of is working on it, so if you'd like to integrate it, along with an eth sign typed data call I'd be more than happy to merge - thank you for surfacing that repo, didn't realize it existed!

@gakonst
Copy link
Owner Author

gakonst commented Sep 10, 2021

Here's another approach:

  1. Define a trait EIP712
  2. Define a struct e.g. struct Name { field1: U256; field2: u65 }
  3. Have a derive macro which implements the EIP712 trait for that struct

The trait could have a fn typehash() which auto generates the typehash from the datatype, similar to this and a fn input(&self) which would abi encode each field of the struct, same as this

And then you’d have a StructName.eip712_encode() which calculates the full hash by calling:

  1. self.domain_separator()
  2. self.typehash
  3. calculate input
  4. hash them together

We could add an extra param on the derive macro which would set name and version for the domain separator, maybe let verifying_contract and chainid be defined at runtime, since you may want to use the same e.g permit struct for many contracts or many chains (it’d be nice if both could work)

Strawman syntax:

#[derive(Debug, EIP712)]
#[eip712(name = “Uniswap V2, version = “1, chainId = 1, verifyingContract = “0xB4e16d0168e52d35CaCD2c6185b44281Ec28C9Dc)]
struct Permit {
    owner: Address,
    spender: Address,
    value: U256,
    nonce: U256,
    deadline: U256,

let permit = Permit {};
dbg!(permit.typed_hash())

It’d basically be very abigen-like, and I think I prefer it to the approach that the Graph took.

The trait may not be necessary by default, by I suspect it’d be useful for 3rd party consumers, to be abstract over the data that you want to typehash for (e.g. wallets)

@Ryanmtate
Copy link
Contributor

Opened #481 as a draft PR. It's a first iteration and is lacking test coverage against solidity and other libs, such as https://crates.io/crates/eip-712.

Additionally, the macro currently does not recurse all nested Eip712 structs.

I welcome early feedback on the code and any thoughts on improvements. Once test coverage and nested structs are completed, I'll move the PR from draft status to open.

@Ryanmtate
Copy link
Contributor

#481 closes this issue. A new issue should be opened to track nested Eip712 structs and another issue for crate restructuring to re-export the macro.

This was referenced Oct 9, 2021
@gakonst gakonst closed this as completed Oct 9, 2021
@gakonst
Copy link
Owner Author

gakonst commented Oct 9, 2021

Issues opened, thank you for the hard work on this.

@Ryanmtate
Copy link
Contributor

Issues opened, thank you for the hard work on this.

No problem! Will keep an eye on those issues and lend a hand where needed! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants