Guardable Module and Modifier

contracts/core/GuardableModifier.sol

@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity >=0.7.0 <0.9.0;
+
+import "../guard/Guardable.sol";
+import "./Modifier.sol";
+
+abstract contract GuardableModifier is Module, Guardable, Modifier {
+    /// @dev Passes a transaction to be executed by the avatar.
+    /// @notice Can only be called by this contract.
+    /// @param to Destination address of module transaction.
+    /// @param value Ether value of module transaction.
+    /// @param data Data payload of module transaction.
+    /// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
+    function exec(
+        address to,
+        uint256 value,
+        bytes memory data,
+        Enum.Operation operation
+    ) internal virtual override returns (bool success) {
+        address currentGuard = guard;
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkTransaction(
+                /// Transaction info used by module transactions.
+                to,
+                value,
+                data,
+                operation,
+                /// Zero out the redundant transaction information only used for Safe multisig transctions.
+                0,
+                0,
+                0,
+                address(0),
+                payable(0),
+                "",
+                msg.sender
+            );
+        }
+        success = IAvatar(target).execTransactionFromModule(
+            to,
+            value,
+            data,
+            operation
+        );
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
+        }
+    }
+
+    /// @dev Passes a transaction to be executed by the target and returns data.
+    /// @notice Can only be called by this contract.
+    /// @param to Destination address of module transaction.
+    /// @param value Ether value of module transaction.
+    /// @param data Data payload of module transaction.
+    /// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
+    function execAndReturnData(
+        address to,
+        uint256 value,
+        bytes memory data,
+        Enum.Operation operation
+    )
+        internal
+        virtual
+        override
+        returns (bool success, bytes memory returnData)
+    {
+        address currentGuard = guard;
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkTransaction(
+                /// Transaction info used by module transactions.
+                to,
+                value,
+                data,
+                operation,
+                /// Zero out the redundant transaction information only used for Safe multisig transctions.
+                0,
+                0,
+                0,
+                address(0),
+                payable(0),
+                "",
+                msg.sender
+            );
+        }
+
+        (success, returnData) = IAvatar(target)
+            .execTransactionFromModuleReturnData(to, value, data, operation);
+
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
+        }
+    }
+}

contracts/core/GuardableModule.sol

@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity >=0.7.0 <0.9.0;
+
+import "../guard/Guardable.sol";
+import "./Module.sol";
+
+/// @title GuardableModule  - A contract that can pass messages to a Module Manager contract if enabled by that contract.
+abstract contract GuardableModule is Module, Guardable {
+    /// @dev Passes a transaction to be executed by the avatar.
+    /// @notice Can only be called by this contract.
+    /// @param to Destination address of module transaction.
+    /// @param value Ether value of module transaction.
+    /// @param data Data payload of module transaction.
+    /// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
+    function exec(
+        address to,
+        uint256 value,
+        bytes memory data,
+        Enum.Operation operation
+    ) internal override returns (bool success) {
+        address currentGuard = guard;
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkTransaction(
+                /// Transaction info used by module transactions.
+                to,
+                value,
+                data,
+                operation,
+                /// Zero out the redundant transaction information only used for Safe multisig transctions.
+                0,
+                0,
+                0,
+                address(0),
+                payable(0),
+                "",
+                msg.sender
+            );
+        }
+        success = IAvatar(target).execTransactionFromModule(
+            to,
+            value,
+            data,
+            operation
+        );
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
+        }
+    }
+
+    /// @dev Passes a transaction to be executed by the target and returns data.
+    /// @notice Can only be called by this contract.
+    /// @param to Destination address of module transaction.
+    /// @param value Ether value of module transaction.
+    /// @param data Data payload of module transaction.
+    /// @param operation Operation type of module transaction: 0 == call, 1 == delegate call.
+    function execAndReturnData(
+        address to,
+        uint256 value,
+        bytes memory data,
+        Enum.Operation operation
+    )
+        internal
+        virtual
+        override
+        returns (bool success, bytes memory returnData)
+    {
+        address currentGuard = guard;
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkTransaction(
+                /// Transaction info used by module transactions.
+                to,
+                value,
+                data,
+                operation,
+                /// Zero out the redundant transaction information only used for Safe multisig transctions.
+                0,
+                0,
+                0,
+                address(0),
+                payable(0),
+                "",
+                msg.sender
+            );
+        }
+
+        (success, returnData) = IAvatar(target)
+            .execTransactionFromModuleReturnData(to, value, data, operation);
+
+        if (currentGuard != address(0)) {
+            IGuard(currentGuard).checkAfterExecution(bytes32(0), success);
+        }
+    }
+}

