From 6905c5707c8a81f4df4f92379bdd7e19fc3f6a87 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 2 Mar 2025 18:06:30 +0200 Subject: [PATCH 1/2] refactor _callinnerHandleOp --- contracts/accounts/Simple7702Account.sol | 2 +- contracts/core/EntryPoint.sol | 89 +++++++++++++----------- reports/gas-checker.txt | 32 ++++----- 3 files changed, 65 insertions(+), 58 deletions(-) diff --git a/contracts/accounts/Simple7702Account.sol b/contracts/accounts/Simple7702Account.sol index a4551d46a..d3348344e 100644 --- a/contracts/accounts/Simple7702Account.sol +++ b/contracts/accounts/Simple7702Account.sol @@ -17,7 +17,7 @@ contract Simple7702Account is BaseAccount, IERC165, IERC1271, ERC1155Holder, ERC // temporary address of entryPoint v0.8 function entryPoint() public pure override returns (IEntryPoint) { - return IEntryPoint(0x8195d7440D459fb0D36c95FC9D7B09CeBf3D54b2); + return IEntryPoint(0x6832Ee376d251B7A37dbdEb158E98F7b0be7b31a); } /** diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 483d585ed..e7b733bbb 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -94,46 +94,14 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT returns (uint256 actualGasCost) { uint256 preGas = gasleft(); bytes memory context = getMemoryBytesFromOffset(opInfo.contextOffset); - bool innerSuccess; - bytes32 returnData; - uint256 actualGas; - { - uint256 saveFreePtr = getFreePtr(); - 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)); - } - assembly ("memory-safe") { - innerSuccess := call(gas(), address(), 0, add(innerCall, 0x20), mload(innerCall), 0, 64) - returnData := mload(0) - // returned by either INNER_REVERT_LOW_PREFUND or successful return. - actualGas := mload(32) - } - restoreFreePtr(saveFreePtr); - } - bool executionSuccess; - if (innerSuccess) { - executionSuccess = returnData != 0; - } else { - bytes32 innerRevertCode = returnData; - if (innerRevertCode == INNER_OUT_OF_GAS) { + (uint256 returnData, uint256 actualGas) = _callInnerHandleOp(userOp, opInfo, context); + if (returnData > 1) { + if (bytes32(returnData) == INNER_OUT_OF_GAS) { // handleOps was called with gas limit too low. abort entire bundle. // can only be caused by bundler (leaving not enough gas for inner call) revert FailedOp(opIndex, "AA95 out of gas"); } - if (innerRevertCode != INNER_REVERT_LOW_PREFUND) { + if (bytes32(returnData) != INNER_REVERT_LOW_PREFUND) { actualGas = preGas - gasleft(); actualGas += _getUnusedGasPenalty(actualGas, opInfo.mUserOp.callGasLimit + opInfo.mUserOp.paymasterPostOpGasLimit); emit PostOpRevertReason( @@ -147,6 +115,8 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT // and postInnerCall below will call _emitPrefundTooLow } + bool executionSuccess = returnData == 0; + bool innerSuccess = returnData <= 1; return _postInnerCall( opInfo, executionSuccess, @@ -155,6 +125,45 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT ); } + /** + * call innerHandleOp + * call innerHandleOp either with userOp.callData or (if it starts with executeUserOp selector) build an executeUserOp call + * @return returnData - 0 - execution success + * 1 - execution call reverted + * anything else - revert forwarded from innerHandleOp + */ + function _callInnerHandleOp( + PackedUserOperation calldata userOp, + UserOpInfo memory opInfo, + bytes memory context + ) internal virtual returns (uint256 returnData, uint256 actualGas) { + bool innerSuccess; + uint256 saveFreePtr = getFreePtr(); + 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)); + } + assembly ("memory-safe") { + innerSuccess := call(gas(), address(), 0, add(innerCall, 0x20), mload(innerCall), 0, 64) + returnData := mload(0) + // returned by either INNER_REVERT_LOW_PREFUND or successful return. + actualGas := mload(32) + } + restoreFreePtr(saveFreePtr); + } + /** * Process the output of innerCall * - calculate paid gas. @@ -164,7 +173,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT * @param opInfo - UserOp fields and info collected during validation. * @param executionSuccess - Whether account execution was successful. * @param actualGas - actual gas used for execution and postOp - * @param innerSuccess - Whether inner call succeeded or reverted + * @param innerSuccess - Whether inner call succeeded or reverted (it can only revert on postOp revert or low prefund) */ function _postInnerCall( UserOpInfo memory opInfo, @@ -368,14 +377,12 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT * @param callData - The callData to execute. * @param opInfo - The UserOpInfo struct. * @param context - The context bytes. - * @return callSuccess - return status of sender callData - * @return actualGasUsed - gas used by this call, including unused gas penalty */ function innerHandleOp( bytes memory callData, UserOpInfo memory opInfo, bytes calldata context - ) external returns (bool callSuccess, uint256 actualGasUsed) { + ) external returns (bool executionReverted, uint256 actualGasUsed) { uint256 preGas = gasleft(); require(msg.sender == address(this), "AA92 internal call only"); MemoryUserOp memory mUserOp = opInfo.mUserOp; @@ -418,7 +425,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT unchecked { uint256 executionGas = preGas - gasleft(); uint256 actualGas = _postExecution(mode, opInfo, context, executionGas); - return (mode == IPaymaster.PostOpMode.opSucceeded, actualGas); + return (mode != IPaymaster.PostOpMode.opSucceeded, actualGas); } } diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 28e8cd08c..6049f6543 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -12,36 +12,36 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 77235 │ │ ║ +║ simple │ 1 │ 77228 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41292 │ 12293 ║ +║ simple - diff from previous │ 2 │ │ 41285 │ 12286 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 448970 │ │ ║ +║ simple │ 10 │ 448912 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41400 │ 12401 ║ +║ simple - diff from previous │ 11 │ │ 41333 │ 12334 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83095 │ │ ║ +║ simple paymaster │ 1 │ 83124 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 39889 │ 10890 ║ +║ simple paymaster with diff │ 2 │ │ 39918 │ 10919 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 442210 │ │ ║ +║ simple paymaster │ 10 │ 442392 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 39897 │ 10898 ║ +║ simple paymaster with diff │ 11 │ │ 39926 │ 10927 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167018 │ │ ║ +║ big tx 5k │ 1 │ 167011 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130513 │ 16073 ║ +║ big tx - diff from previous │ 2 │ │ 130506 │ 16066 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1341879 │ │ ║ +║ big tx 5k │ 10 │ 1341821 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130560 │ 16120 ║ +║ big tx - diff from previous │ 11 │ │ 130517 │ 16077 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84549 │ │ ║ +║ paymaster+postOp │ 1 │ 84542 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41319 │ 12320 ║ +║ paymaster+postOp with diff │ 2 │ │ 41288 │ 12289 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 456540 │ │ ║ +║ paymaster+postOp │ 10 │ 456446 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41283 │ 12284 ║ +║ paymaster+postOp with diff │ 11 │ │ 41360 │ 12361 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 1b63bac85f06dd84053df661248834e112d700f1 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 2 Mar 2025 18:20:08 +0200 Subject: [PATCH 2/2] reordered, fir simpler diff. --- contracts/accounts/Simple7702Account.sol | 2 +- contracts/core/EntryPoint.sol | 80 ++++++++++++------------ reports/gas-checker.txt | 32 +++++----- 3 files changed, 56 insertions(+), 58 deletions(-) diff --git a/contracts/accounts/Simple7702Account.sol b/contracts/accounts/Simple7702Account.sol index d3348344e..8355d9b7a 100644 --- a/contracts/accounts/Simple7702Account.sol +++ b/contracts/accounts/Simple7702Account.sol @@ -17,7 +17,7 @@ contract Simple7702Account is BaseAccount, IERC165, IERC1271, ERC1155Holder, ERC // temporary address of entryPoint v0.8 function entryPoint() public pure override returns (IEntryPoint) { - return IEntryPoint(0x6832Ee376d251B7A37dbdEb158E98F7b0be7b31a); + return IEntryPoint(0x3C965AC44c8C3946de6aBdc65b1Df5FeDCaeb06B); } /** diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index e7b733bbb..f4c163189 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -78,6 +78,44 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT require(success, "AA91 failed send to beneficiary"); } + /** + * call innerHandleOp + * call innerHandleOp either with userOp.callData or (if it starts with executeUserOp selector) build an executeUserOp call + * @return returnData - 0 - execution success + * 1 - execution call reverted + * anything else - revert forwarded from innerHandleOp + */ + function _callInnerHandleOp( + PackedUserOperation calldata userOp, + UserOpInfo memory opInfo + ) internal virtual returns (uint256 returnData, uint256 actualGas) { + uint256 saveFreePtr = getFreePtr(); + bytes memory context = getMemoryBytesFromOffset(opInfo.contextOffset); + 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)); + } + assembly ("memory-safe") { + pop(call(gas(), address(), 0, add(innerCall, 0x20), mload(innerCall), 0, 64)) + returnData := mload(0) + // returned by either INNER_REVERT_LOW_PREFUND or successful return. + actualGas := mload(32) + } + restoreFreePtr(saveFreePtr); + } + /** * Execute a user operation. * @param opIndex - Index into the opInfo array. @@ -93,8 +131,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT internal virtual returns (uint256 actualGasCost) { uint256 preGas = gasleft(); - bytes memory context = getMemoryBytesFromOffset(opInfo.contextOffset); - (uint256 returnData, uint256 actualGas) = _callInnerHandleOp(userOp, opInfo, context); + (uint256 returnData, uint256 actualGas) = _callInnerHandleOp(userOp, opInfo); if (returnData > 1) { if (bytes32(returnData) == INNER_OUT_OF_GAS) { // handleOps was called with gas limit too low. abort entire bundle. @@ -125,45 +162,6 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT ); } - /** - * call innerHandleOp - * call innerHandleOp either with userOp.callData or (if it starts with executeUserOp selector) build an executeUserOp call - * @return returnData - 0 - execution success - * 1 - execution call reverted - * anything else - revert forwarded from innerHandleOp - */ - function _callInnerHandleOp( - PackedUserOperation calldata userOp, - UserOpInfo memory opInfo, - bytes memory context - ) internal virtual returns (uint256 returnData, uint256 actualGas) { - bool innerSuccess; - uint256 saveFreePtr = getFreePtr(); - 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)); - } - assembly ("memory-safe") { - innerSuccess := call(gas(), address(), 0, add(innerCall, 0x20), mload(innerCall), 0, 64) - returnData := mload(0) - // returned by either INNER_REVERT_LOW_PREFUND or successful return. - actualGas := mload(32) - } - restoreFreePtr(saveFreePtr); - } - /** * Process the output of innerCall * - calculate paid gas. diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 6049f6543..73af3ea86 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -12,36 +12,36 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 77228 │ │ ║ +║ simple │ 1 │ 77219 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41285 │ 12286 ║ +║ simple - diff from previous │ 2 │ │ 41288 │ 12289 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 448912 │ │ ║ +║ simple │ 10 │ 448930 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41333 │ 12334 ║ +║ simple - diff from previous │ 11 │ │ 41420 │ 12421 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83124 │ │ ║ +║ simple paymaster │ 1 │ 83139 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 39918 │ 10919 ║ +║ simple paymaster with diff │ 2 │ │ 39897 │ 10898 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 442392 │ │ ║ +║ simple paymaster │ 10 │ 442422 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 39926 │ 10927 ║ +║ simple paymaster with diff │ 11 │ │ 39953 │ 10954 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167011 │ │ ║ +║ big tx 5k │ 1 │ 167014 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130506 │ 16066 ║ +║ big tx - diff from previous │ 2 │ │ 130521 │ 16081 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1341821 │ │ ║ +║ big tx 5k │ 10 │ 1341767 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130517 │ 16077 ║ +║ big tx - diff from previous │ 11 │ │ 130544 │ 16104 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84542 │ │ ║ +║ paymaster+postOp │ 1 │ 84533 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41288 │ 12289 ║ +║ paymaster+postOp with diff │ 2 │ │ 41327 │ 12328 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 456446 │ │ ║ +║ paymaster+postOp │ 10 │ 456488 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41360 │ 12361 ║ +║ paymaster+postOp with diff │ 11 │ │ 41339 │ 12340 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