From ff5ffd754afc0692d506b5494269e87410d2402c Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Tue, 28 Nov 2023 14:35:54 +0200 Subject: [PATCH] implement call to executeUserOp --- contracts/core/EntryPoint.sol | 18 +++++- contracts/interfaces/IAccountExecute.sol | 6 +- contracts/test/TestExecAccount.sol | 75 ++++++++++++++++++++++++ reports/gas-checker.txt | 34 +++++------ test/testExecAccount.test.ts | 56 ++++++++++++++++++ 5 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 contracts/test/TestExecAccount.sol create mode 100644 test/testExecAccount.test.ts diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index ae0b7cff..2f7e9740 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -5,6 +5,7 @@ pragma solidity ^0.8.12; /* solhint-disable no-inline-assembly */ import "../interfaces/IAccount.sol"; +import "../interfaces/IAccountExecute.sol"; import "../interfaces/IPaymaster.sol"; import "../interfaces/IEntryPoint.sol"; @@ -83,7 +84,22 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, assembly { saveFreePtr := mload(0x40) } - bytes memory innerCall = abi.encodeCall(this.innerHandleOp,(userOp.callData, opInfo, context)); + bytes calldata callData = userOp.callData; + bytes memory innerCall; + bytes4 methodSig; + assembly { + let len := callData.length + if gt(len,3) { + methodSig := calldataload(callData.offset) + } + } + if (methodSig == IAccountExecute.executeUserOp.selector) { + bytes memory executeUserOp = abi.encodeCall(IAccountExecute.executeUserOp, (userOp, opInfo.userOpHash)); + innerCall = abi.encodeCall(this.innerHandleOp, (executeUserOp, opInfo, context)); + } else + { + innerCall = abi.encodeCall(this.innerHandleOp, (callData, opInfo, context)); + } bool success; assembly { success := call(gas(), address(), 0, add(innerCall, 0x20), mload(innerCall), 0, 32) diff --git a/contracts/interfaces/IAccountExecute.sol b/contracts/interfaces/IAccountExecute.sol index 2e0fb43c..440bd4b0 100644 --- a/contracts/interfaces/IAccountExecute.sol +++ b/contracts/interfaces/IAccountExecute.sol @@ -5,9 +5,11 @@ import "./UserOperation.sol"; interface IAccountExecute { /** - * Account MAY implement this execute method. - * passing this methodSig as callData will cause the entryPoint to pass the full UserOp (and hash) + * Account may implement this execute method. + * passing this methodSig at the beginning of callData will cause the entryPoint to pass the full UserOp (and hash) * to the account. + * The account should skip the methodSig, and use the callData (and optionally, other UserOp fields) + * * @param userOp - The operation that was just validated. * @param userOpHash - Hash of the user's request data. */ diff --git a/contracts/test/TestExecAccount.sol b/contracts/test/TestExecAccount.sol new file mode 100644 index 00000000..9a624f03 --- /dev/null +++ b/contracts/test/TestExecAccount.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-3.0 + +/* solhint-disable one-contract-per-file */ +/* solhint-disable avoid-low-level-calls */ +pragma solidity ^0.8.15; + +import "@openzeppelin/contracts/utils/Create2.sol"; +import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +import "../samples/SimpleAccount.sol"; +import "../interfaces/IAccountExecute.sol"; + +/** + * a sample account with execUserOp. + * Note that this account does nothing special with the userop, just extract + * call to execute. In theory, such account can reference the signature, the hash, etc. + */ +contract TestExecAccount is SimpleAccount, IAccountExecute { + + constructor(IEntryPoint anEntryPoint) SimpleAccount(anEntryPoint){ + } + + event Executed(UserOperation userOp, bytes innerCallRet); + + function executeUserOp(UserOperation calldata userOp, bytes32 /*userOpHash*/) external { + _requireFromEntryPointOrOwner(); + + // read from the userOp.callData, but skip the "magic" prefix (executeUserOp sig), + // which caused it to call this method. + bytes calldata innerCall = userOp.callData[4 :]; + + bytes memory innerCallRet; + if (innerCall.length > 0) { + (address target, bytes memory data) = abi.decode(innerCall, (address, bytes)); + bool success; + (success, innerCallRet) = target.call(data); + require(success, "inner call failed"); + } + + emit Executed(userOp, innerCallRet); + } +} + +contract TestExecAccountFactory { + TestExecAccount public immutable accountImplementation; + + constructor(IEntryPoint _entryPoint) { + accountImplementation = new TestExecAccount(_entryPoint); + } + + function createAccount(address owner, uint256 salt) public returns (address ret) { + address addr = getAddress(owner, salt); + uint codeSize = addr.code.length; + if (codeSize > 0) { + return addr; + } + ret = address(new ERC1967Proxy{salt: bytes32(salt)}( + address(accountImplementation), + abi.encodeCall(SimpleAccount.initialize, (owner)) + )); + } + + /** + * calculate the counterfactual address of this account as it would be returned by createAccount() + */ + function getAddress(address owner, uint256 salt) public view returns (address) { + return Create2.computeAddress(bytes32(salt), keccak256(abi.encodePacked( + type(ERC1967Proxy).creationCode, + abi.encode( + address(accountImplementation), + abi.encodeCall(SimpleAccount.initialize, (owner)) + ) + ))); + } +} diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index f5eb0b8d..db06aa07 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -4,7 +4,7 @@ ╔══════════════════════════╤════════╗ ║ gas estimate "simple" │ 29014 ║ ╟──────────────────────────┼────────╢ -║ gas estimate "big tx 5k" │ 125248 ║ +║ gas estimate "big tx 5k" │ 125260 ║ ╚══════════════════════════╧════════╝ ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗ @@ -12,36 +12,36 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 81808 │ │ ║ +║ simple │ 1 │ 81961 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 44041 │ 15027 ║ +║ simple - diff from previous │ 2 │ │ 44206 │ 15192 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 478604 │ │ ║ +║ simple │ 10 │ 480110 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 44096 │ 15082 ║ +║ simple - diff from previous │ 11 │ │ 44249 │ 15235 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 89670 │ │ ║ +║ simple paymaster │ 1 │ 89871 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 44671 │ 15657 ║ +║ simple paymaster with diff │ 2 │ │ 44812 │ 15798 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 491926 │ │ ║ +║ simple paymaster │ 10 │ 493672 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 44752 │ 15738 ║ +║ simple paymaster with diff │ 11 │ │ 44833 │ 15819 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 182841 │ │ ║ +║ big tx 5k │ 1 │ 183018 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 144576 │ 19328 ║ +║ big tx - diff from previous │ 2 │ │ 144729 │ 19469 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1484244 │ │ ║ +║ big tx 5k │ 10 │ 1485774 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 144620 │ 19372 ║ +║ big tx - diff from previous │ 11 │ │ 144809 │ 19549 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster │ 1 │ 148138 │ │ ║ +║ token paymaster │ 1 │ 148291 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 2 │ │ 72794 │ 43780 ║ +║ token paymaster with diff │ 2 │ │ 72947 │ 43933 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster │ 10 │ 803643 │ │ ║ +║ token paymaster │ 10 │ 805221 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 11 │ │ 72922 │ 43908 ║ +║ token paymaster with diff │ 11 │ │ 73015 │ 44001 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ diff --git a/test/testExecAccount.test.ts b/test/testExecAccount.test.ts new file mode 100644 index 00000000..4a1e5035 --- /dev/null +++ b/test/testExecAccount.test.ts @@ -0,0 +1,56 @@ +import { before } from 'mocha' +import { + EntryPoint, + TestExecAccount, + TestExecAccount__factory, + TestExecAccountFactory__factory +} from '../typechain' +import { createAccountOwner, deployEntryPoint, fund, objdump } from './testutils' +import { fillAndSign } from './UserOp' +import { Signer, Wallet } from 'ethers' +import { ethers } from 'hardhat' +import { defaultAbiCoder, hexConcat, hexStripZeros } from 'ethers/lib/utils' +import { expect } from 'chai' + +describe('IAccountExecute', () => { + let ethersSigner: Signer + let entryPoint: EntryPoint + let account: TestExecAccount + let owner: Wallet + before(async () => { + const provider = ethers.provider + ethersSigner = provider.getSigner() + entryPoint = await deployEntryPoint() + const factory = await new TestExecAccountFactory__factory(ethersSigner).deploy(entryPoint.address) + owner = createAccountOwner() + await factory.createAccount(owner.getAddress(), 0) + const accountAddress = await factory.callStatic.createAccount(owner.getAddress(), 0) + account = TestExecAccount__factory.connect(accountAddress, provider) + await fund(accountAddress) + }) + + it('should execute ', async () => { + const execSig = account.interface.getSighash('executeUserOp') + // innerCall, as TestExecAccount.executeUserOp will try to decode it: + const innerCall = defaultAbiCoder.encode(['address', 'bytes'], [ + account.address, + account.interface.encodeFunctionData('entryPoint') + ]) + + const userOp = await fillAndSign({ + sender: account.address, + callGasLimit: 100000, // normal estimate also chokes on this callData + callData: hexConcat([execSig, innerCall]) + }, owner, entryPoint) + + await entryPoint.handleOps([userOp], ethersSigner.getAddress()) + + const e = + await account.queryFilter(account.filters.Executed()) + + expect(e.length).to.eq(1, "didn't call inner execUserOp (no Executed event)") + console.log(e[0].event, objdump(e[0].args)) + // validate we retrieved the return value of the called "entryPoint()" function: + expect(hexStripZeros(e[0].args.innerCallRet)).to.eq(hexStripZeros(entryPoint.address)) + }) +})