module events

OpenZeppelin · Jul 23, 2024 · bc4524a · bc4524a
1 parent d6b7580
commit bc4524a
Show file tree

Hide file tree

Showing 10 changed files with 43 additions and 9 deletions.
diff --git a/contracts/abstraction/account/ERC7579Account.sol b/contracts/abstraction/account/ERC7579Account.sol
@@ -182,8 +182,6 @@ abstract contract ERC7579Account is
         if (moduleTypeId != MODULE_TYPE_SIGNER && !IERC7579Module(module).isModuleType(moduleTypeId))
             revert MismatchModuleTypeId(moduleTypeId, module);
         _installModule(moduleTypeId, module, initData);
-        /// TODO: silent unreachable and re-enable this event
-        // emit ModuleInstalled(moduleTypeId, module);
     }
 
     /// @inheritdoc IERC7579ModuleConfig
@@ -193,8 +191,6 @@ abstract contract ERC7579Account is
         bytes calldata deInitData
     ) public virtual onlyEntryPointOrSelf {
         _uninstallModule(moduleTypeId, module, deInitData);
-        /// TODO: silent unreachable and re-enable this event
-        // emit ModuleUninstalled(moduleTypeId, module);
     }
 
     /// @inheritdoc IERC7579ModuleConfig

diff --git a/contracts/abstraction/account/modules/AccountEIP7702.sol b/contracts/abstraction/account/modules/AccountEIP7702.sol
@@ -6,8 +6,14 @@ import {AccountValidateECDSA} from "./validation/AccountValidateECDSA.sol";
 import {ERC4337Utils} from "../../utils/ERC4337Utils.sol";
 import {PackedUserOperation} from "../../../interfaces/IERC4337.sol";
 
+/**
+ * @dev Extension of AccountValidateECDSA that grants access to the private key which address matches the one of this
+ * Account. In the case of an account instanciated at the address of an EOA using EIP-7702, using this module will
+ * allow the EOA's private key to control the account.
+ */
 abstract contract Account7702 is AccountValidateECDSA {
+    /// @inheritdoc AccountValidateECDSA
     function _isSigner(address signer) internal view virtual override returns (bool) {
-        return signer == address(this);
+        return signer == address(this) || super._isSigner(signer);
     }
 }
