-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Move EIP-712 to Last Call #5271
Conversation
All tests passed; auto-merging...(pass) eip-712.md
|
So looking forward to it |
Please see the files tab where a new bot has made a bunch of comments (they don't currently trigger any email/notification). |
@MicahZoltu If I understand correctly these errors are due to EIP format requirements that have changed or become stricter since this document was written. Should I make the changes required to fit the new format? Will those interfere with the attempt to move to Last Call status? |
Yes please!
Nope. Status changes are usually where we make sure EIPs are up to date with the latest format requirements. Do note, however, that substantial changes after the EIP enters last call might require resetting the last call deadline. |
This should be ready to merge once one of the authors approves it. |
@SamWilsn This has got author approval. Just need editor approval now. 🙂 |
The commit 451b062 (as a parent of 213ef96) contains errors. Please inspect the Run Summary for details. |
Ok I've moved the existing section about replay attacks into a Security Considerations section, I think we should be good now. cc @SamWilsn |
I've added a small note concerning the distribution of the signature and in particular frontrunning attacks, as suggested by @xinbenlv, which should be uncontroversial. But this PR really shouldn't result in any further modifications as it already has author approval. Please merge as soon as possible! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try this again.
@frangio please push an empty commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI should be fixed
* Move EIP-712 to Last Call * fix sections * fix eip references * remove external links * shorten title * reformat notes * remove self references * add links to referenced eips * add description * add tentative last call deadline * remove standard from description * add specification header * convert image to relative Co-authored-by: Sam Wilson <[email protected]> * use relative image src * remove mention of EIP-719 * remove implementation section * simplify abstract * add security considerations section * fix section order * add note on frontrunning attacks * empty commit Co-authored-by: Sam Wilson <[email protected]>
There have been no substantial changes to this EIP in over a year.
This PR also includes non-normative changes to adapt the EIP to new formatting requirements. A mention of "EIP-719" was removed because said EIP doesn't exist. The "Implementation" section was removed as it consisted of an implementation log that doesn't belong in the EIP format.