Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover Signed Messages #2

Closed
dternyak opened this issue Sep 15, 2018 · 7 comments
Closed

Recover Signed Messages #2

dternyak opened this issue Sep 15, 2018 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@dternyak
Copy link

First off - thanks a ton for your work on this! Getting tooling around EIP-712 is going to be critical for it to be adopted so I applaud this effort!

I wanted to see if you had any intention of implementing recovery / verification for EIP-712, so that you can verify signed messages, e.g. where they are generated on the client and sent to a backend.

@rmeissner rmeissner self-assigned this Sep 15, 2018
@rmeissner rmeissner added the enhancement New feature or request label Sep 15, 2018
@rmeissner
Copy link
Owner

I assume you are talking about something like signTypedData and recoverTypedSignature in https://github.com/MetaMask/eth-sig-util

I was planning adding similar methods as provided by the js version. Currently we only use the hash generation in the libs for the Gnosis Safe Relay:

  1. We generate the hash for our transaction (https://github.com/gnosis/gnosis-py/blob/master/gnosis/safe/safe_service.py#L455)
  2. Use ethereum.utils to recover (https://github.com/gnosis/gnosis-py/blob/master/gnosis/safe/ethereum_service.py#L146)

So the idea would be to add a method for this and a method to generate the signature, correct?

@dternyak
Copy link
Author

dternyak commented Sep 15, 2018 via email

@rmeissner
Copy link
Owner

I can add that (I don't have time next week, but I should manage the week after)

@dternyak
Copy link
Author

dternyak commented Sep 15, 2018 via email

rmeissner added a commit that referenced this issue Sep 16, 2018
Added signing methods (Closes #2)
@rmeissner
Copy link
Owner

@dternyak I added a sign_typed_data(data, private_key) method, a recover_typed_data(data, v, r, s) and some helper methods. Let me know if there are any questions ;)

@dternyak
Copy link
Author

dternyak commented Sep 17, 2018

Amazing! Thank you so so much. I'm blocked by this on a project I'm working on, so I'm super appreciative on the quick changes.

Unfortunately, I've realized that that there appears to be multiple revisions of EIP-712, with recover_typed_data targeting an older revision that isn't compatible with MetaMask's signing format targeting the newest revision.

The newer revision has a format like so:

[
    {
        "type": 'string',
        "name": 'Message',
        "value": 'Hi, Alice!',
    },
    {
        "type": 'uint32',
        "name": 'A number',
        "value": '1337',
    },
]

That decision seems to have been made here: MetaMask/metamask-extension#4803 (comment).

My method for generating the signed message is via MetaMask, which uses the newer revision compared to the python based message signing that was in your linked examples.

I've been able to confirm eth-sig-util's recoverTypedSignature works with MetaMask's message signing.

So, I think that to get this working with the newest revision, we'll need to port this as well.

For reference, this is how I'm generating the signed message with MetaMask today:

  async signMessage() {
    console.log('AM SIGNING MESSAGE');
    const { accounts, web3 } = this.props;
    const from = accounts[0];
    console.log(from)
    const msgParams = [
      {
        type: 'string',
        name: 'Message',
        value: 'Hi, Alice!',
      },
      {
        type: 'uint32',
        name: 'A number',
        value: '1337',
      },
    ];

    /*  web3.eth.signTypedData not yet implemented!!!
     *  We're going to have to assemble the tx manually!
     *  This is what it would probably look like, though:
      web3.eth.signTypedData(msg, from) function (err, result) {
        if (err) return console.error(err)
        console.log('PERSONAL SIGNED:' + result)
      })
    */

    console.log('CLICKED, SENDING PERSONAL SIGN REQ');
    const params = [msgParams, from];
    console.dir(params);
    const method = 'eth_signTypedData';

    web3.currentProvider.sendAsync(
      {
        method,
        params,
        from,
      },
      (err, result) => {
        if (err) return console.dir(err);
        if (result.error) {
          alert(result.error.message);
        }
        console.log(result.result)
      },
    );
  }

I'd be happy to take a shot at porting the newest recover myself, and I really appreciate all the work you've done here already.

I'll check back tomorrow to see if you've had a chance to mull over this wall of text, and I'll also attempt to add the new recover methods if you agree with my analysis here. I'm pretty new to EIP-712, so I wouldn't be surprised if my understanding of the spec (and it's multiple revisions) is incorrect.

@rmeissner
Copy link
Owner

rmeissner commented Sep 17, 2018

I am actually running against the latest tests of the sig utils of meta mask. eth_signTypedData still points to an older version of signTypedData that is currently supported in metamask.

See MetaMask/metamask-extension#4803 (comment)

Only once that PR is merged you will be able to make use of the new EIP712 standard by using eth_signTypedData_v3

To recover from that you need to use https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L318

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

No branches or pull requests

2 participants