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(docs): EIP-712 Tutorial #252

Merged
merged 5 commits into from
May 27, 2022
Merged

Conversation

kulkarohan
Copy link
Contributor

resolves #159

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the example - thank you for taking the time for this.

But maybe something that is easier for people to grok would be just showing how to use permit in ERC2612 / DAI? That is something that you'll encounter more frequently!

i.e. show that instead of doing approve-> function that calls transferFrom you can do permit off-chain and then function that calls transferFrom including the v,r,s or the signature?

On the code pattern:

Can you replace DSTest with forge-std/Test? Would let you remove the need to specify VM internal vm and instantiate it.

Also would recommend moving EIP_712_DOMAIN_SEPARATOR and SIGNED_TICKET_TYPEHASH into helper fns / state vars?

some helper fns which make it clearer:
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol#L228
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/draft-EIP712.sol#L53-L103

@kulkarohan
Copy link
Contributor Author

hmmm yeah agreed. i'll update

@gigamesh
Copy link

gigamesh commented Apr 29, 2022

Sorry to jump in on this thread but something I've been trying to figure out is how to use forge to test a signed message that is provided to the function as a single string:
https://github.com/soundxyz/protocol/blob/63f5219dca6e91ef27e2922c96473dfacaccb400/contracts/ArtistV4.sol#L450

Is there a way to convert the v, r, s returned from vm.sign() into a bytes signature?

For context, the way I'm creating the signature off chain is with ethers.js's wallet._signTypedData

@kulkarohan
Copy link
Contributor Author

hey @gigamesh! bytes signature = abi.encodePacked(r, s, v) should do the trick

@gigamesh
Copy link

gigamesh commented May 1, 2022

ah ok super easy - thank you!

@kulkarohan
Copy link
Contributor Author

@gakonst @onbjerg sorry for the absurd delay on this. let me know if any further changes are desired and i'll update ASAP!!

Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's iterate on it if needed :)

@onbjerg onbjerg merged commit f11b317 into foundry-rs:master May 27, 2022
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.

Add guide for EIP-712 signatures with vm.sign / vm.addr
4 participants