Update open zeppelin libraries v4 to v5 and fixed some test cases

zkemail · Sep 25, 2024 · 54018ea · 54018ea
1 parent 4d381a9
commit 54018ea
Show file tree

Hide file tree

Showing 35 changed files with 175 additions and 136 deletions.
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
@@ -8,8 +8,8 @@
     "lint": "solhint 'src/**/*.sol'"
   },
   "dependencies": {
-    "@openzeppelin/contracts": "^4.9.2",
-    "@openzeppelin/contracts-upgradeable": "^4.9.2",
+    "@openzeppelin/contracts": "^5.0.0",
+    "@openzeppelin/contracts-upgradeable": "^5.0.0",
     "@safe-global/safe-contracts": "1.4.1",
     "@uniswap/v3-core": "https://github.com/Uniswap/v3-core#0.8",
     "@uniswap/v3-periphery": "https://github.com/Uniswap/v3-periphery.git",

diff --git a/packages/contracts/remappings.txt b/packages/contracts/remappings.txt
@@ -1,4 +1,5 @@
-@openzeppelin/=../../node_modules/@openzeppelin
+@openzeppelin/=node_modules/@openzeppelin
+@openzeppelin/contracts-upgradeable/=node_modules/@openzeppelin/contracts-upgradeable
 @zk-email/=../../node_modules/@zk-email
 @uniswap/=../../node_modules/@uniswap
 forge-std/=../../node_modules/forge-std/src

diff --git a/packages/contracts/script/03_DeployDKIMRegistry.s.sol b/packages/contracts/script/03_DeployDKIMRegistry.s.sol
@@ -14,7 +14,7 @@ contract Deploy is Script {
         }
 
         vm.startBroadcast(deployerPrivateKey);
-        DKIMRegistry dkim = new DKIMRegistry();
+        DKIMRegistry dkim = new DKIMRegistry(msg.sender);
         vm.stopBroadcast();
 
         console.log("DKIMRegistry deployed at: %s", address(dkim));

diff --git a/packages/contracts/script/MintTestNFT.s.sol b/packages/contracts/script/MintTestNFT.s.sol
@@ -7,7 +7,7 @@ import "forge-std/Script.sol";
 contract MyToken is ERC721, ERC721URIStorage, ERC721Burnable, Ownable {
     constructor()
         ERC721("MyToken", "MTK")
-        Ownable()
+        Ownable(msg.sender)
     {}
 
     function safeMint(address to, uint256 tokenId, string memory uri)
@@ -37,10 +37,6 @@ contract MyToken is ERC721, ERC721URIStorage, ERC721Burnable, Ownable {
     {
         return super.supportsInterface(interfaceId);
     }
-
-    function _burn(uint256 tokenId) internal override(ERC721, ERC721URIStorage) {
-        super._burn(tokenId);
-    }
 }
 
 contract Deploy is Script {

diff --git a/packages/contracts/script/UpgradeCore.s.sol b/packages/contracts/script/UpgradeCore.s.sol
@@ -27,7 +27,7 @@ contract Upgrade is Script {
         EmailWalletCore coreImpl = new EmailWalletCore();
 
         EmailWalletCore coreProxy = EmailWalletCore(payable(core));
-        coreProxy.upgradeTo(address(coreImpl));
+        coreProxy.upgradeToAndCall(address(coreImpl), new bytes(0));
 
         // If you want to call some v2 function, refer to the following steps
         //

diff --git a/packages/contracts/script/UpgradeTokenRegistry.s.sol b/packages/contracts/script/UpgradeTokenRegistry.s.sol
@@ -27,7 +27,7 @@ contract Upgrade is Script {
         TokenRegistry tokenRegistryImpl = new TokenRegistry();
 
         TokenRegistry tokenRegistryProxy = TokenRegistry(payable(address(tokenRegistry)));
-        tokenRegistryProxy.upgradeTo(address(tokenRegistryImpl));
+        tokenRegistryProxy.upgradeToAndCall(address(tokenRegistryImpl), new bytes(0));
 
         // If you want to call some v2 function, refer to the following steps
         //

diff --git a/packages/contracts/src/EmailWalletCore.sol b/packages/contracts/src/EmailWalletCore.sol
@@ -5,7 +5,6 @@ import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Ini
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
-import {Create2Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/Create2Upgradeable.sol";
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
 import {IERC20, ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
@@ -27,6 +26,35 @@ import "./interfaces/Types.sol";
 import "./interfaces/Commands.sol";
 import "./interfaces/Events.sol";
 
+/// @notice Struct to store initialization parameters
+/// @dev This struct is used to initialize the EmailWalletCore contract
+/// @param _relayerHandler Address of the relayer handler contract
+/// @param _accountHandler Address of the account handler contract
+/// @param _unclaimsHandler Address of the unclaims handler contract
+/// @param _extensionHandler Address of the extension handler contract
+/// @param _verifier Address of the ZK proof verifier
+/// @param _tokenRegistry Address of the token registry contract
+/// @param _priceOracle Address of the price oracle contract
+/// @param _wethContract Address of the WETH contract
+/// @param _maxFeePerGas Max fee per gas in wei that relayer can set in a UserOp
+/// @param _emailValidityDuration Time period until which a email is valid for EmailOp based on the timestamp of the email signature
+/// @param _unclaimedFundClaimGas Gas required to claim unclaimed funds
+/// @param _unclaimedStateClaimGas Gas required to claim unclaimed state
+
+struct InitParams {
+    address relayerHandler;
+    address accountHandler;
+    address unclaimsHandler;
+    address extensionHandler;
+    address verifier;
+    address tokenRegistry;
+    address priceOracle;
+    address wethContract;
+    uint256 maxFeePerGas;
+    uint256 emailValidityDuration;
+    uint256 unclaimedFundClaimGas;
+    uint256 unclaimedStateClaimGas;
+}
 contract EmailWalletCore is Initializable, UUPSUpgradeable, OwnableUpgradeable {
     using SafeERC20 for IERC20;
     // ZK proof verifier
@@ -77,45 +105,24 @@ contract EmailWalletCore is Initializable, UUPSUpgradeable, OwnableUpgradeable {
     }
 
     /// @notice Constructor
-    /// @param _relayerHandler Address of the relayer handler contract
-    /// @param _accountHandler Address of the account handler contract
-    /// @param _unclaimsHandler Address of the unclaims handler contract
-    /// @param _extensionHandler Address of the extension handler contract
-    /// @param _verifier Address of the ZK proof verifier
-    /// @param _tokenRegistry Address of the token registry contract
-    /// @param _priceOracle Address of the price oracle contract
-    /// @param _wethContract Address of the WETH contract
-    /// @param _maxFeePerGas Max fee per gas in wei that relayer can set in a UserOp
-    /// @param _emailValidityDuration Time period until which a email is valid for EmailOp based on the timestamp of the email signature
-    /// @param _unclaimedFundClaimGas Gas required to claim unclaimed funds
-    /// @param _unclaimedStateClaimGas Gas required to claim unclaimed state
+    /// @param initParams Initialization parameters
+    /// @dev This function is called by the proxy contract during deployment
     function initialize(
-        address _relayerHandler,
-        address _accountHandler,
-        address _unclaimsHandler,
-        address _extensionHandler,
-        address _verifier,
-        address _tokenRegistry,
-        address _priceOracle,
-        address _wethContract,
-        uint256 _maxFeePerGas,
-        uint256 _emailValidityDuration,
-        uint256 _unclaimedFundClaimGas,
-        uint256 _unclaimedStateClaimGas
+        InitParams memory initParams
     ) public initializer {
-        __Ownable_init();
-        relayerHandler = RelayerHandler(_relayerHandler);
-        accountHandler = AccountHandler(_accountHandler);
-        unclaimsHandler = UnclaimsHandler(payable(_unclaimsHandler));
-        extensionHandler = ExtensionHandler(_extensionHandler);
-        verifier = IVerifier(_verifier);
-        tokenRegistry = TokenRegistry(_tokenRegistry);
-        priceOracle = IPriceOracle(_priceOracle);
-        wethContract = _wethContract;
-        maxFeePerGas = _maxFeePerGas;
-        emailValidityDuration = _emailValidityDuration;
-        unclaimedFundClaimGas = _unclaimedFundClaimGas;
-        unclaimedStateClaimGas = _unclaimedStateClaimGas;
+        __Ownable_init(msg.sender);
+        relayerHandler = RelayerHandler(initParams.relayerHandler);
+        accountHandler = AccountHandler(initParams.accountHandler);
+        unclaimsHandler = UnclaimsHandler(payable(initParams.unclaimsHandler));
+        extensionHandler = ExtensionHandler(initParams.extensionHandler);
+        verifier = IVerifier(initParams.verifier);
+        tokenRegistry = TokenRegistry(initParams.tokenRegistry);
+        priceOracle = IPriceOracle(initParams.priceOracle);
+        wethContract = initParams.wethContract;
+        maxFeePerGas = initParams.maxFeePerGas;
+        emailValidityDuration = initParams.emailValidityDuration;
+        unclaimedFundClaimGas = initParams.unclaimedFundClaimGas;
+        unclaimedStateClaimGas = initParams.unclaimedStateClaimGas;
     }
 
     function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
@@ -374,7 +381,7 @@ contract EmailWalletCore is Initializable, UUPSUpgradeable, OwnableUpgradeable {
 
         // Prevent extension from calling ERC20 tokens directly (tokenName should be empty)
         require(
-            Address.isContract(target) &&
+            address(target).code.length > 0 &&
                 target != currContext.walletAddr &&
                 bytes(tokenRegistry.getTokenNameOfAddress(target)).length == 0,
             "invalid target for executeAsExtension"
@@ -547,9 +554,15 @@ contract EmailWalletCore is Initializable, UUPSUpgradeable, OwnableUpgradeable {
             } catch Error(string memory reason) {
                 success = false;
                 returnData = bytes(reason);
-            } catch {
-                success = false;
-                returnData = bytes("err executing extension");
+            } catch (bytes memory lowLevelData) {
+                bytes4 selector;
+                assembly {
+                    selector := mload(add(lowLevelData, 0x20))
+                }
+                if (selector == bytes4(keccak256("ERC721NonexistentToken(uint256)"))) {
+                    return (false, bytes("ERC721NonexistentToken"));
+                }
+                return (false, bytes("err executing extension"));
             }
         }
     }
@@ -576,7 +589,14 @@ contract EmailWalletCore is Initializable, UUPSUpgradeable, OwnableUpgradeable {
             success = true;
         } catch Error(string memory reason) {
             return (false, bytes(reason));
-        } catch {
+        } catch (bytes memory lowLevelData) {
+            bytes4 selector;
+            assembly {
+                selector := mload(add(lowLevelData, 0x20))
+            }
+            if (selector == bytes4(keccak256("ERC20InsufficientBalance(address,uint256,uint256)"))) {
+                return (false, bytes("ERC20InsufficientBalance"));
+            }
             return (false, bytes("err unknown wallet exec"));
         }
     }

diff --git a/packages/contracts/src/Wallet.sol b/packages/contracts/src/Wallet.sol
@@ -45,7 +45,7 @@ contract Wallet is TokenCallbackHandler, OwnableUpgradeable, UUPSUpgradeable {
 
     /// @notice Initialize the contract
     function initialize() public initializer {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
     }
 
     /// @notice Execute a function on an external contract

diff --git a/packages/contracts/src/extensions/NFTExtension.sol b/packages/contracts/src/extensions/NFTExtension.sol
@@ -30,7 +30,7 @@ contract NFTExtension is Extension, IERC721Receiver, Initializable, UUPSUpgradea
     }
 
     function initialize(address coreAddr) initializer public {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
         core = EmailWalletCore(payable(coreAddr));
         templates = new string[][](2);
         templates[0] = ["NFT", "Send", "{uint}", "of", "{string}", "to", "{recipient}"];

diff --git a/packages/contracts/src/extensions/Safe2FAExtension.sol b/packages/contracts/src/extensions/Safe2FAExtension.sol
@@ -25,7 +25,7 @@ contract Safe2FAExtension is Extension, Initializable, UUPSUpgradeable, OwnableU
     }
 
     function initialize(address coreAddr) public initializer {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
         core = EmailWalletCore(payable(coreAddr));
         templates = new string[][](4);
         templates[0] = ["Safe", "Transaction:", "Approve", "{string}", "from", "{address}"];

diff --git a/packages/contracts/src/extensions/UniswapExtension.sol b/packages/contracts/src/extensions/UniswapExtension.sol
@@ -38,7 +38,7 @@ contract UniswapExtension is Extension, Initializable, UUPSUpgradeable, OwnableU
     }
 
     function initialize(address coreAddr, address _tokenReg, address _router, address _factory) initializer public {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
         core = EmailWalletCore(payable(coreAddr));
         tokenRegistry = TokenRegistry(_tokenReg);
         router = ISwapRouter(_router);

diff --git a/packages/contracts/src/handlers/AccountHandler.sol b/packages/contracts/src/handlers/AccountHandler.sol
@@ -6,7 +6,7 @@ import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeab
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 import {IDKIMRegistry} from "@zk-email/contracts/interfaces/IDKIMRegistry.sol";
-import {Create2Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/Create2Upgradeable.sol";
+import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import {Wallet} from "../Wallet.sol";
 import {RelayerHandler} from "./RelayerHandler.sol";
@@ -58,7 +58,7 @@ contract AccountHandler is Initializable, UUPSUpgradeable, OwnableUpgradeable {
         address _walletImplementation,
         uint _emailValidityDuration
     ) public initializer {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
         deployer = _msgSender();
         emailValidityDuration = _emailValidityDuration;
         defaultDkimRegistry = IDKIMRegistry(_defaultDkimRegistry);
@@ -85,7 +85,7 @@ contract AccountHandler is Initializable, UUPSUpgradeable, OwnableUpgradeable {
     /// @param emailProof Proof and instances of the email proof
     function createAccount(EmailProof calldata emailProof) public returns (Wallet wallet) {
         require(emailProof.accountSalt != bytes32(0), "invalid wallet salt");
-        require(Address.isContract(getWalletOfSalt(emailProof.accountSalt)) == false, "wallet already deployed");
+        require(address(getWalletOfSalt(emailProof.accountSalt)).code.length == 0, "wallet already deployed");
         require(emailNullifiers[emailProof.emailNullifier] == false, "email already nullified");
         require(
             isDKIMPublicKeyHashValid(emailProof.accountSalt, emailProof.emailDomain, emailProof.dkimPublicKeyHash),
@@ -111,7 +111,7 @@ contract AccountHandler is Initializable, UUPSUpgradeable, OwnableUpgradeable {
     /// @notice Return true iff the wallet is deployed for the given wallet salt
     /// @param accountSalt Salt used to deploy the wallet
     function isAccountSaltDeployed(bytes32 accountSalt) public view returns (bool) {
-        return Address.isContract(getWalletOfSalt(accountSalt));
+        return address(getWalletOfSalt(accountSalt)).code.length > 0;
     }
 
     /// @notice Return the DKIM public key hash for a given email domain and accountSalt
@@ -142,7 +142,7 @@ contract AccountHandler is Initializable, UUPSUpgradeable, OwnableUpgradeable {
     /// @param salt Salt used to deploy the wallet
     function getWalletOfSalt(bytes32 salt) public view returns (address) {
         return
-            Create2Upgradeable.computeAddress(
+            Create2.computeAddress(
                 salt,
                 keccak256(
                     abi.encodePacked(

diff --git a/packages/contracts/src/handlers/ExtensionHandler.sol b/packages/contracts/src/handlers/ExtensionHandler.sol
@@ -43,7 +43,7 @@ contract ExtensionHandler is Initializable, UUPSUpgradeable, OwnableUpgradeable
     }
 
     function initialize() initializer public {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
         deployer = _msgSender();
     }
 

diff --git a/packages/contracts/src/handlers/RelayerHandler.sol b/packages/contracts/src/handlers/RelayerHandler.sol
@@ -27,7 +27,7 @@ contract RelayerHandler is Initializable, UUPSUpgradeable, OwnableUpgradeable {
     }
 
     function initialize() initializer public {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
         deployer = _msgSender();
     }
 

diff --git a/packages/contracts/src/handlers/UnclaimsHandler.sol b/packages/contracts/src/handlers/UnclaimsHandler.sol
@@ -4,7 +4,7 @@ pragma solidity ^0.8.12;
 import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
 import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
-import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
+import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import "../interfaces/Types.sol";
@@ -76,7 +76,7 @@ contract UnclaimsHandler is ReentrancyGuard, Initializable, UUPSUpgradeable, Own
         uint256 _unclaimsExpiryDuration,
         uint256 _maxFeePerGas
     ) public initializer {
-        __Ownable_init();
+        __Ownable_init(msg.sender);
         deployer = _msgSender();
         relayerHandler = RelayerHandler(_relayerHandler);
         accountHandler = AccountHandler(_accountHandler);
@@ -294,7 +294,16 @@ contract UnclaimsHandler is ReentrancyGuard, Initializable, UUPSUpgradeable, Own
 
         try extension.registerUnclaimedState(us, false) {} catch Error(string memory reason) {
             revert(string.concat("unclaimed state reg err: ", reason));
-        } catch {
+        } catch (bytes memory lowLevelData) {
+            // Handle specific ERC721NonexistentToken error
+            bytes4 selector;
+            assembly {
+                selector := mload(add(lowLevelData, 0x20))
+            }
+            if (selector == bytes4(keccak256("ERC721NonexistentToken(uint256)"))) {
+                revert(string(abi.encodePacked("unclaimed state reg err: ERC721NonexistentToken()")));
+            }
+            // Handle other low-level errors
             revert("unclaimed state reg err");
         }
 

diff --git a/packages/contracts/src/libraries/StringUtils.sol b/packages/contracts/src/libraries/StringUtils.sol
@@ -75,7 +75,7 @@ library StringUtils {
      * @dev Converts a `int256` to its ASCII `string` decimal representation.
      */
     function toString(int256 value) internal pure returns (string memory) {
-        return Strings.toString(value);
+        return Strings.toStringSigned(value);
     }
 
     /**

diff --git a/packages/contracts/src/libraries/SubjectUtils.sol b/packages/contracts/src/libraries/SubjectUtils.sol
@@ -135,7 +135,7 @@ library SubjectUtils {
             (address target, , bytes memory data) = abi.decode(emailOp.executeCallData, (address, uint256, bytes));
 
             require(target != address(0), "invalid execute target");
-            require(Address.isContract(target), "target is not a contract");
+            require(address(target).code.length > 0, "target is not a contract");
 
             require(
                 target != address(core) &&
@@ -249,7 +249,7 @@ library SubjectUtils {
                 // {int} for negative values
                 else if (Strings.equal(matcher, Commands.INT_TEMPLATE)) {
                     int256 num = abi.decode(emailOp.extensionParams.subjectParams[nextParamIndex], (int256));
-                    value = Strings.toString(num);
+                    value = Strings.toStringSigned(num);
                     nextParamIndex++;
                 }
                 // {addres} for wallet address