Clean up SafeEmailRecoveryModule

zkemail · Sep 10, 2024 · ee2c007 · ee2c007
1 parent 374a6f0
commit ee2c007
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 12 deletions.
diff --git a/src/interfaces/ISafe.sol b/src/interfaces/ISafe.sol
@@ -6,14 +6,14 @@ 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);
+        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;

diff --git a/src/modules/SafeEmailRecoveryModule.sol b/src/modules/SafeEmailRecoveryModule.sol
@@ -23,7 +23,7 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
     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(
@@ -79,12 +79,7 @@ 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
@@ -95,14 +90,14 @@ contract SafeEmailRecoveryModule is EmailRecoveryManager {
             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);

diff --git a/test/unit/assertErrorSelectors.t.sol b/test/unit/assertErrorSelectors.t.sol
@@ -65,6 +65,7 @@ contract LogErrorSelectors_Test is Test {
         assertEq(IEmailRecoveryManager.RecoveryRequestExpired.selector, bytes4(0x566ad75e));
         assertEq(IEmailRecoveryManager.InvalidRecoveryDataHash.selector, bytes4(0x9e1a616f));
         assertEq(IEmailRecoveryManager.NoRecoveryInProcess.selector, bytes4(0x87434f51));
+        assertEq(IEmailRecoveryManager.RecoveryHasNotExpired.selector, bytes4(0x93ae1fc6));
         assertEq(IEmailRecoveryManager.RecoveryIsNotActivated.selector, bytes4(0xc737140f));
     }
 
@@ -95,9 +96,10 @@ contract LogErrorSelectors_Test is Test {
     }
 
     function test_SafeEmailRecoveryModule_AssertSelectors() public {
+        assertEq(SafeEmailRecoveryModule.ModuleNotInstalled.selector, bytes4(0x026d9639));
         assertEq(SafeEmailRecoveryModule.InvalidAccount.selector, bytes4(0x4b579b22));
         assertEq(SafeEmailRecoveryModule.InvalidSelector.selector, bytes4(0x12ba286f));
-        assertEq(SafeEmailRecoveryModule.RecoveryFailed.selector, bytes4(0xaa3485bc));
+        assertEq(SafeEmailRecoveryModule.RecoveryFailed.selector, bytes4(0xc9dcc2a0));
         assertEq(SafeEmailRecoveryModule.ResetFailed.selector, bytes4(0x8078983d));
     }