L3: cancelRecovery does not revert when no recovery is in process

zkemail · Jul 19, 2024 · 8f3fa22 · 8f3fa22
1 parent b1811d4
commit 8f3fa22
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 6 deletions.
diff --git a/src/EmailRecoveryManager.sol b/src/EmailRecoveryManager.sol
@@ -445,6 +445,9 @@ contract EmailRecoveryManager is EmailAccountRecovery, Initializable, IEmailReco
      * @dev Deletes the current recovery request associated with the caller's account
      */
     function cancelRecovery() external virtual {
+        if (recoveryRequests[msg.sender].currentWeight == 0) {
+            revert NoRecoveryInProcess();
+        }
         delete recoveryRequests[msg.sender];
         emit RecoveryCancelled(msg.sender);
     }

diff --git a/src/interfaces/IEmailRecoveryManager.sol b/src/interfaces/IEmailRecoveryManager.sol
@@ -85,6 +85,7 @@ interface IEmailRecoveryManager {
     error DelayNotPassed();
     error RecoveryRequestExpired();
     error InvalidCalldataHash();
+    error NoRecoveryInProcess();
     error NotRecoveryModule();
 
     /*//////////////////////////////////////////////////////////////////////////

diff --git a/test/unit/EmailRecoveryManager/cancelRecovery.t.sol b/test/unit/EmailRecoveryManager/cancelRecovery.t.sol
@@ -13,6 +13,12 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
         super.setUp();
     }
 
+    function test_CancelRecovery_RevertWhen_NoRecoveryInProcess() public {
+        vm.startPrank(accountAddress);
+        vm.expectRevert(IEmailRecoveryManager.NoRecoveryInProcess.selector);
+        emailRecoveryManager.cancelRecovery();
+    }
+
     function test_CancelRecovery_CannotCancelWrongRecoveryRequest() public {
         address otherAddress = address(99);
 
@@ -29,13 +35,8 @@ contract EmailRecoveryManager_cancelRecovery_Test is UnitBase {
         assertEq(recoveryRequest.calldataHash, "");
 
         vm.startPrank(otherAddress);
+        vm.expectRevert(IEmailRecoveryManager.NoRecoveryInProcess.selector);
         emailRecoveryManager.cancelRecovery();
-
-        recoveryRequest = emailRecoveryManager.getRecoveryRequest(accountAddress);
-        assertEq(recoveryRequest.executeAfter, 0);
-        assertEq(recoveryRequest.executeBefore, 0);
-        assertEq(recoveryRequest.currentWeight, 1);
-        assertEq(recoveryRequest.calldataHash, "");
     }
 
     function test_CancelRecovery_PartialRequest_Succeeds() public {

diff --git a/test/unit/assertErrorSelectors.t.sol b/test/unit/assertErrorSelectors.t.sol
@@ -48,6 +48,7 @@ contract LogErrorSelectors_Test is Test {
         assertEq(IEmailRecoveryManager.DelayNotPassed.selector, bytes4(0xc806ff6e));
         assertEq(IEmailRecoveryManager.RecoveryRequestExpired.selector, bytes4(0x4c2babb1));
         assertEq(IEmailRecoveryManager.InvalidCalldataHash.selector, bytes4(0xf05609de));
+        assertEq(IEmailRecoveryManager.NoRecoveryInProcess.selector, bytes4(0x87434f51));
         assertEq(IEmailRecoveryManager.NotRecoveryModule.selector, bytes4(0x2f6ef3d6));
     }