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

updated implementation details in EIP-712 #3763

Merged
merged 2 commits into from
Oct 3, 2021
Merged

Conversation

wschwab
Copy link
Contributor

@wschwab wschwab commented Aug 24, 2021

No description provided.

@eth-bot eth-bot enabled auto-merge (squash) August 24, 2021 15:42
@eth-bot
Copy link
Collaborator

eth-bot commented Aug 24, 2021

All tests passed; auto-merging...

Copy link
Contributor

@recmo recmo left a comment

Choose a reason for hiding this comment

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

Awesome! 🙌


* `keccak256` struct hashing

While the `keccak256` function in Solidity cannot be invoked directly on structs, the introduction of `abi.encode` gives a sufficient and well-accepted avenue to hashing structs.
Copy link
Contributor

Choose a reason for hiding this comment

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

And when gas cost matters, dropping to assembly is always an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a look! Do you think this needs to be included in the text, or should it be left as is?

@MicahZoltu MicahZoltu closed this Oct 3, 2021
auto-merge was automatically disabled October 3, 2021 10:54

Pull request was closed

@MicahZoltu MicahZoltu reopened this Oct 3, 2021
@eth-bot eth-bot enabled auto-merge (squash) October 3, 2021 10:54
@MicahZoltu
Copy link
Contributor

CI failing on:

./EIPS/eip-712.md:432: experimnetal ==> experimental
./EIPS/eip-712.md:447: sufficently ==> sufficiently

auto-merge was automatically disabled October 3, 2021 13:34

Head branch was pushed to by a user without write access

@eth-bot eth-bot enabled auto-merge (squash) October 3, 2021 13:34
@eth-bot eth-bot merged commit 332c0c6 into ethereum:master Oct 3, 2021
drortirosh pushed a commit to drortirosh/EIPs that referenced this pull request Oct 3, 2021
* updated implementation details in EIP-712

* fixed typos
PhABC pushed a commit to PhABC/EIPs that referenced this pull request Jan 25, 2022
* updated implementation details in EIP-712

* fixed typos
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