Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AA-217: Charge a penalty payment for unused execution gas limit of a UserOp #356

Merged
merged 37 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
ebbf21a
Makes comments and formatting consistent
gigamesh Mar 3, 2023
64cbdf6
@inheritdoc IEntryPoint
gigamesh Mar 4, 2023
d43bc94
Reverts contracts/test/TestHelpers.sol
gigamesh Mar 6, 2023
105d122
fix: entrypoint does not revert even first postOp reverts with short …
leekt May 30, 2023
3a6b0a5
lint
leekt May 30, 2023
66688cb
gas-report
leekt May 30, 2023
b56ef7b
Merge branch 'next_v0.7' into gigamesh/comments
forshtat Jun 25, 2023
74ed53b
Update IEntryPoint.sol - add missing 'INonceManager'
forshtat Jun 25, 2023
649daf8
Update EntryPoint.sol - fix lint errors
forshtat Jun 25, 2023
aa20d5d
Update gas-checker.txt manually
forshtat Jun 25, 2023
e3111fe
Update gas-checker.txt - add newline for the 'diff'
forshtat Jun 25, 2023
10f0502
Merge pull request #235 from gigamesh/gigamesh/comments
forshtat Jun 25, 2023
6a5ce09
Merge branch 'next_v0.7' into develop
forshtat Jun 25, 2023
8116f81
Update gas-checker.txt manually
forshtat Jun 25, 2023
74a3c37
Merge pull request #293 from leekt/develop
forshtat Jun 25, 2023
f96de6c
Remove "postOp" retry call mode and adjust tests
forshtat Jun 26, 2023
4b76a32
Update gas-calc
forshtat Jun 26, 2023
8903ae1
Update gas-checker.txt
forshtat Jun 26, 2023
ce519f6
Merge branch 'develop' of github.com:eth-infinitism/account-abstracti…
forshtat Oct 18, 2023
9bd5263
Bump 'yarn.lock' versions
forshtat Oct 18, 2023
a218f79
Bump node version for Github Actions
forshtat Oct 18, 2023
318a7ac
Weird
forshtat Oct 18, 2023
0f24290
Disable new solhint rules
forshtat Oct 18, 2023
83cb4e2
Uncomment TestCounter deployment
forshtat Oct 18, 2023
53eda99
Fix
forshtat Oct 18, 2023
ba64217
Fix gas checks
forshtat Oct 18, 2023
30d24ac
Use up to two times 'verificationGasLimit' in the 'requiredGas' calcu…
forshtat Oct 18, 2023
b6a2a64
AA-217: Charge a penalty payment for unused execution gas limit of a …
forshtat Oct 18, 2023
c3e808d
Add test for unused gas limit penalty charge
forshtat Oct 20, 2023
60a0a1c
Fix
forshtat Oct 20, 2023
821f758
Merge branch 'develop' into AA-217-payment-for-unused-gas
forshtat Oct 22, 2023
e2809a7
Use gas estimation instead of a hardcoded value
forshtat Oct 30, 2023
6762541
Fix gas calc
forshtat Oct 30, 2023
cb4e7ca
wtf
forshtat Oct 30, 2023
710f9e9
AA-217: Add the unused gas penalty description to the EIP text (#361)
forshtat Nov 2, 2023
a35c3c3
better estimation gas calculation
drortirosh Nov 2, 2023
d3f1ede
fix warmup.
drortirosh Nov 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions contracts/core/EntryPoint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
bytes32 private constant INNER_OUT_OF_GAS = hex"deaddead";

uint256 private constant REVERT_REASON_MAX_LEN = 2048;
uint256 private constant PENALTY_PERCENT = 10;

/**
* For simulation purposes, validateUserOp (and validatePaymasterUserOp)
Expand Down Expand Up @@ -668,6 +669,23 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
}
}
actualGas += preGas - gasleft();

// Calculating a penalty for unused execution gas
{
uint256 executionGasLimit = mUserOp.callGasLimit;
// Note that 'verificationGasLimit' here is the limit given to the 'postOp' which is part of execution
if (context.length > 0){
executionGasLimit += mUserOp.verificationGasLimit;
}
uint256 executionGasUsed = actualGas - opInfo.preOpGas;

This comment was marked as resolved.

// 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;
if (opInfo.prefund < actualGasCost) {
revert FailedOp(opIndex, "AA51 prefund below actualGasCost");
Expand Down
4 changes: 3 additions & 1 deletion eip/EIPS/eip-4337.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,9 @@ The entry point's `handleOps` function must perform the following steps (we firs
In the execution loop, the `handleOps` call must perform the following steps for each `UserOperation`:

* **Call the account with the `UserOperation`'s calldata**. It's up to the account to choose how to parse the calldata; an expected workflow is for the account to have an `execute` function that parses the remaining calldata as a series of one or more calls that the account should make.
* After the call, refund the account's deposit with the excess gas price that was pre-charged.
* After the call, refund the account's deposit with the excess gas cost that was pre-charged.\
A penalty of `10%` (`UNUSED_GAS_PENALTY_PERCENT`) is applied on the amount of gas that is refunded.\
This penalty is necessary to prevent the UserOps from reserving large parts of the gas space in the bundle but leaving it unused and preventing the bundler from including other UserOperations.
* After the execution of all calls, pay the collected fees from all UserOperations to the bundler's provided address

![](../assets/eip-4337/bundle-seq.svg)
Expand Down
5 changes: 4 additions & 1 deletion eip/EIPS/eip-aa-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ To help make sense of these params, note that a malicious paymaster can at most
* **[OP-062]** Precompiles:
* Only allowed the core 9 precompiles.\
Specifically the computation precompiles that do not access anything in the blockchain state or environment.
* **[OP-070] Transient Storage slots defined in [EIP-1153](./eip-1153.md) and accessed using `TLOAD` (`0x5c`) and `TSTORE` (`0x5d`) opcodes
are treated exactly like persistent storage (SLOAD/SSTORE)

### Code Rules

Expand All @@ -210,7 +212,8 @@ The permanent storage access with `SLOAD` and `SSTORE` instructions within each
* **[STO-022]** There is an `initCode` and the `factory` contract is staked.
* If the entity (`paymaster`, `factory`) is staked, then it is also allowed:
* **[STO-031]** Access the entity's own storage.
* **[STO-032]** Access a storage slot in any non-entity contract that is associated with the entity.
* **[STO-032]** Read/Write Access to storage slots that is associated with the entity, in any non-entity contract.
* **[STO-033]** Read-only access to any storage in non-entity contract.
* **[STO-040]** `UserOperation` may not use an entity address (`factory`/`paymaster`/`aggregator`) that is used as an "account" in another `UserOperation` in the mempool. \
This means that a `Paymaster` and `Factory` contracts cannot practically be an "account" contract as well.
* **[STO-041]** A contract whose Associated Storage slot is accessed during a `UserOperation` may not be an address of a "sender" in another `UserOperation` in the mempool.
Expand Down
24 changes: 12 additions & 12 deletions reports/gas-checker.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,28 @@
║ │ │ │ (delta for │ (compared to ║
║ │ │ │ one UserOp) │ account.exec()) ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple │ 1 │ 81838 │ │ ║
║ simple │ 1 │ 81945 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 2 │ │ 4409515081
║ simple - diff from previous │ 2 │ │ 4422615212
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple │ 10 │ 478810 │ │ ║
║ simple │ 10 │ 480000 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 11 │ │ 4413115117
║ simple - diff from previous │ 11 │ │ 4423815224
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 1 │ 88109 │ │ ║
║ simple paymaster │ 1 │ 88255 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 2 │ │ 4309614082
║ simple paymaster with diff │ 2 │ │ 4320614192
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 10 │ 476022 │ │ ║
║ simple paymaster │ 10 │ 477410 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 11 │ │ 4315514141
║ simple paymaster with diff │ 11 │ │ 4324114227
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 1 │ 182895 │ │ ║
║ big tx 5k │ 1 │ 183014 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 2 │ │ 14459419334
║ big tx - diff from previous │ 2 │ │ 14472519465
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 10 │ 1484454 │ │ ║
║ big tx 5k │ 10 │ 1485632 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 11 │ │ 14461919359
║ big tx - diff from previous │ 11 │ │ 14478619526
╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝

58 changes: 58 additions & 0 deletions test/entrypoint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,64 @@ 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, '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that a good estimate to the callGasUsed is
estimateGas({from:entryPoint, to: account.address, data: op1.callData})

const accountExec = await account.populateTransaction.execute(counter.address, 0, count.data!)
const callGasLimit = await ethersSigner.provider.estimateGas({
from: entryPoint.address,
to: account.address,
data: accountExec.data
})
// expect(callGasLimit.toNumber()).to.be.closeTo(270000, 10000)
const beneficiaryAddress = createAddress()

// "warmup" userop, for better gas calculation, below
await entryPoint.handleOps([await fillAndSign({ sender: account.address, callData: accountExec.data }, accountOwner, entryPoint)], beneficiaryAddress)
await entryPoint.handleOps([await fillAndSign({ sender: account.address, callData: accountExec.data }, accountOwner, entryPoint)], beneficiaryAddress)

const op1 = await fillAndSign({
sender: account.address,
callData: accountExec.data,
verificationGasLimit: 1e5,
callGasLimit: callGasLimit
}, accountOwner, entryPoint)

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)
expect(logs1[0].args.success).to.be.true

const gasUsed1 = logs1[0].args.actualGasUsed.toNumber()
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)

const gasUsed2 = logs2[0].args.actualGasUsed.toNumber()

// we cannot access internal transaction state, so we have to rely on two separate transactions for estimation
// assuming 10% penalty is charged
const expectedGasPenalty = (veryBigCallGasLimit - callGasLimit.toNumber()) * 0.1
const actualGasPenalty = gasUsed2 - gasUsed1

console.log(actualGasPenalty / expectedGasPenalty)
expect(actualGasPenalty).to.be.closeTo(expectedGasPenalty, expectedGasPenalty * 0.001)
})

it('legacy mode (maxPriorityFee==maxFeePerGas) should not use "basefee" opcode', async function () {
const op = await fillAndSign({
sender: account.address,
Expand Down
Loading