Merge pull request #4 from zeroknots/feature/zeroknots-suggestions

small improvements for readability and gas
zkemail · Jun 13, 2024 · 4dd50b4 · 4dd50b4
2 parents 6aac792 + 72057f1
commit 4dd50b4
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 104 deletions.
diff --git a/src/EmailRecoveryManager.sol b/src/EmailRecoveryManager.sol
@@ -80,10 +80,9 @@ contract EmailRecoveryManager is EmailAccountRecoveryNew, IEmailRecoveryManager
         address _dkimRegistry,
         address _emailAuthImpl,
         address _subjectHandler
-    ) {
-        verifierAddr = _verifier;
-        dkimAddr = _dkimRegistry;
-        emailAuthImplementationAddr = _emailAuthImpl;
+    )
+        EmailAccountRecoveryNew(_verifier, _dkimRegistry, _emailAuthImpl)
+    {
         subjectHandler = _subjectHandler;
     }
 

diff --git a/src/experimental/EmailAccountRecoveryNew.sol b/src/experimental/EmailAccountRecoveryNew.sol
@@ -12,9 +12,15 @@ import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy
 /// new guardian and recovering a wallet.
 abstract contract EmailAccountRecoveryNew {
     uint8 constant EMAIL_ACCOUNT_RECOVERY_VERSION_ID = 1;
-    address public verifierAddr;
-    address public dkimAddr;
-    address public emailAuthImplementationAddr;
+    address internal immutable verifierAddr;
+    address internal immutable dkimAddr;
+    address internal immutable emailAuthImplementationAddr;
+
+    constructor(address _verifierAddr, address _dkimAddr, address _emailAuthImplementationAddr) {
+        verifierAddr = _verifierAddr;
+        dkimAddr = _dkimAddr;
+        emailAuthImplementationAddr = _emailAuthImplementationAddr;
+    }
 
     /// @notice Returns the address of the verifier contract.
     /// @dev This function is virtual and can be overridden by inheriting contracts.

diff --git a/src/handlers/EmailRecoverySubjectHandler.sol b/src/handlers/EmailRecoverySubjectHandler.sol
@@ -49,10 +49,10 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler {
 
     function validateAcceptanceSubject(
         uint256 templateIdx,
-        bytes[] memory subjectParams
+        bytes[] calldata subjectParams
     )
         external
-        view
+        pure
         returns (address)
     {
         if (templateIdx != 0) {
@@ -70,7 +70,7 @@ contract EmailRecoverySubjectHandler is IEmailRecoverySubjectHandler {
 
     function validateRecoverySubject(
         uint256 templateIdx,
-        bytes[] memory subjectParams,
+        bytes[] calldata subjectParams,
         address recoveryManager
     )
         public

diff --git a/src/handlers/SafeRecoverySubjectHandler.sol b/src/handlers/SafeRecoverySubjectHandler.sol
@@ -52,10 +52,10 @@ contract SafeRecoverySubjectHandler is IEmailRecoverySubjectHandler {
 
     function validateAcceptanceSubject(
         uint256 templateIdx,
-        bytes[] memory subjectParams
+        bytes[] calldata subjectParams
     )
         external
-        view
+        pure
         returns (address)
     {
         if (templateIdx != 0) {
@@ -73,7 +73,7 @@ contract SafeRecoverySubjectHandler is IEmailRecoverySubjectHandler {
 
     function validateRecoverySubject(
         uint256 templateIdx,
-        bytes[] memory subjectParams,
+        bytes[] calldata subjectParams,
         address recoveryManager
     )
         public

diff --git a/src/libraries/BytesLib.sol b/src/libraries/BytesLib.sol
diff --git a/src/modules/EmailRecoveryModule.sol b/src/modules/EmailRecoveryModule.sol
@@ -4,35 +4,30 @@ pragma solidity ^0.8.25;
 import { ERC7579ExecutorBase } from "@rhinestone/modulekit/src/Modules.sol";
 import { IERC7579Account } from "erc7579/interfaces/IERC7579Account.sol";
 import { IModule } from "erc7579/interfaces/IERC7579Module.sol";
-import { ExecutionLib } from "erc7579/lib/ExecutionLib.sol";
-import { ModeLib } from "erc7579/lib/ModeLib.sol";
-
 import { IRecoveryModule } from "../interfaces/IRecoveryModule.sol";
 import { IEmailRecoveryManager } from "../interfaces/IEmailRecoveryManager.sol";
-import { ISafe } from "../interfaces/ISafe.sol";
-import { BytesLib } from "../libraries/BytesLib.sol";
 
 contract EmailRecoveryModule is ERC7579ExecutorBase, IRecoveryModule {
-    using BytesLib for bytes;
     /*//////////////////////////////////////////////////////////////////////////
                                     CONSTANTS
     //////////////////////////////////////////////////////////////////////////*/
 
-    address public immutable emailRecoveryManager;
+    address public immutable EMAIL_RECOVERY_MANAGER;
 
     event NewValidatorRecovery(address indexed validatorModule, bytes4 recoverySelector);
 
     error NotTrustedRecoveryManager();
     error InvalidSubjectParams();
     error InvalidValidator(address validator);
     error InvalidSelector(bytes4 selector);
+    error InvalidOnInstallData();
 
     mapping(address validatorModule => mapping(address account => bytes4 allowedSelector)) internal
         allowedSelectors;
     mapping(address account => address validator) internal validators;
 
     constructor(address _zkEmailRecovery) {
-        emailRecoveryManager = _zkEmailRecovery;
+        EMAIL_RECOVERY_MANAGER = _zkEmailRecovery;
     }
 
     modifier withoutUnsafeSelector(bytes4 recoverySelector) {
@@ -55,6 +50,7 @@ contract EmailRecoveryModule is ERC7579ExecutorBase, IRecoveryModule {
      * @param data The data to initialize the module with
      */
     function onInstall(bytes calldata data) external {
+        if (data.length == 0) revert InvalidOnInstallData();
         (
             address validator,
             bytes4 selector,
@@ -65,11 +61,11 @@ contract EmailRecoveryModule is ERC7579ExecutorBase, IRecoveryModule {
             uint256 expiry
         ) = abi.decode(data, (address, bytes4, address[], uint256[], uint256, uint256, uint256));
 
-        allowValidatorRecovery(validator, bytes("0"), selector);
+        _allowValidatorRecovery(validator, bytes("0"), selector);
         validators[msg.sender] = validator;
 
         _execute({
-            to: emailRecoveryManager,
+            to: EMAIL_RECOVERY_MANAGER,
             value: 0,
             data: abi.encodeCall(
                 IEmailRecoveryManager.configureRecovery,
@@ -78,7 +74,7 @@ contract EmailRecoveryModule is ERC7579ExecutorBase, IRecoveryModule {
         });
     }
 
-    function allowValidatorRecovery(
+    function _allowValidatorRecovery(
         address validator,
         bytes memory isInstalledContext,
         bytes4 recoverySelector
@@ -107,7 +103,7 @@ contract EmailRecoveryModule is ERC7579ExecutorBase, IRecoveryModule {
         address validator = validators[msg.sender];
         delete allowedSelectors[validator][msg.sender];
         delete validators[msg.sender];
-        IEmailRecoveryManager(emailRecoveryManager).deInitRecoveryFromModule(msg.sender);
+        IEmailRecoveryManager(EMAIL_RECOVERY_MANAGER).deInitRecoveryFromModule(msg.sender);
     }
 
     /**
@@ -116,32 +112,32 @@ contract EmailRecoveryModule is ERC7579ExecutorBase, IRecoveryModule {
      * @return true if the module is initialized, false otherwise
      */
     function isInitialized(address smartAccount) external view returns (bool) {
-        return IEmailRecoveryManager(emailRecoveryManager).getGuardianConfig(smartAccount).threshold
-            != 0;
+        return IEmailRecoveryManager(EMAIL_RECOVERY_MANAGER).getGuardianConfig(smartAccount)
+            .threshold != 0;
     }
 
     /*//////////////////////////////////////////////////////////////////////////
                                      MODULE LOGIC
     //////////////////////////////////////////////////////////////////////////*/
 
-    function recover(address account, bytes memory recoveryCalldata) external {
-        if (msg.sender != emailRecoveryManager) {
+    function recover(address account, bytes calldata recoveryCalldata) external {
+        if (msg.sender != EMAIL_RECOVERY_MANAGER) {
             revert NotTrustedRecoveryManager();
         }
 
-        bytes4 selector = bytes4(recoveryCalldata.slice({ _start: 0, _length: 4 }));
+        bytes4 selector = bytes4(recoveryCalldata[:4]);
 
         address validator = validators[account];
         bytes4 allowedSelector = allowedSelectors[validator][account];
         if (allowedSelector != selector) {
             revert InvalidSelector(selector);
         }
 
-        _execute({ account: account, to: validators[account], value: 0, data: recoveryCalldata });
+        _execute({ account: account, to: validator, value: 0, data: recoveryCalldata });
     }
 
     function getTrustedRecoveryManager() external view returns (address) {
-        return emailRecoveryManager;
+        return EMAIL_RECOVERY_MANAGER;
     }
 
     /*//////////////////////////////////////////////////////////////////////////
@@ -153,7 +149,7 @@ contract EmailRecoveryModule is ERC7579ExecutorBase, IRecoveryModule {
      * @return name The name of the module
      */
     function name() external pure returns (string memory) {
-        return "EmailRecoveryModule";
+        return "ZKEmail.EmailRecoveryModule";
     }
 
     /**