Merge pull request #28 from zkemail/feat/add-is-activated-function

Add functions which returns the user state is in module enabled or recovery is activated.
zkemail · Aug 6, 2024 · 95a3fcb · 95a3fcb
2 parents 03f5547 + ca8fb13
commit 95a3fcb
Show file tree

Hide file tree

Showing 18 changed files with 172 additions and 155 deletions.
diff --git a/package.json b/package.json
@@ -27,11 +27,11 @@
   "dependencies": {
     "@matterlabs": "github:matter-labs/era-contracts",
     "@openzeppelin/contracts-upgradeable": "v5.0.1",
-    "@rhinestone/modulekit": "github:rhinestonewtf/modulekit#test/safe-latest",
+    "@rhinestone/modulekit": "github:rhinestonewtf/modulekit",
     "@zk-email/contracts": "v6.0.3",
     "email-wallet-sdk": "github:zkemail/email-wallet-sdk",
     "erc7579-implementation": "github:erc7579/erc7579-implementation",
-    "ether-email-auth": "github:zkemail/ether-email-auth",
+    "ether-email-auth": "github:zkemail/ether-email-auth#711c7892c244b210febe9d6f8cc28eddbce5fbdc",
     "solidity-stringutils": "github:Arachnid/solidity-stringutils"
   },
   "files": [

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/src/EmailRecoveryManager.sol b/src/EmailRecoveryManager.sol
@@ -103,6 +103,15 @@ abstract contract EmailRecoveryManager is
         return recoveryRequests[account];
     }
 
+    /**
+     * @notice Checks if the recovery is activated for a given account
+     * @param account The address of the account for which the activation status is being checked
+     * @return bool True if the recovery request is activated, false otherwise
+     */
+    function isActivated(address account) public view override returns (bool) {
+        return guardianConfigs[account].threshold > 0;
+    }
+
     /**
      * @notice Returns a two-dimensional array of strings representing the subject templates for an
      * acceptance by a new guardian.
@@ -273,6 +282,10 @@ abstract contract EmailRecoveryManager is
             revert RecoveryInProcess();
         }
 
+        if (!isActivated(account)) {
+            revert RecoveryIsNotActivated();
+        }
+
         // This check ensures GuardianStatus is correct and also implicitly that the
         // account in email is a valid account
         GuardianStorage memory guardianStorage = getGuardian(account, guardian);
@@ -313,6 +326,10 @@ abstract contract EmailRecoveryManager is
             templateIdx, subjectParams, address(this)
         );
 
+        if (!isActivated(account)) {
+            revert RecoveryIsNotActivated();
+        }
+
         GuardianConfig memory guardianConfig = guardianConfigs[account];
         if (guardianConfig.threshold > guardianConfig.acceptedWeight) {
             revert ThresholdExceedsAcceptedWeight(

diff --git a/src/interfaces/IEmailRecoveryManager.sol b/src/interfaces/IEmailRecoveryManager.sol
@@ -77,6 +77,7 @@ interface IEmailRecoveryManager {
     error RecoveryRequestExpired(uint256 blockTimestamp, uint256 executeBefore);
     error InvalidCalldataHash(bytes32 calldataHash, bytes32 expectedCalldataHash);
     error NoRecoveryInProcess();
+    error RecoveryIsNotActivated();
 
     /*//////////////////////////////////////////////////////////////////////////
                                     FUNCTIONS

diff --git a/src/modules/EmailRecoveryModule.sol b/src/modules/EmailRecoveryModule.sol
@@ -97,7 +97,6 @@ contract EmailRecoveryModule is EmailRecoveryManager, ERC7579ExecutorBase, IEmai
         ) {
             revert InvalidValidator(validator);
         }
-
         configureRecovery(guardians, weights, threshold, delay, expiry);
     }
 