contracts/core/Modifier.sol

@@ -1,13 +1,12 @@
 // SPDX-License-Identifier: LGPL-3.0-only
-
-/// @title Modifier Interface - A contract that sits between a Module and an Avatar and enforce some additional logic.
 pragma solidity >=0.7.0 <0.9.0;
 
-import "./Module.sol";
 import "../interfaces/IAvatar.sol";
 import "../signature/SignatureChecker.sol";
+import "./Module.sol";
 
-abstract contract Modifier is Module, IAvatar, SignatureChecker {
+/// @title Modifier Interface - A contract that sits between a Module and an Avatar and enforce some additional logic.
+abstract contract Modifier is Module, SignatureChecker, IAvatar {
     address internal constant SENTINEL_MODULES = address(0x1);
     /// Mapping of modules.
     mapping(address => address) internal modules;
@@ -33,8 +32,10 @@ abstract contract Modifier is Module, IAvatar, SignatureChecker {
 
     /*
     --------------------------------------------------
-    You must override at least one of following two virtual functions,
+    You must override both of the following virtual functions,
     execTransactionFromModule() and execTransactionFromModuleReturnData().
+    It is recommended that implementations of both functions make use the 
+    onlyModule modifier.
     */
 
     /// @dev Passes a transaction to the modifier.
@@ -48,7 +49,7 @@ abstract contract Modifier is Module, IAvatar, SignatureChecker {
         uint256 value,
         bytes calldata data,
         Enum.Operation operation
-    ) public virtual override moduleOnly returns (bool success) {}
+    ) public virtual returns (bool success);
 
     /// @dev Passes a transaction to the modifier, expects return data.
     /// @notice Can only be called by enabled modules.
@@ -61,13 +62,7 @@ abstract contract Modifier is Module, IAvatar, SignatureChecker {
         uint256 value,
         bytes calldata data,
         Enum.Operation operation
-    )
-        public
-        virtual
-        override
-        moduleOnly
-        returns (bool success, bytes memory returnData)
-    {}
+    ) public virtual returns (bool success, bytes memory returnData);
 
     /*
     --------------------------------------------------

contracts/core/Module.sol

@@ -1,13 +1,11 @@
 // SPDX-License-Identifier: LGPL-3.0-only
-
-/// @title Module Interface - A contract that can pass messages to a Module Manager contract if enabled by that contract.
 pragma solidity >=0.7.0 <0.9.0;
 
-import "../interfaces/IAvatar.sol";
 import "../factory/FactoryFriendly.sol";
-import "../guard/Guardable.sol";
+import "../interfaces/IAvatar.sol";
 
-abstract contract Module is FactoryFriendly, Guardable {
+/// @title Module Interface - A contract that can pass messages to a Module Manager contract if enabled by that contract.
+abstract contract Module is FactoryFriendly {
     /// @dev Address that will ultimately execute function calls.
     address public avatar;
     /// @dev Address that this module will pass transactions to.
@@ -45,8 +43,14 @@ abstract contract Module is FactoryFriendly, Guardable {
         uint256 value,
         bytes memory data,
         Enum.Operation operation
-    ) internal returns (bool success) {
-        (success, ) = _exec(to, value, data, operation);
+    ) internal virtual returns (bool success) {
+        return
+            IAvatar(target).execTransactionFromModule(
+                to,
+                value,
+                data,
+                operation
+            );
     }
 
     /// @dev Passes a transaction to be executed by the target and returns data.
@@ -60,49 +64,13 @@ abstract contract Module is FactoryFriendly, Guardable {
         uint256 value,
         bytes memory data,
         Enum.Operation operation
-    ) internal returns (bool success, bytes memory returnData) {
-        (success, returnData) = _exec(to, value, data, operation);
-    }
-
-    function _exec(
-        address to,
-        uint256 value,
-        bytes memory data,
-        Enum.Operation operation
-    ) private returns (bool success, bytes memory returnData) {
-        address currentGuard = guard;
-        if (currentGuard != address(0)) {
-            IGuard(currentGuard).checkTransaction(
-                /// Transaction info used by module transactions.
+    ) internal virtual returns (bool success, bytes memory returnData) {
+        return
+            IAvatar(target).execTransactionFromModuleReturnData(
                 to,
                 value,
                 data,
-                operation,
-                /// Zero out the redundant transaction information only used for Safe multisig transctions.
-                0,
-                0,
-                0,
-                address(0),
-                payable(0),
-                "",
-                msg.sender
+                operation
             );
-            (success, returnData) = IAvatar(target)
-                .execTransactionFromModuleReturnData(
-                    to,
-                    value,
-                    data,
-                    operation
-                );
-            IGuard(currentGuard).checkAfterExecution("", success);
-        } else {
-            (success, returnData) = IAvatar(target)
-                .execTransactionFromModuleReturnData(
-                    to,
-                    value,
-                    data,
-                    operation
-                );
-        }
     }
 }

contracts/factory/ModuleProxyFactory.sol

@@ -19,10 +19,10 @@ contract ModuleProxyFactory {
     /// @notice Initialization failed.
     error FailedInitialization();
 
-    function createProxy(address target, bytes32 salt)
-        internal
-        returns (address result)
-    {
+    function createProxy(
+        address target,
+        bytes32 salt
+    ) internal returns (address result) {
         if (address(target) == address(0)) revert ZeroAddress(target);
         if (address(target).code.length == 0) revert TargetHasNoCode(target);
         bytes memory deployment = abi.encodePacked(

contracts/guard/BaseGuard.sol

@@ -6,12 +6,9 @@ import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
 import "../interfaces/IGuard.sol";
 
 abstract contract BaseGuard is IERC165 {
-    function supportsInterface(bytes4 interfaceId)
-        external
-        pure
-        override
-        returns (bool)
-    {
+    function supportsInterface(
+        bytes4 interfaceId
+    ) external pure override returns (bool) {
         return
             interfaceId == type(IGuard).interfaceId || // 0xe6d7a83a
             interfaceId == type(IERC165).interfaceId; // 0x01ffc9a7

contracts/interfaces/IAvatar.sol

@@ -64,8 +64,8 @@ interface IAvatar {
     /// @param pageSize Maximum number of modules that should be returned.
     /// @return array Array of modules.
     /// @return next Start of the next page.
-    function getModulesPaginated(address start, uint256 pageSize)
-        external
-        view
-        returns (address[] memory array, address next);
+    function getModulesPaginated(
+        address start,
+        uint256 pageSize
+    ) external view returns (address[] memory array, address next);
 }

contracts/test/TestAvatar.sol

@@ -51,11 +51,10 @@ contract TestAvatar {
         else (success, returnData) = to.call{value: value}(data);
     }
 
-    function getModulesPaginated(address, uint256 pageSize)
-        external
-        view
-        returns (address[] memory array, address next)
-    {
+    function getModulesPaginated(
+        address,
+        uint256 pageSize
+    ) external view returns (address[] memory array, address next) {
         // Init array with max page size
         array = new address[](pageSize);