diff --git a/accounts/safe7579/src/core/ExecutionHelper.sol b/accounts/safe7579/src/core/ExecutionHelper.sol index 0dc64d53..8cf4953d 100644 --- a/accounts/safe7579/src/core/ExecutionHelper.sol +++ b/accounts/safe7579/src/core/ExecutionHelper.sol @@ -2,7 +2,7 @@ pragma solidity >=0.8.0 <0.9.0; import { Execution } from "erc7579/interfaces/IERC7579Account.sol"; -import "../interfaces/ISafe.sol"; +import { ISafe } from "../interfaces/ISafe.sol"; /** * @title Helper contract to execute transactions from a safe diff --git a/accounts/safe7579/src/core/HookManager.sol b/accounts/safe7579/src/core/HookManager.sol index c36fa52a..1330e279 100644 --- a/accounts/safe7579/src/core/HookManager.sol +++ b/accounts/safe7579/src/core/HookManager.sol @@ -9,8 +9,6 @@ import { IHook, IModule } from "erc7579/interfaces/IERC7579Module.sol"; * @author zeroknots.eth | rhinestone.wtf */ abstract contract HookManager is ModuleManager { - /// @custom:storage-location erc7201:hookmanager.storage.msa - mapping(address smartAccount => address hook) internal $hookManager; error HookPostCheckFailed(); @@ -51,17 +49,12 @@ abstract contract HookManager is ModuleManager { }); } - function _setHook(address hook) internal virtual { - $hookManager[msg.sender] = hook; - } - function _installHook(address hook, bytes calldata data) internal virtual { address currentHook = $hookManager[msg.sender]; if (currentHook != address(0)) { revert HookAlreadyInstalled(currentHook); } - _setHook(hook); - + $hookManager[msg.sender] = hook; _execute({ safe: msg.sender, target: hook, @@ -71,7 +64,7 @@ abstract contract HookManager is ModuleManager { } function _uninstallHook(address hook, bytes calldata data) internal virtual { - _setHook(address(0)); + $hookManager[msg.sender] = address(0); _execute({ safe: msg.sender, target: hook, diff --git a/accounts/safe7579/src/core/ModuleManager.sol b/accounts/safe7579/src/core/ModuleManager.sol index fe86487f..cf24ac67 100644 --- a/accounts/safe7579/src/core/ModuleManager.sol +++ b/accounts/safe7579/src/core/ModuleManager.sol @@ -8,6 +8,8 @@ import { ExecutionHelper } from "./ExecutionHelper.sol"; import { Receiver } from "erc7579/core/Receiver.sol"; import { AccessControl } from "./AccessControl.sol"; +import "forge-std/console2.sol"; + struct ModuleManagerStorage { // linked list of executors. List is initialized by initializeAccount() SentinelListLib.SentinelList _executors; @@ -150,10 +152,6 @@ abstract contract ModuleManager is AccessControl, Receiver, ExecutionHelper { return $executors.contains(executor); } - /** - * THIS IS NOT PART OF THE STANDARD - * Helper Function to access linked list - */ function getExecutorsPaginated( address cursor, uint256 size @@ -212,37 +210,18 @@ abstract contract ModuleManager is AccessControl, Receiver, ExecutionHelper { fallback() external payable override(Receiver) receiverFallback { address handler = _getFallbackHandler(); if (handler == address(0)) revert NoFallbackHandler(); - /* solhint-disable no-inline-assembly */ - /// @solidity memory-safe-assembly + + // TODO: gas optimize this! + bytes memory retData = _executeReturnData({ + safe: msg.sender, + target: handler, + value: msg.value, + callData: abi.encodePacked(msg.data, _msgSender()) // ERC2771 + }); + // solhint-disable-next-line no-inline-assembly assembly { - // When compiled with the optimizer, the compiler relies on a certain assumptions on how - // the - // memory is used, therefore we need to guarantee memory safety (keeping the free memory - // point 0x40 slot intact, - // not going beyond the scratch space, etc) - // Solidity docs: https://docs.soliditylang.org/en/latest/assembly.html#memory-safety - function allocate(length) -> pos { - pos := mload(0x40) - mstore(0x40, add(pos, length)) - } - - let calldataPtr := allocate(calldatasize()) - calldatacopy(calldataPtr, 0, calldatasize()) - - // The msg.sender address is shifted to the left by 12 bytes to remove the padding - // Then the address without padding is stored right after the calldata - let senderPtr := allocate(20) - mstore(senderPtr, shl(96, caller())) - - // Add 20 bytes for the address appended add the end - let success := call(gas(), handler, 0, calldataPtr, add(calldatasize(), 20), 0, 0) - - let returnDataPtr := allocate(returndatasize()) - returndatacopy(returnDataPtr, 0, returndatasize()) - if iszero(success) { revert(returnDataPtr, returndatasize()) } - return(returnDataPtr, returndatasize()) + return(add(retData, 0x20), mload(retData)) } - /* solhint-enable no-inline-assembly */ } } diff --git a/accounts/safe7579/test/SafeERC7579.t.sol b/accounts/safe7579/test/SafeERC7579.t.sol index 84406973..f9a1b900 100644 --- a/accounts/safe7579/test/SafeERC7579.t.sol +++ b/accounts/safe7579/test/SafeERC7579.t.sol @@ -5,6 +5,7 @@ import "erc7579/interfaces/IERC7579Account.sol"; import "erc7579/lib/ModeLib.sol"; import "erc7579/lib/ExecutionLib.sol"; import { MockTarget } from "./mocks/MockTarget.sol"; +import { MockFallback } from "./mocks/MockFallback.sol"; import "./Base.t.sol"; contract MSATest is TestBaseUtil { @@ -107,4 +108,16 @@ contract MSATest is TestBaseUtil { assertEq(ret.length, 2); assertEq(abi.decode(ret[0], (uint256)), 1338); } + + function test_fallback() public { + MockFallback _fallback = new MockFallback(); + vm.prank(address(safe)); + IERC7579Account(address(safe)).installModule(3, address(_fallback), ""); + (uint256 ret, address erc2771Sender, address msgSender) = + MockFallback(address(safe)).target(1337); + + assertEq(ret, 1337); + assertEq(erc2771Sender, address(this)); + assertEq(msgSender, address(safe)); + } } diff --git a/accounts/safe7579/test/mocks/MockFallback.sol b/accounts/safe7579/test/mocks/MockFallback.sol new file mode 100644 index 00000000..82dd8aa4 --- /dev/null +++ b/accounts/safe7579/test/mocks/MockFallback.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.23; + +import { IFallback, EncodedModuleTypes } from "erc7579/interfaces/IERC7579Module.sol"; +import { IERC7579Account, Execution } from "erc7579/interfaces/IERC7579Account.sol"; +import { ExecutionLib } from "erc7579/lib/ExecutionLib.sol"; +import { ModeLib } from "erc7579/lib/ModeLib.sol"; +import { HandlerContext } from "@safe-global/safe-contracts/contracts/handler/HandlerContext.sol"; + +contract MockFallback is IFallback, HandlerContext { + function onInstall(bytes calldata data) external override { } + + function onUninstall(bytes calldata data) external override { } + + function target(uint256 value) + external + returns (uint256 _value, address erc2771Sender, address msgSender) + { + _value = value; + erc2771Sender = _msgSender(); + msgSender = msg.sender; + } + + function isModuleType(uint256 typeID) external view returns (bool) { + return typeID == 3; + } + + function getModuleTypes() external view returns (EncodedModuleTypes) { } + + function isInitialized(address smartAccount) external view returns (bool) { + return false; + } +} diff --git a/packages/modulekit/.gas-snapshot b/packages/modulekit/.gas-snapshot index 8ad4fa99..97a85324 100644 --- a/packages/modulekit/.gas-snapshot +++ b/packages/modulekit/.gas-snapshot @@ -1,15 +1,15 @@ -BaseTest:test_transfer() (gas: 712286) -ERC7579DifferentialModuleKitLibTest:testAddExecutor() (gas: 1083999) -ERC7579DifferentialModuleKitLibTest:testAddHook() (gas: 717105) +BaseTest:test_transfer() (gas: 933500) +ERC7579DifferentialModuleKitLibTest:testAddExecutor() (gas: 1313175) +ERC7579DifferentialModuleKitLibTest:testAddHook() (gas: 946237) ERC7579DifferentialModuleKitLibTest:testAddSessionKey() (gas: 166) -ERC7579DifferentialModuleKitLibTest:testAddValidator() (gas: 1245702) -ERC7579DifferentialModuleKitLibTest:testDeployAccount() (gas: 20472236) +ERC7579DifferentialModuleKitLibTest:testAddValidator() (gas: 1495677) +ERC7579DifferentialModuleKitLibTest:testDeployAccount() (gas: 20345899) ERC7579DifferentialModuleKitLibTest:testGetUserOpHash() (gas: 188) -ERC7579DifferentialModuleKitLibTest:testRemoveExecutor() (gas: 1160670) -ERC7579DifferentialModuleKitLibTest:testRemoveValidator() (gas: 1002596) -ERC7579DifferentialModuleKitLibTest:testWriteGas() (gas: 1567493) -ERC7579DifferentialModuleKitLibTest:test_transfer() (gas: 712461) -ERC7579DifferentialModuleKitLibTest:testexec__Given__FourInputs() (gas: 710862) -ERC7579DifferentialModuleKitLibTest:testexec__Given__ThreeInputs() (gas: 711683) -ERC7579DifferentialModuleKitLibTest:testexec__Given__TwoInputs() (gas: 716318) -ERC7579DifferentialModuleKitLibTest:testexec__RevertWhen__UserOperationFails() (gas: 694625) \ No newline at end of file +ERC7579DifferentialModuleKitLibTest:testRemoveExecutor() (gas: 1411255) +ERC7579DifferentialModuleKitLibTest:testRemoveValidator() (gas: 1253998) +ERC7579DifferentialModuleKitLibTest:testWriteGas() (gas: 1787983) +ERC7579DifferentialModuleKitLibTest:test_transfer() (gas: 933743) +ERC7579DifferentialModuleKitLibTest:testexec__Given__FourInputs() (gas: 932130) +ERC7579DifferentialModuleKitLibTest:testexec__Given__ThreeInputs() (gas: 932966) +ERC7579DifferentialModuleKitLibTest:testexec__Given__TwoInputs() (gas: 937887) +ERC7579DifferentialModuleKitLibTest:testexec__RevertWhen__UserOperationFails() (gas: 914927) \ No newline at end of file