From ee2c007ca79dd807c46a65ab9148cc2aea558bb5 Mon Sep 17 00:00:00 2001
From: JohnGuilding <jwguilding@gmail.com>
Date: Tue, 10 Sep 2024 15:40:38 +0100
Subject: [PATCH] Clean up SafeEmailRecoveryModule

---
 src/interfaces/ISafe.sol                |  4 ++--
 src/modules/SafeEmailRecoveryModule.sol | 13 ++++---------
 test/unit/assertErrorSelectors.t.sol    |  4 +++-
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/interfaces/ISafe.sol b/src/interfaces/ISafe.sol
index 784a836..7c11057 100644
--- 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
index bd17ade..da14a01 100644
--- 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
index 5b92830..f6ba593 100644
--- 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));
     }