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

feat: add support for EIP-712 message signing #1097

Merged

Conversation

lost-theory
Copy link
Contributor

This change adds a dependency for the eip712 package and uses it to implement a new LocalAccount.sign_message method.

What I did

Related issue: ApeWorX/eip712#4

How I did it

Added the eip712 dependency to requirements.in, used pip-compile to update requirements.txt, used that new package to implement the LocalAccount.sign_message method, wrote a new test that defines an EIP712Message and signs it using a local account with a private key loaded, verified the test result, ran lint & other pre-commit steps, added docs, added entry in the changelog.

How to verify it

I wrote a new test alongside the existing LocalAccount tests in test_accounts.py

$ pytest -k test_sign_message 
...
== 1 passed in 12.87s ==

Checklist

  • I have confirmed that my PR passes all linting checks
  • I have included test cases
  • I have updated the documentation
  • I have added an entry to the changelog

@lost-theory lost-theory force-pushed the feat/eip712-messages branch 2 times, most recently from 9dbc630 to ee61ea6 Compare June 15, 2021 01:28
@BlinkyStitt
Copy link
Collaborator

This looks great! if I could merge, I would.

@skellet0r
Copy link
Collaborator

@lost-theory pls rebase 🙏

This change adds a dependency for the `eip712` package and uses it to
implement a new `LocalAccount.sign_message` method.
@lost-theory
Copy link
Contributor Author

Rebased @skellet0r.

@iamdefinitelyahuman iamdefinitelyahuman merged commit f0cd282 into eth-brownie:master Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants