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.
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
samples
directory and custom session key plugins #22Add
samples
directory and custom session key plugins #22Changes from 35 commits
0fb2113
e4b429c
f12a0ce
c7fd76f
f77df4c
57cf21d
3b63b0d
e097c21
d3ad5a7
82fc8fe
cfd4289
f2ad9a6
f61b085
3e5d7fb
65e6d1a
f2a74ad
b1367b5
c12d26d
3cb63a6
2252299
4b99521
d21659e
76aa433
efb951f
691a9bb
610533c
fd0cdeb
a5133fa
c38624c
bc48866
b59391c
844b47e
6a63c80
cb031e4
5a64a38
6288888
a8c6d36
f3436ad
7c833ac
0f755fb
874b578
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Area of improvement here would be to be able to clean up all of an account's session keys just based on
msg.sender
, without relying on what is passed intodata
. Ideally, the responsibility of proper cleanup lies with the plugin.This is probably a larger change though. Feel free to tackle this later.
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.
Though it will add some gas overhead, this sounds necessary to keep the consistency in the storage of plugin. I added an additional mapping that stores the information of session key and allowed selector to implement this.
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.
Check the result of tryRecover, and if
err != ECDSA.RecoverError.NoError
, revert withInvalidSignature()
.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.
Delaying SIG_VALIDATION_FAILED like this helps with gas estimation from bundlers when using dummy signatures.
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.
That seems valid. Fixed it.
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.
Nit
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.
Nit:
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.