diff --git a/test/integration/EmailRecoveryManager/EmailRecoveryManager.integration.t.sol b/test/integration/EmailRecoveryManager/EmailRecoveryManager.integration.t.sol
@@ -32,13 +32,7 @@ contract EmailRecoveryManager_Integration_Test is
         EmailAuthMsg memory emailAuthMsg =
             getAcceptanceEmailAuthMessage(accountAddress1, guardians1[0]);
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleAcceptance(emailAuthMsg, templateIdx);
     }
 
@@ -212,23 +206,11 @@ contract EmailRecoveryManager_Integration_Test is
         vm.warp(12 seconds);
 
         emailAuthMsg = getRecoveryEmailAuthMessage(accountAddress1, guardians1[0], calldataHash1);
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.ACCEPTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleRecovery(emailAuthMsg, templateIdx);
 
         emailAuthMsg = getRecoveryEmailAuthMessage(accountAddress1, guardians1[1], calldataHash1);
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.ACCEPTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleRecovery(emailAuthMsg, templateIdx);
     }
 

diff --git a/test/integration/OwnableValidatorRecovery/EmailRecoveryModule/EmailRecoveryModule.t.sol b/test/integration/OwnableValidatorRecovery/EmailRecoveryModule/EmailRecoveryModule.t.sol
@@ -141,13 +141,7 @@ contract OwnableValidatorRecovery_EmailRecoveryModule_Integration_Test is
         EmailAuthMsg memory emailAuthMsg =
             getAcceptanceEmailAuthMessage(accountAddress1, guardians1[0]);
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleAcceptance(emailAuthMsg, templateIdx);
     }
 
@@ -160,13 +154,7 @@ contract OwnableValidatorRecovery_EmailRecoveryModule_Integration_Test is
         EmailAuthMsg memory emailAuthMsg =
             getAcceptanceEmailAuthMessage(accountAddress1, guardians1[1]);
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleAcceptance(emailAuthMsg, templateIdx);
     }
 
@@ -181,13 +169,8 @@ contract OwnableValidatorRecovery_EmailRecoveryModule_Integration_Test is
             getRecoveryEmailAuthMessage(accountAddress1, guardians1[0], calldataHash1);
 
         instance1.uninstallModule(MODULE_TYPE_EXECUTOR, recoveryModuleAddress, "");
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.ACCEPTED)
-            )
-        );
+
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleRecovery(emailAuthMsg, templateIdx);
     }
 
@@ -227,13 +210,7 @@ contract OwnableValidatorRecovery_EmailRecoveryModule_Integration_Test is
         EmailAuthMsg memory emailAuthMsg =
             getAcceptanceEmailAuthMessage(accountAddress1, guardians1[0]);
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleAcceptance(emailAuthMsg, templateIdx);
     }
 

diff --git a/.../OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol b/.../OwnableValidatorRecovery/UniversalEmailRecoveryModule/UniversalEmailRecoveryModule.t.sol
@@ -139,13 +139,7 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test
         EmailAuthMsg memory emailAuthMsg =
             getAcceptanceEmailAuthMessage(accountAddress1, guardians1[0]);
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleAcceptance(emailAuthMsg, templateIdx);
     }
 
@@ -158,13 +152,7 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test
         EmailAuthMsg memory emailAuthMsg =
             getAcceptanceEmailAuthMessage(accountAddress1, guardians1[1]);
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleAcceptance(emailAuthMsg, templateIdx);
     }
 
@@ -179,13 +167,8 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test
             getRecoveryEmailAuthMessage(accountAddress1, guardians1[0], calldataHash1);
 
         instance1.uninstallModule(MODULE_TYPE_EXECUTOR, recoveryModuleAddress, "");
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.ACCEPTED)
-            )
-        );
+
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleRecovery(emailAuthMsg, templateIdx);
     }
 
@@ -225,13 +208,7 @@ contract OwnableValidatorRecovery_UniversalEmailRecoveryModule_Integration_Test
         EmailAuthMsg memory emailAuthMsg =
             getAcceptanceEmailAuthMessage(accountAddress1, guardians1[0]);
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.handleAcceptance(emailAuthMsg, templateIdx);
     }
 

