From a96739ce5a925d1a86dc952e0b6d1e6f14e99faa Mon Sep 17 00:00:00 2001 From: Mikhail Date: Fri, 19 May 2023 18:10:46 +0200 Subject: [PATCH] Remove gasleft in setupModules, add erc4337 compatibility test --- .env.sample | 5 + README.md | 20 ++- contracts/base/ModuleManager.sol | 2 +- .../test/4337/Test4337ModuleAndHandler.sol | 65 +++++++++ hardhat.config.ts | 5 +- test/integration/Safe.ERC4337.spec.ts | 133 ++++++++++++++++++ test/utils/setup.ts | 12 ++ tsconfig.json | 3 +- 8 files changed, 240 insertions(+), 5 deletions(-) create mode 100644 contracts/test/4337/Test4337ModuleAndHandler.sol create mode 100644 test/integration/Safe.ERC4337.spec.ts diff --git a/.env.sample b/.env.sample index f2d066cb3..0f2a3ff2e 100644 --- a/.env.sample +++ b/.env.sample @@ -4,3 +4,8 @@ INFURA_KEY="" # Used for custom network NODE_URL="" ETHERSCAN_API_KEY="" +# (Optional) Used to run ERC-4337 compatibility test. MNEMONIC is also required. +ERC4337_TEST_BUNDLER_URL= +ERC4337_TEST_NODE_URL= +ERC4337_TEST_SINGLETON_ADDRESS= +ERC4337_TEST_SAFE_FACTORY_ADDRESS= \ No newline at end of file diff --git a/README.md b/README.md index 199e5ce84..1db39c1bf 100644 --- a/README.md +++ b/README.md @@ -15,13 +15,31 @@ Usage yarn ``` -### Run all tests: +### Testing + +To run the tests: ```bash yarn build yarn test ``` +Optionally, if you want to run the ERC-4337 compatibility test, it uses a live bundler and node, so it contains some pre-requisites: + +1. Define the environment variables: + +``` +ERC4337_TEST_BUNDLER_URL= +ERC4337_TEST_NODE_URL= +ERC4337_TEST_SINGLETON_ADDRESS= +ERC4337_TEST_SAFE_FACTORY_ADDRESS= +MNEMONIC= +``` + +2. Pre-fund the executor account derived from the mnemonic with some Native Token to cover the deployment of an ERC4337 module and the pre-fund of the Safe for the test operation. + +3. If necessary, edit the hardcoded base fee in `contracts/test/4337/UserOperation.sol` + ### Deployments A collection of the different Safe contract deployments and their addresses can be found in the [Safe deployments](https://github.com/safe-global/safe-deployments) repository. diff --git a/contracts/base/ModuleManager.sol b/contracts/base/ModuleManager.sol index f260c0865..ecc3938ad 100644 --- a/contracts/base/ModuleManager.sol +++ b/contracts/base/ModuleManager.sol @@ -35,7 +35,7 @@ abstract contract ModuleManager is SelfAuthorized, Executor { if (to != address(0)) { require(isContract(to), "GS002"); // Setup has to complete successfully or transaction fails. - require(execute(to, 0, data, Enum.Operation.DelegateCall, gasleft()), "GS000"); + require(execute(to, 0, data, Enum.Operation.DelegateCall, type(uint256).max), "GS000"); } } diff --git a/contracts/test/4337/Test4337ModuleAndHandler.sol b/contracts/test/4337/Test4337ModuleAndHandler.sol new file mode 100644 index 000000000..e0a1074e8 --- /dev/null +++ b/contracts/test/4337/Test4337ModuleAndHandler.sol @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity >=0.7.0 <0.9.0; +pragma abicoder v2; + +import "../../libraries/SafeStorage.sol"; + +struct UserOperation { + address sender; + uint256 nonce; + bytes initCode; + bytes callData; + uint256 callGasLimit; + uint256 verificationGasLimit; + uint256 preVerificationGas; + uint256 maxFeePerGas; + uint256 maxPriorityFeePerGas; + bytes paymasterAndData; + bytes signature; +} + +interface ISafe { + function execTransactionFromModule(address to, uint256 value, bytes memory data, uint8 operation) external returns (bool success); +} + +/// @dev A Dummy 4337 Module/Handler for testing purposes +/// ⚠️ ⚠️ ⚠️ DO NOT USE IN PRODUCTION ⚠️ ⚠️ ⚠️ +/// The module does not perform ANY validation, it just executes validateUserOp and execTransaction +/// to perform the opcode level compliance by the bundler. +contract Test4337ModuleAndHandler is SafeStorage { + address public immutable myAddress; + address public immutable entryPoint; + + address internal constant SENTINEL_MODULES = address(0x1); + + constructor(address entryPointAddress) { + entryPoint = entryPointAddress; + myAddress = address(this); + } + + function validateUserOp(UserOperation calldata userOp, bytes32, uint256 missingAccountFunds) external returns (uint256 validationData) { + address payable safeAddress = payable(userOp.sender); + ISafe senderSafe = ISafe(safeAddress); + + if (missingAccountFunds != 0) { + senderSafe.execTransactionFromModule(entryPoint, missingAccountFunds, "", 0); + } + + return 0; + } + + function execTransaction(address to, uint256 value, bytes calldata data) external payable { + address payable safeAddress = payable(msg.sender); + ISafe safe = ISafe(safeAddress); + require(safe.execTransactionFromModule(to, value, data, 0), "tx failed"); + } + + function enableMyself() public { + require(myAddress != address(this), "You need to DELEGATECALL, sir"); + + // Module cannot be added twice. + require(modules[myAddress] == address(0), "GS102"); + modules[myAddress] = modules[SENTINEL_MODULES]; + modules[SENTINEL_MODULES] = myAddress; + } +} diff --git a/hardhat.config.ts b/hardhat.config.ts index ffd6765ee..b8b99ba8b 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -1,3 +1,4 @@ +import "@nomiclabs/hardhat-ethers"; import type { HardhatUserConfig, HttpNetworkUserConfig } from "hardhat/types"; import "@nomiclabs/hardhat-etherscan"; import "@nomiclabs/hardhat-waffle"; @@ -41,7 +42,7 @@ import { BigNumber } from "@ethersproject/bignumber"; import { DeterministicDeploymentInfo } from "hardhat-deploy/dist/types"; const primarySolidityVersion = SOLIDITY_VERSION || "0.7.6"; -const soliditySettings = !!SOLIDITY_SETTINGS ? JSON.parse(SOLIDITY_SETTINGS) : undefined; +const soliditySettings = SOLIDITY_SETTINGS ? JSON.parse(SOLIDITY_SETTINGS) : undefined; const deterministicDeployment = (network: string): DeterministicDeploymentInfo => { const info = getSingletonFactoryInfo(parseInt(network)); @@ -132,7 +133,7 @@ const userConfig: HardhatUserConfig = { }, }; if (NODE_URL) { - userConfig.networks!!.custom = { + userConfig.networks!.custom = { ...sharedNetworkConfig, url: NODE_URL, }; diff --git a/test/integration/Safe.ERC4337.spec.ts b/test/integration/Safe.ERC4337.spec.ts new file mode 100644 index 000000000..8a943f830 --- /dev/null +++ b/test/integration/Safe.ERC4337.spec.ts @@ -0,0 +1,133 @@ +import hre from "hardhat"; +import { expect } from "chai"; +import { AddressZero } from "@ethersproject/constants"; +import { hexConcat } from "ethers/lib/utils"; +import { getFactoryContract, getSafeSingletonContract } from "../utils/setup"; +import { calculateProxyAddress } from "../../src/utils/proxies"; + +const ERC4337_TEST_ENV_VARIABLES_DEFINED = + typeof process.env.ERC4337_TEST_BUNDLER_URL !== "undefined" && + typeof process.env.ERC4337_TEST_NODE_URL !== "undefined" && + typeof process.env.ERC4337_TEST_SAFE_FACTORY_ADDRESS !== "undefined" && + typeof process.env.ERC4337_TEST_SINGLETON_ADDRESS !== "undefined" && + typeof process.env.MNEMONIC !== "undefined"; + +const itif = ERC4337_TEST_ENV_VARIABLES_DEFINED ? it : it.skip; +const SAFE_FACTORY_ADDRESS = process.env.ERC4337_TEST_SAFE_FACTORY_ADDRESS; +const SINGLETON_ADDRESS = process.env.ERC4337_TEST_SINGLETON_ADDRESS; +const BUNDLER_URL = process.env.ERC4337_TEST_BUNDLER_URL; +const NODE_URL = process.env.ERC4337_TEST_NODE_URL; +const MNEMONIC = process.env.MNEMONIC; + +type UserOperation = { + sender: string; + nonce: string; + initCode: string; + callData: string; + callGasLimit: string; + verificationGasLimit: string; + preVerificationGas: string; + maxFeePerGas: string; + maxPriorityFeePerGas: string; + paymasterAndData: string; + signature: string; +}; + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +describe("Safe.ERC4337", () => { + const setupTests = async () => { + const factory = await getFactoryContract(); + const singleton = await getSafeSingletonContract(); + const bundlerProvider = new hre.ethers.providers.JsonRpcProvider(BUNDLER_URL); + const provider = new hre.ethers.providers.JsonRpcProvider(NODE_URL); + const userWallet = hre.ethers.Wallet.fromMnemonic(MNEMONIC as string).connect(provider); + + const entryPoints = await bundlerProvider.send("eth_supportedEntryPoints", []); + if (entryPoints.length === 0) { + throw new Error("No entry points found"); + } + + return { + factory: factory.attach(SAFE_FACTORY_ADDRESS).connect(userWallet), + singleton: singleton.attach(SINGLETON_ADDRESS).connect(provider), + bundlerProvider, + provider, + userWallet, + entryPoints, + }; + }; + + /** + * This test verifies the ERC4337 based on gas estimation for a user operation + * The user operation deploys a Safe with the ERC4337 module and a handler + * and executes a transaction, thus verifying two things: + * 1. Deployment of the Safe with the ERC4337 module and handler is possible + * 2. Executing a transaction is possible + */ + itif("should pass the ERC4337 validation", async () => { + const { singleton, factory, provider, bundlerProvider, userWallet, entryPoints } = await setupTests(); + const ENTRYPOINT_ADDRESS = entryPoints[0]; + + const erc4337ModuleAndHandlerFactory = (await hre.ethers.getContractFactory("Test4337ModuleAndHandler")).connect(userWallet); + const erc4337ModuleAndHandler = await erc4337ModuleAndHandlerFactory.deploy(ENTRYPOINT_ADDRESS); + // The bundler uses a different node, so we need to allow it sometime to sync + await sleep(10000); + + const feeData = await provider.getFeeData(); + const maxFeePerGas = feeData.maxFeePerGas.toHexString(); + + const maxPriorityFeePerGas = feeData.maxPriorityFeePerGas.toHexString(); + + const moduleInitializer = erc4337ModuleAndHandler.interface.encodeFunctionData("enableMyself", []); + const encodedInitializer = singleton.interface.encodeFunctionData("setup", [ + [userWallet.address], + 1, + erc4337ModuleAndHandler.address, + moduleInitializer, + erc4337ModuleAndHandler.address, + AddressZero, + 0, + AddressZero, + ]); + const deployedAddress = await calculateProxyAddress(factory, singleton.address, encodedInitializer, 73); + + // The initCode contains 20 bytes of the factory address and the rest is the calldata to be forwarded + const initCode = hexConcat([ + factory.address, + factory.interface.encodeFunctionData("createProxyWithNonce", [singleton.address, encodedInitializer, 73]), + ]); + const userOpCallData = erc4337ModuleAndHandler.interface.encodeFunctionData("execTransaction", [userWallet.address, 0, 0]); + + // Native tokens for the pre-fund 💸 + await userWallet.sendTransaction({ to: deployedAddress, value: hre.ethers.utils.parseEther("0.001") }); + // The bundler uses a different node, so we need to allow it sometime to sync + await sleep(10000); + + const userOperation: UserOperation = { + sender: deployedAddress, + nonce: "0x0", + initCode, + callData: userOpCallData, + callGasLimit: "0x7A120", + verificationGasLimit: "0x7A120", + preVerificationGas: "0x186A0", + maxFeePerGas, + maxPriorityFeePerGas, + paymasterAndData: "0x", + signature: "0x", + }; + + const DEBUG_MESSAGE = ` + Using entry point: ${ENTRYPOINT_ADDRESS} + Deployed Safe address: ${deployedAddress} + Module/Handler address: ${erc4337ModuleAndHandler.address} + User operation: + ${JSON.stringify(userOperation, null, 2)} + `; + console.log(DEBUG_MESSAGE); + + const estimatedGas = await bundlerProvider.send("eth_estimateUserOperationGas", [userOperation, ENTRYPOINT_ADDRESS]); + expect(estimatedGas).to.not.be.undefined; + }); +}); diff --git a/test/utils/setup.ts b/test/utils/setup.ts index a44644eb3..946f497cd 100644 --- a/test/utils/setup.ts +++ b/test/utils/setup.ts @@ -28,6 +28,18 @@ export const getSafeSingleton = async () => { return Safe.attach(SafeDeployment.address); }; +export const getSafeSingletonContract = async () => { + const safeSingleton = await hre.ethers.getContractFactory(safeContractUnderTest()); + + return safeSingleton; +}; + +export const getFactoryContract = async () => { + const factory = await hre.ethers.getContractFactory("SafeProxyFactory"); + + return factory; +}; + export const getFactory = async () => { const FactoryDeployment = await deployments.get("SafeProxyFactory"); const Factory = await hre.ethers.getContractFactory("SafeProxyFactory"); diff --git a/tsconfig.json b/tsconfig.json index fe78c009e..9e27a0645 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -18,5 +18,6 @@ "resolveJsonModule": true }, "exclude": ["dist", "node_modules"], - "include": ["./src/index.ts", "./types"] + "include": ["./src/index.ts", "./types"], + "files": ["./hardhat.config.ts"] } \ No newline at end of file