Merge pull request #37 from xWerk/fix/space-self-execution

Allow `Space` self execution

src/Space.sol

@@ -69,9 +69,11 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
                                       MODIFIERS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Checks whether the caller is the EntryPoint contract or the admin.
+    /// @notice Checks whether the caller is the {EntryPoint}, the admin or the contract itself (redirected through `execute()`)
     modifier onlyAdminOrEntrypoint() virtual {
-        require(msg.sender == address(entryPoint()) || isAdmin(msg.sender), "Account: not admin or EntryPoint.");
+        if (!(msg.sender == address(entryPoint()) || isAdmin(msg.sender) || msg.sender == address(this))) {
+            revert Errors.CallerNotEntryPointOrAdmin();
+        }
         _;
     }
 
@@ -314,20 +316,20 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
                                 INTERNAL FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @dev Executes a low-level call on the `module` contract with the `data` data forwarding the `value` value
-    function _call(address module, uint256 value, bytes calldata data) internal returns (bool success) {
-        // Execute the call via assembly
+    /// @dev Executes a low-level call on the `target` contract with the `data` data forwarding the `value` value
+    function _call(address target, uint256 value, bytes calldata data) internal returns (bool success) {
+        // Execute the low-level call
         bytes memory result;
-        (success, result) = module.call{ value: value }(data);
+        (success, result) = target.call{ value: value }(data);
 
-        // Revert with the same error returned by the module contract if the call failed
+        // Revert with the same error returned by the target contract if the call failed
         if (!success) {
             assembly {
                 revert(add(result, 0x20), mload(result))
             }
         } else {
             // Otherwise log the execution success
-            emit ModuleExecutionSucceded(module, value, data);
+            emit ModuleExecutionSucceded(target, value, data);
         }
     }
 }

src/libraries/Errors.sol

