Merge pull request #293 from leekt/develop

fix EntryPoint revert if 'postOp' reverts with short revert reason
eth-infinitism · Jun 25, 2023 · 74a3c37 · 74a3c37
2 parents 10f0502 + 8116f81
commit 74a3c37
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 15 deletions.
diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
@@ -72,8 +72,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
@@ -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, uint256)
+    internal virtual override view
+    returns (bytes memory context, uint256 validationData) {
+        validationData = 0;
+        context = abi.encodePacked(userOp.sender);
+    }
+
+    function _postOp(PostOpMode mode, bytes calldata, uint256) internal override {
+        if(mode == PostOpMode.postOpReverted) {
+            return;
+        }
+
+        revert CustomError();
+    }
+}
diff --git 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 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 
diff --git 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({