-
Notifications
You must be signed in to change notification settings - Fork 879
Update ERC-8063: Move to Review #1500
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
Conversation
- Clarify description: groups of Ethereum accounts (EOAs and contracts) - Expand Abstract with coordination/access control use cases and examples - Rewrite semantics as observable contract behavior (MUST revert conditions) - Allow owner leaveGroup to either revert or transfer ownership first - Add ERC-165 discoverability note for ownership transfer extension - Link reference implementation files - Remove RFC2119 keywords from Security Considerations (non-normative) - Fix assets README path reference
|
✅ All reviewers have approved. |
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
Major changes based on Ethereum Magicians review: - Redefine Groups as ERC-20 tokens with balance capped at 1 - Require ERC-5679 for mint/burn instead of custom addMember/removeMember - Remove owner() from interface, recommend ERC-173 for ownership - Add canMint/canBurn for access control introspection - Make group-friendly aliases (addMember, isMember, etc.) optional - Remove redundant ERC8063ERC20.sol (ERC-20 is now the base) This provides instant compatibility with existing wallets, explorers, and tooling while maintaining the group membership semantics.
Head branch was pushed to by a user without write access
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
Credit @SamWilsn for significant design contributions during review. Also update author name from James Savechives to Cheng Qian.
Head branch was pushed to by a user without write access
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
Head branch was pushed to by a user without write access
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
jochem-brouwer
left a comment
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.
The changes look good, however the move of the files from the erc-8063 folder into the eip-8063 folder is not correct (should be inside the erc-8063 folder)
Co-authored-by: Cursor <cursoragent@cursor.com>
ok, done! |
|
The commit 7ed1ffc (as a parent of a054828) contains errors. |
jochem-brouwer
left a comment
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.
LGTM!
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
|
Ah, the CI is failing, that should be fixed! |
jochem-brouwer
left a comment
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 is failing and should be fixed
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
Co-authored-by: Cursor <cursoragent@cursor.com>
Head branch was pushed to by a user without write access
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
eip-review-bot
left a comment
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.
All Reviewers Have Approved; Performing Automatic Merge...
Thanks! Done! |
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: