Skip to content

Commit

Permalink
remove AA40 - global verification gas check
Browse files Browse the repository at this point in the history 
remove AA40, and instead add separate gas check for paymaster and for
factory/account
This way, staked entity that cause verification over-gas usage can't
escape from blaming.
AA40 was a global "over
  • Loading branch information
@drortirosh
drortirosh committed Feb 8, 2024
1 parent c1c2aab commit 40a4af2
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 46 deletions.
60 changes: 34 additions & 26 deletions contracts/core/EntryPoint.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,14 +441,16 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
uint256 opIndex,
PackedUserOperation calldata op,
UserOpInfo memory opInfo,
uint256 requiredPrefund
uint256 requiredPrefund,
uint256 verificationGasLimit
)
internal
returns (
uint256 validationData
)
internal
returns (
uint256 validationData
)
{
unchecked {
uint256 preGas = gasleft();
MemoryUserOp memory mUserOp = opInfo.mUserOp;
address sender = mUserOp.sender;
_createSenderIfNeeded(opIndex, opInfo, op.initCode);
Expand All @@ -461,8 +463,8 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
: requiredPrefund - bal;
}
try
IAccount(sender).validateUserOp{
gas: mUserOp.verificationGasLimit
IAccount(sender).validateUserOp{
gas: verificationGasLimit
}(op, opInfo.userOpHash, missingAccountFunds)
returns (uint256 _validationData) {
validationData = _validationData;
Expand All @@ -477,6 +479,10 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
}
senderInfo.deposit = deposit - requiredPrefund;
}

if (preGas - gasleft() > verificationGasLimit) {
revert FailedOp(opIndex, "AA26 over verificationGasLimit");
}
}
}

Expand All @@ -498,6 +504,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
uint256 requiredPreFund
) internal returns (bytes memory context, uint256 validationData) {
unchecked {
uint256 preGas = gasleft();
MemoryUserOp memory mUserOp = opInfo.mUserOp;
address paymaster = mUserOp.paymaster;
DepositInfo storage paymasterInfo = deposits[paymaster];
Expand All @@ -506,18 +513,22 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
revert FailedOp(opIndex, "AA31 paymaster deposit too low");
}
paymasterInfo.deposit = deposit - requiredPreFund;
uint256 pmVerificationGasLimit = mUserOp.paymasterVerificationGasLimit;
try
IPaymaster(paymaster).validatePaymasterUserOp{gas: mUserOp.paymasterVerificationGasLimit}(
op,
opInfo.userOpHash,
requiredPreFund
)
IPaymaster(paymaster).validatePaymasterUserOp{gas: pmVerificationGasLimit}(
op,
opInfo.userOpHash,
requiredPreFund
)
returns (bytes memory _context, uint256 _validationData) {
context = _context;
validationData = _validationData;
} catch {
revert FailedOpWithRevert(opIndex, "AA33 reverted", Exec.getReturnData(REVERT_REASON_MAX_LEN));
}
if (preGas - gasleft() > pmVerificationGasLimit) {
revert FailedOp(opIndex, "AA36 over verificationGasLimit");
}
}
}

Expand Down Expand Up @@ -597,21 +608,23 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,

// Validate all numeric values in userOp are well below 128 bit, so they can safely be added
// and multiplied without causing overflow.
uint256 verificationGasLimit = mUserOp.verificationGasLimit;
uint256 maxGasValues = mUserOp.preVerificationGas |
mUserOp.verificationGasLimit |
mUserOp.callGasLimit |
mUserOp.paymasterVerificationGasLimit |
mUserOp.paymasterPostOpGasLimit |
mUserOp.maxFeePerGas |
mUserOp.maxPriorityFeePerGas;
verificationGasLimit |
mUserOp.callGasLimit |
mUserOp.paymasterVerificationGasLimit |
mUserOp.paymasterPostOpGasLimit |
mUserOp.maxFeePerGas |
mUserOp.maxPriorityFeePerGas;
require(maxGasValues <= type(uint120).max, "AA94 gas values overflow");

