-
Notifications
You must be signed in to change notification settings - Fork 4
[EXPERIMENTAL] [DO NOT MERGE] chore: ID-3927: ERC 7579 compliance #68
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
base: main
Are you sure you want to change the base?
Conversation
} | ||
|
||
function _removeModule(uint256 moduleTypeId, address module) internal { | ||
address[] storage modules = _moduleList[moduleTypeId]; |
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'd explore storing the modules in a linked list for example SentinelList (audited) or enumerablemap.
if _moduleList becomes big, the for loop'ed sloads could get expensive on gas
ERC-7579 CORE | ||
//////////////////////////////////////////////////////////////////////////*/ | ||
|
||
function execute(bytes32 mode, bytes calldata executionCalldata) external override { |
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 function is usually reserver for self calls or 4337 EPs. Or by something like ERC-7821
7579 Executors will always use executeFromExecutor
bytes calldata data | ||
) external returns (bool success, bytes memory returnData) { | ||
// Only allow the account itself to execute transactions | ||
require(msg.sender == address(this), "ImmutableExecutor: UNAUTHORIZED"); |
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 here for potential reentrancy. if someone manages to coerce the ImmutableExecutor to call it's own onInstall function. it might be possible to chain things together and get arbitrary execution on target
function decodeBatch(bytes calldata executionCalldata) | ||
internal | ||
pure | ||
returns (Execution[] memory executions) |
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.
solady's ERC7579Lib has nice decode helper function here that give you calldata pointers for the Execution[]. saves some malloc gas
* @param executionCalldata The encoded execution calldata | ||
* @return executions Array of decoded delegate executions | ||
*/ | ||
function decodeDelegateCallBatch(bytes calldata executionCalldata) |
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.
usually ERC7579 accounts don't implement this. cause you can just delegatecall a multicall that implements batch delegatecall and get the same behavior. reduces code complexity in the account
No description provided.