diff --git a/test/unit/EmailRecoveryManager/acceptGuardian.t.sol b/test/unit/EmailRecoveryManager/acceptGuardian.t.sol
@@ -40,13 +40,7 @@ contract EmailRecoveryManager_acceptGuardian_Test is UnitBase {
         instance.uninstallModule(MODULE_TYPE_EXECUTOR, recoveryModuleAddress, "");
         vm.stopPrank();
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.REQUESTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.exposed_acceptGuardian(guardian1, templateIdx, subjectParams, nullifier);
     }
 

diff --git a/test/unit/EmailRecoveryManager/configureRecovery.t.sol b/test/unit/EmailRecoveryManager/configureRecovery.t.sol
@@ -42,6 +42,9 @@ contract EmailRecoveryManager_configureRecovery_Test is UnitBase {
         vm.startPrank(accountAddress);
         emailRecoveryModule.workaround_validatorsPush(accountAddress, validatorAddress);
 
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertFalse(isActivated);
+
         vm.expectEmit();
         emit IEmailRecoveryManager.RecoveryConfigured(
             instance.account, guardians.length, totalWeight, threshold
@@ -66,6 +69,9 @@ contract EmailRecoveryManager_configureRecovery_Test is UnitBase {
             emailRecoveryModule.getGuardian(accountAddress, guardians[0]);
         assertEq(uint256(guardian.status), uint256(GuardianStatus.REQUESTED));
         assertEq(guardian.weight, guardianWeights[0]);
+
+        isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertTrue(isActivated);
     }
 
     function test_ConfigureRecovery_RevertWhen_ZeroGuardians() public {

diff --git a/test/unit/EmailRecoveryManager/deInitRecoveryModule.t.sol b/test/unit/EmailRecoveryManager/deInitRecoveryModule.t.sol
@@ -28,6 +28,9 @@ contract EmailRecoveryManager_deInitRecoveryModule_Test is UnitBase {
         acceptGuardian(accountSalt1);
         acceptGuardian(accountSalt2);
 
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertTrue(isActivated);
+
         vm.prank(accountAddress);
         vm.expectEmit();
         emit IEmailRecoveryManager.RecoveryDeInitialized(accountAddress);
@@ -66,5 +69,8 @@ contract EmailRecoveryManager_deInitRecoveryModule_Test is UnitBase {
         assertEq(guardianConfig.totalWeight, 0);
         assertEq(guardianConfig.acceptedWeight, 0);
         assertEq(guardianConfig.threshold, 0);
+
+        isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertFalse(isActivated);
     }
 }
diff --git a/test/unit/EmailRecoveryManager/isActivated.t.sol b/test/unit/EmailRecoveryManager/isActivated.t.sol
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.25;
+
+import { console2 } from "forge-std/console2.sol";
+import { ModuleKitHelpers } from "modulekit/ModuleKit.sol";
+import { MODULE_TYPE_EXECUTOR } from "modulekit/external/ERC7579.sol";
+import { IEmailRecoveryManager } from "src/interfaces/IEmailRecoveryManager.sol";
+import { GuardianStorage, GuardianStatus } from "src/libraries/EnumerableGuardianMap.sol";
+import { UnitBase } from "../UnitBase.t.sol";
+
+contract EmailRecoveryManager_isActivated_Test is UnitBase {
+    using ModuleKitHelpers for *;
+
+    function setUp() public override {
+        super.setUp();
+    }
+
+    function test_isActivated_ReturnsTrueWhenModuleIsInstalled() public {
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertTrue(isActivated);
+    }
+
+    function test_isActivated_ReturnsFalseWhenModuleIsInstalled() public {
+        instance.uninstallModule(MODULE_TYPE_EXECUTOR, recoveryModuleAddress, "");
+
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertFalse(isActivated);
+    }
+}
diff --git a/test/unit/EmailRecoveryManager/processRecovery.t.sol b/test/unit/EmailRecoveryManager/processRecovery.t.sol
@@ -71,13 +71,7 @@ contract EmailRecoveryManager_processRecovery_Test is UnitBase {
         instance.uninstallModule(MODULE_TYPE_EXECUTOR, recoveryModuleAddress, "");
         vm.stopPrank();
 
-        vm.expectRevert(
-            abi.encodeWithSelector(
-                IEmailRecoveryManager.InvalidGuardianStatus.selector,
-                uint256(GuardianStatus.NONE),
-                uint256(GuardianStatus.ACCEPTED)
-            )
-        );
+        vm.expectRevert(IEmailRecoveryManager.RecoveryIsNotActivated.selector);
         emailRecoveryModule.exposed_processRecovery(
             guardian1, templateIdx, subjectParams, nullifier
         );

diff --git a/test/unit/assertErrorSelectors.t.sol b/test/unit/assertErrorSelectors.t.sol
@@ -51,6 +51,7 @@ contract LogErrorSelectors_Test is Test {
         assertEq(IEmailRecoveryManager.RecoveryRequestExpired.selector, bytes4(0x566ad75e));
         assertEq(IEmailRecoveryManager.InvalidCalldataHash.selector, bytes4(0x54d53855));
         assertEq(IEmailRecoveryManager.NoRecoveryInProcess.selector, bytes4(0x87434f51));
+        assertEq(IEmailRecoveryManager.RecoveryIsNotActivated.selector, bytes4(0xc737140f));
     }
 
     function test_EmailRecoveryUniversalFactory_AssertSelectors() public {

diff --git a/test/unit/modules/EmailRecoveryModule/onInstall.t.sol b/test/unit/modules/EmailRecoveryModule/onInstall.t.sol
@@ -49,5 +49,8 @@ contract EmailRecoveryModule_onInstall_Test is EmailRecoveryModuleBase {
 
         bool isInitialized = emailRecoveryModule.isInitialized(accountAddress);
         assertTrue(isInitialized);
+
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertTrue(isActivated);
     }
 }
diff --git a/test/unit/modules/EmailRecoveryModule/onUninstall.t.sol b/test/unit/modules/EmailRecoveryModule/onUninstall.t.sol
@@ -22,5 +22,8 @@ contract EmailRecoveryModule_onUninstall_Test is EmailRecoveryModuleBase {
 
         bool isInitialized = emailRecoveryModule.isInitialized(accountAddress);
         assertFalse(isInitialized);
+
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertFalse(isActivated);
     }
 }
diff --git a/test/unit/modules/UniversalEmailRecoveryModule/onInstall.t.sol b/test/unit/modules/UniversalEmailRecoveryModule/onInstall.t.sol
@@ -54,5 +54,8 @@ contract UniversalEmailRecoveryModule_onInstall_Test is UnitBase {
         bytes4[] memory allowedSelectors = emailRecoveryModule.getAllowedSelectors(accountAddress);
         assertEq(allowedValidators.length, 1);
         assertEq(allowedSelectors.length, 1);
+
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertTrue(isActivated);
     }
 }
diff --git a/test/unit/modules/UniversalEmailRecoveryModule/onUninstall.t.sol b/test/unit/modules/UniversalEmailRecoveryModule/onUninstall.t.sol
@@ -62,5 +62,8 @@ contract UniversalEmailRecoveryModule_onUninstall_Test is UnitBase {
         allowedSelectors = emailRecoveryModule.getAllowedSelectors(accountAddress);
         assertEq(allowedValidators.length, 0);
         assertEq(allowedSelectors.length, 0);
+
+        bool isActivated = emailRecoveryModule.isActivated(accountAddress);
+        assertFalse(isActivated);
     }
 }