From f4724e70e76e2d7e7f19d55cc51877384663a6ff Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 2 Feb 2025 15:07:47 +0200 Subject: [PATCH 01/10] use callValidateUserOp instead of sender.validateUserOp wrap validateUserOp with assembly call. This way, we don't rely on EntryPointSimulation to create FailedOp on sender failures with malformed output (e.g. account not deployed) --- contracts/core/EntryPoint.sol | 34 +++++++++++++++++++++++++--------- reports/gas-checker.txt | 32 ++++++++++++++++---------------- test/entrypoint.test.ts | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 75 insertions(+), 26 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 6ded1a88..7b727c01 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -501,15 +501,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT ? 0 : requiredPrefund - bal; } - try - IAccount(sender).validateUserOp{ - gas: verificationGasLimit - }(op, opInfo.userOpHash, missingAccountFunds) - returns (uint256 _validationData) { - validationData = _validationData; - } catch { - revert FailedOpWithRevert(opIndex, "AA23 reverted", Exec.getReturnData(REVERT_REASON_MAX_LEN)); - } + validationData = _callValidateUserOp(verificationGasLimit, sender, op, opInfo.userOpHash, missingAccountFunds); if (paymaster == address(0)) { DepositInfo storage senderInfo = deposits[sender]; uint256 deposit = senderInfo.deposit; @@ -521,6 +513,30 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT } } + // call sender.validateUserOp + // handle + function _callValidateUserOp(uint256 gasLimit, address sender, PackedUserOperation calldata op, bytes32 userOpHash, uint256 missingAccountFunds) + internal returns (uint256 validationData) { + uint256 saveFreePtr; + assembly ("memory-safe") { + saveFreePtr := mload(0x40) + } + //return sender.validateUserOp{gas: gas}(op, userOpHash, missingAccountFunds); + bytes memory callData = abi.encodeCall(IAccount.validateUserOp, (op, userOpHash, missingAccountFunds)); + bool success; + uint256 dataSize; + assembly ("memory-safe"){ + success := call(gasLimit, sender, 0, add(callData, 0x20), mload(callData), 0, 32) + dataSize := mul(returndatasize(), success) + validationData := mload(0) + mstore(0x40, saveFreePtr) + } + if (dataSize != 32) { + require(sender.code.length > 0, "AA20 account not deployed"); + revert FailedOpWithRevert(0, "AA23 reverted", Exec.getReturnData(REVERT_REASON_MAX_LEN)); + } + } + /** * In case the request has a paymaster: * - Validate paymaster has enough deposit. diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 3cc13daf..06e955ba 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 │ 77450 │ │ ║ +║ simple │ 1 │ 77366 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41574 │ 12315 ║ +║ simple - diff from previous │ 2 │ │ 41513 │ 12254 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451932 │ │ ║ +║ simple │ 10 │ 451092 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41659 │ 12400 ║ +║ simple - diff from previous │ 11 │ │ 41597 │ 12338 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83281 │ │ ║ +║ simple paymaster │ 1 │ 83207 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40106 │ 10847 ║ +║ simple paymaster with diff │ 2 │ │ 40055 │ 10796 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 444654 │ │ ║ +║ simple paymaster │ 10 │ 443949 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 40199 │ 10940 ║ +║ simple paymaster with diff │ 11 │ │ 40086 │ 10827 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167209 │ │ ║ +║ big tx 5k │ 1 │ 167124 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130811 │ 16109 ║ +║ big tx - diff from previous │ 2 │ │ 130738 │ 16036 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1344642 │ │ ║ +║ big tx 5k │ 10 │ 1343796 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130788 │ 16086 ║ +║ big tx - diff from previous │ 11 │ │ 130761 │ 16059 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84458 │ │ ║ +║ paymaster+postOp │ 1 │ 84384 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41318 │ 12059 ║ +║ paymaster+postOp with diff │ 2 │ │ 41232 │ 11973 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 456453 │ │ ║ +║ paymaster+postOp │ 10 │ 455616 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41369 │ 12110 ║ +║ paymaster+postOp with diff │ 11 │ │ 41292 │ 12033 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts index f5eba41d..876b4421 100644 --- a/test/entrypoint.test.ts +++ b/test/entrypoint.test.ts @@ -28,7 +28,8 @@ import { TestRevertAccount__factory, TestSignatureAggregator, TestSignatureAggregator__factory, - TestWarmColdAccount__factory + TestWarmColdAccount__factory, + SimpleAccount__factory } from '../typechain' import { AddressZero, @@ -54,6 +55,7 @@ import { DefaultsForUserOp, fillAndSign, fillSignAndPack, + fillUserOp, getUserOpHash, packUserOp, simulateValidation @@ -649,6 +651,37 @@ describe('EntryPoint', function () { expect(await getBalance(account.address)).to.eq(inititalAccountBalance) }) + it('should fail with AA20 if account not deployed', async () => { + const userop = await fillUserOp({ + sender: createAddress(), + nonce: 0 + }, entryPoint) + const beneficiary = createAddress() + await expect(entryPoint.handleOps([packUserOp(userop)], beneficiary)).to.revertedWith('AA20 account not deployed') + }) + + it('should fail with AA23 if account reverts', async () => { + const userop = await fillUserOp({ + sender: entryPoint.address, // existing but not a real account + nonce: 0 + }, entryPoint) + const beneficiary = createAddress() + await expect(entryPoint.handleOps([packUserOp(userop)], beneficiary).catch(rethrow())).to.be + .revertedWith('FailedOpWithRevert(0,"AA23 reverted",)') + }) + + it('should fail with AA23 (and original error) if account reverts', async () => { + // deploy an account with broken entrypoint, so it always reverts with "not from EntryPoint" + const revertingAccount = await new SimpleAccount__factory(ethersSigner).deploy(createAddress()) + const userop = await fillUserOp({ + sender: revertingAccount.address, + nonce: 0 + }, entryPoint) + const beneficiary = createAddress() + await expect(entryPoint.handleOps([packUserOp(userop)], beneficiary).catch(rethrow())).to.be + .revertedWith('FailedOpWithRevert(0,"AA23 reverted",Error(account: not from EntryPoint)') + }) + it('account should pay a penalty for requiring too much gas and leaving it unused', async function () { if (process.env.COVERAGE != null) { return From 6048b9545262d320ef6e836054d6d699674c5c3b Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 2 Feb 2025 18:21:29 +0200 Subject: [PATCH 02/10] optmized but actually costs more optimized callValidateUserOp (less parameters), but actually costs more (16 per useop, but still...) --- contracts/core/EntryPoint.sol | 17 ++++++++--------- reports/gas-checker.txt | 32 ++++++++++++++++---------------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 7b727c01..384250ea 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -481,8 +481,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT uint256 opIndex, PackedUserOperation calldata op, UserOpInfo memory opInfo, - uint256 requiredPrefund, - uint256 verificationGasLimit + uint256 requiredPrefund ) internal returns ( @@ -501,7 +500,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT ? 0 : requiredPrefund - bal; } - validationData = _callValidateUserOp(verificationGasLimit, sender, op, opInfo.userOpHash, missingAccountFunds); + validationData = _callValidateUserOp(op, opInfo, missingAccountFunds); if (paymaster == address(0)) { DepositInfo storage senderInfo = deposits[sender]; uint256 deposit = senderInfo.deposit; @@ -515,18 +514,19 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT // call sender.validateUserOp // handle - function _callValidateUserOp(uint256 gasLimit, address sender, PackedUserOperation calldata op, bytes32 userOpHash, uint256 missingAccountFunds) + function _callValidateUserOp(PackedUserOperation calldata op, UserOpInfo memory opInfo, uint256 missingAccountFunds) internal returns (uint256 validationData) { uint256 saveFreePtr; assembly ("memory-safe") { saveFreePtr := mload(0x40) } //return sender.validateUserOp{gas: gas}(op, userOpHash, missingAccountFunds); - bytes memory callData = abi.encodeCall(IAccount.validateUserOp, (op, userOpHash, missingAccountFunds)); - bool success; + bytes memory callData = abi.encodeCall(IAccount.validateUserOp, (op, opInfo.userOpHash, missingAccountFunds)); + uint256 gasLimit = opInfo.mUserOp.verificationGasLimit; + address sender = opInfo.mUserOp.sender; uint256 dataSize; assembly ("memory-safe"){ - success := call(gasLimit, sender, 0, add(callData, 0x20), mload(callData), 0, 32) + let success := call(gasLimit, sender, 0, add(callData, 0x20), mload(callData), 0, 32) dataSize := mul(returndatasize(), success) validationData := mload(0) mstore(0x40, saveFreePtr) @@ -674,8 +674,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT opIndex, userOp, outOpInfo, - requiredPreFund, - verificationGasLimit + requiredPreFund ); if (!_validateAndUpdateNonce(mUserOp.sender, mUserOp.nonce)) { diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 06e955ba..54a19ee6 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 │ 77366 │ │ ║ +║ simple │ 1 │ 77382 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41513 │ 12254 ║ +║ simple - diff from previous │ 2 │ │ 41529 │ 12270 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451092 │ │ ║ +║ simple │ 10 │ 451252 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41597 │ 12338 ║ +║ simple - diff from previous │ 11 │ │ 41565 │ 12306 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83207 │ │ ║ +║ simple paymaster │ 1 │ 83230 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40055 │ 10796 ║ +║ simple paymaster with diff │ 2 │ │ 40054 │ 10795 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 443949 │ │ ║ +║ simple paymaster │ 10 │ 444083 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 40086 │ 10827 ║ +║ simple paymaster with diff │ 11 │ │ 40109 │ 10850 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167124 │ │ ║ +║ big tx 5k │ 1 │ 167128 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130738 │ 16036 ║ +║ big tx - diff from previous │ 2 │ │ 130766 │ 16064 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1343796 │ │ ║ +║ big tx 5k │ 10 │ 1343944 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130761 │ 16059 ║ +║ big tx - diff from previous │ 11 │ │ 130741 │ 16039 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84384 │ │ ║ +║ paymaster+postOp │ 1 │ 84407 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41232 │ 11973 ║ +║ paymaster+postOp with diff │ 2 │ │ 41267 │ 12008 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 455616 │ │ ║ +║ paymaster+postOp │ 10 │ 455966 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41292 │ 12033 ║ +║ paymaster+postOp with diff │ 11 │ │ 41267 │ 12008 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From e5f9787125f47057104873eef3046b168fd28dfa Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 2 Feb 2025 18:30:19 +0200 Subject: [PATCH 03/10] added opIndex to FailedOp --- contracts/core/EntryPoint.sol | 16 +++++++++------- reports/gas-checker.txt | 32 ++++++++++++++++---------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 384250ea..734cad64 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -500,7 +500,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT ? 0 : requiredPrefund - bal; } - validationData = _callValidateUserOp(op, opInfo, missingAccountFunds); + validationData = _callValidateUserOp(op, opInfo, missingAccountFunds, opIndex); if (paymaster == address(0)) { DepositInfo storage senderInfo = deposits[sender]; uint256 deposit = senderInfo.deposit; @@ -512,15 +512,14 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT } } - // call sender.validateUserOp - // handle - function _callValidateUserOp(PackedUserOperation calldata op, UserOpInfo memory opInfo, uint256 missingAccountFunds) + // call sender.validateUserOp() + // handle wrong output size with FailedOp + function _callValidateUserOp(PackedUserOperation calldata op, UserOpInfo memory opInfo, uint256 missingAccountFunds, uint256 opIndex) internal returns (uint256 validationData) { uint256 saveFreePtr; assembly ("memory-safe") { saveFreePtr := mload(0x40) } - //return sender.validateUserOp{gas: gas}(op, userOpHash, missingAccountFunds); bytes memory callData = abi.encodeCall(IAccount.validateUserOp, (op, opInfo.userOpHash, missingAccountFunds)); uint256 gasLimit = opInfo.mUserOp.verificationGasLimit; address sender = opInfo.mUserOp.sender; @@ -532,8 +531,11 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT mstore(0x40, saveFreePtr) } if (dataSize != 32) { - require(sender.code.length > 0, "AA20 account not deployed"); - revert FailedOpWithRevert(0, "AA23 reverted", Exec.getReturnData(REVERT_REASON_MAX_LEN)); + if(sender.code.length == 0) { + revert FailedOp(opIndex, "AA20 account not deployed"); + } else { + revert FailedOpWithRevert(opIndex, "AA23 reverted", Exec.getReturnData(REVERT_REASON_MAX_LEN)); + } } } diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 54a19ee6..f9942f93 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 │ 77382 │ │ ║ +║ simple │ 1 │ 77383 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41529 │ 12270 ║ +║ simple - diff from previous │ 2 │ │ 41530 │ 12271 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451252 │ │ ║ +║ simple │ 10 │ 451298 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41565 │ 12306 ║ +║ simple - diff from previous │ 11 │ │ 41590 │ 12331 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83230 │ │ ║ +║ simple paymaster │ 1 │ 83200 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40054 │ 10795 ║ +║ simple paymaster with diff │ 2 │ │ 40072 │ 10813 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 444083 │ │ ║ +║ simple paymaster │ 10 │ 443927 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 40109 │ 10850 ║ +║ simple paymaster with diff │ 11 │ │ 40067 │ 10808 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167128 │ │ ║ +║ big tx 5k │ 1 │ 167141 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130766 │ 16064 ║ +║ big tx - diff from previous │ 2 │ │ 130743 │ 16041 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1343944 │ │ ║ +║ big tx 5k │ 10 │ 1344014 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130741 │ 16039 ║ +║ big tx - diff from previous │ 11 │ │ 130754 │ 16052 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84407 │ │ ║ +║ paymaster+postOp │ 1 │ 84401 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41267 │ 12008 ║ +║ paymaster+postOp with diff │ 2 │ │ 41261 │ 12002 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 455966 │ │ ║ +║ paymaster+postOp │ 10 │ 455870 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41267 │ 12008 ║ +║ paymaster+postOp with diff │ 11 │ │ 41309 │ 12050 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From b61bccf6b7ec4f60c79aa8194a6f715314798383 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 2 Feb 2025 19:28:39 +0200 Subject: [PATCH 04/10] more FailedOp instead of Error --- contracts/core/EntryPoint.sol | 4 ++-- reports/gas-checker.txt | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 734cad64..bfa6ff8d 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -227,7 +227,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT //address(1) is special marker of "signature error" require( address(aggregator) != address(1), - "AA96 invalid aggregator" + FailedOp(totalOps, "AA96 invalid aggregator") ); if (address(aggregator) != address(0)) { @@ -669,7 +669,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT mUserOp.paymasterPostOpGasLimit | mUserOp.maxFeePerGas | mUserOp.maxPriorityFeePerGas; - require(maxGasValues <= type(uint120).max, "AA94 gas values overflow"); + require(maxGasValues <= type(uint120).max, FailedOp(opIndex, "AA94 gas values overflow")); uint256 requiredPreFund = _getRequiredPrefund(mUserOp); validationData = _validateAccountPrepayment( diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index f9942f93..2a3b501c 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -16,32 +16,32 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple - diff from previous │ 2 │ │ 41530 │ 12271 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451298 │ │ ║ +║ simple │ 10 │ 451250 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41590 │ 12331 ║ +║ simple - diff from previous │ 11 │ │ 41614 │ 12355 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83200 │ │ ║ +║ simple paymaster │ 1 │ 83224 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40072 │ 10813 ║ +║ simple paymaster with diff │ 2 │ │ 40084 │ 10825 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 443927 │ │ ║ +║ simple paymaster │ 10 │ 444083 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 40067 │ 10808 ║ +║ simple paymaster with diff │ 11 │ │ 40103 │ 10844 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx 5k │ 1 │ 167141 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130743 │ 16041 ║ +║ big tx - diff from previous │ 2 │ │ 130755 │ 16053 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1344014 │ │ ║ +║ big tx 5k │ 10 │ 1343978 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130754 │ 16052 ║ +║ big tx - diff from previous │ 11 │ │ 130766 │ 16064 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ paymaster+postOp │ 1 │ 84401 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ paymaster+postOp with diff │ 2 │ │ 41261 │ 12002 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 455870 │ │ ║ +║ paymaster+postOp │ 10 │ 455822 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41309 │ 12050 ║ +║ paymaster+postOp with diff │ 11 │ │ 41285 │ 12026 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 43124e5685b1916d926d34f29ddb1088393d0781 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Tue, 4 Feb 2025 15:08:51 +0200 Subject: [PATCH 05/10] cleanup saveFreePtr/restoreFreePtr --- contracts/core/EntryPoint.sol | 39 +++++++++++++++++++++-------------- reports/gas-checker.txt | 24 ++++++++++----------- test/UserOp.ts | 1 - 3 files changed, 36 insertions(+), 28 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index bfa6ff8d..1255d20c 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -92,10 +92,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT bytes memory context = getMemoryBytesFromOffset(opInfo.contextOffset); bool success; { - uint256 saveFreePtr; - assembly ("memory-safe") { - saveFreePtr := mload(0x40) - } + uint256 saveFreePtr = getFreePtr(); bytes calldata callData = userOp.callData; bytes memory innerCall; bytes4 methodSig; @@ -115,8 +112,8 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT assembly ("memory-safe") { success := call(gas(), address(), 0, add(innerCall, 0x20), mload(innerCall), 0, 32) collected := mload(0) - mstore(0x40, saveFreePtr) } + restoreFreePtr(saveFreePtr); } if (!success) { bytes32 innerRevertCode; @@ -516,10 +513,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT // handle wrong output size with FailedOp function _callValidateUserOp(PackedUserOperation calldata op, UserOpInfo memory opInfo, uint256 missingAccountFunds, uint256 opIndex) internal returns (uint256 validationData) { - uint256 saveFreePtr; - assembly ("memory-safe") { - saveFreePtr := mload(0x40) - } + uint256 saveFreePtr = getFreePtr(); bytes memory callData = abi.encodeCall(IAccount.validateUserOp, (op, opInfo.userOpHash, missingAccountFunds)); uint256 gasLimit = opInfo.mUserOp.verificationGasLimit; address sender = opInfo.mUserOp.sender; @@ -528,8 +522,8 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT let success := call(gasLimit, sender, 0, add(callData, 0x20), mload(callData), 0, 32) dataSize := mul(returndatasize(), success) validationData := mload(0) - mstore(0x40, saveFreePtr) } + restoreFreePtr(saveFreePtr); if (dataSize != 32) { if(sender.code.length == 0) { revert FailedOp(opIndex, "AA20 account not deployed"); @@ -789,7 +783,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT /** * The gas price this UserOp agrees to pay. - * Relayer/block builder might submit the TX with higher priorityFee, but the user should not. + * Relayer/block builder might submit the TX with higher priorityFee, but the user should not be affected. * @param mUserOp - The userOp to get the gas price from. */ function getUserOpGasPrice( @@ -806,6 +800,12 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT } } + /// @inheritdoc IEntryPoint + function delegateAndRevert(address target, bytes calldata data) external { + (bool success, bytes memory ret) = target.delegatecall(data); + revert DelegateAndRevert(success, ret); + } + /** * The offset of the given bytes in memory. * @param data - The bytes to get the offset of. @@ -830,9 +830,18 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT } } - /// @inheritdoc IEntryPoint - function delegateAndRevert(address target, bytes calldata data) external { - (bool success, bytes memory ret) = target.delegatecall(data); - revert DelegateAndRevert(success, ret); + // safe free memory pointer. + function getFreePtr() internal pure returns (uint256 ptr) { + assembly ("memory-safe") { + ptr := mload(0x40) + } + } + + // restore free memory pointer. + // no allocated memory since saveFreePtr was called is allowed to be accessed after this call. + function restoreFreePtr(uint256 ptr) internal pure { + assembly ("memory-safe") { + mstore(0x40, ptr) + } } } diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 2a3b501c..354a746d 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -16,32 +16,32 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple - diff from previous │ 2 │ │ 41530 │ 12271 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451250 │ │ ║ +║ simple │ 10 │ 451214 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41614 │ 12355 ║ +║ simple - diff from previous │ 11 │ │ 41638 │ 12379 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83224 │ │ ║ +║ simple paymaster │ 1 │ 83212 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40084 │ 10825 ║ +║ simple paymaster with diff │ 2 │ │ 40096 │ 10837 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 444083 │ │ ║ +║ simple paymaster │ 10 │ 444119 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 40103 │ 10844 ║ +║ simple paymaster with diff │ 11 │ │ 40055 │ 10796 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx 5k │ 1 │ 167141 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130755 │ 16053 ║ +║ big tx - diff from previous │ 2 │ │ 130743 │ 16041 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1343978 │ │ ║ +║ big tx 5k │ 10 │ 1343990 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130766 │ 16064 ║ +║ big tx - diff from previous │ 11 │ │ 130754 │ 16052 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84401 │ │ ║ +║ paymaster+postOp │ 1 │ 84389 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ paymaster+postOp with diff │ 2 │ │ 41261 │ 12002 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 455822 │ │ ║ +║ paymaster+postOp │ 10 │ 455870 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41285 │ 12026 ║ +║ paymaster+postOp with diff │ 11 │ │ 41333 │ 12074 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ diff --git a/test/UserOp.ts b/test/UserOp.ts index 8d29175a..2f984ebd 100644 --- a/test/UserOp.ts +++ b/test/UserOp.ts @@ -224,7 +224,6 @@ export async function fillAndPack (op: Partial, entryPoint?: Entr export function getDomainSeparator (entryPoint: string, chainId: number): string { const domainData = getErc4337TypedDataDomain(entryPoint, chainId) - console.log('data=', domainData) return keccak256(defaultAbiCoder.encode( ['bytes32', 'bytes32', 'bytes32', 'uint256', 'address'], [ From c57101158c39fa1bce6c456231600d8a185e5510 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Tue, 4 Feb 2025 19:54:28 +0200 Subject: [PATCH 06/10] cleaup copy of legacy contracts --- scripts/prepack-contracts-package.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/prepack-contracts-package.sh b/scripts/prepack-contracts-package.sh index eac8c94b..b0c90e0c 100755 --- a/scripts/prepack-contracts-package.sh +++ b/scripts/prepack-contracts-package.sh @@ -14,4 +14,4 @@ cd contracts rm -rf artifacts mkdir -p artifacts -cp `find ../artifacts/contracts -type f | grep -v -E 'test|Test|dbg|bls|IOracle'` artifacts/ +cp `find ../artifacts/contracts -type f | grep -v -E 'test|Test|dbg|bls|IOracle|v06'` artifacts/ From d09fc319964fbd9d68e98155452fd4a11b7304f2 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Wed, 5 Feb 2025 16:57:11 +0200 Subject: [PATCH 07/10] pr issue --- contracts/core/EntryPoint.sol | 2 +- reports/gas-checker.txt | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 1255d20c..664a439c 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -224,7 +224,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT //address(1) is special marker of "signature error" require( address(aggregator) != address(1), - FailedOp(totalOps, "AA96 invalid aggregator") + FailedOp(totalOps + i, "AA96 invalid aggregator") ); if (address(aggregator) != address(0)) { diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 354a746d..d49f5384 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -14,34 +14,34 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple │ 1 │ 77383 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41530 │ 12271 ║ +║ simple - diff from previous │ 2 │ │ 41518 │ 12259 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451214 │ │ ║ +║ simple │ 10 │ 451262 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41638 │ 12379 ║ +║ simple - diff from previous │ 11 │ │ 41590 │ 12331 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83212 │ │ ║ +║ simple paymaster │ 1 │ 83224 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40096 │ 10837 ║ +║ simple paymaster with diff │ 2 │ │ 40084 │ 10825 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 444119 │ │ ║ +║ simple paymaster │ 10 │ 444107 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple paymaster with diff │ 11 │ │ 40055 │ 10796 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167141 │ │ ║ +║ big tx 5k │ 1 │ 167129 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130743 │ 16041 ║ +║ big tx - diff from previous │ 2 │ │ 130755 │ 16053 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1343990 │ │ ║ +║ big tx 5k │ 10 │ 1344002 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130754 │ 16052 ║ +║ big tx - diff from previous │ 11 │ │ 130706 │ 16004 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84389 │ │ ║ +║ paymaster+postOp │ 1 │ 84377 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41261 │ 12002 ║ +║ paymaster+postOp with diff │ 2 │ │ 41237 │ 11978 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 455870 │ │ ║ +║ paymaster+postOp │ 10 │ 455642 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41333 │ 12074 ║ +║ paymaster+postOp with diff │ 11 │ │ 41249 │ 11990 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From ad585802aa19a469532791105ff936027ee4e63b Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 9 Feb 2025 12:05:40 +0200 Subject: [PATCH 08/10] assembly: use if instead of mul use if success { ... } instead of mul(success,....) also, for EntryPointSimulations: - had to reduce code size (so cleared simulation-unrelated "supportsInterface") - fixed maxgap --- contracts/core/EntryPoint.sol | 7 +++++-- contracts/core/EntryPointSimulations.sol | 3 +++ test/entrypointsimulations.test.ts | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 477d0f5b..4d8182bd 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -520,8 +520,11 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT uint256 dataSize; assembly ("memory-safe"){ let success := call(gasLimit, sender, 0, add(callData, 0x20), mload(callData), 0, 32) - dataSize := mul(returndatasize(), success) - validationData := mload(0) + if success { + // ignore returndatasize, in case of revert + dataSize := returndatasize() + validationData := mload(0) + } } restoreFreePtr(saveFreePtr); if (dataSize != 32) { diff --git a/contracts/core/EntryPointSimulations.sol b/contracts/core/EntryPointSimulations.sol index 3dda963e..0bf73ee4 100644 --- a/contracts/core/EntryPointSimulations.sol +++ b/contracts/core/EntryPointSimulations.sol @@ -215,4 +215,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations { return __domainSeparatorV4; } + function supportsInterface(bytes4) public view virtual override returns (bool) { + return false; + } } diff --git a/test/entrypointsimulations.test.ts b/test/entrypointsimulations.test.ts index 80a77a38..34c5d9eb 100644 --- a/test/entrypointsimulations.test.ts +++ b/test/entrypointsimulations.test.ts @@ -295,7 +295,7 @@ describe('EntryPointSimulations', function () { describe(`compare to execution ${withPaymaster} paymaster`, () => { let execVgl: number let execPmVgl: number - const diff = 500 + const diff = 600 before(async () => { execPmVgl = withPaymaster === 'without' ? 0 : await findUserOpWithMin(async n => userOpWithGas(1e6, n), false, entryPoint, 1, 500000) execVgl = await findUserOpWithMin(async n => userOpWithGas(n, execPmVgl), false, entryPoint, 1, 500000) From cf22e10d7824a480d02880bba997b34a3e882bb3 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sun, 9 Feb 2025 14:14:29 +0200 Subject: [PATCH 09/10] gascalc --- reports/gas-checker.txt | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index c02eb480..a4113096 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 │ 77367 │ │ ║ +║ simple │ 1 │ 77410 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41478 │ 12219 ║ +║ simple - diff from previous │ 2 │ │ 41545 │ 12286 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451114 │ │ ║ +║ simple │ 10 │ 451592 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41622 │ 12363 ║ +║ simple - diff from previous │ 11 │ │ 41581 │ 12322 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83216 │ │ ║ +║ simple paymaster │ 1 │ 83258 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40064 │ 10805 ║ +║ simple paymaster with diff │ 2 │ │ 40118 │ 10859 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 443991 │ │ ║ +║ simple paymaster │ 10 │ 444435 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 40107 │ 10848 ║ +║ simple paymaster with diff │ 11 │ │ 40089 │ 10830 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167125 │ │ ║ +║ big tx 5k │ 1 │ 167168 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130715 │ 16013 ║ +║ big tx - diff from previous │ 2 │ │ 130758 │ 16056 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1343770 │ │ ║ +║ big tx 5k │ 10 │ 1344200 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130714 │ 16012 ║ +║ big tx - diff from previous │ 11 │ │ 130829 │ 16127 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84547 │ │ ║ +║ paymaster+postOp │ 1 │ 84601 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41407 │ 12148 ║ +║ paymaster+postOp with diff │ 2 │ │ 41461 │ 12202 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 457318 │ │ ║ +║ paymaster+postOp │ 10 │ 457894 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41443 │ 12184 ║ +║ paymaster+postOp with diff │ 11 │ │ 41449 │ 12190 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 1575598889d4914db4c448131c10d2abe0874767 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 10 Feb 2025 17:03:21 +0200 Subject: [PATCH 10/10] return value failure check --- contracts/core/EntryPoint.sol | 13 +++++++------ reports/gas-checker.txt | 34 +++++++++++++++++----------------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 4d8182bd..47c2be16 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -518,16 +518,17 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuardT uint256 gasLimit = opInfo.mUserOp.verificationGasLimit; address sender = opInfo.mUserOp.sender; uint256 dataSize; + bool success; assembly ("memory-safe"){ - let success := call(gasLimit, sender, 0, add(callData, 0x20), mload(callData), 0, 32) - if success { - // ignore returndatasize, in case of revert - dataSize := returndatasize() - validationData := mload(0) + success := call(gasLimit, sender, 0, add(callData, 0x20), mload(callData), 0, 32) + validationData := mload(0) + // any return data size other than 32 is considered failure + if iszero(eq(returndatasize(), 32)) { + success := 0 } } restoreFreePtr(saveFreePtr); - if (dataSize != 32) { + if (!success) { if(sender.code.length == 0) { revert FailedOp(opIndex, "AA20 account not deployed"); } else { diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index a4113096..eb164913 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -4,7 +4,7 @@ ╔══════════════════════════╤════════╗ ║ gas estimate "simple" │ 29259 ║ ╟──────────────────────────┼────────╢ -║ gas estimate "big tx 5k" │ 114702 ║ +║ gas estimate "big tx 5k" │ 114690 ║ ╚══════════════════════════╧════════╝ ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗ @@ -12,36 +12,36 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 77410 │ │ ║ +║ simple │ 1 │ 77367 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 41545 │ 12286 ║ +║ simple - diff from previous │ 2 │ │ 41538 │ 12279 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 451592 │ │ ║ +║ simple │ 10 │ 451258 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 41581 │ 12322 ║ +║ simple - diff from previous │ 11 │ │ 41526 │ 12267 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 83258 │ │ ║ +║ simple paymaster │ 1 │ 83228 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 40118 │ 10859 ║ +║ simple paymaster with diff │ 2 │ │ 40088 │ 10829 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 444435 │ │ ║ +║ simple paymaster │ 10 │ 444135 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 40089 │ 10830 ║ +║ simple paymaster with diff │ 11 │ │ 40107 │ 10848 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 167168 │ │ ║ +║ big tx 5k │ 1 │ 167113 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 130758 │ 16056 ║ +║ big tx - diff from previous │ 2 │ │ 130751 │ 16061 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1344200 │ │ ║ +║ big tx 5k │ 10 │ 1343914 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 130829 │ 16127 ║ +║ big tx - diff from previous │ 11 │ │ 130714 │ 16024 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 84601 │ │ ║ +║ paymaster+postOp │ 1 │ 84571 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 41461 │ 12202 ║ +║ paymaster+postOp with diff │ 2 │ │ 41419 │ 12160 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 457894 │ │ ║ +║ paymaster+postOp │ 10 │ 457594 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 41449 │ 12190 ║ +║ paymaster+postOp with diff │ 11 │ │ 41455 │ 12196 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