diff --git a/contracts/abstraction/account/modules/ERC7579AccountModuleExecutor.sol b/contracts/abstraction/account/modules/ERC7579AccountModuleExecutor.sol
@@ -35,6 +35,7 @@ abstract contract ERC7579AccountModuleExecutor is ERC7579Account {
         if (moduleTypeId == MODULE_TYPE_EXECUTOR) {
             if (!_executors.add(module)) revert ModuleAlreadyInstalled(moduleTypeId, module);
             IERC7579Module(module).onInstall(initData);
+            emit ModuleInstalled(moduleTypeId, module);
         } else {
             super._installModule(moduleTypeId, module, initData);
         }
@@ -49,6 +50,7 @@ abstract contract ERC7579AccountModuleExecutor is ERC7579Account {
         if (moduleTypeId == MODULE_TYPE_EXECUTOR) {
             if (!_executors.remove(module)) revert ModuleNotInstalled(moduleTypeId, module);
             IERC7579Module(module).onUninstall(deInitData);
+            emit ModuleUninstalled(moduleTypeId, module);
         } else {
             super._uninstallModule(moduleTypeId, module, deInitData);
         }

diff --git a/contracts/abstraction/account/modules/ERC7579AccountModuleFallback.sol b/contracts/abstraction/account/modules/ERC7579AccountModuleFallback.sol
@@ -40,6 +40,7 @@ abstract contract ERC7579AccountModuleFallback is ERC7579Account {
             _fallbacks[selector] = module;
 
             IERC7579Module(module).onInstall(initData[4:]);
+            emit ModuleInstalled(moduleTypeId, module);
         } else {
             super._installModule(moduleTypeId, module, initData);
         }
@@ -59,6 +60,7 @@ abstract contract ERC7579AccountModuleFallback is ERC7579Account {
             delete _fallbacks[selector];
 
             IERC7579Module(module).onUninstall(deInitData[4:]);
+            emit ModuleUninstalled(moduleTypeId, module);
         } else {
             super._uninstallModule(moduleTypeId, module, deInitData);
         }

diff --git a/contracts/abstraction/account/modules/ERC7579AccountModuleHook.sol b/contracts/abstraction/account/modules/ERC7579AccountModuleHook.sol
@@ -43,6 +43,7 @@ abstract contract ERC7579AccountModuleHook is ERC7579Account {
             if (_hook != address(0)) revert ModuleNotInstalled(moduleTypeId, _hook);
             _hook = module;
             IERC7579Module(module).onInstall(initData);
+            emit ModuleInstalled(moduleTypeId, module);
         } else {
             super._installModule(moduleTypeId, module, initData);
         }
@@ -58,6 +59,7 @@ abstract contract ERC7579AccountModuleHook is ERC7579Account {
             if (_hook != module) revert ModuleAlreadyInstalled(moduleTypeId, module);
             delete _hook;
             IERC7579Module(module).onUninstall(deInitData);
+            emit ModuleUninstalled(moduleTypeId, module);
         } else {
             super._uninstallModule(moduleTypeId, module, deInitData);
         }

diff --git a/contracts/abstraction/account/modules/ERC7579AccountModuleSigner.sol b/contracts/abstraction/account/modules/ERC7579AccountModuleSigner.sol
@@ -35,6 +35,7 @@ abstract contract ERC7579AccountModuleSigner is ERC7579Account {
             if (!_signers.add(module)) revert ModuleAlreadyInstalled(moduleTypeId, module);
             // Do not install signers
             // IERC7579Module(module).onInstall(initData);
+            emit ModuleInstalled(moduleTypeId, module);
         } else {
             super._installModule(moduleTypeId, module, initData);
         }
@@ -50,6 +51,7 @@ abstract contract ERC7579AccountModuleSigner is ERC7579Account {
             if (!_signers.remove(module)) revert ModuleNotInstalled(moduleTypeId, module);
             // Do not uninstall signers
             // IERC7579Module(module).onUninstall(deInitData);
+            emit ModuleUninstalled(moduleTypeId, module);
         } else {
             super._uninstallModule(moduleTypeId, module, deInitData);
         }

diff --git a/contracts/abstraction/account/modules/ERC7579AccountModuleValidator.sol b/contracts/abstraction/account/modules/ERC7579AccountModuleValidator.sol
@@ -34,6 +34,7 @@ abstract contract ERC7579AccountModuleValidator is ERC7579Account {
         if (moduleTypeId == MODULE_TYPE_VALIDATOR) {
             if (!_validators.add(module)) revert ModuleAlreadyInstalled(moduleTypeId, module);
             IERC7579Module(module).onInstall(initData);
+            emit ModuleInstalled(moduleTypeId, module);
         } else {
             super._installModule(moduleTypeId, module, initData);
         }
@@ -48,6 +49,7 @@ abstract contract ERC7579AccountModuleValidator is ERC7579Account {
         if (moduleTypeId == MODULE_TYPE_VALIDATOR) {
             if (!_validators.remove(module)) revert ModuleNotInstalled(moduleTypeId, module);
             IERC7579Module(module).onUninstall(deInitData);
+            emit ModuleUninstalled(moduleTypeId, module);
         } else {
             super._uninstallModule(moduleTypeId, module, deInitData);
         }

diff --git a/contracts/abstraction/account/modules/ERC7579AccountMultisig.sol b/contracts/abstraction/account/modules/ERC7579AccountMultisig.sol
@@ -12,16 +12,20 @@ import {AccountValidateECDSA} from "./validation/AccountValidateECDSA.sol";
 import {AccountValidateERC7579} from "./validation/AccountValidateERC7579.sol";
 
 abstract contract ERC7579AccountMultisig is ERC7579Account, AccountValidateECDSA, AccountValidateERC7579 {
+    /// @dev Number of distinct signers/validators required for a userOperation to be valid
     function requiredSignatures() public view virtual returns (uint256);
 
+    /// @inheritdoc AccountValidateECDSA
     function _isSigner(address signer) internal view virtual override returns (bool) {
-        return isModuleInstalled(MODULE_TYPE_SIGNER, signer, _zeroBytesCalldata());
+        return isModuleInstalled(MODULE_TYPE_SIGNER, signer, _zeroBytesCalldata()) || super._isSigner(signer);
     }
 
+    /// @inheritdoc AccountValidateERC7579
     function _isValidator(address module) internal view virtual override returns (bool) {
-        return isModuleInstalled(MODULE_TYPE_VALIDATOR, module, _zeroBytesCalldata());
+        return isModuleInstalled(MODULE_TYPE_VALIDATOR, module, _zeroBytesCalldata()) || super._isValidator(module);
     }
 
+    /// @inheritdoc Account
     function _validateUserOp(
         PackedUserOperation calldata userOp,
         bytes32 userOpHash,

diff --git a/contracts/abstraction/account/modules/validation/AccountValidateECDSA.sol b/contracts/abstraction/account/modules/validation/AccountValidateECDSA.sol
@@ -9,8 +9,17 @@ import {MessageHashUtils} from "../../../../utils/cryptography/MessageHashUtils.
 import {PackedUserOperation} from "../../../../interfaces/IERC4337.sol";
 
 abstract contract AccountValidateECDSA is Account {
-    function _isSigner(address) internal view virtual returns (bool);
+    /**
+     * @dev Hook used to verify the validity of recovered signers.
+     *
+     * Must be implemented by some access control management system to validate which EOA is authorised to sign user
+     * operations for this account.
+     */
+    function _isSigner(address) internal view virtual returns (bool) {
+        return false;
+    }
 
+    /// @inheritdoc Account
     function _validateUserOp(
         PackedUserOperation calldata /*userOp*/,
         bytes32 userOpHash,

diff --git a/contracts/abstraction/account/modules/validation/AccountValidateERC7579.sol b/contracts/abstraction/account/modules/validation/AccountValidateERC7579.sol
@@ -10,8 +10,17 @@ import {PackedUserOperation} from "../../../../interfaces/IERC4337.sol";
 import {IERC7579Validator} from "../../../../interfaces/IERC7579Module.sol";
 
 abstract contract AccountValidateERC7579 is Account {
-    function _isValidator(address) internal view virtual returns (bool);
+    /**
+     * @dev Hook used to verify the validity of validator used.
+     *
+     * Must be implemented by some access control management system to validate which validator is authorised to sign
+     * user operations for this account.
+     */
+    function _isValidator(address) internal view virtual returns (bool) {
+        return false;
+    }
 
+    /// @inheritdoc Account
     function _validateUserOp(
         PackedUserOperation calldata userOp,
         bytes32 userOpHash,