Merge branch 'main' into feat/body-parsing

.solhint.json

@@ -13,6 +13,7 @@
     "no-empty-blocks": "off",
     "not-rely-on-time": "off",
     "one-contract-per-file": "off",
-    "var-name-mixedcase": "off"
+    "var-name-mixedcase": "off",
+    "immutable-vars-naming": "off"
   }
 }

src/EmailRecoveryManager.sol

@@ -186,8 +186,8 @@ abstract contract EmailRecoveryManager is
      * @notice Configures recovery for the caller's account. This is the first core function
      * that must be called during the end-to-end recovery flow
      * @dev Can only be called once for configuration. Sets up the guardians, and validates config
-     * parameters, ensuring that no recovery is in process. It is possible to configure guardians at
-     * a later stage if neccessary
+     * parameters, ensuring that no recovery is in process. It is possible to update configuration
+     * at a later stage if neccessary
      * @param guardians An array of guardian addresses
      * @param weights An array of weights corresponding to each guardian
      * @param threshold The threshold weight required for recovery
@@ -201,7 +201,7 @@ abstract contract EmailRecoveryManager is
         uint256 delay,
         uint256 expiry
     )
-        public
+        internal
     {
         address account = msg.sender;
 
@@ -226,7 +226,9 @@ abstract contract EmailRecoveryManager is
      * that no recovery is in process.
      * @param recoveryConfig The new recovery configuration to be set for the caller's account
      */
-    function updateRecoveryConfig(RecoveryConfig memory recoveryConfig)
+    function updateRecoveryConfig(
+        RecoveryConfig memory recoveryConfig
+    )
         public
         onlyWhenNotRecovering
     {
@@ -350,6 +352,8 @@ abstract contract EmailRecoveryManager is
 
         if (recoveryRequest.recoveryDataHash == bytes32(0)) {
             recoveryRequest.recoveryDataHash = recoveryDataHash;
+            uint256 executeBefore = block.timestamp + recoveryConfigs[account].expiry;
+            recoveryRequest.executeBefore = executeBefore;
         }
 
         if (recoveryRequest.recoveryDataHash != recoveryDataHash) {
@@ -360,11 +364,11 @@ abstract contract EmailRecoveryManager is
 
         if (recoveryRequest.currentWeight >= guardianConfig.threshold) {
             uint256 executeAfter = block.timestamp + recoveryConfigs[account].delay;
-            uint256 executeBefore = block.timestamp + recoveryConfigs[account].expiry;
             recoveryRequest.executeAfter = executeAfter;
-            recoveryRequest.executeBefore = executeBefore;
 
-            emit RecoveryProcessed(account, guardian, executeAfter, executeBefore, recoveryDataHash);
+            emit RecoveryProcessed(
+                account, guardian, executeAfter, recoveryRequest.executeBefore, recoveryDataHash
+            );
         }
     }
 
@@ -422,6 +426,19 @@ abstract contract EmailRecoveryManager is
         emit RecoveryCompleted(account);
     }
 
+    /**
+     * @notice Called during completeRecovery to finalize recovery. Contains recovery module
+     * implementation-specific logic to recover an account/module
+     * @dev this is the only function that must be implemented by consuming contracts to use the
+     * email recovery manager. This does not encompass other important logic such as module
+     * installation, that logic is specific to each module and must be implemeted separately
+     * @param account The address of the account for which the recovery is being completed
+     * @param recoveryData The data that is passed to recover the validator or account.
+     * recoveryData = abi.encode(validatorOrAccount, recoveryFunctionCalldata). Although, it is
+     * possible to design a recovery module using this manager without encoding the validator or
+     * account, depending on how the handler.parseRecoveryDataHash() and module.recover() functions
+     * are implemented
+     */
     function recover(address account, bytes calldata recoveryData) internal virtual;
 
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
@@ -441,17 +458,49 @@ abstract contract EmailRecoveryManager is
     }
 
     /**
-     * @notice Removes all state related to an account.
+     * @notice Cancels the recovery request for a given account if it is expired.
+     * @dev Deletes the current recovery request associated with the given account if the recovery
+     * request has expired.
+     * @param account The address of the account for which the recovery is being cancelled
+     */
+    function cancelExpiredRecovery(address account) external {
+        if (recoveryRequests[account].currentWeight == 0) {
+            revert NoRecoveryInProcess();
+        }
+        if (recoveryRequests[account].executeBefore > block.timestamp) {
+            revert RecoveryHasNotExpired(
+                account, block.timestamp, recoveryRequests[account].executeBefore
+            );
+        }
+        delete recoveryRequests[account];
+        emit RecoveryCancelled(account);
+    }
+
+    /**
+     * @notice Removes all state related to msg.sender.
      * @dev In order to prevent unexpected behaviour when reinstalling account modules, the module
-     * should be deinitialized. This should include remove state accociated with an account.
+     * should be deinitialized. This should include removing state accociated with an account.
      */
     function deInitRecoveryModule() internal onlyWhenNotRecovering {
-        delete recoveryConfigs[msg.sender];
-        delete recoveryRequests[msg.sender];
+        address account = msg.sender;
+        deInitRecoveryModule(account);
+    }
+
+    /**
+     * @notice Removes all state related to an account.
+     * @dev Although this function is internal, it should be used carefully as it can be called by
+     * anyone. In order to prevent unexpected behaviour when reinstalling account modules, the
+     * module should be deinitialized. This should include removing state accociated with an
+     * account
+     * @param account The address of the account for which recovery is being deinitialized
+     */
+    function deInitRecoveryModule(address account) internal onlyWhenNotRecovering {
+        delete recoveryConfigs[account];
+        delete recoveryRequests[account];
 
-        removeAllGuardians(msg.sender);
-        delete guardianConfigs[msg.sender];
+        removeAllGuardians(account);
+        delete guardianConfigs[account];
 
-        emit RecoveryDeInitialized(msg.sender);
+        emit RecoveryDeInitialized(account);
     }
 }

