-
Notifications
You must be signed in to change notification settings - Fork 479
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
Add ERC: Signature Aggregation for ERC-4337 #626
Add ERC: Signature Aggregation for ERC-4337 #626
Conversation
✅ All reviewers have approved. |
ERCS/erc-4337-agg.md
Outdated
description: An ERC-4337 improvement to aggregation of all UserOperation signatures in a bundle | ||
title: Signature Aggregation for Account Abstraction | ||
author: Vitalik Buterin (@vbuterin), Yoav Weiss (@yoavw), Dror Tirosh (@drortirosh), Shahaf Nacson (@shahafn), Alex Forshtat (@forshtat) | ||
discussions-to: |
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.
Please create a discussions topic in Eth Magicians
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.
Created.
ERCS/erc-4337-agg.md
Outdated
@@ -0,0 +1,145 @@ | |||
--- | |||
eip: | |||
description: An ERC-4337 improvement to aggregation of all UserOperation signatures in a bundle |
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.
Description should be after title
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.
Fixed.
Co-authored-by: Andrew B Coathup <[email protected]>
The commit a44004c (as a parent of dcc3d53) contains errors. |
@abcoathup , what blocks this PR from being closed? |
@drortirosh you need an ERC editor to review (I am not an editor). There is only a small number of editors, so it can take some time to get reviewed. In the meantime, ensure that the validator doesn’t raise any errors & fix any issues. There is a fortnightly office hours that you could attend: ethcatherders/EIPIP#360 |
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.
Most of the comments are fine to fix in draft. The only blocker is the external link.
ERCS/erc-7766.md
Outdated
* **code: -32506** - transaction rejected because wallet specified unsupported signature aggregator | ||
* The `data` field SHOULD contain an `aggregator` value |
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.
This is a touch vague I think.
ERCS/erc-7766.md
Outdated
## Reference Implementation | ||
|
||
See `https://github.com/eth-infinitism/account-abstraction/tree/main/contracts` |
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.
This is still an external link. Please either copy your reference implementation to your assets/
directory, or remove this section entirely. Note that we cannot accept GPL'd code because it unfairly burdens implementers trying to follow your standard.
Co-authored-by: Sam Wilson <[email protected]>
Co-authored-by: Sam Wilson <[email protected]>
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.
- Be careful to always put inline code in backticks
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...
We're extracting the signature aggregator outside of ERC-4337, so we want first to create a new ERC.
PR for the new ERC is here: #627