uint256 requiredPreFund = _getRequiredPrefund(mUserOp);
validationData = _validateAccountPrepayment(
opIndex,
userOp,
outOpInfo,
requiredPreFund
requiredPreFund,
verificationGasLimit
);

if (!_validateAndUpdateNonce(mUserOp.sender, mUserOp.nonce)) {
Expand All @@ -628,11 +641,6 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
);
}
unchecked {
uint256 gasUsed = preGas - gasleft();

if (mUserOp.verificationGasLimit + mUserOp.paymasterVerificationGasLimit < gasUsed) {
revert FailedOp(opIndex, "AA40 over verificationGasLimit");
}
outOpInfo.prefund = requiredPreFund;
outOpInfo.contextOffset = getOffsetOfMemoryBytes(context);
outOpInfo.preOpGas = preGas - gasleft() + userOp.preVerificationGas;
Expand Down Expand Up @@ -671,8 +679,8 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
actualGasCost = actualGas * gasPrice;
if (mode != IPaymaster.PostOpMode.postOpReverted) {
try IPaymaster(paymaster).postOp{
gas: mUserOp.paymasterPostOpGasLimit
}(mode, context, actualGasCost, gasPrice)
gas: mUserOp.paymasterPostOpGasLimit
}(mode, context, actualGasCost, gasPrice)
// solhint-disable-next-line no-empty-blocks
{} catch {
bytes memory reason = Exec.getReturnData(REVERT_REASON_MAX_LEN);
Expand Down
40 changes: 20 additions & 20 deletions reports/gas-checker.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,44 +12,44 @@
║ │ │ │ (delta for │ (compared to ║
║ │ │ │ one UserOp) │ account.exec()) ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple │ 1 │ 79958 │ │ ║
║ simple │ 1 │ 79916 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 2 │ │ 42156 │ 13177 ║
║ simple - diff from previous │ 2 │ │ 42138 │ 13159 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple │ 10 │ 459549 │ │ ║
║ simple │ 10 │ 459309 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 11 │ │ 42211 │ 13232 ║
║ simple - diff from previous │ 11 │ │ 42205 │ 13226 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 1 │ 86060 │ │ ║
║ simple paymaster │ 1 │ 86066 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 2 │ │ 40971 │ 11992 ║
║ simple paymaster with diff │ 2 │ │ 40977 │ 11998 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 10 │ 454974 │ │ ║
║ simple paymaster │ 10 │ 455142 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 11 │ │ 40975 │ 11996 ║
║ simple paymaster with diff │ 11 │ │ 41041 │ 12062 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 1 │ 181002 │ │ ║
║ big tx 5k │ 1 │ 180972 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 2 │ │ 142666 │ 17442 ║
║ big tx - diff from previous │ 2 │ │ 142660 │ 17436 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 10 │ 1465047 │ │ ║
║ big tx 5k │ 10 │ 1464891 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 11 │ │ 142710 │ 17486 ║
║ big tx - diff from previous │ 11 │ │ 142644 │ 17420 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp │ 1 │ 87725 │ │ ║
║ paymaster+postOp │ 1 │ 87743 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp with diff │ 2 │ │ 42636 │ 13657 ║
║ paymaster+postOp with diff │ 2 │ │ 42642 │ 13663 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp │ 10 │ 471608 │ │ ║
║ paymaster+postOp │ 10 │ 471800 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp with diff │ 11 │ │ 42681 │ 13702 ║
║ paymaster+postOp with diff │ 11 │ │ 42687 │ 13708 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster │ 1 │ 128754 │ │ ║
║ token paymaster │ 1 │ 128772 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster with diff │ 2 │ │ 66351 │ 37372 ║
║ token paymaster with diff │ 2 │ │ 66381 │ 37402 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster │ 10 │ 726214 │ │ ║
║ token paymaster │ 10 │ 726382 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster with diff │ 11 │ │ 66467 │ 37488 ║
║ token paymaster with diff │ 11 │ │ 66473 │ 37494 ║
╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝

0 comments on commit 40a4af2

Please sign in to comment.