From 105d1222fea92be98e181e31a6ef725cd7093732 Mon Sep 17 00:00:00 2001 From: leekt Date: Wed, 31 May 2023 03:03:49 +0900 Subject: [PATCH 1/4] fix: entrypoint does not revert even first postOp reverts with short revert reason --- contracts/core/EntryPoint.sol | 7 +++-- .../test/TestPaymasterRevertCustomError.sol | 30 +++++++++++++++++++ test/entrypoint.test.ts | 21 ++++++++++++- 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 contracts/test/TestPaymasterRevertCustomError.sol diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index b5473a5e..ce0deec5 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -66,8 +66,11 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard } catch { bytes32 innerRevertCode; assembly { - returndatacopy(0, 0, 32) - innerRevertCode := mload(0) + let len := returndatasize() + if eq(32,len) { + returndatacopy(0, 0, 32) + innerRevertCode := mload(0) + } } // handleOps was called with gas limit too low. abort entire bundle. if (innerRevertCode == INNER_OUT_OF_GAS) { diff --git a/contracts/test/TestPaymasterRevertCustomError.sol b/contracts/test/TestPaymasterRevertCustomError.sol new file mode 100644 index 00000000..61782555 --- /dev/null +++ b/contracts/test/TestPaymasterRevertCustomError.sol @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.8.12; + +import "../core/BasePaymaster.sol"; + +/** + * test postOp revert with custom error + */ +error CustomError(); + +contract TestPaymasterRevertCustomError is BasePaymaster { + // solhint-disable no-empty-blocks + constructor(IEntryPoint _entryPoint) BasePaymaster(_entryPoint) + {} + + function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) + internal virtual override view + returns (bytes memory context, uint256 validationData) { + validationData = 0; + context = abi.encodePacked(userOp.sender); + } + + function _postOp(PostOpMode mode, bytes calldata context, uint256 actualGasCost) internal override { + if(mode == PostOpMode.postOpReverted) { + return; + } + + revert CustomError(); + } +} diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts index 253f087e..082badb6 100644 --- a/test/entrypoint.test.ts +++ b/test/entrypoint.test.ts @@ -20,7 +20,8 @@ import { TestSignatureAggregator, TestSignatureAggregator__factory, MaliciousAccount__factory, - TestWarmColdAccount__factory + TestWarmColdAccount__factory, + TestPaymasterRevertCustomError__factory } from '../typechain' import { AddressZero, @@ -1171,6 +1172,24 @@ describe('EntryPoint', function () { await expect(entryPoint.handleOps([op], beneficiaryAddress)).to.revertedWith('"AA31 paymaster deposit too low"') }) + it('should not revert when paymaster reverts with custom error on postOp', async function () { + const account3Owner = createAccountOwner() + const errorPostOp = await new TestPaymasterRevertCustomError__factory(ethersSigner).deploy(entryPoint.address) + await errorPostOp.addStake(globalUnstakeDelaySec, { value: paymasterStake }) + await errorPostOp.deposit({ value: ONE_ETH }) + + const op = await fillAndSign({ + paymasterAndData: errorPostOp.address, + callData: accountExecFromEntryPoint.data, + initCode: getAccountInitCode(account3Owner.address, simpleAccountFactory), + + verificationGasLimit: 3e6, + callGasLimit: 1e6 + }, account3Owner, entryPoint) + const beneficiaryAddress = createAddress() + await entryPoint.handleOps([op], beneficiaryAddress) + }) + it('paymaster should pay for tx', async function () { await paymaster.deposit({ value: ONE_ETH }) const op = await fillAndSign({ From 3a6b0a5e4c451abe636631283a54dab0c6acc7e8 Mon Sep 17 00:00:00 2001 From: leekt Date: Wed, 31 May 2023 03:12:28 +0900 Subject: [PATCH 2/4] lint --- contracts/test/TestPaymasterRevertCustomError.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/test/TestPaymasterRevertCustomError.sol b/contracts/test/TestPaymasterRevertCustomError.sol index 61782555..b0e7f425 100644 --- a/contracts/test/TestPaymasterRevertCustomError.sol +++ b/contracts/test/TestPaymasterRevertCustomError.sol @@ -13,14 +13,14 @@ contract TestPaymasterRevertCustomError is BasePaymaster { constructor(IEntryPoint _entryPoint) BasePaymaster(_entryPoint) {} - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) + function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32, uint256) internal virtual override view returns (bytes memory context, uint256 validationData) { validationData = 0; context = abi.encodePacked(userOp.sender); } - function _postOp(PostOpMode mode, bytes calldata context, uint256 actualGasCost) internal override { + function _postOp(PostOpMode mode, bytes calldata, uint256) internal override { if(mode == PostOpMode.postOpReverted) { return; } From 66688cbe50cd719ffdf9897918b78e50c9bd00c0 Mon Sep 17 00:00:00 2001 From: leekt Date: Wed, 31 May 2023 03:12:34 +0900 Subject: [PATCH 3/4] gas-report --- reports/gas-checker.txt | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 17dea2e6..75f64b0b 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -12,28 +12,28 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 81901 │ │ ║ +║ simple │ 1 │ 81921 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 44212 │ 15198 ║ +║ simple - diff from previous │ 2 │ │ 44184 │ 15170 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 479854 │ │ ║ +║ simple │ 10 │ 479946 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 44236 │ 15222 ║ +║ simple - diff from previous │ 11 │ │ 44220 │ 15206 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 88172 │ │ ║ +║ simple paymaster │ 1 │ 88180 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 43165 │ 14151 ║ +║ simple paymaster with diff │ 2 │ │ 43185 │ 14171 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 476994 │ │ ║ +║ simple paymaster │ 10 │ 477134 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 43260 │ 14246 ║ +║ simple paymaster with diff │ 11 │ │ 43220 │ 14206 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 182958 │ │ ║ +║ big tx 5k │ 1 │ 182978 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 144723 │ 19463 ║ +║ big tx - diff from previous │ 2 │ │ 144719 │ 19459 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1485438 │ │ ║ +║ big tx 5k │ 10 │ 1485494 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 144712 │ 19452 ║ +║ big tx - diff from previous │ 11 │ │ 144792 │ 19532 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 8116f81fe226cfacb55734206fcd341e5ae6bbd0 Mon Sep 17 00:00:00 2001 From: Alex Forshtat Date: Sun, 25 Jun 2023 23:03:40 +0300 Subject: [PATCH 4/4] Update gas-checker.txt manually --- reports/gas-checker.txt | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 4aa9f835..a519fb50 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -12,28 +12,28 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 81913 │ │ ║ +║ simple │ 1 │ 81921 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 44200 │ 15186 ║ +║ simple - diff from previous │ 2 │ │ 44208 │ 15194 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 479770 │ │ ║ +║ simple │ 10 │ 479922 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 44272 │ 15258 ║ +║ simple - diff from previous │ 11 │ │ 44244 │ 15230 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 88196 │ │ ║ +║ simple paymaster │ 1 │ 88192 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 43189 │ 14175 ║ +║ simple paymaster with diff │ 2 │ │ 43197 │ 14183 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 477078 │ │ ║ +║ simple paymaster │ 10 │ 477170 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 43200 │ 14186 ║ +║ simple paymaster with diff │ 11 │ │ 43220 │ 14206 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 182970 │ │ ║ +║ big tx 5k │ 1 │ 182978 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 144711 │ 19451 ║ +║ big tx - diff from previous │ 2 │ │ 144683 │ 19423 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1485498 │ │ ║ +║ big tx 5k │ 10 │ 1485506 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 144748 │ 19488 ║ +║ big tx - diff from previous │ 11 │ │ 144744 │ 19484 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