-
Notifications
You must be signed in to change notification settings - Fork 5
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
Split ERC-7821 executor into its own contract #61
Conversation
…zedExecutor function
function _erc7821AuthorizedExecutor( | ||
bytes32 /* mode */, | ||
bytes calldata /* executionData */ | ||
) internal view virtual returns (bool) { |
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.
any reason to to remove address caller
from the parameter, and using msg.sender
implicitelly. I have the feeling its nicer if this internal function can be called to check the permission of someone else.
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 wanted to make this function composable with onlyOwner
, onlyRole
or restricted
modifiers in the same way authorizeUpgrade
is in UUPSUpgradeable. I start to think that the AccessManaged
contract must expose a variant of the restricted
modifier that allow passing in a custom address. Also note those modifiers generally revert and not return a boolean
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.
Worth noting that if we want this function to be composable with AccessManaged.restricted
, then it can't have view
visibility :(
} | ||
|
||
// slither-disable-next-line write-after-write | ||
function _emptyCalldataBytes() private pure returns (bytes calldata result) { |
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.
We'll need an helper for that at some point. (but not in this PR)
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.
haha yeah I thought the same
Motivation
ERC-7821 might be used for EOAs with EIP-7702 to achieve batched execution without any ERC-4337 capability