-
Notifications
You must be signed in to change notification settings - Fork 194
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
feat(world-modules): register delegation with signature #2480
Conversation
🦋 Changeset detectedLatest commit: 25ed862 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
} | ||
|
||
function install(bytes memory) public pure { | ||
revert Module_RootInstallNotSupported(); |
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.
revert Module_RootInstallNotSupported(); | |
revert Module_NonRootInstallNotSupported(); |
// If the message was not signed by the delegator or is invalid, revert | ||
address signer = ECDSA.recover(hash, signature); | ||
if (signer != delegator) { | ||
revert InvalidSigner(delegator, delegatee); |
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.
I think InvalidSignature
would be more descriptive here, since the signer would not match the delegator if any piece of the signature doesn't match (delegatee, nonce, etc)
revert InvalidSigner(delegator, delegatee); | |
revert InvalidSignature(signer); |
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.
since this file exports a single constant called delegationWithSignatureTypes
we should call the file the same
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.
I want to really make sure it's clear this module is not meant for stable usage yet and we will remove it in the near future once it's audited and moved into the World. Should we rename it to Unstable_DelegationWithSignatureSystem
for now?
Addresses #2383
This PR adds a new
registerDelegationWithSignature
function to the World:and a new
UserDelegationNonces
table, to prevent reply attacks:Anyone can call
registerDelegationWithSignature
to register a delegation fordelegator
, provided that the given EIP-1271 signature is valid (ie. was signed for the same contract address, chain ID, nonce, delegation details etc.)If the signature is valid, the delegation is created and the nonce is incremented.