@@ -18,6 +18,9 @@ library Errors {
     /// @notice Thrown when `msg.sender` is not the {Space} contract owner
     error CallerNotSpaceOwner();
 
+    /// @notice Thrown when `msg.sender` is not the {EntryPoint} or the admin
+    error CallerNotEntryPointOrAdmin();
+
     /// @notice Thrown when a native token (ETH) withdrawal fails
     error NativeWithdrawFailed();
 

test/mocks/MockBadSpace.sol

@@ -58,9 +58,11 @@ contract MockBadSpace is ISpace, AccountCore, ERC1271, ModuleManager {
                                       MODIFIERS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Checks whether the caller is the EntryPoint contract or the admin.
+    /// @notice Checks whether the caller is the {EntryPoint}, the admin or the contract itself (redirected through `execute()`)
     modifier onlyAdminOrEntrypoint() virtual {
-        require(msg.sender == address(entryPoint()) || isAdmin(msg.sender), "Account: not admin or EntryPoint.");
+        if (!(msg.sender == address(entryPoint()) || isAdmin(msg.sender) || msg.sender == address(this))) {
+            revert Errors.CallerNotEntryPointOrAdmin();
+        }
         _;
     }
 

test/unit/concrete/space/disable-module/disableModule.t.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.26;
 import { Space_Unit_Concrete_Test } from "../Space.t.sol";
 import { MockModule } from "../../../../mocks/MockModule.sol";
 import { Events } from "../../../../utils/Events.sol";
+import { Errors } from "../../../../utils/Errors.sol";
 
 contract DisableModule_Unit_Concrete_Test is Space_Unit_Concrete_Test {
     function setUp() public virtual override {
@@ -14,8 +15,8 @@ contract DisableModule_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.disableModule({ module: address(0x1) });

test/unit/concrete/space/disable-module/disableModule.tree

@@ -1,6 +1,6 @@
 disableModule.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     └── given module enabled
         ├── it should mark the module as disabled

test/unit/concrete/space/enable-module/enableModule.t.sol

@@ -15,8 +15,8 @@ contract EnableModule_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.enableModule({ module: address(0x1) });

test/unit/concrete/space/enable-module/enableModule.tree

@@ -1,6 +1,6 @@
 enableModule.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     ├── when the module IS NOT allowlisted
     │   └── it should revert with the {ModuleNotAllowlisted} error

test/unit/concrete/space/execute-batch/executeBatch.t.sol

@@ -28,8 +28,8 @@ contract ExecuteBatch_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.executeBatch(modules, values, data);

test/unit/concrete/space/execute-batch/executeBatch.tree

@@ -1,6 +1,6 @@
 executeBatch.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     ├── when one array have a different length than the others
     │   └── it should revert with the {WrongArrayLengths} error

test/unit/concrete/space/execute/execute.t.sol

@@ -14,8 +14,8 @@ contract Execute_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.execute({ module: address(mockModule), value: 0, data: "" });

test/unit/concrete/space/execute/execute.tree

@@ -1,6 +1,6 @@
 execute.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     ├── when the module IS NOT enabled
     │   └── it should revert with the {ModuleNotEnabled} error

test/unit/concrete/space/withdraw-erc1155/withdrawERC1155.t.sol

@@ -26,8 +26,8 @@ contract WithdrawERC1155_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.withdrawERC1155({ to: users.bob, collection: IERC1155(address(0x0)), ids: ids, amounts: amounts });

test/unit/concrete/space/withdraw-erc1155/withdrawERC1155.tree

@@ -1,6 +1,6 @@
 withdrawERC1155.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     ├── when there the ERC-1155 balance IS NOT sufficient
     │   └── it should revert with the {ERC1155InsufficientBalance} error

test/unit/concrete/space/withdraw-erc20/withdrawERC20.t.sol

@@ -15,8 +15,8 @@ contract WithdrawERC20_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.withdrawERC20({ to: users.bob, asset: IERC20(address(0x0)), amount: 100e6 });

test/unit/concrete/space/withdraw-erc20/withdrawERC20.tree

@@ -1,6 +1,6 @@
 withdrawERC20.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     ├── when space ERC-20 token balance IS INSUFFICIENT to support the withdrawal
     │   └── it should revert with the {InsufficientERC20ToWithdraw} error

test/unit/concrete/space/withdraw-erc721/withdrawERC721.t.sol

@@ -15,8 +15,8 @@ contract WithdrawERC721_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.withdrawERC721({ to: users.bob, collection: IERC721(address(0x0)), tokenId: 1 });

test/unit/concrete/space/withdraw-erc721/withdrawERC721.tree

@@ -1,6 +1,6 @@
 withdrawERC721.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     ├── when there is no existing ERC-721 token to be transferred
     │   └── it should revert with the {ERC721WithdrawalFailed} error

test/unit/concrete/space/withdraw-native/withdrawNative.t.sol

@@ -28,8 +28,8 @@ contract WithdrawNative_Unit_Concrete_Test is Space_Unit_Concrete_Test {
         // Make Bob the caller for this test suite who is not the owner of the space
         vm.startPrank({ msgSender: users.bob });
 
-        // Expect the next call to revert with the "Account: not admin or EntryPoint." error
-        vm.expectRevert("Account: not admin or EntryPoint.");
+        // Expect the next call to revert with the {CallerNotEntryPointOrAdmin} error
+        vm.expectRevert(Errors.CallerNotEntryPointOrAdmin.selector);
 
         // Run the test
         space.withdrawNative({ to: users.bob, amount: 2 ether });

test/unit/concrete/space/withdraw-native/withdrawNative.tree

@@ -1,6 +1,6 @@
 withdrawNative.t.sol
 ├── when the caller IS NOT the space owner
-│   └── it should revert with the "Account: not admin or EntryPoint." error
+│   └── it should revert with the {CallerNotEntryPointOrAdmin} error
 └── when the caller IS the space owner
     ├── when space native token (ETH) balance IS INSUFFICIENT to support the withdrawal
     │   └── it should revert with the {InsufficientERC20ToWithdraw} error

test/utils/Errors.sol

@@ -10,12 +10,15 @@ library Errors {
     error CallerNotStationOwner();
 
     /*//////////////////////////////////////////////////////////////////////////
-                                    CONTAINER
+                                        SPACE
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @notice Thrown when `msg.sender` is not the {Space} contract owner
     error CallerNotSpaceOwner();
 
+    /// @notice Thrown when `msg.sender` is not the {EntryPoint} or the admin
+    error CallerNotEntryPointOrAdmin();
+
     /// @notice Thrown when a native token (ETH) withdrawal fails
     error NativeWithdrawFailed();