Merge branch 'safe-global:master' into patch-1

4337/README.md

@@ -48,22 +48,18 @@ Important to note that [ERC-4337](https://eips.ethereum.org/EIPS/eip-4337#first-
 
 > The initCode field (if non-zero length) is parsed as a 20-byte address, followed by "calldata" to pass to this address.
 
+To deploy a Safe with 4337 directly enabled, we require a setup library that enables multiple modules (`AddModulesLib`). This is necessary because to enable a Module, the Safe has to do a call to itself. Before calling the setup method, we do not know the address of the Safe yet (as the address depends on the setup parameters) and using MultiSend will not result in the correct `msg.sender` for a selfcall.
+
 The `initCode` for the Safe with a 4337 module enabled is composed in the following way:
 
 ```solidity
 /** Enable Modules **/
-bytes memory enableModuleCalldata = abi.encodeWithSignature("enableModule", 4337_MODULE_ADDRESS);
-bytes memory enableEntryPointCalldata = abi.encodeWithSignature("enableModule", ENTRY_POINT_ADDRESS);
-bytes memory initExecutor = MULTISEND_ADDRESS;
-// Might be more gas efficient to use delegate call to the singleton here, but also more error prone.
-bytes memory initData = abi.encodePacked(
-    uint8(0), sender, uint256(0), enableModuleCalldata.length, enableModuleCalldata,
-    uint8(0), sender, uint256(0), enableEntryPointCalldata.length, enableEntryPointCalldata
-);
+bytes memory initExecutor = ADD_MODULES_LIB_ADDRESS;
+bytes memory initData =  abi.encodeWithSignature("enableModules", [4337_MODULE_ADDRESS, ENTRY_POINT_ADDRESS]);
 
 /** Setup Safe **/
 // We do not want to use any payment logic therefore this is all set to 0
-bytes memory setupData = abi.encodeWithSignature("setup", owners, threhsold, initExecutor, initData, address(0), 0, address(0));
+bytes memory setupData = abi.encodeWithSignature("setup", owners, threshold, initExecutor, initData, 4337_MODULE_ADDRESS, address(0), 0, address(0));
 
 /** Deploy Proxy **/
 bytes memory deployData = abi.encodeWithSignature("createProxyWithNonce", SAFE_SINGLETON_ADDRESS, setupData, salt);

4337/contracts/AddModulesLib.sol

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity >=0.8.0 <0.9.0;
+
+import {ISafe} from "./interfaces/Safe.sol";
+
+/// @title AddModulesLib
+contract AddModulesLib {
+    function enableModules(address[] calldata modules) external {
+        for (uint256 i = modules.length; i > 0; i--) {
+            // This call will only work properly if used via a delegatecall
+            ISafe(address(this)).enableModule(modules[i - 1]);
+        }
+    }
+}

4337/contracts/EIP4337Module.sol

@@ -1,13 +1,13 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 pragma solidity >=0.8.0 <0.9.0;
 
-import "@gnosis.pm/safe-contracts/contracts/handler/HandlerContext.sol";
-import "./vendor/CompatibilityFallbackHandler.sol";
-import "./UserOperation.sol";
-import "./interfaces/Safe.sol";
+import {HandlerContext} from "@safe-global/safe-contracts/contracts/handler/HandlerContext.sol";
+import {CompatibilityFallbackHandler} from "@safe-global/safe-contracts/contracts/handler/CompatibilityFallbackHandler.sol";
+import {UserOperation, UserOperationLib} from "./UserOperation.sol";
+import {INonceManager} from "./interfaces/ERC4337.sol";
+import {ISafe} from "./interfaces/Safe.sol";
 
 /// @title EIP4337Module
-/// TODO should implement default fallback methods
 abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler {
     using UserOperationLib for UserOperation;
     bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = keccak256("EIP712Domain(uint256 chainId,address verifyingContract)");
@@ -51,7 +51,7 @@ abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler
         _validateSignatures(entryPoint, userOp);
 
         if (requiredPrefund != 0) {
-            Safe(safeAddress).execTransactionFromModule(entryPoint, requiredPrefund, "", 0);
+            ISafe(safeAddress).execTransactionFromModule(entryPoint, requiredPrefund, "", 0);
         }
         return 0;
     }
@@ -129,7 +129,7 @@ abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler
             );
     }
 
-    /// @dev Validates that the user operation is correctly signed. Users methods from Gnosis Safe contract, reverts if signatures are invalid
+    /// @dev Validates that the user operation is correctly signed. Users methods from Safe contract, reverts if signatures are invalid
     /// @param entryPoint Address of the entry point
     /// @param userOp User operation struct
     function _validateSignatures(address entryPoint, UserOperation calldata userOp) internal view {
@@ -145,85 +145,22 @@ abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler
             entryPoint
         );
 
-        Safe(payable(userOp.sender)).checkSignatures(operationHash, "", userOp.signature);
+        ISafe(payable(userOp.sender)).checkSignatures(operationHash, "", userOp.signature);
     }
 }
 
 contract Simple4337Module is EIP4337Module {
-    // NOTE There is a change proposed to EIP-4337 to move nonce tracking to the entrypoint
-    mapping(address => mapping(bytes32 => uint64)) private nonces;
-
     constructor(address entryPoint)
         EIP4337Module(entryPoint, bytes4(keccak256("execTransactionFromModule(address,uint256,bytes,uint8)")))
     {}
 
     function validateReplayProtection(UserOperation calldata userOp) internal override {
-        // We need to increase the nonce to make it impossible to drain the safe by making it send prefunds for the same transaction
+        // The entrypoints handles the increase of the nonce
         // Right shifting fills up with 0s from the left
-        bytes32 key = bytes32(userOp.nonce >> 64);
-        uint64 safeNonce = nonces[userOp.sender][key];
-        nonces[userOp.sender][key]++;
-
-        // Casting to uint64 to remove the key segment
-        require(safeNonce == uint64(userOp.nonce), "Invalid Nonce");
-    }
-}
-
-contract DoubleCheck4337Module is EIP4337Module {
-    bytes32 private constant SAFE_4337_EXECUTION_TYPEHASH =
-        keccak256("Safe4337Execution(address safe,address target,uint256 value,bytes calldata data,uint8 operation,uint256 nonce)");
-
-    struct ExecutionStatus {
-        bool approved;
-        bool executed;
-    }
-
-    mapping(address => mapping(bytes32 => ExecutionStatus)) private hashes;
-
-    constructor(address entryPoint)
-        EIP4337Module(entryPoint, bytes4(keccak256("checkAndExecTransaction(address,address,uint256,bytes,uint8,uint256)")))
-    {}
+        uint192 key = uint192(userOp.nonce >> 64);
+        uint256 safeNonce = INonceManager(supportedEntryPoint).getNonce(userOp.sender, key);
 
-    function encodeSafeExecutionData(
-        address safe,
-        address target,
-        uint256 value,
-        bytes memory data,
-        uint8 operation,
-        uint256 nonce
-    ) public view returns (bytes memory) {
-        bytes32 safeExecutionTypeData = keccak256(
-            abi.encode(SAFE_4337_EXECUTION_TYPEHASH, safe, target, value, keccak256(data), operation, nonce)
-        );
-
-        return abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeExecutionTypeData);
-    }
-
-    function validateReplayProtection(UserOperation calldata userOp) internal override {
-        (address safe, address target, uint256 value, bytes memory data, uint8 operation, uint256 nonce) = abi.decode(
-            userOp.callData,
-            (address, address, uint256, bytes, uint8, uint256)
-        );
-        bytes32 executionHash = keccak256(encodeSafeExecutionData(safe, target, value, data, operation, nonce));
-        require(userOp.sender == safe, "Unexpected Safe in calldata");
-        require(userOp.nonce == nonce, "Unexpected nonce in calldata");
-        ExecutionStatus memory status = hashes[userOp.sender][executionHash];
-        require(!status.approved && !status.executed, "Unexpected status");
-        hashes[userOp.sender][executionHash].approved = true;
-    }
-
-    function checkAndExecTransactionFromModule(
-        address safe,
-        address target,
-        uint256 value,
-        bytes calldata data,
-        uint8 operation,
-        uint256 nonce
-    ) external {
-        bytes32 executionHash = keccak256(encodeSafeExecutionData(safe, target, value, data, operation, nonce));
-        ExecutionStatus memory status = hashes[safe][executionHash];
-        require(status.approved && !status.executed, "Unexpected status");
-        hashes[safe][executionHash].executed = true;
-        Safe(safe).execTransactionFromModule(target, value, data, operation);
+        // Check returned nonce against the user operation nonce
+        require(safeNonce == userOp.nonce, "Invalid Nonce");
     }
 }

4337/contracts/interfaces/ERC4337.sol

@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity >=0.7.0 <0.9.0;
+
+import {UserOperation} from "../UserOperation.sol";
+
+interface INonceManager {
+    /**
+     * Return the next nonce for this sender.
+     * Within a given key, the nonce values are sequenced (starting with zero, and incremented by one on each userop)
+     * But UserOp with different keys can come with arbitrary order.
+     *
+     * @param sender the account address
+     * @param key the high 192 bit of the nonce
+     * @return nonce a full nonce to pass for next UserOp with this sender.
+     */
+    function getNonce(address sender, uint192 key) external view returns (uint256 nonce);
+
+    /**
+     * Manually increment the nonce of the sender.
+     * This method is exposed just for completeness..
+     * Account does NOT need to call it, neither during validation, nor elsewhere,
+     * as the EntryPoint will update the nonce regardless.
+     * Possible use-case is call it with various keys to "initialize" their nonces to one, so that future
+     * UserOperations will not pay extra for the first transaction with a given key.
+     */
+    function incrementNonce(uint192 key) external;
+}
+
+interface IAccount {
+    /**
+     * Validate user's signature and nonce
+     * the entryPoint will make the call to the recipient only if this validation call returns successfully.
+     * signature failure should be reported by returning SIG_VALIDATION_FAILED (1).
+     * This allows making a "simulation call" without a valid signature
+     * Other failures (e.g. nonce mismatch, or invalid signature format) should still revert to signal failure.
+     *
+     * @dev Must validate caller is the entryPoint.
+     *      Must validate the signature and nonce
+     * @param userOp the operation that is about to be executed.
+     * @param userOpHash hash of the user's request data. can be used as the basis for signature.
+     * @param missingAccountFunds missing funds on the account's deposit in the entrypoint.
+     *      This is the minimum amount to transfer to the sender(entryPoint) to be able to make the call.
+     *      The excess is left as a deposit in the entrypoint, for future calls.
+     *      can be withdrawn anytime using "entryPoint.withdrawTo()"
+     *      In case there is a paymaster in the request (or the current deposit is high enough), this value will be zero.
+     * @return validationData packaged ValidationData structure. use `_packValidationData` and `_unpackValidationData` to encode and decode
+     *      <20-byte> sigAuthorizer - 0 for valid signature, 1 to mark signature failure,
+     *         otherwise, an address of an "authorizer" contract.
+     *      <6-byte> validUntil - last timestamp this operation is valid. 0 for "indefinite"
+     *      <6-byte> validAfter - first timestamp this operation is valid
+     *      If an account doesn't use time-range, it is enough to return SIG_VALIDATION_FAILED value (1) for signature failure.
+     *      Note that the validation code cannot use block.timestamp (or block.number) directly.
+     */
+    function validateUserOp(
+        UserOperation calldata userOp,
+        bytes32 userOpHash,
+        uint256 missingAccountFunds
+    ) external returns (uint256 validationData);
+}

4337/contracts/interfaces/Safe.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 pragma solidity >=0.7.0 <0.9.0;
 
-interface Safe {
+interface ISafe {
     /**
      * @dev Reads `length` bytes of storage in the currents contract
      * @param offset - the offset in the current contract's storage in words to start reading from
@@ -49,4 +49,11 @@ interface Safe {
      * @return next Start of the next page.
      */
     function getModulesPaginated(address start, uint256 pageSize) external view returns (address[] memory array, address next);
+
+    /**
+     * @notice Enables the module `module` for the Safe.
+     * @dev This can only be done via a Safe transaction.
+     * @param module Module to be enabled.
+     */
+    function enableModule(address module) external;
 }

4337/contracts/test/SafeMock.sol

@@ -3,7 +3,10 @@ pragma solidity >=0.8.0;
 
 import "../UserOperation.sol";
 
-import "@gnosis.pm/safe-contracts/contracts/proxies/GnosisSafeProxyFactory.sol";
+import "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol";
+import "@safe-global/safe-contracts/contracts/SafeL2.sol";
+
+import {INonceManager} from "../interfaces/ERC4337.sol";
 
 contract SafeMock {
     address public immutable supportedEntryPoint;
@@ -228,7 +231,7 @@ contract Safe4337Mock is SafeMock {
         return block.chainid;
     }
 
-    /// @dev Validates that the user operation is correctly signed. Users methods from Gnosis Safe contract, reverts if signatures are invalid
+    /// @dev Validates that the user operation is correctly signed. Users methods from Safe contract, reverts if signatures are invalid
     /// @param entryPoint Address of the entry point
     /// @param userOp User operation struct
     function _validateSignatures(address entryPoint, UserOperation calldata userOp) internal view {
@@ -248,16 +251,13 @@ contract Safe4337Mock is SafeMock {
         checkSignatures(operationHash, operationData, userOp.signature);
     }
 
-    mapping(address => mapping(bytes32 => uint64)) private nonces;
-
     function validateReplayProtection(UserOperation calldata userOp) internal {
-        // We need to increase the nonce to make it impossible to drain the safe by making it send prefunds for the same transaction
+        // The entrypoints handles the increase of the nonce
         // Right shifting fills up with 0s from the left
-        bytes32 key = bytes32(userOp.nonce >> 64);
-        uint64 safeNonce = nonces[userOp.sender][key];
-        nonces[userOp.sender][key]++;
+        uint192 key = uint192(userOp.nonce >> 64);
+        uint256 safeNonce = INonceManager(supportedEntryPoint).getNonce(userOp.sender, key);
 
-        // Casting to uint64 to remove the key segment
-        require(safeNonce == uint64(userOp.nonce), "Invalid Nonce");
+        // Check returned nonce against the user operation nonce
+        require(safeNonce == userOp.nonce, "Invalid Nonce");
     }
 }