From 035f905fba48a4d7366ec7337a6c1014c5b1577e Mon Sep 17 00:00:00 2001 From: Richard Meissner Date: Fri, 13 Oct 2023 14:42:52 +0200 Subject: [PATCH] Closes #83: Add integration tests (#90) * Add integration tests * Formatting * Make compatible with stackup * Minor refactortings * Remove nonce check from module, as done by Entrypoint * Add more comments --- 4337/README.md | 2 + 4337/contracts/EIP4337Module.sol | 93 ++---- 4337/contracts/test/SafeMock.sol | 26 +- 4337/contracts/test/TestEntryPoint.sol | 7 +- 4337/scripts/runOp.ts | 125 +++++--- 4337/src/utils/safe.ts | 275 ++++++++++++++++-- 4337/src/utils/userOp.ts | 18 +- .../eip4337/EIP4337ModuleExisting.spec.ts | 34 ++- 4337/test/eip4337/EIP4337ModuleNew.spec.ts | 50 +++- 4337/test/eip4337/EIP4337Safe.spec.ts | 4 +- 10 files changed, 469 insertions(+), 165 deletions(-) diff --git a/4337/README.md b/4337/README.md index 9d1246968..4da8ff80b 100644 --- a/4337/README.md +++ b/4337/README.md @@ -1,5 +1,7 @@ # Safe Module/Fallback handler for EIP4337 Support +:warning: **This module MUST only be used with Safe 1.4.1 or newer** :warning: + ## Execution Flow The diagram below outlines the flow that is triggered when a user operation is submitted to the entrypoint. Additionally the gas overhead compared to a native implementation is mentioned. diff --git a/4337/contracts/EIP4337Module.sol b/4337/contracts/EIP4337Module.sol index 3a5e43b05..533402ff7 100644 --- a/4337/contracts/EIP4337Module.sol +++ b/4337/contracts/EIP4337Module.sol @@ -10,11 +10,16 @@ import {ISafe} from "./interfaces/Safe.sol"; /// @title EIP4337Module abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler { using UserOperationLib for UserOperation; + + // value in case of signature failure, with no time-range. + // equivalent to _packValidationData(true,0,0); + uint256 internal constant SIG_VALIDATION_FAILED = 1; + bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = keccak256("EIP712Domain(uint256 chainId,address verifyingContract)"); bytes32 private constant SAFE_OP_TYPEHASH = keccak256( - "SafeOp(address safe,bytes callData,uint256 nonce,uint256 verificationGas,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,uint256 callGas,address entryPoint)" + "SafeOp(address safe,bytes callData,uint256 nonce,uint256 preVerificationGas,uint256 verificationGasLimit,uint256 callGasLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,address entryPoint)" ); address public immutable supportedEntryPoint; @@ -32,32 +37,28 @@ abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler UserOperation calldata userOp, bytes32, uint256 requiredPrefund - ) external returns (uint256) { + ) external returns (uint256 validationResult) { address payable safeAddress = payable(userOp.sender); // The entryPoint address is appended to the calldata in `HandlerContext` contract - // Because of this, the relayer may be manipulate the entryPoint address, therefore we have to verify that + // Because of this, the relayer may manipulate the entryPoint address, therefore we have to verify that // the sender is the Safe specified in the userOperation require(safeAddress == msg.sender, "Invalid Caller"); - validateReplayProtection(userOp); - + // We verify that the userOp calls the expected execution function require(expectedExecutionFunctionId == bytes4(userOp.callData), "Unsupported execution function id"); - // We need to make sure that the entryPoint's requested prefund is in bounds - require(requiredPrefund <= userOp.requiredPreFund(), "Prefund too high"); - address entryPoint = _msgSender(); require(entryPoint == supportedEntryPoint, "Unsupported entry point"); - _validateSignatures(entryPoint, userOp); + // The userOp nonce is validated in the Entrypoint (for 0.6.0+), therefore we will not check it again + validationResult = validateSignatures(entryPoint, userOp); + // We trust the entrypoint to set the correct prefund value, based on the operation params + // We need to perform this even if the signature is not valid, else the simulation function of the Entrypoint will not work if (requiredPrefund != 0) { ISafe(safeAddress).execTransactionFromModule(entryPoint, requiredPrefund, "", 0); } - return 0; } - function validateReplayProtection(UserOperation calldata userOp) internal virtual; - function domainSeparator() public view returns (bytes32) { return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, block.chainid, this)); } @@ -66,86 +67,62 @@ abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler /// @param safe Safe address /// @param callData Call data /// @param nonce Nonce of the operation - /// @param verificationGas Gas required for verification /// @param preVerificationGas Gas required for pre-verification (e.g. for EOA signature verification) + /// @param verificationGasLimit Gas required for verification + /// @param callGasLimit Gas available during the execution of the call /// @param maxFeePerGas Max fee per gas /// @param maxPriorityFeePerGas Max priority fee per gas - /// @param callGas Gas available during the execution of the call /// @param entryPoint Address of the entry point /// @return Operation hash bytes - function encodeOperationData( + function getOperationHash( address safe, bytes calldata callData, uint256 nonce, - uint256 verificationGas, uint256 preVerificationGas, + uint256 verificationGasLimit, + uint256 callGasLimit, uint256 maxFeePerGas, uint256 maxPriorityFeePerGas, - uint256 callGas, address entryPoint - ) public view returns (bytes memory) { + ) public view returns (bytes32) { bytes32 safeOperationHash = keccak256( abi.encode( SAFE_OP_TYPEHASH, safe, keccak256(callData), nonce, - verificationGas, preVerificationGas, + verificationGasLimit, + callGasLimit, maxFeePerGas, maxPriorityFeePerGas, - callGas, entryPoint ) ); - - return abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeOperationHash); - } - - function getOperationHash( - address safe, - bytes calldata callData, - uint256 nonce, - uint256 verificationGas, - uint256 preVerificationGas, - uint256 maxFeePerGas, - uint256 maxPriorityFeePerGas, - uint256 callGas, - address entryPoint - ) public view returns (bytes32) { - return - keccak256( - encodeOperationData( - safe, - callData, - nonce, - verificationGas, - preVerificationGas, - maxFeePerGas, - maxPriorityFeePerGas, - callGas, - entryPoint - ) - ); + return keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeOperationHash)); } /// @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 { + function validateSignatures(address entryPoint, UserOperation calldata userOp) internal view returns (uint256) { bytes32 operationHash = getOperationHash( payable(userOp.sender), userOp.callData, userOp.nonce, - userOp.verificationGasLimit, userOp.preVerificationGas, + userOp.verificationGasLimit, + userOp.callGasLimit, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, - userOp.callGasLimit, entryPoint ); - ISafe(payable(userOp.sender)).checkSignatures(operationHash, "", userOp.signature); + try ISafe(payable(userOp.sender)).checkSignatures(operationHash, "", userOp.signature) { + return 0; + } catch { + return SIG_VALIDATION_FAILED; + } } } @@ -153,14 +130,4 @@ contract Simple4337Module is EIP4337Module { constructor(address entryPoint) EIP4337Module(entryPoint, bytes4(keccak256("execTransactionFromModule(address,uint256,bytes,uint8)"))) {} - - function validateReplayProtection(UserOperation calldata userOp) internal override { - // The entrypoints handles the increase of the nonce - // Right shifting fills up with 0s from the left - uint192 key = uint192(userOp.nonce >> 64); - uint256 safeNonce = INonceManager(supportedEntryPoint).getNonce(userOp.sender, key); - - // Check returned nonce against the user operation nonce - require(safeNonce == userOp.nonce, "Invalid Nonce"); - } } diff --git a/4337/contracts/test/SafeMock.sol b/4337/contracts/test/SafeMock.sol index 0c4c966ca..1b23e92a8 100644 --- a/4337/contracts/test/SafeMock.sol +++ b/4337/contracts/test/SafeMock.sol @@ -122,7 +122,7 @@ contract Safe4337Mock is SafeMock { bytes32 private constant SAFE_OP_TYPEHASH = keccak256( - "SafeOp(address safe,bytes callData,uint256 nonce,uint256 verificationGas,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,uint256 callGas,address entryPoint)" + "SafeOp(address safe,bytes callData,uint256 nonce,uint256 preVerificationGas,uint256 verificationGasLimit,uint256 callGasLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,address entryPoint)" ); bytes4 public immutable expectedExecutionFunctionId; @@ -164,22 +164,22 @@ contract Safe4337Mock is SafeMock { /// @param safe Safe address /// @param callData Call data /// @param nonce Nonce of the operation - /// @param verificationGas Gas required for verification /// @param preVerificationGas Gas required for pre-verification (e.g. for EOA signature verification) + /// @param verificationGasLimit Gas required for verification + /// @param callGasLimit Gas available during the execution of the call /// @param maxFeePerGas Max fee per gas /// @param maxPriorityFeePerGas Max priority fee per gas - /// @param callGas Gas available during the execution of the call /// @param entryPoint Address of the entry point /// @return Operation hash bytes function encodeOperationData( address safe, bytes calldata callData, uint256 nonce, - uint256 verificationGas, uint256 preVerificationGas, + uint256 verificationGasLimit, + uint256 callGasLimit, uint256 maxFeePerGas, uint256 maxPriorityFeePerGas, - uint256 callGas, address entryPoint ) public view returns (bytes memory) { bytes32 safeOperationHash = keccak256( @@ -188,11 +188,11 @@ contract Safe4337Mock is SafeMock { safe, keccak256(callData), nonce, - verificationGas, preVerificationGas, + verificationGasLimit, + callGasLimit, maxFeePerGas, maxPriorityFeePerGas, - callGas, entryPoint ) ); @@ -204,11 +204,11 @@ contract Safe4337Mock is SafeMock { address safe, bytes calldata callData, uint256 nonce, - uint256 verificationGas, uint256 preVerificationGas, + uint256 verificationGasLimit, + uint256 callGasLimit, uint256 maxFeePerGas, uint256 maxPriorityFeePerGas, - uint256 callGas, address entryPoint ) public view returns (bytes32) { return @@ -217,11 +217,11 @@ contract Safe4337Mock is SafeMock { safe, callData, nonce, - verificationGas, preVerificationGas, + verificationGasLimit, + callGasLimit, maxFeePerGas, maxPriorityFeePerGas, - callGas, entryPoint ) ); @@ -239,11 +239,11 @@ contract Safe4337Mock is SafeMock { payable(userOp.sender), userOp.callData, userOp.nonce, - userOp.verificationGasLimit, userOp.preVerificationGas, + userOp.verificationGasLimit, + userOp.callGasLimit, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, - userOp.callGasLimit, entryPoint ); bytes32 operationHash = keccak256(operationData); diff --git a/4337/contracts/test/TestEntryPoint.sol b/4337/contracts/test/TestEntryPoint.sol index 3986518f6..b400f8034 100644 --- a/4337/contracts/test/TestEntryPoint.sol +++ b/4337/contracts/test/TestEntryPoint.sol @@ -59,7 +59,12 @@ contract TestEntryPoint is INonceManager { require(gasleft() > userOp.verificationGasLimit, "Not enough gas for verification"); - Account(userOp.sender).validateUserOp{gas: userOp.verificationGasLimit}(userOp, bytes32(0), requiredPrefund); + uint256 validationResult = Account(userOp.sender).validateUserOp{gas: userOp.verificationGasLimit}( + userOp, + bytes32(0), + requiredPrefund + ); + require(validationResult == 0, "Signature validation failed"); uint256 userBalance = balances[userOp.sender]; if (userBalance < requiredPrefund) { revert NotEnoughFunds(requiredPrefund, userBalance); diff --git a/4337/scripts/runOp.ts b/4337/scripts/runOp.ts index 137dce06d..deb0163b0 100644 --- a/4337/scripts/runOp.ts +++ b/4337/scripts/runOp.ts @@ -1,75 +1,109 @@ import { parseEther } from '@ethersproject/units' import hre, { ethers } from 'hardhat' import '@nomiclabs/hardhat-ethers' -import { buildSignatureBytes, signHash } from '../src/utils/execution' import { - buildSafeUserOp, getRequiredPrefund, - calculateSafeOperationHash, - buildUserOperationFromSafeUserOperation, getSupportedEntryPoints, - buildSafeUserOpTransaction, - signSafeOp, } from '../src/utils/userOp' import { chainId } from '../test/utils/encoding' import { getSimple4337Module } from '../test/utils/setup' +import { Result } from 'ethers/lib/utils' +import { GlobalConfig, MultiProvider4337, Safe4337 } from '../src/utils/safe' const DEBUG = process.env.SCRIPT_DEBUG || false const MNEMONIC = process.env.SCRIPT_MNEMONIC -const BUNLDER_URL = process.env.SCRIPT_BUNDLER_URL -const SAFE_ADDRESS = process.env.SCRIPT_SAFE_ADDRESS -const MODULE_ADDRESS = process.env.SCRIPT_MODULE_ADDRESS +const BUNDLER_URL = process.env.SCRIPT_BUNDLER_URL +const SAFE_SINGLETON_ADDRESS = process.env.SCRIPT_SAFE_SINGLETON_ADDRESS!! +const PROXY_FACTORY_ADDRESS = process.env.SCRIPT_PROXY_FACTORY_ADDRESS!! +const ADD_MODULES_LIB_ADDRESS = process.env.SCRIPT_ADD_MODULES_LIB_ADDRESS!! +const MODULE_ADDRESS = process.env.SCRIPT_MODULE_ADDRESS!! + +const INTERFACES = new ethers.utils.Interface([ + 'function enableModule(address)', + 'function setup(address[],uint256,address,bytes,address,address,uint256,address)', + 'function createProxyWithNonce(address,bytes,uint256) returns (address)', + 'function proxyCreationCode() returns (bytes)', + 'function enableModules(address[])', + 'function getNonce(address,uint192) returns (uint256 nonce)', + 'function supportedEntryPoint() returns (address)', + 'function getOwners() returns (address[])', + 'function getModulesPaginated(address, uint256) returns (address[], address)', + 'function getOperationHash(address,bytes,uint256,uint256,uint256,uint256,uint256,uint256,address)' +]) const buildData = (method: string, params?: any[]): string => { const iface = new ethers.utils.Interface([`function ${method}`]) return iface.encodeFunctionData(method, params) } +const callInterface = async(contract: string, method: string, params: any[] = []): Promise => { + const result = await hre.ethers.provider.call({ + to: contract, + data: INTERFACES.encodeFunctionData(method, params) + }) + return INTERFACES.decodeFunctionResult(method, result) +} + const runOp = async () => { const user1 = MNEMONIC ? hre.ethers.Wallet.fromMnemonic(MNEMONIC).connect(hre.ethers.provider) : (await hre.ethers.getSigners())[0] // This node only allows eth_chainId, eth_getSupportedEntrypoints, eth_sendUserOperation // All other methods return an error - const accountAbstractionProvider = new hre.ethers.providers.JsonRpcProvider(BUNLDER_URL) - const moduleAddress = MODULE_ADDRESS ?? ((await getSimple4337Module()).address) - const safeAddress = SAFE_ADDRESS!! + const accountAbstractionProvider = new MultiProvider4337(BUNDLER_URL!!, hre.ethers.provider) const entryPoints = await getSupportedEntryPoints(accountAbstractionProvider) const entryPoint = entryPoints[0] + const moduleAddress = MODULE_ADDRESS ?? ((await getSimple4337Module()).address) + const moduleSupportedEntrypoint = await user1.call({ to: moduleAddress, data: INTERFACES.encodeFunctionData("supportedEntryPoint")}) + console.log({moduleAddress, moduleSupportedEntrypoint}) + + const proxyCreationCode = (await callInterface(PROXY_FACTORY_ADDRESS, "proxyCreationCode"))[0] - const feeData = await ethers.provider.getFeeData() - console.log({feeData}) - - const legacyFee = feeData.maxFeePerGas!!.add(feeData.maxPriorityFeePerGas!!).toString() - const safeOp = buildSafeUserOpTransaction( - safeAddress, - '0x02270bd144e70cE6963bA02F575776A16184E1E6', - parseEther('0.1'), - '0x', - '0', + const globalConfig: GlobalConfig = { entryPoint, - false, - { - maxFeePerGas: legacyFee, - maxPriorityFeePerGas: legacyFee, - callGas: '1000000', - }, - ) - - console.log(await chainId()) - console.log(await user1.address) - const safeOpHash = calculateSafeOperationHash(moduleAddress, safeOp, await chainId()) - const signature = buildSignatureBytes([await signSafeOp(user1, moduleAddress, safeOp, await chainId())]) - console.log({ safeOpHash, signature }) - const userOp = buildUserOperationFromSafeUserOperation({ - safeAddress, - safeOp, - signature, + safeSingleton: SAFE_SINGLETON_ADDRESS, + erc4337module: MODULE_ADDRESS, + proxyFactory: PROXY_FACTORY_ADDRESS, + proxyCreationCode, + addModulesLib: ADD_MODULES_LIB_ADDRESS, + chainId: await chainId() + } + const safe = await Safe4337.withSigner(user1.address, globalConfig) + + safe.connect(accountAbstractionProvider) + + console.log(safe.address) + const safeBalance = await ethers.provider.getBalance(safe.address) + const minBalance = ethers.utils.parseEther("0.01") + console.log(safeBalance) + if (safeBalance < minBalance) { + await(await user1.sendTransaction({to: safe.address, value: ethers.utils.parseEther("0.01")})).wait() + } + + const operation = await safe.operate({ + to: '0x02270bd144e70cE6963bA02F575776A16184E1E6', + value: parseEther('0.0001'), + data: '0x', + operation: 0 }) - console.log(userOp) + + await operation.authorize(user1); + + const userOp = await operation.userOperation() + console.log({userOp}) + + console.log("checkSignatures", await ethers.provider.send("eth_call", [{ + from: entryPoint, + to: safe.address, + data: buildData("checkSignatures(bytes32,bytes,bytes)", [ + operation.operationHash(), + "0x", + await operation.encodedSignatures() + ]), + }, "latest"])) console.log("validateUserOp", await ethers.provider.send("eth_call", [{ from: entryPoint, - to: safeAddress, + to: safe.address, data: buildData("validateUserOp((address,uint256,bytes,bytes,uint256,uint256,uint256,uint256,uint256,bytes,bytes),bytes32,uint256)", [ [ userOp.sender, @@ -89,15 +123,12 @@ const runOp = async () => { ]), }, "latest"])) - console.log(await accountAbstractionProvider.send('eth_estimateUserOperationGas', [{ - ...userOp - }, entryPoint])) - if (DEBUG) { console.log('Usign account with address:', user1.address) console.log('Using EIP4337Diatomic deployed at:', moduleAddress) - console.log('Using Safe contract deployed at:', safeAddress) - console.log('Using entrypoint at:', entryPoints[1]) + console.log('Using Safe contract deployed at:', safe.address) + console.log('Using entrypoint at:', entryPoint) + console.log('Balance of Safe:', ethers.utils.formatEther(await ethers.provider.getBalance(safe.address)), "ETH") } await accountAbstractionProvider.send('eth_sendUserOperation', [userOp, entryPoint]) diff --git a/4337/src/utils/safe.ts b/4337/src/utils/safe.ts index 17ac6601b..f05bedbd2 100644 --- a/4337/src/utils/safe.ts +++ b/4337/src/utils/safe.ts @@ -1,4 +1,11 @@ -import { constants, ethers, providers } from 'ethers' +import { constants, ethers, utils, providers } from 'ethers' +import { TypedDataSigner } from '@ethersproject/abstract-signer' +// Import from Safe contracts repo once fixed +import { MetaTransaction, SafeSignature, buildSignatureBytes } from './execution' +import { UserOperation } from './userOp' + +const AddressOne = "0x0000000000000000000000000000000000000001"; + const INTERFACES = new ethers.utils.Interface([ 'function enableModule(address)', @@ -6,23 +13,51 @@ const INTERFACES = new ethers.utils.Interface([ 'function createProxyWithNonce(address,bytes,uint256) returns (address)', 'function proxyCreationCode() returns (bytes)', 'function enableModules(address[])', + 'function execTransactionFromModule(address to, uint256 value, bytes calldata data, uint8 operation) external payable returns (bool success)', 'function getNonce(address,uint192) returns (uint256 nonce)', 'function supportedEntryPoint() returns (address)', 'function getOwners() returns (address[])', + 'function getThreshold() view returns (uint256)', 'function getModulesPaginated(address, uint256) returns (address[], address)', 'function getOperationHash(address,bytes,uint256,uint256,uint256,uint256,uint256,uint256,address)' - ]) +]) + + +const EIP712_SAFE_OPERATION_TYPE = { + // "SafeOp(address safe,bytes callData,uint256 nonce,uint256 preVerificationGas,uint256 verificationGasLimit,uint256 callGasLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,address entryPoint)" + SafeOp: [ + { type: 'address', name: 'safe' }, + { type: 'bytes', name: 'callData' }, + { type: 'uint256', name: 'nonce' }, + { type: 'uint256', name: 'preVerificationGas' }, + { type: 'uint256', name: 'verificationGasLimit' }, + { type: 'uint256', name: 'callGasLimit' }, + { type: 'uint256', name: 'maxFeePerGas' }, + { type: 'uint256', name: 'maxPriorityFeePerGas' }, + { type: 'address', name: 'entryPoint' }, + ], + } -interface GlobalConfig { +export interface OperationParams { + nonce: bigint, + preVerificationGas: bigint, + verificationGasLimit: bigint, + callGasLimit: bigint, + maxFeePerGas: bigint, + maxPriorityFeePerGas: bigint +} + +export interface GlobalConfig { safeSingleton: string, entryPoint: string, erc4337module: string, proxyFactory: string, - proxyCreationCode: string - addModulesLib: string + proxyCreationCode: string, + addModulesLib: string, + chainId: number } -interface SafeConfig { +export interface SafeConfig { signers: string[] threshold: number, nonce: number @@ -50,18 +85,224 @@ const buildInitParamsForConfig = (safeConfig: SafeConfig, globalConfig: GlobalCo } } +const callInterface = async(provider: providers.Provider, contract: string, method: string, params: any[]): Promise => { + const result = await provider.call({ + to: contract, + data: INTERFACES.encodeFunctionData(method, params) + }) + console.log(result) + return INTERFACES.decodeFunctionResult(method, result) +} + +const actionCalldata = (action: MetaTransaction): string => { + return INTERFACES.encodeFunctionData( + 'execTransactionFromModule', + [action.to, action.value, action.data, action.operation] + ) +} + +export class MultiProvider4337 extends providers.JsonRpcProvider { + generalProvider: providers.JsonRpcProvider + constructor(aaProviderUrl: string, generalProvider: providers.JsonRpcProvider) { + super(aaProviderUrl) + this.generalProvider = generalProvider; + } + + send(method: string, params: any[]): Promise { + if ([ + 'eth_chainId', + 'eth_supportedEntryPoints', + 'eth_estimateUserOperationGas', + 'eth_sendUserOperation', + 'eth_getUserOperationByHash', + 'eth_getUserOperationReceipt' + ].indexOf(method) >= 0) { + return super.send(method, params); + } else { + return this.generalProvider.send(method, params) + } + } +} + +export class Safe4337Operation { + + private safe: Safe4337 + private action: MetaTransaction + private params: OperationParams + private globalConfig: GlobalConfig + private signatures: SafeSignature[] = [] + + constructor(safe: Safe4337, action: MetaTransaction, params: OperationParams, globalConfig: GlobalConfig) { + this.safe = safe; + this.action = action; + this.params = params; + this.globalConfig = globalConfig + } + + operationHash(): string { + return utils._TypedDataEncoder.hash( + { chainId: this.globalConfig.chainId, verifyingContract: this.globalConfig.erc4337module }, + EIP712_SAFE_OPERATION_TYPE, + { + safe: this.safe.address, + callData: actionCalldata(this.action), + entryPoint: this.globalConfig.entryPoint, + ...this.params + } + ) + } + + async encodedSignatures(): Promise { + return buildSignatureBytes(this.signatures) + } + + async userOperation(paymasterAndData: string = "0x"): Promise { + const initCode = await this.safe.isDeployed() ? "0x" : this.safe.getInitCode() + return { + nonce: utils.hexlify(this.params.nonce), + callData: actionCalldata(this.action), + verificationGasLimit: utils.hexValue(this.params.verificationGasLimit), + preVerificationGas: utils.hexlify(this.params.preVerificationGas), + callGasLimit: utils.hexlify(this.params.callGasLimit), + maxFeePerGas: utils.hexlify(this.params.maxFeePerGas), + maxPriorityFeePerGas: utils.hexlify(this.params.maxPriorityFeePerGas), + initCode, + paymasterAndData, + sender: this.safe.address, + signature: await this.encodedSignatures(), + } + } + + async authorize(signer: ethers.Signer & TypedDataSigner) { + const validSigners = await this.safe.getSigners() + const signerAddress = await signer.getAddress() + if (validSigners.indexOf(signerAddress) < 0) throw Error("Invalid Signer") + if (this.signatures.findIndex((signature) => signature.signer === signerAddress) >= 0) throw Error("Already signed") + this.signatures.push({ + signer: signerAddress, + data: await signer._signTypedData( + { chainId: this.globalConfig.chainId, verifyingContract: this.globalConfig.erc4337module }, + EIP712_SAFE_OPERATION_TYPE, + { + safe: this.safe.address, + callData: actionCalldata(this.action), + entryPoint: this.globalConfig.entryPoint, + ...this.params + } + ) + }) + console.log(this.signatures) + } + + static async build(provider: providers.JsonRpcProvider, safe: Safe4337, action: MetaTransaction, globalConfig: GlobalConfig): Promise { + const initCode = await safe.isDeployed() ? "0x" : safe.getInitCode() + const nonce = (await callInterface(provider, globalConfig.entryPoint, "getNonce", [safe.address, 0]))[0] + const estimateOperation = { + sender: safe.address, + callData: actionCalldata(action), + paymasterAndData: "0x", + nonce: utils.hexlify(nonce), + initCode, + signature: "0x".padEnd(130, "a"), + // For some providers we need to set some really high values to allow estimation + preVerificationGas: utils.hexlify(1000000), + verificationGasLimit: utils.hexlify(1000000), + callGasLimit: utils.hexlify(10000000), + // To keep the required funds low, the gas fee is set close to the minimum + maxFeePerGas: "0x10", + maxPriorityFeePerGas: "0x10", + } + const estimates = await provider.send('eth_estimateUserOperationGas', [{ + ...estimateOperation + }, globalConfig.entryPoint]) + console.log(estimates) + + const feeData = await provider.getFeeData() + const params: OperationParams = { + nonce, + maxFeePerGas: feeData.maxFeePerGas!!.toBigInt(), + maxPriorityFeePerGas: feeData.maxPriorityFeePerGas!!.toBigInt(), + // Add a small margin as some dataoverhead calculation is not always accurate + preVerificationGas: BigInt(estimates.preVerificationGas) + BigInt(1000), + // Add 20% to the gas limits to account for inaccurate estimations + verificationGasLimit: BigInt(estimates.verificationGasLimit) * BigInt(12) / BigInt(10), + callGasLimit: BigInt(estimates.callGasLimit) * BigInt(12) / BigInt(10), + } + return new Safe4337Operation(safe, action, params, globalConfig) + } +} + export class Safe4337 { public address: string; - private configs: { safe: SafeConfig, global: GlobalConfig } | undefined; + private globalConfig: GlobalConfig; + private safeConfig: SafeConfig | undefined; + private provider: providers.JsonRpcProvider | undefined; - constructor(address: string, configs?: { safe: SafeConfig, global: GlobalConfig }) { - if (configs) { - const initParams = buildInitParamsForConfig(configs.safe, configs.global) + constructor(address: string, globalConfig: GlobalConfig, safeConfig?: SafeConfig) { + if (safeConfig) { + const initParams = buildInitParamsForConfig(safeConfig, globalConfig) if (address !== initParams.safeAddress) throw Error("Invalid configs"); } this.address = address; - this.configs = configs; + this.globalConfig = globalConfig; + this.safeConfig = safeConfig + } + + connect(provider: providers.JsonRpcProvider): Safe4337 { + this.provider = provider; + return this; + } + + disconnect() { + this.provider = undefined; + } + + async isDeployed(): Promise { + if (!this.provider) throw Error("Not connected!"); + + const code = await this.provider.getCode(this.address, "latest"); + return code !== '0x'; + } + + getInitCode(): string { + if (!this.safeConfig) throw Error("Init code not available"); + const initParams = buildInitParamsForConfig(this.safeConfig, this.globalConfig); + return initParams.initCode; + } + + async getSigners(): Promise { + if (!await this.isDeployed()) { + if (!this.safeConfig) throw Error("Not deployed and no config available"); + return this.safeConfig.signers; + } + const result = await callInterface(this.provider!!, this.address, "getOwners", []) + return result[0] + } + + async getModules(): Promise { + if (!await this.isDeployed()) { + if (!this.safeConfig) throw Error("Not deployed and no config available"); + return [this.globalConfig.erc4337module, this.globalConfig.entryPoint]; + } + const result = await callInterface(this.provider!!, this.address, "getModulesPaginated", [AddressOne, 10]) + return result[0] + } + + async getThreshold(): Promise { + if (!await this.isDeployed()) { + if (!this.safeConfig) throw Error("Not deployed and no config available"); + return this.safeConfig.threshold; + } + const result = await callInterface(this.provider!!, this.address, "getThreshold", []) + return result[0] + } + + async operate(action: MetaTransaction): Promise { + if (!this.provider) throw Error("Missing provider") + return Safe4337Operation.build( + this.provider, this, action, this.globalConfig + ) } static async withSigner(signer: string, globalConfig: GlobalConfig): Promise { @@ -73,18 +314,8 @@ export class Safe4337 { return Safe4337.withConfigs(safeConfig, globalConfig) } - getInitCode(): string { - if (!this.configs) throw Error("Init code not available"); - const initParams = buildInitParamsForConfig(this.configs.safe, this.configs.global) - return initParams.initCode - } - - async getSigners(): Promise { - return this.configs?.safe.signers!! - } - static async withConfigs(safeConfig: SafeConfig, globalConfig: GlobalConfig): Promise { const initParams = buildInitParamsForConfig(safeConfig, globalConfig) - return new Safe4337(initParams.safeAddress, { safe: safeConfig, global: globalConfig }) + return new Safe4337(initParams.safeAddress, globalConfig, safeConfig) } } \ No newline at end of file diff --git a/4337/src/utils/userOp.ts b/4337/src/utils/userOp.ts index 5a85e1550..a099365f6 100644 --- a/4337/src/utils/userOp.ts +++ b/4337/src/utils/userOp.ts @@ -23,25 +23,25 @@ export interface SafeUserOperation { safe: string callData: string nonce: string - verificationGas: string preVerificationGas: string + verificationGasLimit: string + callGasLimit: string maxFeePerGas: string maxPriorityFeePerGas: string - callGas: string entryPoint: string } export const EIP712_SAFE_OPERATION_TYPE = { - // "SafeOp(address safe,bytes callData,uint256 nonce,uint256 verificationGas,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,uint256 callGas,address entryPoint)" + // "SafeOp(address safe,bytes callData,uint256 nonce,uint256 preVerificationGas,uint256 verificationGasLimit,uint256 callGasLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,address entryPoint)" SafeOp: [ { type: 'address', name: 'safe' }, { type: 'bytes', name: 'callData' }, { type: 'uint256', name: 'nonce' }, - { type: 'uint256', name: 'verificationGas' }, { type: 'uint256', name: 'preVerificationGas' }, + { type: 'uint256', name: 'verificationGasLimit' }, + { type: 'uint256', name: 'callGasLimit' }, { type: 'uint256', name: 'maxFeePerGas' }, { type: 'uint256', name: 'maxPriorityFeePerGas' }, - { type: 'uint256', name: 'callGas' }, { type: 'address', name: 'entryPoint' }, ], } @@ -71,9 +71,9 @@ export const buildSafeUserOp = (template: OptionalExceptFor { safe.address, operation.callData, operation.nonce, - operation.verificationGas, operation.preVerificationGas, + operation.verificationGasLimit, + operation.callGasLimit, operation.maxFeePerGas, operation.maxPriorityFeePerGas, - operation.callGas, operation.entryPoint, ) @@ -50,6 +50,19 @@ describe('EIP4337Module', async () => { }) describe('execTransaction - existing account', () => { + + it('should revert with invalid signature', async () => { + const { safe, entryPoint } = await setupTests() + + await user1.sendTransaction({to: safe.address, value: ethers.utils.parseEther("1.0")}) + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("1.0")) + const safeOp = buildSafeUserOpTransaction(safe.address, user1.address, ethers.utils.parseEther("0.5"), "0x", '0', entryPoint.address) + const signature = buildSignatureBytes([await signHash(user1, ethers.utils.keccak256("0xbaddad42"))]) + const userOp = buildUserOperationFromSafeUserOperation({safeAddress: safe.address, safeOp, signature}) + await expect(entryPoint.executeUserOp(userOp, 0)).to.be.revertedWith("Signature validation failed") + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("1.0")) + }) + it('should execute contract calls without fee', async () => { const { safe, validator, entryPoint } = await setupTests() @@ -66,6 +79,23 @@ describe('EIP4337Module', async () => { expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("0.5")) }) + it('should not be able to execute contract calls twice', async () => { + const { safe, validator, entryPoint } = await setupTests() + + await user1.sendTransaction({to: safe.address, value: ethers.utils.parseEther("1.0")}) + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("1.0")) + const safeOp = buildSafeUserOpTransaction(safe.address, user1.address, ethers.utils.parseEther("0.5"), "0x", '0', entryPoint.address) + const safeOpHash = calculateSafeOperationHash(validator.address, safeOp, await chainId()) + const signature = buildSignatureBytes([await signHash(user1, safeOpHash)]) + const userOp = buildUserOperationFromSafeUserOperation({safeAddress: safe.address, safeOp, signature}) + await logGas( + "Execute UserOp without fee payment", + entryPoint.executeUserOp(userOp, 0) + ) + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("0.5")) + await expect(entryPoint.executeUserOp(userOp, 0)).to.be.revertedWith("InvalidNonce(0)") + }) + it('should execute contract calls with fee', async () => { const { safe, validator, entryPoint } = await setupTests() diff --git a/4337/test/eip4337/EIP4337ModuleNew.spec.ts b/4337/test/eip4337/EIP4337ModuleNew.spec.ts index 4a40f9584..1cccf6280 100644 --- a/4337/test/eip4337/EIP4337ModuleNew.spec.ts +++ b/4337/test/eip4337/EIP4337ModuleNew.spec.ts @@ -1,11 +1,9 @@ import { expect } from 'chai' import { deployments, ethers, waffle } from 'hardhat' import '@nomiclabs/hardhat-ethers' -import { getTestSafe, getSimple4337Module, getEntryPoint, getFactory, getAddModulesLib, getSafeL2Singleton } from '../utils/setup' -import { buildSignatureBytes, signHash, logGas } from '../../src/utils/execution' +import { getSimple4337Module, getEntryPoint, getFactory, getAddModulesLib, getSafeL2Singleton } from '../utils/setup' +import { buildSignatureBytes, logGas } from '../../src/utils/execution' import { - buildSafeUserOp, - calculateSafeOperationHash, buildUserOperationFromSafeUserOperation, buildSafeUserOpTransaction, signSafeOp, @@ -31,7 +29,8 @@ describe('EIP4337Module', async () => { erc4337module: module.address, proxyFactory: proxyFactory.address, addModulesLib: addModulesLib.address, - proxyCreationCode + proxyCreationCode, + chainId: await chainId() }) return { @@ -43,7 +42,25 @@ describe('EIP4337Module', async () => { } }) - describe('execTransaction - existing account', () => { + describe('execTransaction - new account', () => { + + it('should revert with invalid signature', async () => { + const { safe, entryPoint } = await setupTests() + + await user1.sendTransaction({to: safe.address, value: ethers.utils.parseEther("1.0")}) + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("1.0")) + const safeOp = buildSafeUserOpTransaction(safe.address, user1.address, ethers.utils.parseEther("0.5"), "0x", '0', entryPoint.address) + const signature = buildSignatureBytes([await signSafeOp(user1, user1.address, safeOp, await chainId())]) + const userOp = buildUserOperationFromSafeUserOperation({ + safeAddress: safe.address, + safeOp, + signature, + initCode: safe.getInitCode() + }) + await expect(entryPoint.executeUserOp(userOp, 0)).to.be.revertedWith("Signature validation failed") + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("1.0")) + }) + it('should execute contract calls without fee', async () => { const { safe, validator, entryPoint } = await setupTests() @@ -64,6 +81,27 @@ describe('EIP4337Module', async () => { expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("0.5")) }) + it('should not be able to execute contract calls twice', async () => { + const { safe, validator, entryPoint } = await setupTests() + + await user1.sendTransaction({to: safe.address, value: ethers.utils.parseEther("1.0")}) + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("1.0")) + const safeOp = buildSafeUserOpTransaction(safe.address, user1.address, ethers.utils.parseEther("0.5"), "0x", '0', entryPoint.address) + const signature = buildSignatureBytes([await signSafeOp(user1, validator.address, safeOp, await chainId())]) + const userOp = buildUserOperationFromSafeUserOperation({ + safeAddress: safe.address, + safeOp, + signature, + initCode: safe.getInitCode() + }) + await logGas( + "Execute UserOp without fee payment", + entryPoint.executeUserOp(userOp, 0) + ) + expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.utils.parseEther("0.5")) + await expect(entryPoint.executeUserOp(userOp, 0)).to.be.revertedWith("InvalidNonce(0)") + }) + it('should execute contract calls with fee', async () => { const { safe, validator, entryPoint } = await setupTests() diff --git a/4337/test/eip4337/EIP4337Safe.spec.ts b/4337/test/eip4337/EIP4337Safe.spec.ts index 20f7d1727..27a6240dd 100644 --- a/4337/test/eip4337/EIP4337Safe.spec.ts +++ b/4337/test/eip4337/EIP4337Safe.spec.ts @@ -38,11 +38,11 @@ describe('EIP4337Safe', async () => { safe.address, operation.callData, operation.nonce, - operation.verificationGas, operation.preVerificationGas, + operation.verificationGasLimit, + operation.callGasLimit, operation.maxFeePerGas, operation.maxPriorityFeePerGas, - operation.callGas, operation.entryPoint, )