This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 13
219 additional zk email recovery reference implementations #224
Merged
jacque006
merged 12 commits into
main
from
219-additional-zk-email-recovery-reference-implementations
Apr 2, 2024
Merged
219 additional zk email recovery reference implementations #224
jacque006
merged 12 commits into
main
from
219-additional-zk-email-recovery-reference-implementations
Apr 2, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 29, 2024
jacque006
approved these changes
Apr 2, 2024
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.
Did not dig too deeply into SimpleAccount
, ERC-7579, & ERC-6900 modules per notes, will do so with tests & EmailAuth integrations later. Nice work & good notes 👍
jacque006
reviewed
Apr 2, 2024
Comment on lines
+11
to
+13
/*////////////////////////////////////////////////////////////////////////// | ||
THIS CONTRACT IS STILL IN ACTIVE DEVELOPMENT. NOT FOR PRODUCTION USE | ||
//////////////////////////////////////////////////////////////////////////*/ |
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.
Thanks for adding these. In the future, I think we may need a better way to delineate this, such as placing them in a different directory when ready for production use.
jacque006
deleted the
219-additional-zk-email-recovery-reference-implementations
branch
April 2, 2024 05:54
FYI @voltrevo |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this PR doing?
@account-abstraction/utils
v0.7.0 has not been published, so some helper functions that are compatible with 0.7.0 contracts were pulled frometh-infinitism/bundler
. The helper functions are located in the new fileuserOpUtils.ts
- these functions are basically a copy and paste except updating to ethers v6 and copying one ethers v5 function that has behaviour that zero pads variables in a way that the new functions expect (the ethers v6 equivalent broke things).Since this PR has got quite big from the 0.7.0 update, and we have that deadline (which involves the Safe plugins rather than these new implementations), going to add tests for the new implementations in a separate issue - #227. Note: while the new contracts compile and abide by the required interfaces, there will be issues with the logic so a meticulous review of these probably isn't needed until #227. Feel free to point out any issues regardless ofc.
How can these changes be manually tested?
Does this PR resolve or contribute to any issues?
Checklist
Guidelines
resolve conversation
button is for reviewers, not authors