Add test for unused gas limit penalty charge

eth-infinitism · Oct 20, 2023 · c3e808d · c3e808d
1 parent b6a2a64
commit c3e808d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 5 deletions.
diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol
@@ -19,8 +19,6 @@ import "./UserOperationLib.sol";
 import "@openzeppelin/contracts/utils/introspection/ERC165.sol" as OpenZeppelin;
 import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 
-import "hardhat/console.sol";
-
 /*
  * Account-Abstraction (EIP-4337) singleton EntryPoint implementation.
  * Only one instance required on each chain.
@@ -680,9 +678,12 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
                     executionGasLimit += mUserOp.verificationGasLimit;
                 }
                 uint256 executionGasUsed = actualGas - opInfo.preOpGas;
-                uint256 unusedGas = executionGasLimit - executionGasUsed;
-                uint256 unusedGasPenalty = (unusedGas * PENALTY_PERCENT) / 100;
-                actualGas += unusedGasPenalty;
+                // this check is required for the gas used within EntryPoint and not covered by explicit gas limits
+                if (executionGasLimit > executionGasUsed) {
+                    uint256 unusedGas = executionGasLimit - executionGasUsed;
+                    uint256 unusedGasPenalty = (unusedGas * PENALTY_PERCENT) / 100;
+                    actualGas += unusedGasPenalty;
+                }
             }
 
             actualGasCost = actualGas * gasPrice;

diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts
@@ -543,6 +543,47 @@ describe('EntryPoint', function () {
         expect(await getBalance(account.address)).to.eq(inititalAccountBalance)
       })
 
+      it('account should pay a penalty for requiring too much gas and leaving it unused', async function () {
+        if (process.env.COVERAGE != null) {
+          return
+        }
+        const iterations = 10
+        const count = await counter.populateTransaction.gasWaster(iterations, '')
+        const accountExec = await account.populateTransaction.execute(counter.address, 0, count.data!)
+        const op1 = await fillAndSign({
+          sender: account.address,
+          callData: accountExec.data,
+          verificationGasLimit: 1e5,
+          callGasLimit: 265000
+        }, accountOwner, entryPoint)
+
+        const beneficiaryAddress = createAddress()
+        const rcpt1 = await entryPoint.handleOps([op1], beneficiaryAddress, {
+          maxFeePerGas: 1e9,
+          gasLimit: 20000000
+        }).then(async t => await t.wait())
+        const logs1 = await entryPoint.queryFilter(entryPoint.filters.UserOperationEvent(), rcpt1.blockHash)
+        assert.equal(logs1[0].args.success, true)
+
+        const veryBigCallGasLimit = 10000000
+        const op2 = await fillAndSign({
+          sender: account.address,
+          callData: accountExec.data,
+          verificationGasLimit: 1e5,
+          callGasLimit: veryBigCallGasLimit
+        }, accountOwner, entryPoint)
+        const rcpt2 = await entryPoint.handleOps([op2], beneficiaryAddress, {
+          maxFeePerGas: 1e9,
+          gasLimit: 20000000
+        }).then(async t => await t.wait())
+        const logs2 = await entryPoint.queryFilter(entryPoint.filters.UserOperationEvent(), rcpt2.blockHash)
+        // we cannot access internal transaction state, so we have to rely on two separate transactions for estimation
+        const approximateUnusedGas = veryBigCallGasLimit - logs1[0].args.actualGasUsed.toNumber()
+        const approximatePenalty = logs2[0].args.actualGasUsed.sub(logs1[0].args.actualGasUsed)
+        // assuming 10% penalty is charged
+        expect(approximatePenalty.toNumber()).to.be.closeTo(approximateUnusedGas / 10, 40000)
+      })
+
       it('legacy mode (maxPriorityFee==maxFeePerGas) should not use "basefee" opcode', async function () {
         const op = await fillAndSign({
           sender: account.address,