src/handlers/EmailRecoveryCommandHandler.sol

@@ -124,7 +124,7 @@ contract EmailRecoveryCommandHandler is IEmailRecoveryCommandHandler {
         bytes[] calldata commandParams
     )
         public
-        view
+        pure
         returns (address)
     {
         if (templateIdx != 0) {

src/interfaces/IEmailRecoveryManager.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.25;
 
-import { GuardianStorage, GuardianStatus } from "../libraries/EnumerableGuardianMap.sol";
+import { GuardianStatus } from "../libraries/EnumerableGuardianMap.sol";
 
 interface IEmailRecoveryManager {
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
@@ -13,12 +13,13 @@ interface IEmailRecoveryManager {
      * Config should be maintained over subsequent recovery attempts unless explicitly modified
      */
     struct RecoveryConfig {
-        uint256 delay; // the time from when recovery is started until the recovery request can be
-        // executed
+        uint256 delay; // the time from when the threshold for a recovery request has passed (when
+            // the attempt is successful), until the recovery request can be executed
         uint256 expiry; // the time from when recovery is started until the recovery request becomes
             // invalid. The recovery expiry encourages the timely execution of successful recovery
             // attempts, and reduces the risk of unauthorized access through stale or outdated
-            // requests.
+            // requests. After the recovery expiry has passed, anyone can cancel the recovery
+            // request
     }
 
     /**
@@ -77,6 +78,7 @@ interface IEmailRecoveryManager {
     error RecoveryRequestExpired(uint256 blockTimestamp, uint256 executeBefore);
     error InvalidRecoveryDataHash(bytes32 recoveryDataHash, bytes32 expectedRecoveryDataHash);
     error NoRecoveryInProcess();
+    error RecoveryHasNotExpired(address account, uint256 blockTimestamp, uint256 executeBefore);
     error RecoveryIsNotActivated();
 
     /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/

src/interfaces/ISafe.sol

@@ -6,13 +6,17 @@ interface ISafe {
     function getOwners() external view returns (address[] memory);
     function setFallbackHandler(address handler) external;
     function setGuard(address guard) external;
-    function execTransactionFromModule(
+    function execTransactionFromModuleReturnData(
         address to,
         uint256 value,
         bytes memory data,
         uint8 operation
     )
         external
-        returns (bool success);
-     function isModuleEnabled(address module) external view returns (bool);
+        returns (bool success, bytes memory returnData);
+    function isModuleEnabled(address module) external view returns (bool);
+    function addOwnerWithThreshold(address owner, uint256 _threshold) external;
+    function removeOwner(address prevOwner, address owner, uint256 _threshold) external;
+    function swapOwner(address prevOwner, address oldOwner, address newOwner) external;
+    function changeThreshold(uint256 _threshold) external;
 }

src/modules/EmailRecoveryModule.sol

@@ -7,7 +7,6 @@ import { IModule } from "erc7579/interfaces/IERC7579Module.sol";
 import { ISafe } from "../interfaces/ISafe.sol";
 import { IEmailRecoveryModule } from "../interfaces/IEmailRecoveryModule.sol";
 import { EmailRecoveryManager } from "../EmailRecoveryManager.sol";
-import { GuardianManager } from "../GuardianManager.sol";
 
 /**
  * @title EmailRecoveryModule
@@ -53,13 +52,21 @@ contract EmailRecoveryModule is EmailRecoveryManager, ERC7579ExecutorBase, IEmai
         if (_validator == address(0)) {
             revert InvalidValidator(_validator);
         }
-        if (
-            _selector == IModule.onUninstall.selector || _selector == IModule.onInstall.selector
-                || _selector == IERC7579Account.execute.selector
-                || _selector == ISafe.setFallbackHandler.selector
-                || _selector == ISafe.setGuard.selector || _selector == bytes4(0)
-        ) {
-            revert InvalidSelector(_selector);
+        if (_validator == msg.sender) {
+            if (
+                _selector != ISafe.addOwnerWithThreshold.selector
+                    && _selector != ISafe.removeOwner.selector && _selector != ISafe.swapOwner.selector
+                    && _selector != ISafe.changeThreshold.selector
+            ) {
+                revert InvalidSelector(_selector);
+            }
+        } else {
+            if (
+                _selector == IModule.onInstall.selector || _selector == IModule.onUninstall.selector
+                    || _selector == bytes4(0)
+            ) {
+                revert InvalidSelector(_selector);
+            }
         }
 
         validator = _validator;
@@ -140,14 +147,10 @@ contract EmailRecoveryModule is EmailRecoveryManager, ERC7579ExecutorBase, IEmai
      * being recovered. recoveryData = abi.encode(validator, recoveryFunctionCalldata)
      */
     function recover(address account, bytes calldata recoveryData) internal override {
-        (address validator, bytes memory recoveryCalldata) =
-            abi.decode(recoveryData, (address, bytes));
-
-        if (validator == address(0)) {
-            revert InvalidValidator(validator);
-        }
+        (, bytes memory recoveryCalldata) = abi.decode(recoveryData, (address, bytes));
 
         bytes4 calldataSelector;
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             calldataSelector := mload(add(recoveryCalldata, 32))
         }

src/modules/SafeEmailRecoveryModule.sol

@@ -20,9 +20,10 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
 
     event RecoveryExecuted(address indexed account);
 
+    error ModuleNotInstalled(address account);
     error InvalidAccount(address account);
     error InvalidSelector(bytes4 selector);
-    error RecoveryFailed(address account);
+    error RecoveryFailed(address account, bytes returnData);
     error ResetFailed(address account);
 
     constructor(
@@ -34,6 +35,31 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
         EmailRecoveryManager(verifier, dkimRegistry, emailAuthImpl, commandHandler)
     { }
 
+    /**
+     * @notice Configures recovery for the caller's account
+     * @dev This function ensures that the module is installed before configuring recovery. Calls
+     * internal configureRecovery function
+     * @param guardians An array of guardian addresses
+     * @param weights An array of weights corresponding to each guardian
+     * @param threshold The threshold weight required for recovery
+     * @param delay The delay period before recovery can be executed
+     * @param expiry The expiry time after which the recovery attempt is invalid
+     */
+    function configureSafeRecovery(
+        address[] memory guardians,
+        uint256[] memory weights,
+        uint256 threshold,
+        uint256 delay,
+        uint256 expiry
+    )
+        public
+    {
+        if (!ISafe(msg.sender).isModuleEnabled(address(this))) {
+            revert ModuleNotInstalled(msg.sender);
+        }
+        configureRecovery(guardians, weights, threshold, delay, expiry);
+    }
+
     /**
      * Check if a recovery request can be initiated based on guardian acceptance
      * @param account The smart account to check
@@ -53,29 +79,25 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
      * being recovered. recoveryData = abi.encode(safeAccount, recoveryFunctionCalldata)
      */
     function recover(address account, bytes calldata recoveryData) internal override {
-        (address encodedAccount, bytes memory recoveryCalldata) =
-            abi.decode(recoveryData, (address, bytes));
-
-        if (encodedAccount == address(0) || encodedAccount != account) {
-            revert InvalidAccount(encodedAccount);
-        }
+        (, bytes memory recoveryCalldata) = abi.decode(recoveryData, (address, bytes));
 
         bytes4 calldataSelector;
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             calldataSelector := mload(add(recoveryCalldata, 32))
         }
         if (calldataSelector != selector) {
             revert InvalidSelector(calldataSelector);
         }
 
-        bool success = ISafe(account).execTransactionFromModule({
+        (bool success, bytes memory returnData) = ISafe(account).execTransactionFromModuleReturnData({
             to: account,
             value: 0,
             data: recoveryCalldata,
             operation: uint8(Enum.Operation.Call)
         });
         if (!success) {
-            revert RecoveryFailed(account);
+            revert RecoveryFailed(account, returnData);
         }
 
         emit RecoveryExecuted(account);
@@ -89,9 +111,9 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
         if (account == address(0)) {
             revert InvalidAccount(account);
         }
-        if (ISafe(account).isModuleEnabled(address(this)) == true) {
+        if (ISafe(account).isModuleEnabled(address(this))) {
             revert ResetFailed(account);
         }
-        deInitRecoveryModule();
+        deInitRecoveryModule(account);
     }
 }