From 3181436922a56bd200688fb8091767b44662394a Mon Sep 17 00:00:00 2001 From: shahafn Date: Thu, 2 Nov 2023 13:52:30 +0200 Subject: [PATCH 01/24] Packing gas limits - Adding packed paymasterGasLimits for validatePaymasterUserOp and postOp - Packing gas limits of validateUserOp and call gas limits into accountGasLimits --- contracts/core/EntryPoint.sol | 49 ++++++++++--------- contracts/core/UserOperationLib.sol | 18 +++++-- contracts/interfaces/UserOperation.sol | 8 +-- contracts/samples/DepositPaymaster.sol | 2 +- contracts/samples/LegacyTokenPaymaster.sol | 3 +- contracts/samples/VerifyingPaymaster.sol | 4 +- .../samples/bls/BLSSignatureAggregator.sol | 4 +- contracts/samples/gnosis/EIP4337Manager.sol | 2 +- contracts/test/MaliciousAccount.sol | 7 ++- 9 files changed, 60 insertions(+), 37 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 74a096bf..e7387e2c 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -225,8 +225,8 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, struct MemoryUserOp { address sender; uint256 nonce; - uint256 callGasLimit; - uint256 verificationGasLimit; + bytes32 accountGasLimits; + bytes32 paymasterGasLimits; uint256 preVerificationGas; address paymaster; uint256 maxFeePerGas; @@ -257,11 +257,16 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, require(msg.sender == address(this), "AA92 internal call only"); MemoryUserOp memory mUserOp = opInfo.mUserOp; - uint callGasLimit = mUserOp.callGasLimit; + uint callGasLimit = UserOperationLib.getExecutionGasLimit(mUserOp.accountGasLimits); unchecked { // handleOps was called with gas limit too low. abort entire bundle. if ( - gasleft() < callGasLimit + mUserOp.verificationGasLimit + 5000 + gasleft() < + callGasLimit + + UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) + + UserOperationLib.getValidationGasLimit(mUserOp.paymasterGasLimits) + + UserOperationLib.getExecutionGasLimit(mUserOp.paymasterGasLimits) + + 5000 ) { assembly { mstore(0, INNER_OUT_OF_GAS) @@ -313,8 +318,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, ) internal pure { mUserOp.sender = userOp.sender; mUserOp.nonce = userOp.nonce; - mUserOp.callGasLimit = userOp.callGasLimit; - mUserOp.verificationGasLimit = userOp.verificationGasLimit; + mUserOp.accountGasLimits = userOp.accountGasLimits; mUserOp.preVerificationGas = userOp.preVerificationGas; mUserOp.maxFeePerGas = userOp.maxFeePerGas; mUserOp.maxPriorityFeePerGas = userOp.maxPriorityFeePerGas; @@ -325,8 +329,10 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, "AA93 invalid paymasterAndData" ); mUserOp.paymaster = address(bytes20(paymasterAndData[:20])); + mUserOp.paymasterGasLimits = userOp.paymasterGasLimits; } else { mUserOp.paymaster = address(0); + mUserOp.paymasterGasLimits = bytes32(0); } } @@ -338,12 +344,10 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, MemoryUserOp memory mUserOp ) internal pure returns (uint256 requiredPrefund) { unchecked { - // When using a Paymaster, the verificationGasLimit is used also to as a limit for the postOp call. - // Our security model might call postOp eventually twice. - uint256 mul = mUserOp.paymaster != address(0) ? 2 : 1; - uint256 requiredGas = mUserOp.callGasLimit + - mUserOp.verificationGasLimit * - mul + + uint256 requiredGas = UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) + + UserOperationLib.getExecutionGasLimit(mUserOp.accountGasLimits) + + UserOperationLib.getValidationGasLimit(mUserOp.paymasterGasLimits) + + UserOperationLib.getExecutionGasLimit(mUserOp.paymasterGasLimits) + mUserOp.preVerificationGas; requiredPrefund = requiredGas * mUserOp.maxFeePerGas; @@ -366,7 +370,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, if (sender.code.length != 0) revert FailedOp(opIndex, "AA10 sender already constructed"); address sender1 = senderCreator.createSender{ - gas: opInfo.mUserOp.verificationGasLimit + gas: UserOperationLib.getValidationGasLimit(opInfo.mUserOp.accountGasLimits) }(initCode); if (sender1 == address(0)) revert FailedOp(opIndex, "AA13 initCode failed or OOG"); @@ -427,7 +431,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, } try IAccount(sender).validateUserOp{ - gas: mUserOp.verificationGasLimit + gas: UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) }(op, opInfo.userOpHash, missingAccountFunds) returns (uint256 _validationData) { validationData = _validationData; @@ -472,13 +476,12 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, ) internal returns (bytes memory context, uint256 validationData) { unchecked { MemoryUserOp memory mUserOp = opInfo.mUserOp; - uint256 verificationGasLimit = mUserOp.verificationGasLimit; + // TODO: Do we actually need that still? + uint256 verificationGasLimit = UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits); require( verificationGasLimit > gasUsedByValidateAccountPrepayment, "AA41 too little verificationGas" ); - uint256 gas = verificationGasLimit - - gasUsedByValidateAccountPrepayment; address paymaster = mUserOp.paymaster; DepositInfo storage paymasterInfo = deposits[paymaster]; @@ -488,7 +491,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, } paymasterInfo.deposit = uint112(deposit - requiredPreFund); try - IPaymaster(paymaster).validatePaymasterUserOp{gas: gas}( + IPaymaster(paymaster).validatePaymasterUserOp{gas: uint128(bytes16(mUserOp.paymasterGasLimits))}( op, opInfo.userOpHash, requiredPreFund @@ -582,8 +585,10 @@ 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 maxGasValues = mUserOp.preVerificationGas | - mUserOp.verificationGasLimit | - mUserOp.callGasLimit | + UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) | + UserOperationLib.getExecutionGasLimit(mUserOp.accountGasLimits) | + UserOperationLib.getValidationGasLimit(mUserOp.paymasterGasLimits) | + UserOperationLib.getExecutionGasLimit(mUserOp.paymasterGasLimits) | userOp.maxFeePerGas | userOp.maxPriorityFeePerGas; require(maxGasValues <= type(uint120).max, "AA94 gas values overflow"); @@ -621,7 +626,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, unchecked { uint256 gasUsed = preGas - gasleft(); - if (userOp.verificationGasLimit < gasUsed) { + if (UserOperationLib.getValidationGasLimit(userOp.accountGasLimits) + UserOperationLib.getValidationGasLimit(userOp.paymasterGasLimits) < gasUsed) { revert FailedOp(opIndex, "AA40 over verificationGasLimit"); } outOpInfo.prefund = requiredPreFund; @@ -662,7 +667,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, actualGasCost = actualGas * gasPrice; if (mode != IPaymaster.PostOpMode.postOpReverted) { IPaymaster(paymaster).postOp{ - gas: mUserOp.verificationGasLimit + gas: uint128(uint256(mUserOp.paymasterGasLimits)) }(mode, context, actualGasCost); } } diff --git a/contracts/core/UserOperationLib.sol b/contracts/core/UserOperationLib.sol index 19b678a4..8b8f054b 100644 --- a/contracts/core/UserOperationLib.sol +++ b/contracts/core/UserOperationLib.sol @@ -55,8 +55,8 @@ library UserOperationLib { uint256 nonce = userOp.nonce; bytes32 hashInitCode = calldataKeccak(userOp.initCode); bytes32 hashCallData = calldataKeccak(userOp.callData); - uint256 callGasLimit = userOp.callGasLimit; - uint256 verificationGasLimit = userOp.verificationGasLimit; + bytes32 accountGasLimits = userOp.accountGasLimits; + bytes32 paymasterGasLimits = userOp.paymasterGasLimits; uint256 preVerificationGas = userOp.preVerificationGas; uint256 maxFeePerGas = userOp.maxFeePerGas; uint256 maxPriorityFeePerGas = userOp.maxPriorityFeePerGas; @@ -65,12 +65,24 @@ library UserOperationLib { return abi.encode( sender, nonce, hashInitCode, hashCallData, - callGasLimit, verificationGasLimit, preVerificationGas, + accountGasLimits, paymasterGasLimits, preVerificationGas, maxFeePerGas, maxPriorityFeePerGas, hashPaymasterAndData ); } + function getValidationGasLimit( + bytes32 packedGasLimits + ) internal pure returns(uint128) { + return uint128(bytes16(packedGasLimits)); + } + + function getExecutionGasLimit( + bytes32 packedGasLimits + ) internal pure returns(uint128) { + return uint128(uint256(packedGasLimits)); + } + /** * Hash the user operation data. * @param userOp - The user operation data. diff --git a/contracts/interfaces/UserOperation.sol b/contracts/interfaces/UserOperation.sol index 8185d1c4..84694235 100644 --- a/contracts/interfaces/UserOperation.sol +++ b/contracts/interfaces/UserOperation.sol @@ -7,8 +7,8 @@ pragma solidity ^0.8.12; * @param nonce - Unique value the sender uses to verify it is not a replay. * @param initCode - If set, the account contract will be created by this constructor/ * @param callData - The method call to execute on this account. - * @param callGasLimit - The gas limit passed to the callData method call. - * @param verificationGasLimit - Gas used for validateUserOp and validatePaymasterUserOp. + * @param accountGasLimits - Packed gas limits for validateUserOp and gas limit passed to the callData method call. + * @param paymasterGasLimits - Packed gas limits for validatePaymasterUserOp and gas limit passed to postOp method call. * @param preVerificationGas - Gas not calculated by the handleOps method, but added to the gas paid. * Covers batch overhead. * @param maxFeePerGas - Same as EIP-1559 gas parameter. @@ -22,8 +22,8 @@ struct UserOperation { uint256 nonce; bytes initCode; bytes callData; - uint256 callGasLimit; - uint256 verificationGasLimit; + bytes32 accountGasLimits; + bytes32 paymasterGasLimits; uint256 preVerificationGas; uint256 maxFeePerGas; uint256 maxPriorityFeePerGas; diff --git a/contracts/samples/DepositPaymaster.sol b/contracts/samples/DepositPaymaster.sol index 89a107df..5f51b6aa 100644 --- a/contracts/samples/DepositPaymaster.sol +++ b/contracts/samples/DepositPaymaster.sol @@ -131,7 +131,7 @@ contract DepositPaymaster is BasePaymaster { (userOpHash); // verificationGasLimit is dual-purposed, as gas limit for postOp. make sure it is high enough - require(userOp.verificationGasLimit > COST_OF_POST, "DepositPaymaster: gas too low for postOp"); + require(UserOperationLib.getExecutionGasLimit(userOp.paymasterGasLimits) > COST_OF_POST, "DepositPaymaster: gas too low for postOp"); bytes calldata paymasterAndData = userOp.paymasterAndData; require(paymasterAndData.length == 20+20, "DepositPaymaster: paymasterAndData must specify token"); diff --git a/contracts/samples/LegacyTokenPaymaster.sol b/contracts/samples/LegacyTokenPaymaster.sol index d9ebf9bc..291be399 100644 --- a/contracts/samples/LegacyTokenPaymaster.sol +++ b/contracts/samples/LegacyTokenPaymaster.sol @@ -5,6 +5,7 @@ pragma solidity ^0.8.12; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "../core/BasePaymaster.sol"; +import "../core/UserOperationLib.sol"; /** * A sample paymaster that defines itself as a token to pay for gas. @@ -75,7 +76,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 { // verificationGasLimit is dual-purposed, as gas limit for postOp. make sure it is high enough // make sure that verificationGasLimit is high enough to handle postOp - require(userOp.verificationGasLimit > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); + require(UserOperationLib.getExecutionGasLimit(userOp.paymasterGasLimits) > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); if (userOp.initCode.length != 0) { _validateConstructor(userOp); diff --git a/contracts/samples/VerifyingPaymaster.sol b/contracts/samples/VerifyingPaymaster.sol index 5dcf0b15..31c1bca4 100644 --- a/contracts/samples/VerifyingPaymaster.sol +++ b/contracts/samples/VerifyingPaymaster.sol @@ -50,8 +50,8 @@ contract VerifyingPaymaster is BasePaymaster { userOp.nonce, keccak256(userOp.initCode), keccak256(userOp.callData), - userOp.callGasLimit, - userOp.verificationGasLimit, + userOp.accountGasLimits, + userOp.paymasterGasLimits, userOp.preVerificationGas, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, diff --git a/contracts/samples/bls/BLSSignatureAggregator.sol b/contracts/samples/bls/BLSSignatureAggregator.sol index 1866692f..134d9e5c 100644 --- a/contracts/samples/bls/BLSSignatureAggregator.sol +++ b/contracts/samples/bls/BLSSignatureAggregator.sol @@ -88,8 +88,8 @@ contract BLSSignatureAggregator is IAggregator { userOp.nonce, keccak256(userOp.initCode), keccak256(userOp.callData), - userOp.callGasLimit, - userOp.verificationGasLimit, + userOp.accountGasLimits, + userOp.paymasterGasLimits, userOp.preVerificationGas, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol index 2a687ada..f38a4f5f 100644 --- a/contracts/samples/gnosis/EIP4337Manager.sol +++ b/contracts/samples/gnosis/EIP4337Manager.sol @@ -163,7 +163,7 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor { sig[2] = bytes1(uint8(1)); sig[35] = bytes1(uint8(1)); uint256 nonce = uint256(IEntryPoint(manager.entryPoint()).getNonce(address(safe), 0)); - UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", 0, 1000000, 0, 0, 0, "", sig); + UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", bytes32(bytes3(0x0f4240)), bytes32(0), 0, 0, 0, "", sig); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; IEntryPoint _entryPoint = IEntryPoint(payable(manager.entryPoint())); diff --git a/contracts/test/MaliciousAccount.sol b/contracts/test/MaliciousAccount.sol index 57325ddc..62bc4d29 100644 --- a/contracts/test/MaliciousAccount.sol +++ b/contracts/test/MaliciousAccount.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.12; import "../interfaces/IAccount.sol"; import "../interfaces/IEntryPoint.sol"; +import "../core/UserOperationLib.sol"; contract MaliciousAccount is IAccount { IEntryPoint private ep; @@ -13,7 +14,11 @@ contract MaliciousAccount is IAccount { ep.depositTo{value : missingAccountFunds}(address(this)); // Now calculate basefee per EntryPoint.getUserOpGasPrice() and compare it to the basefe we pass off-chain in the signature uint256 externalBaseFee = abi.decode(userOp.signature, (uint256)); - uint256 requiredGas = userOp.callGasLimit + userOp.verificationGasLimit + userOp.preVerificationGas; + uint256 requiredGas = UserOperationLib.getValidationGasLimit(userOp.accountGasLimits) + + UserOperationLib.getExecutionGasLimit(userOp.accountGasLimits) + + UserOperationLib.getValidationGasLimit(userOp.paymasterGasLimits) + + UserOperationLib.getExecutionGasLimit(userOp.paymasterGasLimits) + + userOp.preVerificationGas; uint256 gasPrice = missingAccountFunds / requiredGas; uint256 basefee = gasPrice - userOp.maxPriorityFeePerGas; require (basefee == externalBaseFee, "Revert after first validation"); From 1e2fe50a8c2e3211888fc16deac6d78874822249 Mon Sep 17 00:00:00 2001 From: shahafn Date: Thu, 30 Nov 2023 21:56:02 +0200 Subject: [PATCH 02/24] Packing paymaster gas fields into paymasterAndData --- contracts/core/EntryPoint.sol | 50 ++++++++++--------- contracts/core/UserOperationLib.sol | 19 ++++--- contracts/interfaces/UserOperation.sol | 4 +- contracts/samples/DepositPaymaster.sol | 2 +- contracts/samples/LegacyTokenPaymaster.sol | 2 +- contracts/samples/VerifyingPaymaster.sol | 2 +- .../samples/bls/BLSSignatureAggregator.sol | 1 - contracts/samples/gnosis/EIP4337Manager.sol | 2 +- contracts/test/MaliciousAccount.sol | 13 +++-- 9 files changed, 49 insertions(+), 46 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index e7387e2c..ed990f6f 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -225,8 +225,10 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, struct MemoryUserOp { address sender; uint256 nonce; - bytes32 accountGasLimits; - bytes32 paymasterGasLimits; + uint128 verificationGasLimit; + uint128 callGasLimit; + uint128 paymasterVerificationGasLimit; + uint128 paymasterPostOpGasLimit; uint256 preVerificationGas; address paymaster; uint256 maxFeePerGas; @@ -257,15 +259,15 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, require(msg.sender == address(this), "AA92 internal call only"); MemoryUserOp memory mUserOp = opInfo.mUserOp; - uint callGasLimit = UserOperationLib.getExecutionGasLimit(mUserOp.accountGasLimits); + uint callGasLimit = mUserOp.callGasLimit; unchecked { // handleOps was called with gas limit too low. abort entire bundle. if ( gasleft() < callGasLimit + - UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) + - UserOperationLib.getValidationGasLimit(mUserOp.paymasterGasLimits) + - UserOperationLib.getExecutionGasLimit(mUserOp.paymasterGasLimits) + + mUserOp.verificationGasLimit + + mUserOp.paymasterVerificationGasLimit + + mUserOp.paymasterPostOpGasLimit + 5000 ) { assembly { @@ -318,7 +320,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, ) internal pure { mUserOp.sender = userOp.sender; mUserOp.nonce = userOp.nonce; - mUserOp.accountGasLimits = userOp.accountGasLimits; + (mUserOp.verificationGasLimit, mUserOp.callGasLimit) = UserOperationLib.unpackAccountGasLimits(userOp.accountGasLimits); mUserOp.preVerificationGas = userOp.preVerificationGas; mUserOp.maxFeePerGas = userOp.maxFeePerGas; mUserOp.maxPriorityFeePerGas = userOp.maxPriorityFeePerGas; @@ -329,10 +331,12 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, "AA93 invalid paymasterAndData" ); mUserOp.paymaster = address(bytes20(paymasterAndData[:20])); - mUserOp.paymasterGasLimits = userOp.paymasterGasLimits; + mUserOp.paymasterVerificationGasLimit = uint128(bytes16(paymasterAndData[20:36])); + mUserOp.paymasterPostOpGasLimit = uint128(bytes16(paymasterAndData[36:52])); } else { mUserOp.paymaster = address(0); - mUserOp.paymasterGasLimits = bytes32(0); + mUserOp.paymasterVerificationGasLimit = 0; + mUserOp.paymasterPostOpGasLimit = 0; } } @@ -344,10 +348,10 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, MemoryUserOp memory mUserOp ) internal pure returns (uint256 requiredPrefund) { unchecked { - uint256 requiredGas = UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) + - UserOperationLib.getExecutionGasLimit(mUserOp.accountGasLimits) + - UserOperationLib.getValidationGasLimit(mUserOp.paymasterGasLimits) + - UserOperationLib.getExecutionGasLimit(mUserOp.paymasterGasLimits) + + uint256 requiredGas = mUserOp.verificationGasLimit + + mUserOp.callGasLimit + + mUserOp.paymasterVerificationGasLimit + + mUserOp.paymasterPostOpGasLimit + mUserOp.preVerificationGas; requiredPrefund = requiredGas * mUserOp.maxFeePerGas; @@ -370,7 +374,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, if (sender.code.length != 0) revert FailedOp(opIndex, "AA10 sender already constructed"); address sender1 = senderCreator.createSender{ - gas: UserOperationLib.getValidationGasLimit(opInfo.mUserOp.accountGasLimits) + gas: opInfo.mUserOp.verificationGasLimit }(initCode); if (sender1 == address(0)) revert FailedOp(opIndex, "AA13 initCode failed or OOG"); @@ -431,7 +435,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, } try IAccount(sender).validateUserOp{ - gas: UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) + gas: mUserOp.verificationGasLimit }(op, opInfo.userOpHash, missingAccountFunds) returns (uint256 _validationData) { validationData = _validationData; @@ -477,7 +481,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, unchecked { MemoryUserOp memory mUserOp = opInfo.mUserOp; // TODO: Do we actually need that still? - uint256 verificationGasLimit = UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits); + uint256 verificationGasLimit = mUserOp.verificationGasLimit; require( verificationGasLimit > gasUsedByValidateAccountPrepayment, "AA41 too little verificationGas" @@ -491,7 +495,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, } paymasterInfo.deposit = uint112(deposit - requiredPreFund); try - IPaymaster(paymaster).validatePaymasterUserOp{gas: uint128(bytes16(mUserOp.paymasterGasLimits))}( + IPaymaster(paymaster).validatePaymasterUserOp{gas: mUserOp.paymasterVerificationGasLimit}( op, opInfo.userOpHash, requiredPreFund @@ -585,10 +589,10 @@ 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 maxGasValues = mUserOp.preVerificationGas | - UserOperationLib.getValidationGasLimit(mUserOp.accountGasLimits) | - UserOperationLib.getExecutionGasLimit(mUserOp.accountGasLimits) | - UserOperationLib.getValidationGasLimit(mUserOp.paymasterGasLimits) | - UserOperationLib.getExecutionGasLimit(mUserOp.paymasterGasLimits) | + mUserOp.verificationGasLimit | + mUserOp.callGasLimit | + mUserOp.paymasterVerificationGasLimit | + mUserOp.paymasterPostOpGasLimit | userOp.maxFeePerGas | userOp.maxPriorityFeePerGas; require(maxGasValues <= type(uint120).max, "AA94 gas values overflow"); @@ -626,7 +630,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, unchecked { uint256 gasUsed = preGas - gasleft(); - if (UserOperationLib.getValidationGasLimit(userOp.accountGasLimits) + UserOperationLib.getValidationGasLimit(userOp.paymasterGasLimits) < gasUsed) { + if (mUserOp.verificationGasLimit + mUserOp.paymasterVerificationGasLimit < gasUsed) { revert FailedOp(opIndex, "AA40 over verificationGasLimit"); } outOpInfo.prefund = requiredPreFund; @@ -667,7 +671,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, actualGasCost = actualGas * gasPrice; if (mode != IPaymaster.PostOpMode.postOpReverted) { IPaymaster(paymaster).postOp{ - gas: uint128(uint256(mUserOp.paymasterGasLimits)) + gas: mUserOp.paymasterVerificationGasLimit }(mode, context, actualGasCost); } } diff --git a/contracts/core/UserOperationLib.sol b/contracts/core/UserOperationLib.sol index 8b8f054b..d419a0f6 100644 --- a/contracts/core/UserOperationLib.sol +++ b/contracts/core/UserOperationLib.sol @@ -56,7 +56,6 @@ library UserOperationLib { bytes32 hashInitCode = calldataKeccak(userOp.initCode); bytes32 hashCallData = calldataKeccak(userOp.callData); bytes32 accountGasLimits = userOp.accountGasLimits; - bytes32 paymasterGasLimits = userOp.paymasterGasLimits; uint256 preVerificationGas = userOp.preVerificationGas; uint256 maxFeePerGas = userOp.maxFeePerGas; uint256 maxPriorityFeePerGas = userOp.maxPriorityFeePerGas; @@ -65,22 +64,22 @@ library UserOperationLib { return abi.encode( sender, nonce, hashInitCode, hashCallData, - accountGasLimits, paymasterGasLimits, preVerificationGas, + accountGasLimits, preVerificationGas, maxFeePerGas, maxPriorityFeePerGas, hashPaymasterAndData ); } - function getValidationGasLimit( - bytes32 packedGasLimits - ) internal pure returns(uint128) { - return uint128(bytes16(packedGasLimits)); + function unpackAccountGasLimits( + bytes32 accountGasLimits + ) internal pure returns(uint128 validationGasLimit, uint128 callGasLimit) { + return (uint128(bytes16(accountGasLimits)), uint128(uint256(accountGasLimits))); } - function getExecutionGasLimit( - bytes32 packedGasLimits - ) internal pure returns(uint128) { - return uint128(uint256(packedGasLimits)); + function unpackPaymasterStaticFields( + bytes calldata paymasterAndData + ) internal pure returns(address paymaster, uint128 validationGasLimit, uint128 postOp) { + return (address(bytes20(paymasterAndData[:20])), uint128(bytes16(paymasterAndData[20:36])), uint128(bytes16(paymasterAndData[36:52]))); } /** diff --git a/contracts/interfaces/UserOperation.sol b/contracts/interfaces/UserOperation.sol index 84694235..4e1b8a4a 100644 --- a/contracts/interfaces/UserOperation.sol +++ b/contracts/interfaces/UserOperation.sol @@ -8,12 +8,11 @@ pragma solidity ^0.8.12; * @param initCode - If set, the account contract will be created by this constructor/ * @param callData - The method call to execute on this account. * @param accountGasLimits - Packed gas limits for validateUserOp and gas limit passed to the callData method call. - * @param paymasterGasLimits - Packed gas limits for validatePaymasterUserOp and gas limit passed to postOp method call. * @param preVerificationGas - Gas not calculated by the handleOps method, but added to the gas paid. * Covers batch overhead. * @param maxFeePerGas - Same as EIP-1559 gas parameter. * @param maxPriorityFeePerGas - Same as EIP-1559 gas parameter. - * @param paymasterAndData - If set, this field holds the paymaster address and paymaster-specific data. + * @param paymasterAndData - If set, this field holds the paymaster address, verification gas limit, postOp gas limit and paymaster-specific extra data * The paymaster will pay for the transaction instead of the sender. * @param signature - Sender-verified signature over the entire request, the EntryPoint address and the chain ID. */ @@ -23,7 +22,6 @@ struct UserOperation { bytes initCode; bytes callData; bytes32 accountGasLimits; - bytes32 paymasterGasLimits; uint256 preVerificationGas; uint256 maxFeePerGas; uint256 maxPriorityFeePerGas; diff --git a/contracts/samples/DepositPaymaster.sol b/contracts/samples/DepositPaymaster.sol index 5f51b6aa..3e833532 100644 --- a/contracts/samples/DepositPaymaster.sol +++ b/contracts/samples/DepositPaymaster.sol @@ -131,7 +131,7 @@ contract DepositPaymaster is BasePaymaster { (userOpHash); // verificationGasLimit is dual-purposed, as gas limit for postOp. make sure it is high enough - require(UserOperationLib.getExecutionGasLimit(userOp.paymasterGasLimits) > COST_OF_POST, "DepositPaymaster: gas too low for postOp"); + require(uint128(bytes16(userOp.paymasterAndData[36:52])) > COST_OF_POST, "DepositPaymaster: gas too low for postOp"); bytes calldata paymasterAndData = userOp.paymasterAndData; require(paymasterAndData.length == 20+20, "DepositPaymaster: paymasterAndData must specify token"); diff --git a/contracts/samples/LegacyTokenPaymaster.sol b/contracts/samples/LegacyTokenPaymaster.sol index 291be399..58c7dce8 100644 --- a/contracts/samples/LegacyTokenPaymaster.sol +++ b/contracts/samples/LegacyTokenPaymaster.sol @@ -76,7 +76,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 { // verificationGasLimit is dual-purposed, as gas limit for postOp. make sure it is high enough // make sure that verificationGasLimit is high enough to handle postOp - require(UserOperationLib.getExecutionGasLimit(userOp.paymasterGasLimits) > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); + require(uint128(bytes16(userOp.paymasterAndData[36:52])) > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); if (userOp.initCode.length != 0) { _validateConstructor(userOp); diff --git a/contracts/samples/VerifyingPaymaster.sol b/contracts/samples/VerifyingPaymaster.sol index 31c1bca4..6e8d8b0e 100644 --- a/contracts/samples/VerifyingPaymaster.sol +++ b/contracts/samples/VerifyingPaymaster.sol @@ -51,7 +51,7 @@ contract VerifyingPaymaster is BasePaymaster { keccak256(userOp.initCode), keccak256(userOp.callData), userOp.accountGasLimits, - userOp.paymasterGasLimits, + uint128(bytes16(userOp.paymasterAndData[20:52])), userOp.preVerificationGas, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, diff --git a/contracts/samples/bls/BLSSignatureAggregator.sol b/contracts/samples/bls/BLSSignatureAggregator.sol index 134d9e5c..33bb8e66 100644 --- a/contracts/samples/bls/BLSSignatureAggregator.sol +++ b/contracts/samples/bls/BLSSignatureAggregator.sol @@ -89,7 +89,6 @@ contract BLSSignatureAggregator is IAggregator { keccak256(userOp.initCode), keccak256(userOp.callData), userOp.accountGasLimits, - userOp.paymasterGasLimits, userOp.preVerificationGas, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol index f38a4f5f..6abb05c4 100644 --- a/contracts/samples/gnosis/EIP4337Manager.sol +++ b/contracts/samples/gnosis/EIP4337Manager.sol @@ -163,7 +163,7 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor { sig[2] = bytes1(uint8(1)); sig[35] = bytes1(uint8(1)); uint256 nonce = uint256(IEntryPoint(manager.entryPoint()).getNonce(address(safe), 0)); - UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", bytes32(bytes3(0x0f4240)), bytes32(0), 0, 0, 0, "", sig); + UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", bytes32(bytes3(0x0f4240)), 0, 0, 0, "", sig); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; IEntryPoint _entryPoint = IEntryPoint(payable(manager.entryPoint())); diff --git a/contracts/test/MaliciousAccount.sol b/contracts/test/MaliciousAccount.sol index 62bc4d29..d6c18b88 100644 --- a/contracts/test/MaliciousAccount.sol +++ b/contracts/test/MaliciousAccount.sol @@ -14,11 +14,14 @@ contract MaliciousAccount is IAccount { ep.depositTo{value : missingAccountFunds}(address(this)); // Now calculate basefee per EntryPoint.getUserOpGasPrice() and compare it to the basefe we pass off-chain in the signature uint256 externalBaseFee = abi.decode(userOp.signature, (uint256)); - uint256 requiredGas = UserOperationLib.getValidationGasLimit(userOp.accountGasLimits) + - UserOperationLib.getExecutionGasLimit(userOp.accountGasLimits) + - UserOperationLib.getValidationGasLimit(userOp.paymasterGasLimits) + - UserOperationLib.getExecutionGasLimit(userOp.paymasterGasLimits) + - userOp.preVerificationGas; + (uint128 verificationGasLimit, uint128 callGasLimit) = UserOperationLib.unpackAccountGasLimits(userOp.accountGasLimits); + uint128 paymasterVerificationGasLimit = uint128(bytes16(userOp.paymasterAndData[20:36])); + uint128 paymasterPostOpGasLimit = uint128(bytes16(userOp.paymasterAndData[36:52])); + uint256 requiredGas = verificationGasLimit + + callGasLimit + + paymasterVerificationGasLimit + + paymasterPostOpGasLimit + + userOp.preVerificationGas; uint256 gasPrice = missingAccountFunds / requiredGas; uint256 basefee = gasPrice - userOp.maxPriorityFeePerGas; require (basefee == externalBaseFee, "Revert after first validation"); From bef7e4dc16ab13a7945c032222bb1ad917fe048c Mon Sep 17 00:00:00 2001 From: shahafn Date: Sun, 3 Dec 2023 16:52:34 +0200 Subject: [PATCH 03/24] bug fixes --- contracts/core/EntryPoint.sol | 4 +--- contracts/samples/VerifyingPaymaster.sol | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index ed990f6f..cd0f0f8b 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -265,8 +265,6 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, if ( gasleft() < callGasLimit + - mUserOp.verificationGasLimit + - mUserOp.paymasterVerificationGasLimit + mUserOp.paymasterPostOpGasLimit + 5000 ) { @@ -671,7 +669,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, actualGasCost = actualGas * gasPrice; if (mode != IPaymaster.PostOpMode.postOpReverted) { IPaymaster(paymaster).postOp{ - gas: mUserOp.paymasterVerificationGasLimit + gas: mUserOp.paymasterPostOpGasLimit }(mode, context, actualGasCost); } } diff --git a/contracts/samples/VerifyingPaymaster.sol b/contracts/samples/VerifyingPaymaster.sol index 6e8d8b0e..eaa7e432 100644 --- a/contracts/samples/VerifyingPaymaster.sol +++ b/contracts/samples/VerifyingPaymaster.sol @@ -51,7 +51,7 @@ contract VerifyingPaymaster is BasePaymaster { keccak256(userOp.initCode), keccak256(userOp.callData), userOp.accountGasLimits, - uint128(bytes16(userOp.paymasterAndData[20:52])), + uint256(bytes32(userOp.paymasterAndData[20:52])), userOp.preVerificationGas, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, From 41bad54203ef1c3c2aedfdfa3fac70824bf59fad Mon Sep 17 00:00:00 2001 From: shahafn Date: Mon, 4 Dec 2023 20:50:46 +0200 Subject: [PATCH 04/24] Fixing entrypoint tests --- contracts/test/MaliciousAccount.sol | 4 - contracts/test/TestExpirePaymaster.sol | 2 +- test/UserOp.ts | 95 ++++++++------- test/UserOperation.ts | 22 +++- test/entrypoint.test.ts | 156 ++++++++++++++----------- test/simple-wallet.test.ts | 4 +- test/solidityTypes.ts | 1 + test/testutils.ts | 15 ++- 8 files changed, 180 insertions(+), 119 deletions(-) diff --git a/contracts/test/MaliciousAccount.sol b/contracts/test/MaliciousAccount.sol index d6c18b88..a4f53825 100644 --- a/contracts/test/MaliciousAccount.sol +++ b/contracts/test/MaliciousAccount.sol @@ -15,12 +15,8 @@ contract MaliciousAccount is IAccount { // Now calculate basefee per EntryPoint.getUserOpGasPrice() and compare it to the basefe we pass off-chain in the signature uint256 externalBaseFee = abi.decode(userOp.signature, (uint256)); (uint128 verificationGasLimit, uint128 callGasLimit) = UserOperationLib.unpackAccountGasLimits(userOp.accountGasLimits); - uint128 paymasterVerificationGasLimit = uint128(bytes16(userOp.paymasterAndData[20:36])); - uint128 paymasterPostOpGasLimit = uint128(bytes16(userOp.paymasterAndData[36:52])); uint256 requiredGas = verificationGasLimit + callGasLimit + - paymasterVerificationGasLimit + - paymasterPostOpGasLimit + userOp.preVerificationGas; uint256 gasPrice = missingAccountFunds / requiredGas; uint256 basefee = gasPrice - userOp.maxPriorityFeePerGas; diff --git a/contracts/test/TestExpirePaymaster.sol b/contracts/test/TestExpirePaymaster.sol index 51036a1c..abf9897b 100644 --- a/contracts/test/TestExpirePaymaster.sol +++ b/contracts/test/TestExpirePaymaster.sol @@ -15,7 +15,7 @@ contract TestExpirePaymaster is BasePaymaster { internal virtual override view returns (bytes memory context, uint256 validationData) { (userOp, userOpHash, maxCost); - (uint48 validAfter, uint48 validUntil) = abi.decode(userOp.paymasterAndData[20 :], (uint48, uint48)); + (uint48 validAfter, uint48 validUntil) = abi.decode(userOp.paymasterAndData[52 :], (uint48, uint48)); validationData = _packValidationData(false, validUntil, validAfter); context = ""; } diff --git a/test/UserOp.ts b/test/UserOp.ts index daa0decd..29639f26 100644 --- a/test/UserOp.ts +++ b/test/UserOp.ts @@ -5,12 +5,12 @@ import { keccak256 } from 'ethers/lib/utils' import { BigNumber, Contract, Signer, Wallet } from 'ethers' -import { AddressZero, callDataCost, rethrow } from './testutils' +import { AddressZero, callDataCost, packAccountGasLimits, packPaymasterData, rethrow } from './testutils' import { ecsign, toRpcSig, keccak256 as keccak256_buffer } from 'ethereumjs-util' import { EntryPoint, EntryPointSimulations__factory } from '../typechain' -import { UserOperation } from './UserOperation' +import { PackedUserOperation, UserOperation } from './UserOperation' import { Create2Factory } from '../src/Create2Factory' import { TransactionRequest } from '@ethersproject/abstract-provider' @@ -18,55 +18,50 @@ import EntryPointSimulationsJson from '../artifacts/contracts/core/EntryPointSim import { ethers } from 'hardhat' import { IEntryPointSimulations } from '../typechain/contracts/core/EntryPointSimulations' -export function packUserOp (op: UserOperation, forSignature = true): string { + +export function packUserOp (userOp: UserOperation): PackedUserOperation { + const accountGasLimits = packAccountGasLimits(userOp.verificationGasLimit, userOp.callGasLimit) + let paymasterAndData = '0x' + if (userOp.paymaster.length >= 20) { + paymasterAndData = packPaymasterData(userOp.paymaster as string, userOp.paymasterVerificationGasLimit, userOp.paymasterPostOpGasLimit, userOp.paymasterData as string) + } + return { + sender: userOp.sender, + nonce: userOp.nonce, + callData: userOp.callData, + accountGasLimits, + initCode: userOp.initCode, + preVerificationGas: userOp.preVerificationGas, + maxFeePerGas: userOp.maxFeePerGas, + maxPriorityFeePerGas: userOp.maxPriorityFeePerGas, + paymasterAndData, + signature: userOp.signature + } +} +export function encodeUserOp (userOp: UserOperation, forSignature = true): string { + const packedUserOp = packUserOp(userOp) if (forSignature) { return defaultAbiCoder.encode( ['address', 'uint256', 'bytes32', 'bytes32', - 'uint256', 'uint256', 'uint256', 'uint256', 'uint256', + 'uint256', 'uint256', 'uint256', 'uint256', 'bytes32'], - [op.sender, op.nonce, keccak256(op.initCode), keccak256(op.callData), - op.callGasLimit, op.verificationGasLimit, op.preVerificationGas, op.maxFeePerGas, op.maxPriorityFeePerGas, - keccak256(op.paymasterAndData)]) + [packedUserOp.sender, packedUserOp.nonce, keccak256(packedUserOp.initCode), keccak256(packedUserOp.callData), + packedUserOp.accountGasLimits, packedUserOp.preVerificationGas, packedUserOp.maxFeePerGas, packedUserOp.maxPriorityFeePerGas, + keccak256(packedUserOp.paymasterAndData)]) } else { // for the purpose of calculating gas cost encode also signature (and no keccak of bytes) return defaultAbiCoder.encode( ['address', 'uint256', 'bytes', 'bytes', - 'uint256', 'uint256', 'uint256', 'uint256', 'uint256', + 'uint256', 'uint256', 'uint256', 'uint256', 'bytes', 'bytes'], - [op.sender, op.nonce, op.initCode, op.callData, - op.callGasLimit, op.verificationGasLimit, op.preVerificationGas, op.maxFeePerGas, op.maxPriorityFeePerGas, - op.paymasterAndData, op.signature]) + [packedUserOp.sender, packedUserOp.nonce, packedUserOp.initCode, packedUserOp.callData, + packedUserOp.accountGasLimits, packedUserOp.preVerificationGas, packedUserOp.maxFeePerGas, packedUserOp.maxPriorityFeePerGas, + packedUserOp.paymasterAndData, packedUserOp.signature]) } } -export function packUserOp1 (op: UserOperation): string { - return defaultAbiCoder.encode([ - 'address', // sender - 'uint256', // nonce - 'bytes32', // initCode - 'bytes32', // callData - 'uint256', // callGasLimit - 'uint256', // verificationGasLimit - 'uint256', // preVerificationGas - 'uint256', // maxFeePerGas - 'uint256', // maxPriorityFeePerGas - 'bytes32' // paymasterAndData - ], [ - op.sender, - op.nonce, - keccak256(op.initCode), - keccak256(op.callData), - op.callGasLimit, - op.verificationGasLimit, - op.preVerificationGas, - op.maxFeePerGas, - op.maxPriorityFeePerGas, - keccak256(op.paymasterAndData) - ]) -} - export function getUserOpHash (op: UserOperation, entryPoint: string, chainId: number): string { - const userOpHash = keccak256(packUserOp(op, true)) + const userOpHash = keccak256(encodeUserOp(op, true)) const enc = defaultAbiCoder.encode( ['bytes32', 'address', 'uint256'], [userOpHash, entryPoint, chainId]) @@ -83,7 +78,10 @@ export const DefaultsForUserOp: UserOperation = { preVerificationGas: 21000, // should also cover calldata cost. maxFeePerGas: 0, maxPriorityFeePerGas: 1e9, - paymasterAndData: '0x', + paymaster: '0x', + paymasterData: '0x', + paymasterVerificationGasLimit: 2e5, + paymasterPostOpGasLimit: 0, signature: '0x' } @@ -177,6 +175,14 @@ export async function fillUserOp (op: Partial, entryPoint?: Entry // estimateGas assumes direct call from entryPoint. add wrapper cost. op1.callGasLimit = gasEtimated // .add(55000) } + if (op1.paymaster != null) { + if (op1.paymasterVerificationGasLimit == null) { + op1.paymasterVerificationGasLimit = DefaultsForUserOp.paymasterVerificationGasLimit + } + if (op1.paymasterPostOpGasLimit == null) { + op1.paymasterPostOpGasLimit = DefaultsForUserOp.paymasterPostOpGasLimit + } + } if (op1.maxFeePerGas == null) { if (provider == null) throw new Error('must have entryPoint to autofill maxFeePerGas') const block = await provider.getBlock('latest') @@ -191,7 +197,7 @@ export async function fillUserOp (op: Partial, entryPoint?: Entry // eslint-disable-next-line @typescript-eslint/no-base-to-string if (op2.preVerificationGas.toString() === '0') { // TODO: we don't add overhead, which is ~21000 for a single TX, but much lower in a batch. - op2.preVerificationGas = callDataCost(packUserOp(op2, false)) + op2.preVerificationGas = callDataCost(encodeUserOp(op2, false)) } return op2 } @@ -216,6 +222,11 @@ export async function fillAndSign (op: Partial, signer: Wallet | } } +export async function fillSignAndPack (op: Partial, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'getNonce'): Promise { + const filledAndSignedOp = await fillAndSign(op, signer, entryPoint, getNonceFunction) + return packUserOp(filledAndSignedOp) +} + /** * This function relies on a "state override" functionality of the 'eth_call' RPC method * in order to provide the details of a simulated validation call to the bundler @@ -224,7 +235,7 @@ export async function fillAndSign (op: Partial, signer: Wallet | * @param txOverrides */ export async function simulateValidation ( - userOp: UserOperation, + userOp: PackedUserOperation, entryPointAddress: string, txOverrides?: any): Promise { const entryPointSimulations = EntryPointSimulations__factory.createInterface() @@ -257,7 +268,7 @@ export async function simulateValidation ( // TODO: this code is very much duplicated but "encodeFunctionData" is based on 20 overloads // TypeScript is not able to resolve overloads with variables: https://github.com/microsoft/TypeScript/issues/14107 export async function simulateHandleOp ( - userOp: UserOperation, + userOp: PackedUserOperation, target: string, targetCallData: string, entryPointAddress: string, diff --git a/test/UserOperation.ts b/test/UserOperation.ts index 8bed5ab3..480923b0 100644 --- a/test/UserOperation.ts +++ b/test/UserOperation.ts @@ -6,8 +6,26 @@ export interface UserOperation { nonce: typ.uint256 initCode: typ.bytes callData: typ.bytes - callGasLimit: typ.uint256 - verificationGasLimit: typ.uint256 + callGasLimit: typ.uint128 + verificationGasLimit: typ.uint128 + preVerificationGas: typ.uint256 + maxFeePerGas: typ.uint256 + maxPriorityFeePerGas: typ.uint256 + paymaster: typ.bytes + paymasterVerificationGasLimit: typ.uint128 + paymasterPostOpGasLimit: typ.uint128 + paymasterData: typ.bytes + signature: typ.bytes +} + + +export interface PackedUserOperation { + + sender: typ.address + nonce: typ.uint256 + initCode: typ.bytes + callData: typ.bytes + accountGasLimits: typ.bytes32 preVerificationGas: typ.uint256 maxFeePerGas: typ.uint256 maxPriorityFeePerGas: typ.uint256 diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts index b1fb051f..0422cd32 100644 --- a/test/entrypoint.test.ts +++ b/test/entrypoint.test.ts @@ -48,8 +48,8 @@ import { getAggregatedAccountInitCode, decodeRevertReason } from './testutils' -import { DefaultsForUserOp, fillAndSign, getUserOpHash, simulateValidation } from './UserOp' -import { UserOperation } from './UserOperation' +import { DefaultsForUserOp, fillAndSign, fillSignAndPack, getUserOpHash, packUserOp, simulateValidation } from './UserOp' +import { PackedUserOperation, UserOperation } from './UserOperation' import { PopulatedTransaction } from 'ethers/lib/ethers' import { ethers } from 'hardhat' import { arrayify, defaultAbiCoder, hexConcat, hexZeroPad, parseEther } from 'ethers/lib/utils' @@ -86,7 +86,8 @@ describe('EntryPoint', function () { // sanity: validate helper functions const sampleOp = await fillAndSign({ sender: account.address }, accountOwner, entryPoint) - expect(getUserOpHash(sampleOp, entryPoint.address, chainId)).to.eql(await entryPoint.getUserOpHash(sampleOp)) + const packedOp = packUserOp(sampleOp) + expect(getUserOpHash(sampleOp, entryPoint.address, chainId)).to.eql(await entryPoint.getUserOpHash(packedOp)) }) describe('Stake Management', () => { @@ -230,7 +231,7 @@ describe('EntryPoint', function () { // note: for the actual opcode and storage rule restrictions see the reference bundler ValidationManager it('should not use banned ops during simulateValidation', async () => { - const op1 = await fillAndSign({ + const op1 = await fillSignAndPack({ initCode: getAccountInitCode(accountOwner1.address, simpleAccountFactory), sender: await getAccountAddress(accountOwner1.address, simpleAccountFactory) }, accountOwner1, entryPoint) @@ -275,14 +276,18 @@ describe('EntryPoint', function () { // we need maxFeeperGas > block.basefee + maxPriorityFeePerGas so requiredPrefund onchain is basefee + maxPriorityFeePerGas maxFeePerGas: block.baseFeePerGas.mul(3), maxPriorityFeePerGas: block.baseFeePerGas, - paymasterAndData: '0x' + paymaster: '0x', + paymasterData: '0x', + paymasterVerificationGasLimit: 0, + paymasterPostOpGasLimit: 0 } + const userOpPacked = packUserOp(userOp) try { - await simulateValidation(userOp, entryPoint.address, { gasLimit: 1e6 }) + await simulateValidation(userOpPacked, entryPoint.address, { gasLimit: 1e6 }) console.log('after first simulation') await ethers.provider.send('evm_mine', []) - await expect(simulateValidation(userOp, entryPoint.address, { gasLimit: 1e6 })) + await expect(simulateValidation(userOpPacked, entryPoint.address, { gasLimit: 1e6 })) .to.revertedWith('Revert after first validation') // if we get here, it means the userOp passed first sim and reverted second expect.fail(null, null, 'should fail on first simulation') @@ -306,9 +311,10 @@ describe('EntryPoint', function () { callData: badData.data! } const beneficiaryAddress = createAddress() - await simulateValidation(badOp, entryPoint.address, { gasLimit: 3e5 }) + const badOpPacked = packUserOp(badOp) + await simulateValidation(badOpPacked, entryPoint.address, { gasLimit: 3e5 }) - const tx = await entryPoint.handleOps([badOp], beneficiaryAddress) // { gasLimit: 3e5 }) + const tx = await entryPoint.handleOps([badOpPacked], beneficiaryAddress) // { gasLimit: 3e5 }) const receipt = await tx.wait() const userOperationRevertReasonEvent = receipt.events?.find(event => event.event === 'UserOperationRevertReason') expect(userOperationRevertReasonEvent?.event).to.equal('UserOperationRevertReason') @@ -326,13 +332,14 @@ describe('EntryPoint', function () { nonce: TOUCH_GET_AGGREGATOR, sender: testWarmColdAccount.address } + const badOpPacked = packUserOp(badOp) const beneficiaryAddress = createAddress() try { - await simulateValidation(badOp, entryPoint.address, { gasLimit: 1e6 }) + await simulateValidation(badOpPacked, entryPoint.address, { gasLimit: 1e6 }) throw new Error('should revert') } catch (e: any) { if ((e as Error).message.includes('ValidationResult')) { - const tx = await entryPoint.handleOps([badOp], beneficiaryAddress, { gasLimit: 1e6 }) + const tx = await entryPoint.handleOps([badOpPacked], beneficiaryAddress, { gasLimit: 1e6 }) await tx.wait() } else { expect(e.message).to.include('AA23 reverted (or OOG)') @@ -348,16 +355,18 @@ describe('EntryPoint', function () { const badOp: UserOperation = { ...DefaultsForUserOp, nonce: TOUCH_PAYMASTER, - paymasterAndData: paymaster.address, + paymaster: paymaster.address, + paymasterVerificationGasLimit: 150000, sender: testWarmColdAccount.address } const beneficiaryAddress = createAddress() + const badOpPacked = packUserOp(badOp) try { - await simulateValidation(badOp, entryPoint.address, { gasLimit: 1e6 }) + await simulateValidation(badOpPacked, entryPoint.address, { gasLimit: 1e6 }) throw new Error('should revert') } catch (e: any) { if ((e as Error).message.includes('ValidationResult')) { - const tx = await entryPoint.handleOps([badOp], beneficiaryAddress, { gasLimit: 1e6 }) + const tx = await entryPoint.handleOps([badOpPacked], beneficiaryAddress, { gasLimit: 1e6 }) await tx.wait() } else { expect(e.message).to.include('AA23 reverted (or OOG)') @@ -380,7 +389,7 @@ describe('EntryPoint', function () { }) it('should fail nonce with new key and seq!=0', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender, nonce: keyShifted.add(1) }, accountOwner, entryPoint) @@ -389,7 +398,7 @@ describe('EntryPoint', function () { describe('with key=1, seq=1', () => { before(async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender, nonce: keyShifted }, accountOwner, entryPoint) @@ -401,7 +410,7 @@ describe('EntryPoint', function () { }) it('should allow to increment nonce of different key', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender, nonce: await entryPoint.getNonce(sender, key) }, accountOwner, entryPoint) @@ -413,7 +422,7 @@ describe('EntryPoint', function () { const incNonceKey = 5 const incrementCallData = entryPoint.interface.encodeFunctionData('incrementNonce', [incNonceKey]) const callData = account.interface.encodeFunctionData('execute', [entryPoint.address, 0, incrementCallData]) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender, callData, nonce: await entryPoint.getNonce(sender, key) @@ -423,7 +432,7 @@ describe('EntryPoint', function () { expect(await entryPoint.getNonce(sender, incNonceKey)).to.equal(BigNumber.from(incNonceKey).shl(64).add(1)) }) it('should fail with nonsequential seq', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender, nonce: keyShifted.add(3) }, accountOwner, entryPoint) @@ -445,7 +454,7 @@ describe('EntryPoint', function () { it('should revert on signature failure', async () => { // wallet-reported signature failure should revert in handleOps const wrongOwner = createAccountOwner() - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address }, wrongOwner, entryPoint) const beneficiaryAddress = createAddress() @@ -453,7 +462,7 @@ describe('EntryPoint', function () { }) it('account should pay for tx', async function () { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: accountExecFromEntryPoint.data, verificationGasLimit: 1e6, @@ -485,7 +494,7 @@ describe('EntryPoint', function () { const iterations = 45 const count = await counter.populateTransaction.gasWaster(iterations, '') const accountExec = await account.populateTransaction.execute(counter.address, 0, count.data!) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: accountExec.data, verificationGasLimit: 1e5, @@ -518,7 +527,7 @@ describe('EntryPoint', function () { const iterations = 45 const count = await counter.populateTransaction.gasWaster(iterations, '') const accountExec = await account.populateTransaction.execute(counter.address, 0, count.data!) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: accountExec.data, verificationGasLimit: 1e5, @@ -544,7 +553,7 @@ describe('EntryPoint', function () { }) it('legacy mode (maxPriorityFee==maxFeePerGas) should not use "basefee" opcode', async function () { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: accountExecFromEntryPoint.data, maxPriorityFeePerGas: 10e9, @@ -567,7 +576,7 @@ describe('EntryPoint', function () { it('if account has a deposit, it should use it to pay', async function () { await account.addDeposit({ value: ONE_ETH }) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: accountExecFromEntryPoint.data, verificationGasLimit: 1e6, @@ -602,7 +611,7 @@ describe('EntryPoint', function () { }) it('should pay for reverted tx', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: '0xdeadface', verificationGasLimit: 1e6, @@ -623,7 +632,7 @@ describe('EntryPoint', function () { it('#handleOp (single)', async () => { const beneficiaryAddress = createAddress() - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: accountExecFromEntryPoint.data }, accountOwner, entryPoint) @@ -644,7 +653,7 @@ describe('EntryPoint', function () { const callHandleOps = entryPoint.interface.encodeFunctionData('handleOps', [[], beneficiaryAddress]) const execHandlePost = account.interface.encodeFunctionData('execute', [entryPoint.address, 0, callHandleOps]) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, callData: execHandlePost }, accountOwner, entryPoint) @@ -657,14 +666,14 @@ describe('EntryPoint', function () { expect(decodeRevertReason(error?.args?.revertReason)).to.eql('Error(ReentrancyGuard: reentrant call)', 'execution of handleOps inside a UserOp should revert') }) it('should report failure on insufficient verificationGas after creation', async () => { - const op0 = await fillAndSign({ + const op0 = await fillSignAndPack({ sender: account.address, verificationGasLimit: 5e5 }, accountOwner, entryPoint) // must succeed with enough verification gas await simulateValidation(op0, entryPoint.address) - const op1 = await fillAndSign({ + const op1 = await fillSignAndPack({ sender: account.address, verificationGasLimit: 10000 }, accountOwner, entryPoint) @@ -677,11 +686,11 @@ describe('EntryPoint', function () { if (process.env.COVERAGE != null) { return } - let createOp: UserOperation + let createOp: PackedUserOperation const beneficiaryAddress = createAddress() // 1 it('should reject create if sender address is wrong', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ initCode: getAccountInitCode(accountOwner.address, simpleAccountFactory), verificationGasLimit: 2e6, sender: '0x'.padEnd(42, '1') @@ -693,7 +702,7 @@ describe('EntryPoint', function () { }) it('should reject create if account not funded', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ initCode: getAccountInitCode(accountOwner.address, simpleAccountFactory, 100), verificationGasLimit: 2e6 }, accountOwner, entryPoint) @@ -712,7 +721,7 @@ describe('EntryPoint', function () { const salt = 20 const preAddr = await getAccountAddress(accountOwner.address, simpleAccountFactory, salt) await fund(preAddr) - createOp = await fillAndSign({ + createOp = await fillSignAndPack({ initCode: getAccountInitCode(accountOwner.address, simpleAccountFactory, salt), callGasLimit: 1e6, verificationGasLimit: 2e6 @@ -772,14 +781,14 @@ describe('EntryPoint', function () { await fund(account1) await fund(account2.address) // execute and increment counter - const op1 = await fillAndSign({ + const op1 = await fillSignAndPack({ initCode: getAccountInitCode(accountOwner1.address, simpleAccountFactory), callData: accountExecCounterFromEntryPoint.data, callGasLimit: 2e6, verificationGasLimit: 2e6 }, accountOwner1, entryPoint) - const op2 = await fillAndSign({ + const op2 = await fillSignAndPack({ callData: accountExecCounterFromEntryPoint.data, sender: account2.address, callGasLimit: 2e6, @@ -819,7 +828,7 @@ describe('EntryPoint', function () { await ethersSigner.sendTransaction({ to: aggAccount2.address, value: parseEther('0.1') }) }) it('should fail to execute aggregated account without an aggregator', async () => { - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: aggAccount.address }, accountOwner, entryPoint) @@ -827,7 +836,7 @@ describe('EntryPoint', function () { await expect(entryPoint.handleOps([userOp], beneficiaryAddress)).to.revertedWith('AA24 signature error') }) it('should fail to execute aggregated account with wrong aggregator', async () => { - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: aggAccount.address }, accountOwner, entryPoint) @@ -846,7 +855,7 @@ describe('EntryPoint', function () { const address1 = hexZeroPad('0x1', 20) const aggAccount1 = await new TestAggregatedAccount__factory(ethersSigner).deploy(entryPoint.address, address1) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: aggAccount1.address, maxFeePerGas: 0 }, accountOwner, entryPoint) @@ -863,7 +872,7 @@ describe('EntryPoint', function () { }) it('should fail to execute aggregated account with wrong agg. signature', async () => { - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: aggAccount.address }, accountOwner, entryPoint) @@ -882,16 +891,16 @@ describe('EntryPoint', function () { const aggAccount3 = await new TestAggregatedAccount__factory(ethersSigner).deploy(entryPoint.address, aggregator3.address) await ethersSigner.sendTransaction({ to: aggAccount3.address, value: parseEther('0.1') }) - const userOp1 = await fillAndSign({ + const userOp1 = await fillSignAndPack({ sender: aggAccount.address }, accountOwner, entryPoint) - const userOp2 = await fillAndSign({ + const userOp2 = await fillSignAndPack({ sender: aggAccount2.address }, accountOwner, entryPoint) - const userOp_agg3 = await fillAndSign({ + const userOp_agg3 = await fillSignAndPack({ sender: aggAccount3.address }, accountOwner, entryPoint) - const userOp_noAgg = await fillAndSign({ + const userOp_noAgg = await fillSignAndPack({ sender: account.address }, accountOwner, entryPoint) @@ -957,13 +966,13 @@ describe('EntryPoint', function () { context('create account', () => { let initCode: BytesLike let addr: string - let userOp: UserOperation + let userOp: PackedUserOperation before(async () => { const factory = await new TestAggregatedAccountFactory__factory(ethersSigner).deploy(entryPoint.address, aggregator.address) initCode = await getAggregatedAccountInitCode(entryPoint.address, factory) addr = await entryPoint.callStatic.getSenderAddress(initCode).catch(e => e.errorArgs.sender) await ethersSigner.sendTransaction({ to: addr, value: parseEther('0.1') }) - userOp = await fillAndSign({ + userOp = await fillSignAndPack({ initCode }, accountOwner, entryPoint) }) @@ -1003,8 +1012,10 @@ describe('EntryPoint', function () { it('should fail with nonexistent paymaster', async () => { const pm = createAddress() - const op = await fillAndSign({ - paymasterAndData: pm, + const op = await fillSignAndPack({ + paymaster: pm, + paymasterData: '0x', + paymasterVerificationGasLimit: 3e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(account2Owner.address, simpleAccountFactory), verificationGasLimit: 3e6, @@ -1014,8 +1025,10 @@ describe('EntryPoint', function () { }) it('should fail if paymaster has no deposit', async function () { - const op = await fillAndSign({ - paymasterAndData: paymaster.address, + const op = await fillSignAndPack({ + paymaster: paymaster.address, + paymasterData: '0x', + paymasterVerificationGasLimit: 3e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(account2Owner.address, simpleAccountFactory), @@ -1032,8 +1045,10 @@ describe('EntryPoint', function () { await errorPostOp.addStake(globalUnstakeDelaySec, { value: paymasterStake }) await errorPostOp.deposit({ value: ONE_ETH }) - const op = await fillAndSign({ - paymasterAndData: errorPostOp.address, + const op = await fillSignAndPack({ + paymaster: errorPostOp.address, + paymasterData: '0x', + paymasterVerificationGasLimit: 3e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(account3Owner.address, simpleAccountFactory), @@ -1046,8 +1061,10 @@ describe('EntryPoint', function () { it('paymaster should pay for tx', async function () { await paymaster.deposit({ value: ONE_ETH }) - const op = await fillAndSign({ - paymasterAndData: paymaster.address, + const op = await fillSignAndPack({ + paymaster: paymaster.address, + paymasterData: '0x', + paymasterVerificationGasLimit: 1e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(account2Owner.address, simpleAccountFactory) }, account2Owner, entryPoint) @@ -1063,8 +1080,10 @@ describe('EntryPoint', function () { await paymaster.deposit({ value: ONE_ETH }) const anOwner = createAccountOwner() - const op = await fillAndSign({ - paymasterAndData: paymaster.address, + const op = await fillSignAndPack({ + paymaster: paymaster.address, + paymasterData: '0x', + paymasterVerificationGasLimit: 1e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(anOwner.address, simpleAccountFactory) }, anOwner, entryPoint) @@ -1097,7 +1116,7 @@ describe('EntryPoint', function () { describe('validateUserOp time-range', function () { it('should accept non-expired owner', async () => { - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address }, sessionOwner, entryPoint) const ret = await simulateValidation(userOp, entryPoint.address) @@ -1108,7 +1127,7 @@ describe('EntryPoint', function () { it('should not reject expired owner', async () => { const expiredOwner = createAccountOwner() await account.addTemporaryOwner(expiredOwner.address, 123, now - 60) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address }, expiredOwner, entryPoint) const ret = await simulateValidation(userOp, entryPoint.address) @@ -1130,9 +1149,10 @@ describe('EntryPoint', function () { it('should accept non-expired paymaster request', async () => { const timeRange = defaultAbiCoder.encode(['uint48', 'uint48'], [123, now + 60]) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address, - paymasterAndData: hexConcat([paymaster.address, timeRange]) + paymaster: paymaster.address, + paymasterData: timeRange }, ethersSigner, entryPoint) const ret = await simulateValidation(userOp, entryPoint.address) expect(ret.returnInfo.validUntil).to.eql(now + 60) @@ -1141,9 +1161,10 @@ describe('EntryPoint', function () { it('should not reject expired paymaster request', async () => { const timeRange = defaultAbiCoder.encode(['uint48', 'uint48'], [321, now - 60]) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address, - paymasterAndData: hexConcat([paymaster.address, timeRange]) + paymaster: paymaster.address, + paymasterData: timeRange, }, ethersSigner, entryPoint) const ret = await simulateValidation(userOp, entryPoint.address) expect(ret.returnInfo.validUntil).to.eql(now - 60) @@ -1151,11 +1172,12 @@ describe('EntryPoint', function () { }) // helper method - async function createOpWithPaymasterParams (owner: Wallet, after: number, until: number): Promise { + async function createOpWithPaymasterParams (owner: Wallet, after: number, until: number): Promise { const timeRange = defaultAbiCoder.encode(['uint48', 'uint48'], [after, until]) - return await fillAndSign({ + return await fillSignAndPack({ sender: account.address, - paymasterAndData: hexConcat([paymaster.address, timeRange]) + paymaster: paymaster.address, + paymasterData: timeRange, }, owner, entryPoint) } @@ -1197,7 +1219,7 @@ describe('EntryPoint', function () { it('should revert on expired account', async () => { const expiredOwner = createAccountOwner() await account.addTemporaryOwner(expiredOwner.address, 1, 2) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address }, expiredOwner, entryPoint) await expect(entryPoint.handleOps([userOp], beneficiary)) @@ -1207,7 +1229,7 @@ describe('EntryPoint', function () { it('should revert on date owner', async () => { const futureOwner = createAccountOwner() await account.addTemporaryOwner(futureOwner.address, now + 100, now + 200) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address }, futureOwner, entryPoint) await expect(entryPoint.handleOps([userOp], beneficiary)) diff --git a/test/simple-wallet.test.ts b/test/simple-wallet.test.ts index 12d2ca70..8e455310 100644 --- a/test/simple-wallet.test.ts +++ b/test/simple-wallet.test.ts @@ -20,7 +20,7 @@ import { ONE_ETH, HashZero, deployEntryPoint } from './testutils' -import { fillUserOpDefaults, getUserOpHash, packUserOp, signUserOp } from './UserOp' +import { fillUserOpDefaults, getUserOpHash, encodeUserOp, signUserOp } from './UserOp' import { parseEther } from 'ethers/lib/utils' import { UserOperation } from './UserOperation' @@ -53,7 +53,7 @@ describe('SimpleAccount', function () { it('should pack in js the same as solidity', async () => { const op = await fillUserOpDefaults({ sender: accounts[0] }) - const packed = packUserOp(op) + const packed = encodeUserOp(op) expect(await testUtil.packUserOp(op)).to.equal(packed) }) diff --git a/test/solidityTypes.ts b/test/solidityTypes.ts index 5026ef9e..5a051e99 100644 --- a/test/solidityTypes.ts +++ b/test/solidityTypes.ts @@ -6,5 +6,6 @@ export type address = string export type uint256 = BigNumberish export type uint = BigNumberish export type uint48 = BigNumberish +export type uint128 = BigNumberish export type bytes = BytesLike export type bytes32 = BytesLike diff --git a/test/testutils.ts b/test/testutils.ts index a773f364..82fe2065 100644 --- a/test/testutils.ts +++ b/test/testutils.ts @@ -1,7 +1,7 @@ import { ethers } from 'hardhat' import { arrayify, - hexConcat, + hexConcat, hexlify, hexZeroPad, keccak256, parseEther } from 'ethers/lib/utils' @@ -22,6 +22,7 @@ import { expect } from 'chai' import { Create2Factory } from '../src/Create2Factory' import { debugTransaction } from './debugTx' import { UserOperation } from './UserOperation' +import { Hexable } from '@ethersproject/bytes/src.ts' export const AddressZero = ethers.constants.AddressZero export const HashZero = ethers.constants.HashZero @@ -286,3 +287,15 @@ export async function createAccount ( proxy } } + +export function packAccountGasLimits (validationGasLimit: BytesLike | Hexable | number | bigint, callGasLimit: BytesLike | Hexable | number | bigint): string { + return ethers.utils.hexConcat([hexZeroPad(hexlify(validationGasLimit, {hexPad: 'left'}),16), hexZeroPad(hexlify(callGasLimit, {hexPad: 'left'}),16)]) +} + +export function packPaymasterData (paymaster: string, paymasterVerificationGasLimit: BytesLike | Hexable | number | bigint, postOpGasLimit: BytesLike | Hexable | number | bigint, paymasterData: string): string { + return ethers.utils.hexConcat([paymaster, hexZeroPad(hexlify(paymasterVerificationGasLimit, {hexPad: 'left'}),16), hexZeroPad(hexlify(postOpGasLimit, {hexPad: 'left'}),16), paymasterData]) +} + +export function unpackAccountGasLimits (accountGasLimits: string): {validationGasLimit: number, callGasLimit: number} { + return { validationGasLimit: parseInt(accountGasLimits.slice(2, 34), 16), callGasLimit: parseInt(accountGasLimits.slice(34), 16) } +} From f27b2d6bd93c67ad16f959a6d523918fd3ec9646 Mon Sep 17 00:00:00 2001 From: shahafn Date: Mon, 4 Dec 2023 22:11:27 +0200 Subject: [PATCH 05/24] Fixing (almost all) tests --- contracts/samples/VerifyingPaymaster.sol | 4 +- contracts/samples/gnosis/EIP4337Manager.sol | 4 +- test/UserOp.ts | 6 ++- test/entrypointsimulations.test.ts | 30 +++++++------- test/gnosis.test.ts | 18 ++++----- test/paymaster.test.ts | 43 ++++++++++++--------- test/simple-wallet.test.ts | 13 ++++--- test/verifying_paymaster.test.ts | 39 ++++++++++++------- test/y.bls.test.ts | 18 ++++----- 9 files changed, 99 insertions(+), 76 deletions(-) diff --git a/contracts/samples/VerifyingPaymaster.sol b/contracts/samples/VerifyingPaymaster.sol index eaa7e432..fad2a99f 100644 --- a/contracts/samples/VerifyingPaymaster.sol +++ b/contracts/samples/VerifyingPaymaster.sol @@ -23,9 +23,9 @@ contract VerifyingPaymaster is BasePaymaster { address public immutable verifyingSigner; - uint256 private constant VALID_TIMESTAMP_OFFSET = 20; + uint256 private constant VALID_TIMESTAMP_OFFSET = 52; - uint256 private constant SIGNATURE_OFFSET = 84; + uint256 private constant SIGNATURE_OFFSET = 116; constructor(IEntryPoint _entryPoint, address _verifyingSigner) BasePaymaster(_entryPoint) { verifyingSigner = _verifyingSigner; diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol index 6abb05c4..be0beaf0 100644 --- a/contracts/samples/gnosis/EIP4337Manager.sol +++ b/contracts/samples/gnosis/EIP4337Manager.sol @@ -151,8 +151,8 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor { /** * Validate this gnosisSafe is callable through the EntryPoint. - * the test is might be incomplete: we check that we reach our validateUserOp and fail on signature. - * we don't test full transaction + * the test might be incomplete: we check that we reach our validateUserOp and fail on signature. + * we don't test a full transaction */ function validateEip4337(GnosisSafe safe, EIP4337Manager manager) public { diff --git a/test/UserOp.ts b/test/UserOp.ts index 29639f26..828356c9 100644 --- a/test/UserOp.ts +++ b/test/UserOp.ts @@ -80,7 +80,7 @@ export const DefaultsForUserOp: UserOperation = { maxPriorityFeePerGas: 1e9, paymaster: '0x', paymasterData: '0x', - paymasterVerificationGasLimit: 2e5, + paymasterVerificationGasLimit: 3e5, paymasterPostOpGasLimit: 0, signature: '0x' } @@ -202,6 +202,10 @@ export async function fillUserOp (op: Partial, entryPoint?: Entry return op2 } +export async function fillAndPack (op: Partial, entryPoint?: EntryPoint, getNonceFunction = 'getNonce'): Promise { + return packUserOp(await fillUserOp(op, entryPoint, getNonceFunction)) +} + export async function fillAndSign (op: Partial, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'getNonce'): Promise { const provider = entryPoint?.provider const op2 = await fillUserOp(op, entryPoint, getNonceFunction) diff --git a/test/entrypointsimulations.test.ts b/test/entrypointsimulations.test.ts index 8920057a..91510039 100644 --- a/test/entrypointsimulations.test.ts +++ b/test/entrypointsimulations.test.ts @@ -18,7 +18,7 @@ import { getBalance, deployEntryPoint } from './testutils' -import { fillAndSign, simulateHandleOp, simulateValidation } from './UserOp' +import { fillSignAndPack, simulateHandleOp, simulateValidation } from './UserOp' import { BigNumber, Wallet } from 'ethers' import { hexConcat } from 'ethers/lib/utils' @@ -115,7 +115,7 @@ describe('EntryPointSimulations', function () { it('should fail if validateUserOp fails', async () => { // using wrong nonce - const op = await fillAndSign({ sender: account.address, nonce: 1234 }, accountOwner, entryPoint) + const op = await fillSignAndPack({ sender: account.address, nonce: 1234 }, accountOwner, entryPoint) await expect(simulateValidation(op, entryPoint.address)).to .revertedWith('AA25 invalid account nonce') }) @@ -124,13 +124,13 @@ describe('EntryPointSimulations', function () { // (this is actually a feature of the wallet, not the entrypoint) // using wrong owner for account1 // (zero gas price so that it doesn't fail on prefund) - const op = await fillAndSign({ sender: account1.address, maxFeePerGas: 0 }, accountOwner, entryPoint) + const op = await fillSignAndPack({ sender: account1.address, maxFeePerGas: 0 }, accountOwner, entryPoint) const { returnInfo } = await simulateValidation(op, entryPoint.address) expect(returnInfo.sigFailed).to.be.true }) it('should revert if wallet not deployed (and no initCode)', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: createAddress(), nonce: 0, verificationGasLimit: 1000 @@ -140,19 +140,19 @@ describe('EntryPointSimulations', function () { }) it('should revert on oog if not enough verificationGas', async () => { - const op = await fillAndSign({ sender: account.address, verificationGasLimit: 1000 }, accountOwner, entryPoint) + const op = await fillSignAndPack({ sender: account.address, verificationGasLimit: 1000 }, accountOwner, entryPoint) await expect(simulateValidation(op, entryPoint.address)).to .revertedWith('AA23 reverted (or OOG)') }) it('should succeed if validateUserOp succeeds', async () => { - const op = await fillAndSign({ sender: account1.address }, accountOwner1, entryPoint) + const op = await fillSignAndPack({ sender: account1.address }, accountOwner1, entryPoint) await fund(account1) await simulateValidation(op, entryPoint.address) }) it('should return empty context if no paymaster', async () => { - const op = await fillAndSign({ sender: account1.address, maxFeePerGas: 0 }, accountOwner1, entryPoint) + const op = await fillSignAndPack({ sender: account1.address, maxFeePerGas: 0 }, accountOwner1, entryPoint) const { returnInfo } = await simulateValidation(op, entryPoint.address) expect(returnInfo.paymasterContext).to.eql('0x') }) @@ -163,14 +163,14 @@ describe('EntryPointSimulations', function () { const { proxy: account2 } = await createAccount(ethersSigner, await ethersSigner.getAddress(), entryPoint.address) await fund(account2) await account2.execute(entryPoint.address, stakeValue, entryPoint.interface.encodeFunctionData('addStake', [unstakeDelay])) - const op = await fillAndSign({ sender: account2.address }, ethersSigner, entryPoint) + const op = await fillSignAndPack({ sender: account2.address }, ethersSigner, entryPoint) const result = await simulateValidation(op, entryPoint.address) expect(result.senderInfo.stake).to.equal(stakeValue) expect(result.senderInfo.unstakeDelaySec).to.equal(unstakeDelay) }) it('should prevent overflows: fail if any numeric value is more than 120 bits', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ preVerificationGas: BigNumber.from(2).pow(130), sender: account1.address }, accountOwner1, entryPoint) @@ -180,7 +180,7 @@ describe('EntryPointSimulations', function () { }) it('should fail creation for wrong sender', async () => { - const op1 = await fillAndSign({ + const op1 = await fillSignAndPack({ initCode: getAccountInitCode(accountOwner1.address, simpleAccountFactory), sender: '0x'.padEnd(42, '1'), verificationGasLimit: 30e6 @@ -192,7 +192,7 @@ describe('EntryPointSimulations', function () { it('should report failure on insufficient verificationGas (OOG) for creation', async () => { const initCode = getAccountInitCode(accountOwner1.address, simpleAccountFactory) const sender = await entryPoint.callStatic.getSenderAddress(initCode).catch(e => e.errorArgs.sender) - const op0 = await fillAndSign({ + const op0 = await fillSignAndPack({ initCode, sender, verificationGasLimit: 5e5, @@ -201,7 +201,7 @@ describe('EntryPointSimulations', function () { // must succeed with enough verification gas. await simulateValidation(op0, entryPoint.address, { gas: '0xF4240' }) - const op1 = await fillAndSign({ + const op1 = await fillSignAndPack({ initCode, sender, verificationGasLimit: 1e5, @@ -213,7 +213,7 @@ describe('EntryPointSimulations', function () { it('should succeed for creating an account', async () => { const sender = await getAccountAddress(accountOwner1.address, simpleAccountFactory) - const op1 = await fillAndSign({ + const op1 = await fillSignAndPack({ sender, initCode: getAccountInitCode(accountOwner1.address, simpleAccountFactory) }, accountOwner1, entryPoint) @@ -226,7 +226,7 @@ describe('EntryPointSimulations', function () { // a possible attack: call an account's execFromEntryPoint through initCode. This might lead to stolen funds. const { proxy: account } = await createAccount(ethersSigner, await accountOwner.getAddress(), entryPoint.address) const sender = createAddress() - const op1 = await fillAndSign({ + const op1 = await fillSignAndPack({ initCode: hexConcat([ account.address, account.interface.encodeFunctionData('execute', [sender, 0, '0x']) @@ -248,7 +248,7 @@ describe('EntryPointSimulations', function () { const count = counter.interface.encodeFunctionData('count') const callData = account.interface.encodeFunctionData('execute', [counter.address, 0, count]) // deliberately broken signature. simulate should work with it too. - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address, callData }, accountOwner1, entryPoint) diff --git a/test/gnosis.test.ts b/test/gnosis.test.ts index 24819edf..35cd132a 100644 --- a/test/gnosis.test.ts +++ b/test/gnosis.test.ts @@ -25,7 +25,7 @@ import { HashZero, isDeployed } from './testutils' -import { fillAndSign } from './UserOp' +import { fillSignAndPack } from './UserOp' import { defaultAbiCoder, hexConcat, hexZeroPad, parseEther } from 'ethers/lib/utils' import { expect } from 'chai' @@ -101,7 +101,7 @@ describe('Gnosis Proxy', function () { }) it('should fail from wrong entrypoint', async function () { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: proxy.address }, owner, entryPoint, 'getNonce') @@ -111,7 +111,7 @@ describe('Gnosis Proxy', function () { }) it('should fail on invalid userop', async function () { - let op = await fillAndSign({ + let op = await fillSignAndPack({ sender: proxy.address, nonce: 1234, callGasLimit: 1e6, @@ -119,18 +119,18 @@ describe('Gnosis Proxy', function () { }, owner, entryPoint, 'getNonce') await expect(entryPoint.handleOps([op], beneficiary)).to.revertedWith('AA25 invalid account nonce') - op = await fillAndSign({ + op = await fillSignAndPack({ sender: proxy.address, callGasLimit: 1e6, callData: safe_execTxCallData }, owner, entryPoint, 'getNonce') // invalidate the signature - op.callGasLimit = 1 + op.maxFeePerGas = 1 await expect(entryPoint.handleOps([op], beneficiary)).to.revertedWith('FailedOp(0, "AA24 signature error")') }) it('should exec', async function () { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: proxy.address, callGasLimit: 1e6, callData: safe_execTxCallData @@ -147,7 +147,7 @@ describe('Gnosis Proxy', function () { const counter_countFailCallData = counter.interface.encodeFunctionData('countFail') const safe_execFailTxCallData = manager.interface.encodeFunctionData('executeAndRevert', [counter.address, 0, counter_countFailCallData, 0]) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: proxy.address, callGasLimit: 1e6, callData: safe_execFailTxCallData @@ -180,7 +180,7 @@ describe('Gnosis Proxy', function () { to: counterfactualAddress, value: parseEther('0.1') }) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: counterfactualAddress, initCode, verificationGasLimit: 400000 @@ -198,7 +198,7 @@ describe('Gnosis Proxy', function () { if (counterfactualAddress == null) this.skip() expect(await isDeployed(counterfactualAddress)) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: counterfactualAddress, callData: safe_execTxCallData }, owner, entryPoint, 'getNonce') diff --git a/test/paymaster.test.ts b/test/paymaster.test.ts index 5fcb410e..a494099b 100644 --- a/test/paymaster.test.ts +++ b/test/paymaster.test.ts @@ -24,9 +24,9 @@ import { createAccount, getAccountAddress } from './testutils' -import { fillAndSign, simulateValidation } from './UserOp' +import { fillSignAndPack, simulateValidation } from './UserOp' import { hexConcat, parseEther } from 'ethers/lib/utils' -import { UserOperation } from './UserOperation' +import { PackedUserOperation } from './UserOperation' import { hexValue } from '@ethersproject/bytes' describe('EntryPoint with paymaster', function () { @@ -95,9 +95,10 @@ describe('EntryPoint with paymaster', function () { calldata = await account.populateTransaction.execute(account.address, 0, updateEntryPoint).then(tx => tx.data!) }) it('paymaster should reject if account doesn\'t have tokens', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account.address, - paymasterAndData: paymaster.address, + paymaster: paymaster.address, + paymasterPostOpGasLimit: 3e5, callData: calldata }, accountOwner, entryPoint) await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress, { @@ -110,15 +111,16 @@ describe('EntryPoint with paymaster', function () { }) describe('create account', () => { - let createOp: UserOperation + let createOp: PackedUserOperation let created = false const beneficiaryAddress = createAddress() it('should reject if account not funded', async () => { - const op = await fillAndSign({ + const op = await fillSignAndPack({ initCode: getAccountDeployer(entryPoint.address, accountOwner.address, 1), verificationGasLimit: 1e7, - paymasterAndData: paymaster.address + paymaster: paymaster.address, + paymasterPostOpGasLimit: 3e5, }, accountOwner, entryPoint) await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress, { gasLimit: 1e7 @@ -126,10 +128,11 @@ describe('EntryPoint with paymaster', function () { }) it('should succeed to create account with tokens', async () => { - createOp = await fillAndSign({ + createOp = await fillSignAndPack({ initCode: getAccountDeployer(entryPoint.address, accountOwner.address, 3), verificationGasLimit: 2e6, - paymasterAndData: paymaster.address, + paymaster: paymaster.address, + paymasterPostOpGasLimit: 3e5, nonce: 0 }, accountOwner, entryPoint) @@ -180,16 +183,17 @@ describe('EntryPoint with paymaster', function () { const justEmit = testCounter.interface.encodeFunctionData('justemit') const execFromSingleton = account.interface.encodeFunctionData('execute', [testCounter.address, 0, justEmit]) - const ops: UserOperation[] = [] + const ops: PackedUserOperation[] = [] const accounts: SimpleAccount[] = [] for (let i = 0; i < 4; i++) { const { proxy: aAccount } = await createAccount(ethersSigner, await accountOwner.getAddress(), entryPoint.address) await paymaster.mintTokens(aAccount.address, parseEther('1')) - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: aAccount.address, callData: execFromSingleton, - paymasterAndData: paymaster.address + paymaster: paymaster.address, + paymasterPostOpGasLimit: 3e5 }, accountOwner, entryPoint) accounts.push(aAccount) @@ -222,10 +226,11 @@ describe('EntryPoint with paymaster', function () { await paymaster.mintTokens(account.address, parseEther('1')) approveCallData = paymaster.interface.encodeFunctionData('approve', [account.address, ethers.constants.MaxUint256]) // need to call approve from account2. use paymaster for that - const approveOp = await fillAndSign({ + const approveOp = await fillSignAndPack({ sender: account2.address, callData: account2.interface.encodeFunctionData('execute', [paymaster.address, 0, approveCallData]), - paymasterAndData: paymaster.address + paymaster: paymaster.address, + paymasterPostOpGasLimit: 3e5 }, accountOwner, entryPoint) await entryPoint.handleOps([approveOp], beneficiaryAddress) expect(await paymaster.allowance(account2.address, account.address)).to.eq(ethers.constants.MaxUint256) @@ -240,17 +245,19 @@ describe('EntryPoint with paymaster', function () { const withdrawTokens = paymaster.interface.encodeFunctionData('transferFrom', [account2.address, account.address, withdrawAmount]) const execFromEntryPoint = account.interface.encodeFunctionData('execute', [paymaster.address, 0, withdrawTokens]) - const userOp1 = await fillAndSign({ + const userOp1 = await fillSignAndPack({ sender: account.address, callData: execFromEntryPoint, - paymasterAndData: paymaster.address + paymaster: paymaster.address, + paymasterPostOpGasLimit: 3e5 }, accountOwner, entryPoint) // account2's operation is unimportant, as it is going to be reverted - but the paymaster will have to pay for it. - const userOp2 = await fillAndSign({ + const userOp2 = await fillSignAndPack({ sender: account2.address, callData: execFromEntryPoint, - paymasterAndData: paymaster.address, + paymaster: paymaster.address, + paymasterPostOpGasLimit: 3e5, callGasLimit: 1e6 }, accountOwner, entryPoint) diff --git a/test/simple-wallet.test.ts b/test/simple-wallet.test.ts index 8e455310..7ef73912 100644 --- a/test/simple-wallet.test.ts +++ b/test/simple-wallet.test.ts @@ -20,7 +20,7 @@ import { ONE_ETH, HashZero, deployEntryPoint } from './testutils' -import { fillUserOpDefaults, getUserOpHash, encodeUserOp, signUserOp } from './UserOp' +import { fillUserOpDefaults, getUserOpHash, encodeUserOp, signUserOp, packUserOp } from './UserOp' import { parseEther } from 'ethers/lib/utils' import { UserOperation } from './UserOperation' @@ -53,8 +53,9 @@ describe('SimpleAccount', function () { it('should pack in js the same as solidity', async () => { const op = await fillUserOpDefaults({ sender: accounts[0] }) - const packed = encodeUserOp(op) - expect(await testUtil.packUserOp(op)).to.equal(packed) + const encoded = encodeUserOp(op) + const packed = packUserOp(op) + expect(await testUtil.packUserOp(packed)).to.equal(encoded) }) describe('#executeBatch', () => { @@ -135,7 +136,8 @@ describe('SimpleAccount', function () { expectedPay = actualGasPrice * (callGasLimit + verificationGasLimit) preBalance = await getBalance(account.address) - const ret = await account.validateUserOp(userOp, userOpHash, expectedPay, { gasPrice: actualGasPrice }) + const packedOp = packUserOp(userOp) + const ret = await account.validateUserOp(packedOp, userOpHash, expectedPay, { gasPrice: actualGasPrice }) await ret.wait() }) @@ -146,7 +148,8 @@ describe('SimpleAccount', function () { it('should return NO_SIG_VALIDATION on wrong signature', async () => { const userOpHash = HashZero - const deadline = await account.callStatic.validateUserOp({ ...userOp, nonce: 1 }, userOpHash, 0) + const packedOp = packUserOp(userOp) + const deadline = await account.callStatic.validateUserOp({ ...packedOp, nonce: 1 }, userOpHash, 0) expect(deadline).to.eq(1) }) }) diff --git a/test/verifying_paymaster.test.ts b/test/verifying_paymaster.test.ts index 5103f2cb..06711a55 100644 --- a/test/verifying_paymaster.test.ts +++ b/test/verifying_paymaster.test.ts @@ -12,9 +12,9 @@ import { createAccountOwner, createAddress, deployEntryPoint } from './testutils' -import { fillAndSign, simulateValidation } from './UserOp' -import { arrayify, defaultAbiCoder, hexConcat, parseEther } from 'ethers/lib/utils' -import { UserOperation } from './UserOperation' +import { DefaultsForUserOp, fillAndSign, fillSignAndPack, packUserOp, simulateValidation } from './UserOp' +import { arrayify, defaultAbiCoder, hexConcat, hexlify, hexZeroPad, parseEther } from 'ethers/lib/utils' +import { PackedUserOperation } from './UserOperation' const MOCK_VALID_UNTIL = '0x00000000deadbeef' const MOCK_VALID_AFTER = '0x0000000000001234' @@ -43,9 +43,13 @@ describe('EntryPoint with VerifyingPaymaster', function () { describe('#parsePaymasterAndData', () => { it('should parse data properly', async () => { - const paymasterAndData = hexConcat([paymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), MOCK_SIG]) + const paymasterAndData = hexConcat([paymaster.address, hexZeroPad(hexlify(DefaultsForUserOp.paymasterVerificationGasLimit, {hexPad: 'left'}),16), hexZeroPad(hexlify(DefaultsForUserOp.paymasterPostOpGasLimit, {hexPad: 'left'}),16), defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), MOCK_SIG]) console.log(paymasterAndData) const res = await paymaster.parsePaymasterAndData(paymasterAndData) + console.log('MOCK_VALID_UNTIL, MOCK_VALID_AFTER', MOCK_VALID_UNTIL, MOCK_VALID_AFTER) + console.log('validUntil after', res.validUntil, res.validAfter) + console.log('MOCK SIG', MOCK_SIG) + console.log('sig', res.signature) expect(res.validUntil).to.be.equal(ethers.BigNumber.from(MOCK_VALID_UNTIL)) expect(res.validAfter).to.be.equal(ethers.BigNumber.from(MOCK_VALID_AFTER)) expect(res.signature).equal(MOCK_SIG) @@ -54,29 +58,32 @@ describe('EntryPoint with VerifyingPaymaster', function () { describe('#validatePaymasterUserOp', () => { it('should reject on no signature', async () => { - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address, - paymasterAndData: hexConcat([paymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x1234']) + paymaster: paymaster.address, + paymasterData: hexConcat([defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x1234']) }, accountOwner, entryPoint) await expect(simulateValidation(userOp, entryPoint.address)).to.be.revertedWith('invalid signature length in paymasterAndData') }) it('should reject on invalid signature', async () => { - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address, - paymasterAndData: hexConcat([paymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) + paymaster: paymaster.address, + paymasterData: hexConcat([defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) }, accountOwner, entryPoint) await expect(simulateValidation(userOp, entryPoint.address)).to.be.revertedWith('ECDSA: invalid signature') }) describe('with wrong signature', () => { - let wrongSigUserOp: UserOperation + let wrongSigUserOp: PackedUserOperation const beneficiaryAddress = createAddress() before(async () => { const sig = await offchainSigner.signMessage(arrayify('0xdead')) - wrongSigUserOp = await fillAndSign({ + wrongSigUserOp = await fillSignAndPack({ sender: account.address, - paymasterAndData: hexConcat([paymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), sig]) + paymaster: paymaster.address, + paymasterData: hexConcat([defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), sig]) }, accountOwner, entryPoint) }) @@ -93,13 +100,15 @@ describe('EntryPoint with VerifyingPaymaster', function () { it('succeed with valid signature', async () => { const userOp1 = await fillAndSign({ sender: account.address, - paymasterAndData: hexConcat([paymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) + paymaster: paymaster.address, + paymasterData: hexConcat([defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) }, accountOwner, entryPoint) - const hash = await paymaster.getHash(userOp1, MOCK_VALID_UNTIL, MOCK_VALID_AFTER) + const hash = await paymaster.getHash(packUserOp(userOp1), MOCK_VALID_UNTIL, MOCK_VALID_AFTER) const sig = await offchainSigner.signMessage(arrayify(hash)) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ ...userOp1, - paymasterAndData: hexConcat([paymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), sig]) + paymaster: paymaster.address, + paymasterData: hexConcat([defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), sig]) }, accountOwner, entryPoint) const res = await simulateValidation(userOp, entryPoint.address) expect(res.returnInfo.sigFailed).to.be.false diff --git a/test/y.bls.test.ts b/test/y.bls.test.ts index 66305bc8..3dfcf82b 100644 --- a/test/y.bls.test.ts +++ b/test/y.bls.test.ts @@ -13,7 +13,7 @@ import { } from '../typechain' import { ethers } from 'hardhat' import { createAddress, deployEntryPoint, fund, ONE_ETH } from './testutils' -import { DefaultsForUserOp, fillUserOp, simulateValidation } from './UserOp' +import { DefaultsForUserOp, fillAndPack, fillUserOp, packUserOp, simulateValidation } from './UserOp' import { expect } from 'chai' import { keccak256 } from 'ethereumjs-util' import { hashToPoint } from '@thehubbleproject/bls/dist/mcl' @@ -66,14 +66,14 @@ describe('bls account', function () { const sig1 = signer1.sign('0x1234') const sig2 = signer2.sign('0x5678') const offChainSigResult = hexConcat(aggregate([sig1, sig2])) - const userOp1 = { ...DefaultsForUserOp, signature: hexConcat(sig1) } - const userOp2 = { ...DefaultsForUserOp, signature: hexConcat(sig2) } + const userOp1 = packUserOp({ ...DefaultsForUserOp, signature: hexConcat(sig1) }) + const userOp2 = packUserOp({ ...DefaultsForUserOp, signature: hexConcat(sig2) }) const solidityAggResult = await blsAgg.aggregateSignatures([userOp1, userOp2]) expect(solidityAggResult).to.equal(offChainSigResult) }) it('#userOpToMessage', async () => { - const userOp1 = await fillUserOp({ + const userOp1 = await fillAndPack({ sender: account1.address }, entrypoint) const requestHash = await blsAgg.getUserOpHash(userOp1) @@ -83,7 +83,7 @@ describe('bls account', function () { }) it('#validateUserOpSignature', async () => { - const userOp1 = await fillUserOp({ + const userOp1 = await fillAndPack({ sender: account1.address }, entrypoint) const requestHash = await blsAgg.getUserOpHash(userOp1) @@ -108,7 +108,7 @@ describe('bls account', function () { const res = await brokenAccountFactory.provider.call(deployTx) const acc = brokenAccountFactory.interface.decodeFunctionResult('createAccount', res)[0] await fund(acc) - const userOp = await fillUserOp({ + const userOp = await fillAndPack({ sender: acc, initCode: hexConcat([brokenAccountFactory.address, deployTx.data!]) }, entrypoint) @@ -135,14 +135,14 @@ describe('bls account', function () { it('validateSignatures', async function () { // yes, it does take long on hardhat, but quick on geth. this.timeout(30000) - const userOp1 = await fillUserOp({ + const userOp1 = await fillAndPack({ sender: account1.address }, entrypoint) const requestHash = await blsAgg.getUserOpHash(userOp1) const sig1 = signer1.sign(requestHash) userOp1.signature = hexConcat(sig1) - const userOp2 = await fillUserOp({ + const userOp2 = await fillAndPack({ sender: account2.address }, entrypoint) const requestHash2 = await blsAgg.getUserOpHash(userOp2) @@ -182,7 +182,7 @@ describe('bls account', function () { const verifier = new BlsVerifier(BLS_DOMAIN) const senderAddress = await entrypoint.callStatic.getSenderAddress(initCode).catch(e => e.errorArgs.sender) await fund(senderAddress, '0.01') - const userOp = await fillUserOp({ + const userOp = await fillAndPack({ sender: senderAddress, initCode }, entrypoint) From 287fe6053e5929b95ede50f888b93b4c3421a502 Mon Sep 17 00:00:00 2001 From: shahafn Date: Tue, 5 Dec 2023 00:01:50 +0200 Subject: [PATCH 06/24] Fixing TokenPaymaster tests --- contracts/samples/TokenPaymaster.sol | 4 +- test/samples/TokenPaymaster.test.ts | 75 +++++++++++++++++----------- 2 files changed, 48 insertions(+), 31 deletions(-) diff --git a/contracts/samples/TokenPaymaster.sol b/contracts/samples/TokenPaymaster.sol index ef338d31..f047ff53 100644 --- a/contracts/samples/TokenPaymaster.sol +++ b/contracts/samples/TokenPaymaster.sol @@ -123,7 +123,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { override returns (bytes memory context, uint256 validationResult) {unchecked { uint256 priceMarkup = tokenPaymasterConfig.priceMarkup; - uint256 paymasterAndDataLength = userOp.paymasterAndData.length - 20; + uint256 paymasterAndDataLength = userOp.paymasterAndData.length - 52; require(paymasterAndDataLength == 0 || paymasterAndDataLength == 32, "TPM: invalid data length" ); @@ -131,7 +131,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { // note: as price is in ether-per-token and we want more tokens increasing it means dividing it by markup uint256 cachedPriceWithMarkup = cachedPrice * PRICE_DENOMINATOR / priceMarkup; if (paymasterAndDataLength == 32) { - uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[20 : 52])); + uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[52 : 84])); if (clientSuppliedPrice < cachedPriceWithMarkup) { // note: smaller number means 'more ether per token' cachedPriceWithMarkup = clientSuppliedPrice; diff --git a/test/samples/TokenPaymaster.test.ts b/test/samples/TokenPaymaster.test.ts index b4a5d4ef..cd562a1a 100644 --- a/test/samples/TokenPaymaster.test.ts +++ b/test/samples/TokenPaymaster.test.ts @@ -1,5 +1,5 @@ import { BigNumberish, ContractReceipt, ContractTransaction, Wallet, utils, BigNumber } from 'ethers' -import { Interface, parseEther } from 'ethers/lib/utils' +import { hexlify, hexZeroPad, Interface, parseEther } from 'ethers/lib/utils' import { assert, expect } from 'chai' import { ethers } from 'hardhat' @@ -26,7 +26,7 @@ import { } from '../../typechain/contracts/samples/TokenPaymaster' import { checkForGeth, createAccount, createAccountOwner, deployEntryPoint, fund } from '../testutils' -import { fillUserOp, signUserOp } from '../UserOp' +import { fillUserOp, packUserOp, signUserOp } from '../UserOp' function generatePaymasterAndData (pm: string, tokenPrice?: BigNumberish): string { if (tokenPrice != null) { @@ -134,21 +134,21 @@ describe('TokenPaymaster', function () { it('paymaster should reject if account does not have enough tokens or allowance', async () => { const snapshot = await ethers.provider.send('evm_snapshot', []) - const paymasterAndData = generatePaymasterAndData(paymasterAddress) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) await expect( - entryPoint.handleOps([op], beneficiaryAddress, { gasLimit: 1e7 }) + entryPoint.handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7 }) ).to.be.revertedWith('AA33 reverted: ERC20: insufficient allowance') await token.sudoApprove(account.address, paymaster.address, ethers.constants.MaxUint256) await expect( - entryPoint.handleOps([op], beneficiaryAddress, { gasLimit: 1e7 }) + entryPoint.handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7 }) ).to.revertedWith('AA33 reverted: ERC20: transfer amount exceeds balance') await ethers.provider.send('evm_revert', [snapshot]) }) @@ -158,17 +158,19 @@ describe('TokenPaymaster', function () { await token.transfer(account.address, parseEther('1')) await token.sudoApprove(account.address, paymaster.address, ethers.constants.MaxUint256) - const paymasterAndData = generatePaymasterAndData(paymasterAddress) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, + paymasterVerificationGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5, callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) // for simpler 'gasPrice()' calculation await ethers.provider.send('hardhat_setNextBlockBaseFeePerGas', [utils.hexlify(op.maxFeePerGas)]) const tx = await entryPoint - .handleOps([op], beneficiaryAddress, { + .handleOps([opPacked], beneficiaryAddress, { gasLimit: 3e7, maxFeePerGas: op.maxFeePerGas, maxPriorityFeePerGas: op.maxFeePerGas @@ -211,15 +213,17 @@ describe('TokenPaymaster', function () { await tokenOracle.setPrice(initialPriceToken * 5) await nativeAssetOracle.setPrice(initialPriceEther * 10) - const paymasterAndData = generatePaymasterAndData(paymasterAddress) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, + paymasterVerificationGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5, callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) const tx: ContractTransaction = await entryPoint - .handleOps([op], beneficiaryAddress, { gasLimit: 1e7 }) + .handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7 }) const receipt: ContractReceipt = await tx.wait() const block = await ethers.provider.getBlock(receipt.blockHash) @@ -248,19 +252,23 @@ describe('TokenPaymaster', function () { const currentCachedPrice = await paymaster.cachedPrice() assert.equal((currentCachedPrice as any) / (priceDenominator as any), 0.2) const overrideTokenPrice = priceDenominator.mul(132).div(1000) - const paymasterAndData = generatePaymasterAndData(paymasterAddress, overrideTokenPrice) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, + paymasterVerificationGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5, + paymasterData: hexZeroPad(hexlify(overrideTokenPrice), 32), callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) + console.log('op', opPacked, op) // for simpler 'gasPrice()' calculation await ethers.provider.send('hardhat_setNextBlockBaseFeePerGas', [utils.hexlify(op.maxFeePerGas)]) const tx = await entryPoint - .handleOps([op], beneficiaryAddress, { + .handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7, maxFeePerGas: op.maxFeePerGas, maxPriorityFeePerGas: op.maxFeePerGas @@ -272,7 +280,7 @@ describe('TokenPaymaster', function () { }) const preChargeTokens = decodedLogs[0].args.value - const requiredGas = BigNumber.from(op.callGasLimit).add(BigNumber.from(op.verificationGasLimit).mul(2)).add(op.preVerificationGas).add(40000 /* REFUND_POSTOP_COST */) + const requiredGas = BigNumber.from(op.callGasLimit).add(BigNumber.from(op.verificationGasLimit).add(BigNumber.from(op.paymasterVerificationGasLimit))).add(BigNumber.from(op.paymasterPostOpGasLimit)).add(op.preVerificationGas).add(40000 /* REFUND_POSTOP_COST */) const requiredPrefund = requiredGas.mul(op.maxFeePerGas) const preChargeTokenPrice = requiredPrefund.mul(priceDenominator).div(preChargeTokens) @@ -290,18 +298,21 @@ describe('TokenPaymaster', function () { assert.equal((currentCachedPrice as any) / (priceDenominator as any), 0.2) // note: higher number is lower token price const overrideTokenPrice = priceDenominator.mul(50) - const paymasterAndData = generatePaymasterAndData(paymasterAddress, overrideTokenPrice) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, + paymasterVerificationGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5, + paymasterData: hexZeroPad(hexlify(overrideTokenPrice), 32), callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) // for simpler 'gasPrice()' calculation await ethers.provider.send('hardhat_setNextBlockBaseFeePerGas', [utils.hexlify(op.maxFeePerGas)]) const tx = await entryPoint - .handleOps([op], beneficiaryAddress, { + .handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7, maxFeePerGas: op.maxFeePerGas, maxPriorityFeePerGas: op.maxFeePerGas @@ -313,7 +324,7 @@ describe('TokenPaymaster', function () { }) const preChargeTokens = decodedLogs[0].args.value - const requiredGas = BigNumber.from(op.callGasLimit).add(BigNumber.from(op.verificationGasLimit).mul(2)).add(op.preVerificationGas).add(40000 /* REFUND_POSTOP_COST */) + const requiredGas = BigNumber.from(op.callGasLimit).add(BigNumber.from(op.verificationGasLimit).add(BigNumber.from(op.paymasterVerificationGasLimit))).add(BigNumber.from(op.paymasterPostOpGasLimit)).add(op.preVerificationGas).add(40000 /* REFUND_POSTOP_COST */) const requiredPrefund = requiredGas.mul(op.maxFeePerGas) const preChargeTokenPrice = requiredPrefund.mul(priceDenominator).div(preChargeTokens) @@ -332,15 +343,17 @@ describe('TokenPaymaster', function () { // Cannot happen too fast though await ethers.provider.send('evm_increaseTime', [200]) - const paymasterAndData = generatePaymasterAndData(paymasterAddress) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, + paymasterVerificationGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5, callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) const tx = await entryPoint - .handleOps([op], beneficiaryAddress, { gasLimit: 1e7 }) + .handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7 }) .then(async tx => await tx.wait()) const decodedLogs = tx.logs.map(it => { @@ -378,15 +391,17 @@ describe('TokenPaymaster', function () { const withdrawTokensCall = await token.populateTransaction.transfer(token.address, parseEther('0.009')).then(tx => tx.data!) const callData = await account.populateTransaction.execute(token.address, 0, withdrawTokensCall).then(tx => tx.data!) - const paymasterAndData = generatePaymasterAndData(paymasterAddress) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, + paymasterVerificationGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5, callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) const tx = await entryPoint - .handleOps([op], beneficiaryAddress, { gasLimit: 1e7 }) + .handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7 }) .then(async tx => await tx.wait()) const decodedLogs = tx.logs.map(it => { @@ -408,15 +423,17 @@ describe('TokenPaymaster', function () { // deposit exactly the minimum amount so the next UserOp makes it go under await entryPoint.depositTo(paymaster.address, { value: minEntryPointBalance }) - const paymasterAndData = generatePaymasterAndData(paymasterAddress) let op = await fillUserOp({ sender: account.address, - paymasterAndData, + paymaster: paymasterAddress, + paymasterVerificationGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5, callData }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) + const opPacked = packUserOp(op) const tx = await entryPoint - .handleOps([op], beneficiaryAddress, { gasLimit: 1e7 }) + .handleOps([opPacked], beneficiaryAddress, { gasLimit: 1e7 }) .then(async tx => await tx.wait()) const decodedLogs = tx.logs.map(it => { return testInterface.parseLog(it) From b2a9e61086fcd066d11cdc3199354f2f7efdef41 Mon Sep 17 00:00:00 2001 From: shahafn Date: Tue, 5 Dec 2023 00:34:12 +0200 Subject: [PATCH 07/24] Fixing safe tests --- contracts/samples/gnosis/EIP4337Manager.sol | 2 +- test/gnosis.test.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol index be0beaf0..a89a943d 100644 --- a/contracts/samples/gnosis/EIP4337Manager.sol +++ b/contracts/samples/gnosis/EIP4337Manager.sol @@ -163,7 +163,7 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor { sig[2] = bytes1(uint8(1)); sig[35] = bytes1(uint8(1)); uint256 nonce = uint256(IEntryPoint(manager.entryPoint()).getNonce(address(safe), 0)); - UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", bytes32(bytes3(0x0f4240)), 0, 0, 0, "", sig); + UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", bytes32(bytes16(uint128(0x0f4240))), 0, 0, 0, "", sig); UserOperation[] memory userOps = new UserOperation[](1); userOps[0] = userOp; IEntryPoint _entryPoint = IEntryPoint(payable(manager.entryPoint())); diff --git a/test/gnosis.test.ts b/test/gnosis.test.ts index 35cd132a..99010233 100644 --- a/test/gnosis.test.ts +++ b/test/gnosis.test.ts @@ -97,6 +97,7 @@ describe('Gnosis Proxy', function () { }) it('should validate', async function () { + console.log('manager proxy', manager.address, proxySafe.address) await manager.callStatic.validateEip4337(proxySafe.address, manager.address, { gasLimit: 10e6 }) }) From 46fb3ba86c507ffdf5315949c918bed5360f9124 Mon Sep 17 00:00:00 2001 From: shahafn Date: Tue, 5 Dec 2023 00:42:59 +0200 Subject: [PATCH 08/24] Lint --- test/UserOp.ts | 5 ++--- test/UserOperation.ts | 1 - test/entrypoint.test.ts | 6 +++--- test/paymaster.test.ts | 2 +- test/samples/TokenPaymaster.test.ts | 14 +------------- test/testutils.ts | 11 ++++++++--- test/verifying_paymaster.test.ts | 12 +++++++++--- test/y.bls.test.ts | 2 +- 8 files changed, 25 insertions(+), 28 deletions(-) diff --git a/test/UserOp.ts b/test/UserOp.ts index 828356c9..67afcb85 100644 --- a/test/UserOp.ts +++ b/test/UserOp.ts @@ -18,7 +18,6 @@ import EntryPointSimulationsJson from '../artifacts/contracts/core/EntryPointSim import { ethers } from 'hardhat' import { IEntryPointSimulations } from '../typechain/contracts/core/EntryPointSimulations' - export function packUserOp (userOp: UserOperation): PackedUserOperation { const accountGasLimits = packAccountGasLimits(userOp.verificationGasLimit, userOp.callGasLimit) let paymasterAndData = '0x' @@ -52,7 +51,7 @@ export function encodeUserOp (userOp: UserOperation, forSignature = true): strin // for the purpose of calculating gas cost encode also signature (and no keccak of bytes) return defaultAbiCoder.encode( ['address', 'uint256', 'bytes', 'bytes', - 'uint256', 'uint256', 'uint256', 'uint256', + 'uint256', 'uint256', 'uint256', 'uint256', 'bytes', 'bytes'], [packedUserOp.sender, packedUserOp.nonce, packedUserOp.initCode, packedUserOp.callData, packedUserOp.accountGasLimits, packedUserOp.preVerificationGas, packedUserOp.maxFeePerGas, packedUserOp.maxPriorityFeePerGas, @@ -227,7 +226,7 @@ export async function fillAndSign (op: Partial, signer: Wallet | } export async function fillSignAndPack (op: Partial, signer: Wallet | Signer, entryPoint?: EntryPoint, getNonceFunction = 'getNonce'): Promise { - const filledAndSignedOp = await fillAndSign(op, signer, entryPoint, getNonceFunction) + const filledAndSignedOp = await fillAndSign(op, signer, entryPoint, getNonceFunction) return packUserOp(filledAndSignedOp) } diff --git a/test/UserOperation.ts b/test/UserOperation.ts index 480923b0..c1b2d327 100644 --- a/test/UserOperation.ts +++ b/test/UserOperation.ts @@ -18,7 +18,6 @@ export interface UserOperation { signature: typ.bytes } - export interface PackedUserOperation { sender: typ.address diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts index 0422cd32..5f49e109 100644 --- a/test/entrypoint.test.ts +++ b/test/entrypoint.test.ts @@ -52,7 +52,7 @@ import { DefaultsForUserOp, fillAndSign, fillSignAndPack, getUserOpHash, packUse import { PackedUserOperation, UserOperation } from './UserOperation' import { PopulatedTransaction } from 'ethers/lib/ethers' import { ethers } from 'hardhat' -import { arrayify, defaultAbiCoder, hexConcat, hexZeroPad, parseEther } from 'ethers/lib/utils' +import { arrayify, defaultAbiCoder, hexZeroPad, parseEther } from 'ethers/lib/utils' import { debugTransaction } from './debugTx' import { BytesLike } from '@ethersproject/bytes' import { toChecksumAddress } from 'ethereumjs-util' @@ -1164,7 +1164,7 @@ describe('EntryPoint', function () { const userOp = await fillSignAndPack({ sender: account.address, paymaster: paymaster.address, - paymasterData: timeRange, + paymasterData: timeRange }, ethersSigner, entryPoint) const ret = await simulateValidation(userOp, entryPoint.address) expect(ret.returnInfo.validUntil).to.eql(now - 60) @@ -1177,7 +1177,7 @@ describe('EntryPoint', function () { return await fillSignAndPack({ sender: account.address, paymaster: paymaster.address, - paymasterData: timeRange, + paymasterData: timeRange }, owner, entryPoint) } diff --git a/test/paymaster.test.ts b/test/paymaster.test.ts index a494099b..edaa816f 100644 --- a/test/paymaster.test.ts +++ b/test/paymaster.test.ts @@ -120,7 +120,7 @@ describe('EntryPoint with paymaster', function () { initCode: getAccountDeployer(entryPoint.address, accountOwner.address, 1), verificationGasLimit: 1e7, paymaster: paymaster.address, - paymasterPostOpGasLimit: 3e5, + paymasterPostOpGasLimit: 3e5 }, accountOwner, entryPoint) await expect(entryPoint.callStatic.handleOps([op], beneficiaryAddress, { gasLimit: 1e7 diff --git a/test/samples/TokenPaymaster.test.ts b/test/samples/TokenPaymaster.test.ts index cd562a1a..77f3fdfa 100644 --- a/test/samples/TokenPaymaster.test.ts +++ b/test/samples/TokenPaymaster.test.ts @@ -1,4 +1,4 @@ -import { BigNumberish, ContractReceipt, ContractTransaction, Wallet, utils, BigNumber } from 'ethers' +import { ContractReceipt, ContractTransaction, Wallet, utils, BigNumber } from 'ethers' import { hexlify, hexZeroPad, Interface, parseEther } from 'ethers/lib/utils' import { assert, expect } from 'chai' import { ethers } from 'hardhat' @@ -28,18 +28,6 @@ import { checkForGeth, createAccount, createAccountOwner, deployEntryPoint, fund import { fillUserOp, packUserOp, signUserOp } from '../UserOp' -function generatePaymasterAndData (pm: string, tokenPrice?: BigNumberish): string { - if (tokenPrice != null) { - return utils.hexlify( - utils.concat([pm, utils.hexZeroPad(utils.hexlify(tokenPrice), 32)]) - ) - } else { - return utils.hexlify( - utils.concat([pm]) - ) - } -} - const priceDenominator = BigNumber.from(10).pow(26) describe('TokenPaymaster', function () { diff --git a/test/testutils.ts b/test/testutils.ts index 82fe2065..75ce6e2c 100644 --- a/test/testutils.ts +++ b/test/testutils.ts @@ -289,13 +289,18 @@ export async function createAccount ( } export function packAccountGasLimits (validationGasLimit: BytesLike | Hexable | number | bigint, callGasLimit: BytesLike | Hexable | number | bigint): string { - return ethers.utils.hexConcat([hexZeroPad(hexlify(validationGasLimit, {hexPad: 'left'}),16), hexZeroPad(hexlify(callGasLimit, {hexPad: 'left'}),16)]) + return ethers.utils.hexConcat([ + hexZeroPad(hexlify(validationGasLimit, { hexPad: 'left' }), 16), hexZeroPad(hexlify(callGasLimit, { hexPad: 'left' }), 16) + ]) } export function packPaymasterData (paymaster: string, paymasterVerificationGasLimit: BytesLike | Hexable | number | bigint, postOpGasLimit: BytesLike | Hexable | number | bigint, paymasterData: string): string { - return ethers.utils.hexConcat([paymaster, hexZeroPad(hexlify(paymasterVerificationGasLimit, {hexPad: 'left'}),16), hexZeroPad(hexlify(postOpGasLimit, {hexPad: 'left'}),16), paymasterData]) + return ethers.utils.hexConcat([ + paymaster, hexZeroPad(hexlify(paymasterVerificationGasLimit, { hexPad: 'left' }), 16), + hexZeroPad(hexlify(postOpGasLimit, { hexPad: 'left' }), 16), paymasterData + ]) } -export function unpackAccountGasLimits (accountGasLimits: string): {validationGasLimit: number, callGasLimit: number} { +export function unpackAccountGasLimits (accountGasLimits: string): { validationGasLimit: number, callGasLimit: number } { return { validationGasLimit: parseInt(accountGasLimits.slice(2, 34), 16), callGasLimit: parseInt(accountGasLimits.slice(34), 16) } } diff --git a/test/verifying_paymaster.test.ts b/test/verifying_paymaster.test.ts index 06711a55..80be270e 100644 --- a/test/verifying_paymaster.test.ts +++ b/test/verifying_paymaster.test.ts @@ -43,7 +43,11 @@ describe('EntryPoint with VerifyingPaymaster', function () { describe('#parsePaymasterAndData', () => { it('should parse data properly', async () => { - const paymasterAndData = hexConcat([paymaster.address, hexZeroPad(hexlify(DefaultsForUserOp.paymasterVerificationGasLimit, {hexPad: 'left'}),16), hexZeroPad(hexlify(DefaultsForUserOp.paymasterPostOpGasLimit, {hexPad: 'left'}),16), defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), MOCK_SIG]) + const paymasterAndData = hexConcat([ + paymaster.address, hexZeroPad(hexlify(DefaultsForUserOp.paymasterVerificationGasLimit, { hexPad: 'left' }), 16), + hexZeroPad(hexlify(DefaultsForUserOp.paymasterPostOpGasLimit, { hexPad: 'left' }), 16), + defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), MOCK_SIG + ]) console.log(paymasterAndData) const res = await paymaster.parsePaymasterAndData(paymasterAndData) console.log('MOCK_VALID_UNTIL, MOCK_VALID_AFTER', MOCK_VALID_UNTIL, MOCK_VALID_AFTER) @@ -70,7 +74,8 @@ describe('EntryPoint with VerifyingPaymaster', function () { const userOp = await fillSignAndPack({ sender: account.address, paymaster: paymaster.address, - paymasterData: hexConcat([defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) + paymasterData: hexConcat( + [defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) }, accountOwner, entryPoint) await expect(simulateValidation(userOp, entryPoint.address)).to.be.revertedWith('ECDSA: invalid signature') }) @@ -101,7 +106,8 @@ describe('EntryPoint with VerifyingPaymaster', function () { const userOp1 = await fillAndSign({ sender: account.address, paymaster: paymaster.address, - paymasterData: hexConcat([defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) + paymasterData: hexConcat( + [defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), '0x' + '00'.repeat(65)]) }, accountOwner, entryPoint) const hash = await paymaster.getHash(packUserOp(userOp1), MOCK_VALID_UNTIL, MOCK_VALID_AFTER) const sig = await offchainSigner.signMessage(arrayify(hash)) diff --git a/test/y.bls.test.ts b/test/y.bls.test.ts index 3dfcf82b..2f91b41b 100644 --- a/test/y.bls.test.ts +++ b/test/y.bls.test.ts @@ -13,7 +13,7 @@ import { } from '../typechain' import { ethers } from 'hardhat' import { createAddress, deployEntryPoint, fund, ONE_ETH } from './testutils' -import { DefaultsForUserOp, fillAndPack, fillUserOp, packUserOp, simulateValidation } from './UserOp' +import { DefaultsForUserOp, fillAndPack, packUserOp, simulateValidation } from './UserOp' import { expect } from 'chai' import { keccak256 } from 'ethereumjs-util' import { hashToPoint } from '@thehubbleproject/bls/dist/mcl' From 7494872f15926d81d0b74d7a9c264c25beeea3ed Mon Sep 17 00:00:00 2001 From: shahafn Date: Tue, 5 Dec 2023 13:02:09 +0200 Subject: [PATCH 09/24] Fixing gas-calc, tsc --- gascalc/GasChecker.ts | 12 +- reports/gas-checker.txt | 24 ++-- src/AASigner.ts | 10 +- test/deposit-paymaster.test.ts | 197 --------------------------------- test/testutils.ts | 9 -- 5 files changed, 23 insertions(+), 229 deletions(-) delete mode 100644 test/deposit-paymaster.test.ts diff --git a/gascalc/GasChecker.ts b/gascalc/GasChecker.ts index 32465063..ec1c11e3 100644 --- a/gascalc/GasChecker.ts +++ b/gascalc/GasChecker.ts @@ -14,13 +14,13 @@ import { } from '../typechain' import { BigNumberish, Wallet } from 'ethers' import hre from 'hardhat' -import { fillAndSign, fillUserOp, signUserOp } from '../test/UserOp' +import { fillSignAndPack, fillUserOp, packUserOp, signUserOp } from '../test/UserOp' import { TransactionReceipt } from '@ethersproject/abstract-provider' import { table, TableUserConfig } from 'table' import { Create2Factory } from '../src/Create2Factory' import * as fs from 'fs' import { SimpleAccountInterface } from '../typechain/contracts/samples/SimpleAccount' -import { UserOperation } from '../test/UserOperation' +import { PackedUserOperation } from '../test/UserOperation' const gasCheckerLogFile = './reports/gas-checker.txt' @@ -130,7 +130,7 @@ export class GasChecker { console.log('factaddr', factoryAddress) const fact = SimpleAccountFactory__factory.connect(factoryAddress, ethersSigner) // create accounts - const creationOps: UserOperation[] = [] + const creationOps: PackedUserOperation[] = [] for (const n of range(count)) { const salt = n // const initCode = this.accountInitCode(fact, salt) @@ -149,7 +149,7 @@ export class GasChecker { preVerificationGas: 1, maxFeePerGas: 0 }), this.accountOwner, this.entryPoint().address, await provider.getNetwork().then(net => net.chainId)) - creationOps.push(op) + creationOps.push(packUserOp(op)) this.createdAccounts.add(addr) } @@ -223,14 +223,14 @@ export class GasChecker { } // console.debug('== account est=', accountEst.toString()) accountEst = est.accountEst - const op = await fillAndSign({ + const op = await fillSignAndPack({ sender: account, callData: accountExecFromEntryPoint, maxPriorityFeePerGas: info.gasPrice, maxFeePerGas: info.gasPrice, callGasLimit: accountEst, verificationGasLimit: 1000000, - paymasterAndData: paymaster, + paymaster: paymaster, preVerificationGas: 1 }, accountOwner, GasCheckCollector.inst.entryPoint) // const packed = packUserOp(op, false) diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index b8be925c..e397f02c 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 │ 81838 │ │ ║ +║ simple │ 1 │ 82972 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 44095 │ 15081 ║ +║ simple - diff from previous │ 2 │ │ 45170 │ 16156 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 478810 │ │ ║ +║ simple │ 10 │ 489608 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 44131 │ 15117 ║ +║ simple - diff from previous │ 11 │ │ 45218 │ 16204 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 88109 │ │ ║ +║ simple paymaster │ 1 │ 89827 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 43096 │ 14082 ║ +║ simple paymaster with diff │ 2 │ │ 44750 │ 15736 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 476022 │ │ ║ +║ simple paymaster │ 10 │ 492626 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 43155 │ 14141 ║ +║ simple paymaster with diff │ 11 │ │ 44843 │ 15829 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 182895 │ │ ║ +║ big tx 5k │ 1 │ 184029 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 144594 │ 19334 ║ +║ big tx - diff from previous │ 2 │ │ 145670 │ 20410 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1484454 │ │ ║ +║ big tx 5k │ 10 │ 1495236 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 144619 │ 19359 ║ +║ big tx - diff from previous │ 11 │ │ 145789 │ 20529 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ diff --git a/src/AASigner.ts b/src/AASigner.ts index 4e10955a..8b95a548 100644 --- a/src/AASigner.ts +++ b/src/AASigner.ts @@ -6,8 +6,8 @@ import { BaseProvider, Provider, TransactionRequest } from '@ethersproject/provi import { BigNumber, Bytes, ethers, Event, Signer } from 'ethers' import { clearInterval } from 'timers' import { getAccountAddress, getAccountInitCode } from '../test/testutils' -import { fillAndSign, getUserOpHash } from '../test/UserOp' -import { UserOperation } from '../test/UserOperation' +import { fillAndSign, getUserOpHash, packUserOp } from '../test/UserOp' +import { PackedUserOperation, UserOperation } from '../test/UserOperation' import { EntryPoint, EntryPoint__factory, @@ -132,12 +132,12 @@ async function sendQueuedUserOps (queueSender: QueueSendUserOp, entryPoint: Entr console.log('queue too small/too young. waiting') return } - const ops: UserOperation[] = [] + const ops: PackedUserOperation[] = [] const queue = queueSender.queue Object.keys(queue).forEach(sender => { const op = queue[sender].shift() if (op != null) { - ops.push(op) + ops.push(packUserOp(op)) queueSender.queueSize-- } }) @@ -174,7 +174,7 @@ export function localUserOpSender (entryPointAddress: string, signer: Signer, be } const gasLimit = BigNumber.from(userOp.preVerificationGas).add(userOp.verificationGasLimit).add(userOp.callGasLimit) console.log('calc gaslimit=', gasLimit.toString()) - const ret = await entryPoint.handleOps([userOp], beneficiary ?? await signer.getAddress(), { + const ret = await entryPoint.handleOps([packUserOp(userOp)], beneficiary ?? await signer.getAddress(), { maxPriorityFeePerGas: userOp.maxPriorityFeePerGas, maxFeePerGas: userOp.maxFeePerGas }) diff --git a/test/deposit-paymaster.test.ts b/test/deposit-paymaster.test.ts deleted file mode 100644 index 2e7ca959..00000000 --- a/test/deposit-paymaster.test.ts +++ /dev/null @@ -1,197 +0,0 @@ -import './aa.init' -import { ethers } from 'hardhat' -import { expect } from 'chai' -import { - SimpleAccount, - EntryPoint, - DepositPaymaster, - DepositPaymaster__factory, - TestOracle__factory, - TestCounter, - TestCounter__factory, - TestToken, - TestToken__factory -} from '../typechain' -import { - AddressZero, createAddress, - createAccountOwner, - deployEntryPoint, FIVE_ETH, ONE_ETH, userOpsWithoutAgg, createAccount -} from './testutils' -import { fillAndSign, simulateValidation } from './UserOp' -import { hexConcat, hexZeroPad, parseEther } from 'ethers/lib/utils' - -// TODO: fails after unrelated change in the repo -describe.skip('DepositPaymaster', () => { - let entryPoint: EntryPoint - const ethersSigner = ethers.provider.getSigner() - let token: TestToken - let paymaster: DepositPaymaster - before(async function () { - entryPoint = await deployEntryPoint() - - paymaster = await new DepositPaymaster__factory(ethersSigner).deploy(entryPoint.address) - await paymaster.addStake(1, { value: parseEther('2') }) - await entryPoint.depositTo(paymaster.address, { value: parseEther('1') }) - - token = await new TestToken__factory(ethersSigner).deploy() - const testOracle = await new TestOracle__factory(ethersSigner).deploy() - await paymaster.addToken(token.address, testOracle.address) - - await token.mint(await ethersSigner.getAddress(), FIVE_ETH) - await token.approve(paymaster.address, ethers.constants.MaxUint256) - }) - - describe('deposit', () => { - let account: SimpleAccount - - before(async () => { - ({ proxy: account } = await createAccount(ethersSigner, await ethersSigner.getAddress(), entryPoint.address)) - }) - it('should deposit and read balance', async () => { - await paymaster.addDepositFor(token.address, account.address, 100) - expect(await paymaster.depositInfo(token.address, account.address)).to.eql({ amount: 100 }) - }) - it('should fail to withdraw without unlock', async () => { - const paymasterWithdraw = await paymaster.populateTransaction.withdrawTokensTo(token.address, AddressZero, 1).then(tx => tx.data!) - - await expect( - account.execute(paymaster.address, 0, paymasterWithdraw) - ).to.revertedWith('DepositPaymaster: must unlockTokenDeposit') - }) - it('should fail to withdraw within the same block ', async () => { - const paymasterUnlock = await paymaster.populateTransaction.unlockTokenDeposit().then(tx => tx.data!) - const paymasterWithdraw = await paymaster.populateTransaction.withdrawTokensTo(token.address, AddressZero, 1).then(tx => tx.data!) - - await expect( - account.executeBatch([paymaster.address, paymaster.address], [], [paymasterUnlock, paymasterWithdraw]) - ).to.be.revertedWith('DepositPaymaster: must unlockTokenDeposit') - }) - it('should succeed to withdraw after unlock', async () => { - const paymasterUnlock = await paymaster.populateTransaction.unlockTokenDeposit().then(tx => tx.data!) - const target = createAddress() - const paymasterWithdraw = await paymaster.populateTransaction.withdrawTokensTo(token.address, target, 1).then(tx => tx.data!) - await account.execute(paymaster.address, 0, paymasterUnlock) - await account.execute(paymaster.address, 0, paymasterWithdraw) - expect(await token.balanceOf(target)).to.eq(1) - }) - }) - - describe('#validatePaymasterUserOp', () => { - let account: SimpleAccount - const gasPrice = 1e9 - - before(async () => { - ({ proxy: account } = await createAccount(ethersSigner, await ethersSigner.getAddress(), entryPoint.address)) - }) - - it('should fail if no token', async () => { - const userOp = await fillAndSign({ - sender: account.address, - paymasterAndData: paymaster.address - }, ethersSigner, entryPoint) - await expect(simulateValidation(userOp, entryPoint.address)).to.be.revertedWith('paymasterAndData must specify token') - }) - - it('should fail with wrong token', async () => { - const userOp = await fillAndSign({ - sender: account.address, - paymasterAndData: hexConcat([paymaster.address, hexZeroPad('0x1234', 20)]) - }, ethersSigner, entryPoint) - await expect(simulateValidation(userOp, entryPoint.address, { gasPrice })).to.be.revertedWith('DepositPaymaster: unsupported token') - }) - - it('should reject if no deposit', async () => { - const userOp = await fillAndSign({ - sender: account.address, - paymasterAndData: hexConcat([paymaster.address, hexZeroPad(token.address, 20)]) - }, ethersSigner, entryPoint) - await expect(simulateValidation(userOp, entryPoint.address, { gasPrice })).to.be.revertedWith('DepositPaymaster: deposit too low') - }) - - it('should reject if deposit is not locked', async () => { - await paymaster.addDepositFor(token.address, account.address, ONE_ETH) - - const paymasterUnlock = await paymaster.populateTransaction.unlockTokenDeposit().then(tx => tx.data!) - await account.execute(paymaster.address, 0, paymasterUnlock) - - const userOp = await fillAndSign({ - sender: account.address, - paymasterAndData: hexConcat([paymaster.address, hexZeroPad(token.address, 20)]) - }, ethersSigner, entryPoint) - await expect(simulateValidation(userOp, entryPoint.address, { gasPrice })).to.be.revertedWith('not locked') - }) - - it('succeed with valid deposit', async () => { - // needed only if previous test did unlock. - const paymasterLockTokenDeposit = await paymaster.populateTransaction.lockTokenDeposit().then(tx => tx.data!) - await account.execute(paymaster.address, 0, paymasterLockTokenDeposit) - - const userOp = await fillAndSign({ - sender: account.address, - paymasterAndData: hexConcat([paymaster.address, hexZeroPad(token.address, 20)]) - }, ethersSigner, entryPoint) - await simulateValidation(userOp, entryPoint.address) - }) - }) - describe('#handleOps', () => { - let account: SimpleAccount - const accountOwner = createAccountOwner() - let counter: TestCounter - let callData: string - before(async () => { - ({ proxy: account } = await createAccount(ethersSigner, await accountOwner.getAddress(), entryPoint.address)) - counter = await new TestCounter__factory(ethersSigner).deploy() - const counterJustEmit = await counter.populateTransaction.justemit().then(tx => tx.data!) - callData = await account.populateTransaction.execute(counter.address, 0, counterJustEmit).then(tx => tx.data!) - - await paymaster.addDepositFor(token.address, account.address, ONE_ETH) - }) - it('should pay with deposit (and revert user\'s call) if user can\'t pay with tokens', async () => { - const beneficiary = createAddress() - const userOp = await fillAndSign({ - sender: account.address, - paymasterAndData: hexConcat([paymaster.address, hexZeroPad(token.address, 20)]), - callData - }, accountOwner, entryPoint) - - await entryPoint.handleAggregatedOps(userOpsWithoutAgg([userOp]), beneficiary) - - const [log] = await entryPoint.queryFilter(entryPoint.filters.UserOperationEvent()) - expect(log.args.success).to.eq(false) - expect(await counter.queryFilter(counter.filters.CalledFrom())).to.eql([]) - expect(await ethers.provider.getBalance(beneficiary)).to.be.gt(0) - }) - - it('should pay with tokens if available', async () => { - const beneficiary = createAddress() - const beneficiary1 = createAddress() - const initialTokens = parseEther('1') - await token.mint(account.address, initialTokens) - - // need to "approve" the paymaster to use the tokens. we issue a UserOp for that (which uses the deposit to execute) - const tokenApprovePaymaster = await token.populateTransaction.approve(paymaster.address, ethers.constants.MaxUint256).then(tx => tx.data!) - const execApprove = await account.populateTransaction.execute(token.address, 0, tokenApprovePaymaster).then(tx => tx.data!) - const userOp1 = await fillAndSign({ - sender: account.address, - paymasterAndData: hexConcat([paymaster.address, hexZeroPad(token.address, 20)]), - callData: execApprove - }, accountOwner, entryPoint) - await entryPoint.handleAggregatedOps(userOpsWithoutAgg([userOp1]), beneficiary1) - - const userOp = await fillAndSign({ - sender: account.address, - paymasterAndData: hexConcat([paymaster.address, hexZeroPad(token.address, 20)]), - callData - }, accountOwner, entryPoint) - await entryPoint.handleAggregatedOps(userOpsWithoutAgg([userOp]), beneficiary) - - const [log] = await entryPoint.queryFilter(entryPoint.filters.UserOperationEvent(), await ethers.provider.getBlockNumber()) - expect(log.args.success).to.eq(true) - const charge = log.args.actualGasCost - expect(await ethers.provider.getBalance(beneficiary)).to.eq(charge) - - const targetLogs = await counter.queryFilter(counter.filters.CalledFrom()) - expect(targetLogs.length).to.eq(1) - }) - }) -}) diff --git a/test/testutils.ts b/test/testutils.ts index 75ce6e2c..2e5f60b7 100644 --- a/test/testutils.ts +++ b/test/testutils.ts @@ -255,15 +255,6 @@ export async function isDeployed (addr: string): Promise { return code.length > 2 } -// internal helper function: create a UserOpsPerAggregator structure, with no aggregator or signature -export function userOpsWithoutAgg (userOps: UserOperation[]): IEntryPoint.UserOpsPerAggregatorStruct[] { - return [{ - userOps, - aggregator: AddressZero, - signature: '0x' - }] -} - // Deploys an implementation and a proxy pointing to this implementation export async function createAccount ( ethersSigner: Signer, From 0cc1d675ce70982b4fdc0c5f1989d98db6cc08b8 Mon Sep 17 00:00:00 2001 From: shahafn Date: Tue, 5 Dec 2023 13:09:55 +0200 Subject: [PATCH 10/24] Lint --- test/testutils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/testutils.ts b/test/testutils.ts index 2e5f60b7..4d13016c 100644 --- a/test/testutils.ts +++ b/test/testutils.ts @@ -10,7 +10,6 @@ import { EntryPoint, EntryPoint__factory, IERC20, - IEntryPoint, SimpleAccount, SimpleAccountFactory__factory, SimpleAccount__factory, @@ -21,7 +20,6 @@ import { BytesLike } from '@ethersproject/bytes' import { expect } from 'chai' import { Create2Factory } from '../src/Create2Factory' import { debugTransaction } from './debugTx' -import { UserOperation } from './UserOperation' import { Hexable } from '@ethersproject/bytes/src.ts' export const AddressZero = ethers.constants.AddressZero From 6ffd04215954611a7ba1e5926a0feec39c93bee4 Mon Sep 17 00:00:00 2001 From: shahafn Date: Tue, 5 Dec 2023 13:20:49 +0200 Subject: [PATCH 11/24] ethers tsc --- test/testutils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/testutils.ts b/test/testutils.ts index 4d13016c..b4ac9023 100644 --- a/test/testutils.ts +++ b/test/testutils.ts @@ -16,11 +16,10 @@ import { SimpleAccountFactory, TestAggregatedAccountFactory } from '../typechain' -import { BytesLike } from '@ethersproject/bytes' +import { BytesLike, Hexable } from '@ethersproject/bytes' import { expect } from 'chai' import { Create2Factory } from '../src/Create2Factory' import { debugTransaction } from './debugTx' -import { Hexable } from '@ethersproject/bytes/src.ts' export const AddressZero = ethers.constants.AddressZero export const HashZero = ethers.constants.HashZero From 56a57108e568fbf3979c75e8a15ee51e4a3b4e13 Mon Sep 17 00:00:00 2001 From: shahafn Date: Tue, 5 Dec 2023 13:26:11 +0200 Subject: [PATCH 12/24] gas calc again --- 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 e397f02c..ee826227 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 │ 82972 │ │ ║ +║ simple │ 1 │ 82855 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 45170 │ 16156 ║ +║ simple - diff from previous │ 2 │ │ 45041 │ 16027 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 489608 │ │ ║ +║ simple │ 10 │ 488438 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 45218 │ 16204 ║ +║ simple - diff from previous │ 11 │ │ 45041 │ 16027 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 89827 │ │ ║ +║ simple paymaster │ 1 │ 89734 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 44750 │ 15736 ║ +║ simple paymaster with diff │ 2 │ │ 44633 │ 15619 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 492626 │ │ ║ +║ simple paymaster │ 10 │ 491588 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 44843 │ 15829 ║ +║ simple paymaster with diff │ 11 │ │ 44630 │ 15616 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 184029 │ │ ║ +║ big tx 5k │ 1 │ 183900 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 145670 │ 20410 ║ +║ big tx - diff from previous │ 2 │ │ 145553 │ 20293 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1495236 │ │ ║ +║ big tx 5k │ 10 │ 1494090 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 145789 │ 20529 ║ +║ big tx - diff from previous │ 11 │ │ 145600 │ 20340 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 2e3ceffcc7bc01035785614adea839c3413553a2 Mon Sep 17 00:00:00 2001 From: shahafn Date: Thu, 21 Dec 2023 00:06:08 +0200 Subject: [PATCH 13/24] Fixes --- contracts/core/EntryPoint.sol | 27 ++++------------------ contracts/core/UserOperationLib.sol | 10 +++++--- contracts/samples/LegacyTokenPaymaster.sol | 2 +- contracts/samples/TokenPaymaster.sol | 5 ++-- contracts/test/TestUtil.sol | 4 ++-- test/simple-wallet.test.ts | 2 +- 6 files changed, 18 insertions(+), 32 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index cd0f0f8b..1dc2ff1d 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -328,9 +328,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, paymasterAndData.length >= 20, "AA93 invalid paymasterAndData" ); - mUserOp.paymaster = address(bytes20(paymasterAndData[:20])); - mUserOp.paymasterVerificationGasLimit = uint128(bytes16(paymasterAndData[20:36])); - mUserOp.paymasterPostOpGasLimit = uint128(bytes16(paymasterAndData[36:52])); + (mUserOp.paymaster, mUserOp.paymasterVerificationGasLimit, mUserOp.paymasterPostOpGasLimit) = UserOperationLib.unpackPaymasterStaticFields(paymasterAndData); } else { mUserOp.paymaster = address(0); mUserOp.paymasterVerificationGasLimit = 0; @@ -413,12 +411,10 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, ) internal returns ( - uint256 gasUsedByValidateAccountPrepayment, uint256 validationData ) { unchecked { - uint256 preGas = gasleft(); MemoryUserOp memory mUserOp = opInfo.mUserOp; address sender = mUserOp.sender; _createSenderIfNeeded(opIndex, opInfo, op.initCode); @@ -453,7 +449,6 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, } senderInfo.deposit = uint112(deposit - requiredPrefund); } - gasUsedByValidateAccountPrepayment = preGas - gasleft(); } } @@ -467,24 +462,15 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, * @param op - The user operation. * @param opInfo - The operation info. * @param requiredPreFund - The required prefund amount. - * @param gasUsedByValidateAccountPrepayment - The gas used by _validateAccountPrepayment. */ function _validatePaymasterPrepayment( uint256 opIndex, UserOperation calldata op, UserOpInfo memory opInfo, - uint256 requiredPreFund, - uint256 gasUsedByValidateAccountPrepayment + uint256 requiredPreFund ) internal returns (bytes memory context, uint256 validationData) { unchecked { MemoryUserOp memory mUserOp = opInfo.mUserOp; - // TODO: Do we actually need that still? - uint256 verificationGasLimit = mUserOp.verificationGasLimit; - require( - verificationGasLimit > gasUsedByValidateAccountPrepayment, - "AA41 too little verificationGas" - ); - address paymaster = mUserOp.paymaster; DepositInfo storage paymasterInfo = deposits[paymaster]; uint256 deposit = paymasterInfo.deposit; @@ -595,12 +581,8 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, userOp.maxPriorityFeePerGas; require(maxGasValues <= type(uint120).max, "AA94 gas values overflow"); - uint256 gasUsedByValidateAccountPrepayment; uint256 requiredPreFund = _getRequiredPrefund(mUserOp); - ( - gasUsedByValidateAccountPrepayment, - validationData - ) = _validateAccountPrepayment( + validationData = _validateAccountPrepayment( opIndex, userOp, outOpInfo, @@ -621,8 +603,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, opIndex, userOp, outOpInfo, - requiredPreFund, - gasUsedByValidateAccountPrepayment + requiredPreFund ); } unchecked { diff --git a/contracts/core/UserOperationLib.sol b/contracts/core/UserOperationLib.sol index d419a0f6..d5236105 100644 --- a/contracts/core/UserOperationLib.sol +++ b/contracts/core/UserOperationLib.sol @@ -10,6 +10,10 @@ import {calldataKeccak} from "./Helpers.sol"; * Utility functions helpful when working with UserOperation structs. */ library UserOperationLib { + + uint256 public constant PAYMASTER_VALIDATION_GAS_OFFSET = 20; + uint256 public constant PAYMASTER_POSTOP_GAS_OFFSET = 36; + uint256 public constant PAYMASTER_DATA_OFFSET = 52; /** * Get sender from user operation data. * @param userOp - The user operation data. @@ -48,7 +52,7 @@ library UserOperationLib { * Pack the user operation data into bytes for hashing. * @param userOp - The user operation data. */ - function pack( + function encode( UserOperation calldata userOp ) internal pure returns (bytes memory ret) { address sender = getSender(userOp); @@ -79,7 +83,7 @@ library UserOperationLib { function unpackPaymasterStaticFields( bytes calldata paymasterAndData ) internal pure returns(address paymaster, uint128 validationGasLimit, uint128 postOp) { - return (address(bytes20(paymasterAndData[:20])), uint128(bytes16(paymasterAndData[20:36])), uint128(bytes16(paymasterAndData[36:52]))); + return (address(bytes20(paymasterAndData[:PAYMASTER_VALIDATION_GAS_OFFSET])), uint128(bytes16(paymasterAndData[PAYMASTER_VALIDATION_GAS_OFFSET:PAYMASTER_POSTOP_GAS_OFFSET])), uint128(bytes16(paymasterAndData[PAYMASTER_POSTOP_GAS_OFFSET:PAYMASTER_DATA_OFFSET]))); } /** @@ -89,7 +93,7 @@ library UserOperationLib { function hash( UserOperation calldata userOp ) internal pure returns (bytes32) { - return keccak256(pack(userOp)); + return keccak256(encode(userOp)); } /** diff --git a/contracts/samples/LegacyTokenPaymaster.sol b/contracts/samples/LegacyTokenPaymaster.sol index 58c7dce8..7610b74d 100644 --- a/contracts/samples/LegacyTokenPaymaster.sol +++ b/contracts/samples/LegacyTokenPaymaster.sol @@ -76,7 +76,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 { // verificationGasLimit is dual-purposed, as gas limit for postOp. make sure it is high enough // make sure that verificationGasLimit is high enough to handle postOp - require(uint128(bytes16(userOp.paymasterAndData[36:52])) > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); + require(uint128(bytes16(userOp.paymasterAndData[UserOperationLib.PAYMASTER_POSTOP_GAS_OFFSET:UserOperationLib.PAYMASTER_DATA_OFFSET])) > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); if (userOp.initCode.length != 0) { _validateConstructor(userOp); diff --git a/contracts/samples/TokenPaymaster.sol b/contracts/samples/TokenPaymaster.sol index f047ff53..51a3580e 100644 --- a/contracts/samples/TokenPaymaster.sol +++ b/contracts/samples/TokenPaymaster.sol @@ -8,6 +8,7 @@ import "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; import "../interfaces/IEntryPoint.sol"; import "../core/BasePaymaster.sol"; +import "../core/UserOperationLib.sol"; import "./utils/UniswapHelper.sol"; import "./utils/OracleHelper.sol"; @@ -123,7 +124,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { override returns (bytes memory context, uint256 validationResult) {unchecked { uint256 priceMarkup = tokenPaymasterConfig.priceMarkup; - uint256 paymasterAndDataLength = userOp.paymasterAndData.length - 52; + uint256 paymasterAndDataLength = userOp.paymasterAndData.length - UserOperationLib.PAYMASTER_DATA_OFFSET; require(paymasterAndDataLength == 0 || paymasterAndDataLength == 32, "TPM: invalid data length" ); @@ -131,7 +132,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { // note: as price is in ether-per-token and we want more tokens increasing it means dividing it by markup uint256 cachedPriceWithMarkup = cachedPrice * PRICE_DENOMINATOR / priceMarkup; if (paymasterAndDataLength == 32) { - uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[52 : 84])); + uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[UserOperationLib.PAYMASTER_DATA_OFFSET : 84])); if (clientSuppliedPrice < cachedPriceWithMarkup) { // note: smaller number means 'more ether per token' cachedPriceWithMarkup = clientSuppliedPrice; diff --git a/contracts/test/TestUtil.sol b/contracts/test/TestUtil.sol index 4257c3fb..594868b2 100644 --- a/contracts/test/TestUtil.sol +++ b/contracts/test/TestUtil.sol @@ -7,8 +7,8 @@ import "../core/UserOperationLib.sol"; contract TestUtil { using UserOperationLib for UserOperation; - function packUserOp(UserOperation calldata op) external pure returns (bytes memory){ - return op.pack(); + function encodeUserOp(UserOperation calldata op) external pure returns (bytes memory){ + return op.encode(); } } diff --git a/test/simple-wallet.test.ts b/test/simple-wallet.test.ts index 7ef73912..844bd37e 100644 --- a/test/simple-wallet.test.ts +++ b/test/simple-wallet.test.ts @@ -55,7 +55,7 @@ describe('SimpleAccount', function () { const op = await fillUserOpDefaults({ sender: accounts[0] }) const encoded = encodeUserOp(op) const packed = packUserOp(op) - expect(await testUtil.packUserOp(packed)).to.equal(encoded) + expect(await testUtil.encodeUserOp(packed)).to.equal(encoded) }) describe('#executeBatch', () => { From d95187bc1434b70076eccfd649759164f69e14dd Mon Sep 17 00:00:00 2001 From: shahafn Date: Thu, 21 Dec 2023 00:17:54 +0200 Subject: [PATCH 14/24] Rename UserOperation to PackedUserOperation --- contracts/core/BaseAccount.sol | 6 ++--- contracts/core/BasePaymaster.sol | 4 ++-- contracts/core/EntryPoint.sol | 22 +++++++++---------- contracts/core/EntryPointSimulations.sol | 6 ++--- contracts/core/UserOperationLib.sol | 8 +++---- contracts/interfaces/IAccount.sol | 2 +- contracts/interfaces/IAggregator.sol | 6 ++--- contracts/interfaces/IEntryPoint.sol | 6 ++--- .../interfaces/IEntryPointSimulations.sol | 4 ++-- contracts/interfaces/IPaymaster.sol | 2 +- contracts/interfaces/UserOperation.sol | 2 +- contracts/samples/DepositPaymaster.sol | 4 ++-- contracts/samples/LegacyTokenPaymaster.sol | 4 ++-- contracts/samples/SimpleAccount.sol | 2 +- contracts/samples/TokenPaymaster.sol | 2 +- contracts/samples/VerifyingPaymaster.sol | 6 ++--- contracts/samples/bls/BLSAccount.sol | 2 +- .../samples/bls/BLSSignatureAggregator.sol | 22 +++++++++---------- contracts/samples/gnosis/EIP4337Fallback.sol | 2 +- contracts/samples/gnosis/EIP4337Manager.sol | 6 ++--- contracts/test/BrokenBlsAccount.sol | 2 +- contracts/test/MaliciousAccount.sol | 2 +- contracts/test/TestAggregatedAccount.sol | 2 +- contracts/test/TestExpirePaymaster.sol | 2 +- contracts/test/TestExpiryAccount.sol | 2 +- contracts/test/TestPaymasterAcceptAll.sol | 2 +- .../test/TestPaymasterRevertCustomError.sol | 2 +- contracts/test/TestRevertAccount.sol | 2 +- contracts/test/TestSignatureAggregator.sol | 6 ++--- contracts/test/TestUtil.sol | 4 ++-- contracts/test/TestWarmColdAccount.sol | 2 +- 31 files changed, 73 insertions(+), 73 deletions(-) diff --git a/contracts/core/BaseAccount.sol b/contracts/core/BaseAccount.sol index 5c89e0f4..45b6793d 100644 --- a/contracts/core/BaseAccount.sol +++ b/contracts/core/BaseAccount.sol @@ -15,7 +15,7 @@ import "./UserOperationLib.sol"; * Specific account implementation should inherit it and provide the account-specific logic. */ abstract contract BaseAccount is IAccount { - using UserOperationLib for UserOperation; + using UserOperationLib for PackedUserOperation; /** * Return value in case of signature failure, with no time-range. @@ -48,7 +48,7 @@ abstract contract BaseAccount is IAccount { * to pay for the user operation. */ function validateUserOp( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 missingAccountFunds ) external virtual override returns (uint256 validationData) { @@ -83,7 +83,7 @@ abstract contract BaseAccount is IAccount { * Note that the validation code cannot use block.timestamp (or block.number) directly. */ function _validateSignature( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, bytes32 userOpHash ) internal virtual returns (uint256 validationData); diff --git a/contracts/core/BasePaymaster.sol b/contracts/core/BasePaymaster.sol index b6a4540d..7b6d4f53 100644 --- a/contracts/core/BasePaymaster.sol +++ b/contracts/core/BasePaymaster.sol @@ -22,7 +22,7 @@ abstract contract BasePaymaster is IPaymaster, Ownable { /// @inheritdoc IPaymaster function validatePaymasterUserOp( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost ) external override returns (bytes memory context, uint256 validationData) { @@ -37,7 +37,7 @@ abstract contract BasePaymaster is IPaymaster, Ownable { * @param maxCost - The maximum cost of the user operation. */ function _validatePaymasterUserOp( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost ) internal virtual returns (bytes memory context, uint256 validationData); diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 1dc2ff1d..fb8a1727 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -25,7 +25,7 @@ import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; */ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, OpenZeppelin.ERC165 { - using UserOperationLib for UserOperation; + using UserOperationLib for PackedUserOperation; SenderCreator private senderCreator = new SenderCreator(); @@ -70,7 +70,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, */ function _executeUserOp( uint256 opIndex, - UserOperation calldata userOp, + PackedUserOperation calldata userOp, UserOpInfo memory opInfo ) internal @@ -112,7 +112,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, /// @inheritdoc IEntryPoint function handleOps( - UserOperation[] calldata ops, + PackedUserOperation[] calldata ops, address payable beneficiary ) public nonReentrant { uint256 opslen = ops.length; @@ -154,7 +154,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, uint256 totalOps = 0; for (uint256 i = 0; i < opasLen; i++) { UserOpsPerAggregator calldata opa = opsPerAggregator[i]; - UserOperation[] calldata ops = opa.userOps; + PackedUserOperation[] calldata ops = opa.userOps; IAggregator aggregator = opa.aggregator; //address(1) is special marker of "signature error" @@ -180,7 +180,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, uint256 opIndex = 0; for (uint256 a = 0; a < opasLen; a++) { UserOpsPerAggregator calldata opa = opsPerAggregator[a]; - UserOperation[] calldata ops = opa.userOps; + PackedUserOperation[] calldata ops = opa.userOps; IAggregator aggregator = opa.aggregator; uint256 opslen = ops.length; @@ -205,7 +205,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, for (uint256 a = 0; a < opasLen; a++) { UserOpsPerAggregator calldata opa = opsPerAggregator[a]; emit SignatureAggregatorChanged(address(opa.aggregator)); - UserOperation[] calldata ops = opa.userOps; + PackedUserOperation[] calldata ops = opa.userOps; uint256 opslen = ops.length; for (uint256 i = 0; i < opslen; i++) { @@ -301,7 +301,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, /// @inheritdoc IEntryPoint function getUserOpHash( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) public view returns (bytes32) { return keccak256(abi.encode(userOp.hash(), address(this), block.chainid)); @@ -313,7 +313,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, * @param mUserOp - The memory user operation. */ function _copyUserOpToMemory( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, MemoryUserOp memory mUserOp ) internal pure { mUserOp.sender = userOp.sender; @@ -405,7 +405,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, */ function _validateAccountPrepayment( uint256 opIndex, - UserOperation calldata op, + PackedUserOperation calldata op, UserOpInfo memory opInfo, uint256 requiredPrefund ) @@ -465,7 +465,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, */ function _validatePaymasterPrepayment( uint256 opIndex, - UserOperation calldata op, + PackedUserOperation calldata op, UserOpInfo memory opInfo, uint256 requiredPreFund ) internal returns (bytes memory context, uint256 validationData) { @@ -559,7 +559,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, */ function _validatePrepayment( uint256 opIndex, - UserOperation calldata userOp, + PackedUserOperation calldata userOp, UserOpInfo memory outOpInfo ) internal diff --git a/contracts/core/EntryPointSimulations.sol b/contracts/core/EntryPointSimulations.sol index 9a5f50c9..e02de273 100644 --- a/contracts/core/EntryPointSimulations.sol +++ b/contracts/core/EntryPointSimulations.sol @@ -26,7 +26,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations { /// @inheritdoc IEntryPointSimulations function simulateValidation( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) external returns ( @@ -85,7 +85,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations { /// @inheritdoc IEntryPointSimulations function simulateHandleOp( - UserOperation calldata op, + PackedUserOperation calldata op, address target, bytes calldata targetCallData ) @@ -123,7 +123,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations { } function _simulationOnlyValidations( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) internal view diff --git a/contracts/core/UserOperationLib.sol b/contracts/core/UserOperationLib.sol index d5236105..7cb01bdf 100644 --- a/contracts/core/UserOperationLib.sol +++ b/contracts/core/UserOperationLib.sol @@ -19,7 +19,7 @@ library UserOperationLib { * @param userOp - The user operation data. */ function getSender( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) internal pure returns (address) { address data; //read sender from userOp, which is first userOp member (saves 800 gas...) @@ -35,7 +35,7 @@ library UserOperationLib { * @param userOp - The user operation data. */ function gasPrice( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) internal view returns (uint256) { unchecked { uint256 maxFeePerGas = userOp.maxFeePerGas; @@ -53,7 +53,7 @@ library UserOperationLib { * @param userOp - The user operation data. */ function encode( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) internal pure returns (bytes memory ret) { address sender = getSender(userOp); uint256 nonce = userOp.nonce; @@ -91,7 +91,7 @@ library UserOperationLib { * @param userOp - The user operation data. */ function hash( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) internal pure returns (bytes32) { return keccak256(encode(userOp)); } diff --git a/contracts/interfaces/IAccount.sol b/contracts/interfaces/IAccount.sol index 6609e989..86d96412 100644 --- a/contracts/interfaces/IAccount.sol +++ b/contracts/interfaces/IAccount.sol @@ -32,7 +32,7 @@ interface IAccount { * Note that the validation code cannot use block.timestamp (or block.number) directly. */ function validateUserOp( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 missingAccountFunds ) external returns (uint256 validationData); diff --git a/contracts/interfaces/IAggregator.sol b/contracts/interfaces/IAggregator.sol index 4226a9c1..d8b447f8 100644 --- a/contracts/interfaces/IAggregator.sol +++ b/contracts/interfaces/IAggregator.sol @@ -14,7 +14,7 @@ interface IAggregator { * @param signature - The aggregated signature. */ function validateSignatures( - UserOperation[] calldata userOps, + PackedUserOperation[] calldata userOps, bytes calldata signature ) external view; @@ -27,7 +27,7 @@ interface IAggregator { * (usually empty, unless account and aggregator support some kind of "multisig". */ function validateUserOpSignature( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) external view returns (bytes memory sigForUserOp); /** @@ -38,6 +38,6 @@ interface IAggregator { * @return aggregatedSignature - The aggregated signature. */ function aggregateSignatures( - UserOperation[] calldata userOps + PackedUserOperation[] calldata userOps ) external view returns (bytes memory aggregatedSignature); } diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol index bace9a64..dff80478 100644 --- a/contracts/interfaces/IEntryPoint.sol +++ b/contracts/interfaces/IEntryPoint.sol @@ -99,7 +99,7 @@ interface IEntryPoint is IStakeManager, INonceManager { // UserOps handled, per aggregator. struct UserOpsPerAggregator { - UserOperation[] userOps; + PackedUserOperation[] userOps; // Aggregator address IAggregator aggregator; // Aggregated signature @@ -115,7 +115,7 @@ interface IEntryPoint is IStakeManager, INonceManager { * @param beneficiary - The address to receive the fees. */ function handleOps( - UserOperation[] calldata ops, + PackedUserOperation[] calldata ops, address payable beneficiary ) external; @@ -135,7 +135,7 @@ interface IEntryPoint is IStakeManager, INonceManager { * @param userOp - The user operation to generate the request ID for. */ function getUserOpHash( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) external view returns (bytes32); /** diff --git a/contracts/interfaces/IEntryPointSimulations.sol b/contracts/interfaces/IEntryPointSimulations.sol index bcc822c5..dcd039fb 100644 --- a/contracts/interfaces/IEntryPointSimulations.sol +++ b/contracts/interfaces/IEntryPointSimulations.sol @@ -41,7 +41,7 @@ interface IEntryPointSimulations is IEntryPoint { * @param userOp - The user operation to validate. */ function simulateValidation( - UserOperation calldata userOp + PackedUserOperation calldata userOp ) external returns ( @@ -62,7 +62,7 @@ interface IEntryPointSimulations is IEntryPoint { * @param targetCallData - CallData to pass to target address. */ function simulateHandleOp( - UserOperation calldata op, + PackedUserOperation calldata op, address target, bytes calldata targetCallData ) diff --git a/contracts/interfaces/IPaymaster.sol b/contracts/interfaces/IPaymaster.sol index 7a6e076e..63c8a00d 100644 --- a/contracts/interfaces/IPaymaster.sol +++ b/contracts/interfaces/IPaymaster.sol @@ -37,7 +37,7 @@ interface IPaymaster { * Note that the validation code cannot use block.timestamp (or block.number) directly. */ function validatePaymasterUserOp( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost ) external returns (bytes memory context, uint256 validationData); diff --git a/contracts/interfaces/UserOperation.sol b/contracts/interfaces/UserOperation.sol index 4e1b8a4a..7250ae66 100644 --- a/contracts/interfaces/UserOperation.sol +++ b/contracts/interfaces/UserOperation.sol @@ -16,7 +16,7 @@ pragma solidity ^0.8.12; * The paymaster will pay for the transaction instead of the sender. * @param signature - Sender-verified signature over the entire request, the EntryPoint address and the chain ID. */ -struct UserOperation { +struct PackedUserOperation { address sender; uint256 nonce; bytes initCode; diff --git a/contracts/samples/DepositPaymaster.sol b/contracts/samples/DepositPaymaster.sol index 3e833532..afe8a347 100644 --- a/contracts/samples/DepositPaymaster.sol +++ b/contracts/samples/DepositPaymaster.sol @@ -26,7 +26,7 @@ import "./IOracle.sol"; */ contract DepositPaymaster is BasePaymaster { - using UserOperationLib for UserOperation; + using UserOperationLib for PackedUserOperation; using SafeERC20 for IERC20; //calculated cost of the postOp @@ -126,7 +126,7 @@ contract DepositPaymaster is BasePaymaster { * Note that the sender's balance is not checked. If it fails to pay from its balance, * this deposit will be used to compensate the paymaster for the transaction. */ - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) internal view override returns (bytes memory context, uint256 validationData) { (userOpHash); diff --git a/contracts/samples/LegacyTokenPaymaster.sol b/contracts/samples/LegacyTokenPaymaster.sol index 7610b74d..bc962b95 100644 --- a/contracts/samples/LegacyTokenPaymaster.sol +++ b/contracts/samples/LegacyTokenPaymaster.sol @@ -70,7 +70,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 { * verify the sender has enough tokens. * (since the paymaster is also the token, there is no notion of "approval") */ - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 /*userOpHash*/, uint256 requiredPreFund) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32 /*userOpHash*/, uint256 requiredPreFund) internal view override returns (bytes memory context, uint256 validationData) { uint256 tokenPrefund = getTokenValueOfEth(requiredPreFund); @@ -91,7 +91,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 { // when constructing an account, validate constructor code and parameters // we trust our factory (and that it doesn't have any other public methods) - function _validateConstructor(UserOperation calldata userOp) internal virtual view { + function _validateConstructor(PackedUserOperation calldata userOp) internal virtual view { address factory = address(bytes20(userOp.initCode[0 : 20])); require(factory == theFactory, "TokenPaymaster: wrong account factory"); } diff --git a/contracts/samples/SimpleAccount.sol b/contracts/samples/SimpleAccount.sol index 10f5d12b..234308ca 100644 --- a/contracts/samples/SimpleAccount.sol +++ b/contracts/samples/SimpleAccount.sol @@ -97,7 +97,7 @@ contract SimpleAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, In } /// implement template method of BaseAccount - function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash) + function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash) internal override virtual returns (uint256 validationData) { bytes32 hash = userOpHash.toEthSignedMessageHash(); if (owner != hash.recover(userOp.signature)) diff --git a/contracts/samples/TokenPaymaster.sol b/contracts/samples/TokenPaymaster.sol index 51a3580e..cb341776 100644 --- a/contracts/samples/TokenPaymaster.sol +++ b/contracts/samples/TokenPaymaster.sol @@ -119,7 +119,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { /// @param requiredPreFund The amount of tokens required for pre-funding. /// @return context The context containing the token amount and user sender address (if applicable). /// @return validationResult A uint256 value indicating the result of the validation (always 0 in this implementation). - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32, uint256 requiredPreFund) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32, uint256 requiredPreFund) internal override returns (bytes memory context, uint256 validationResult) {unchecked { diff --git a/contracts/samples/VerifyingPaymaster.sol b/contracts/samples/VerifyingPaymaster.sol index fad2a99f..3ee4bcb2 100644 --- a/contracts/samples/VerifyingPaymaster.sol +++ b/contracts/samples/VerifyingPaymaster.sol @@ -19,7 +19,7 @@ import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; contract VerifyingPaymaster is BasePaymaster { using ECDSA for bytes32; - using UserOperationLib for UserOperation; + using UserOperationLib for PackedUserOperation; address public immutable verifyingSigner; @@ -39,7 +39,7 @@ contract VerifyingPaymaster is BasePaymaster { * note that this signature covers all fields of the UserOperation, except the "paymasterAndData", * which will carry the signature itself. */ - function getHash(UserOperation calldata userOp, uint48 validUntil, uint48 validAfter) + function getHash(PackedUserOperation calldata userOp, uint48 validUntil, uint48 validAfter) public view returns (bytes32) { //can't use userOp.hash(), since it contains also the paymasterAndData itself. address sender = userOp.getSender(); @@ -70,7 +70,7 @@ contract VerifyingPaymaster is BasePaymaster { * paymasterAndData[20:84] : abi.encode(validUntil, validAfter) * paymasterAndData[84:] : signature */ - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 /*userOpHash*/, uint256 requiredPreFund) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32 /*userOpHash*/, uint256 requiredPreFund) internal view override returns (bytes memory context, uint256 validationData) { (requiredPreFund); diff --git a/contracts/samples/bls/BLSAccount.sol b/contracts/samples/bls/BLSAccount.sol index efc64ce1..82954a05 100644 --- a/contracts/samples/bls/BLSAccount.sol +++ b/contracts/samples/bls/BLSAccount.sol @@ -30,7 +30,7 @@ contract BLSAccount is SimpleAccount, IBLSAccount { _setBlsPublicKey(aPublicKey); } - function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash) + function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash) internal override view returns (uint256 validationData) { (userOp, userOpHash); diff --git a/contracts/samples/bls/BLSSignatureAggregator.sol b/contracts/samples/bls/BLSSignatureAggregator.sol index 33bb8e66..45084924 100644 --- a/contracts/samples/bls/BLSSignatureAggregator.sol +++ b/contracts/samples/bls/BLSSignatureAggregator.sol @@ -13,7 +13,7 @@ import "./BLSHelper.sol"; * A BLS-based signature aggregator, to validate aggregated signature of multiple UserOps if BLSAccount */ contract BLSSignatureAggregator is IAggregator { - using UserOperationLib for UserOperation; + using UserOperationLib for PackedUserOperation; bytes32 public constant BLS_DOMAIN = keccak256("eip4337.bls.domain"); @@ -31,7 +31,7 @@ contract BLSSignatureAggregator is IAggregator { * normally public key will be queried from the deployed BLSAccount itself; * the public key will be read from the 'initCode' if the account is not deployed yet; */ - function getUserOpPublicKey(UserOperation memory userOp) public view returns (uint256[4] memory publicKey) { + function getUserOpPublicKey(PackedUserOperation memory userOp) public view returns (uint256[4] memory publicKey) { bytes memory initCode = userOp.initCode; if (initCode.length > 0) { publicKey = getTrailingPublicKey(initCode); @@ -59,7 +59,7 @@ contract BLSSignatureAggregator is IAggregator { } /// @inheritdoc IAggregator - function validateSignatures(UserOperation[] calldata userOps, bytes calldata signature) + function validateSignatures(PackedUserOperation[] calldata userOps, bytes calldata signature) external view override { require(signature.length == 64, "BLS: invalid signature"); (uint256[2] memory blsSignature) = abi.decode(signature, (uint256[2])); @@ -69,7 +69,7 @@ contract BLSSignatureAggregator is IAggregator { uint256[2][] memory messages = new uint256[2][](userOpsLen); for (uint256 i = 0; i < userOpsLen; i++) { - UserOperation memory userOp = userOps[i]; + PackedUserOperation memory userOp = userOps[i]; blsPublicKeys[i] = getUserOpPublicKey(userOp); messages[i] = _userOpToMessage(userOp, _getPublicKeyHash(blsPublicKeys[i])); @@ -82,7 +82,7 @@ contract BLSSignatureAggregator is IAggregator { * NOTE: this hash is not the same as UserOperation.hash() * (slightly less efficient, since it uses memory userOp) */ - function internalUserOpHash(UserOperation memory userOp) internal pure returns (bytes32) { + function internalUserOpHash(PackedUserOperation memory userOp) internal pure returns (bytes32) { return keccak256(abi.encode( userOp.sender, userOp.nonce, @@ -100,22 +100,22 @@ contract BLSSignatureAggregator is IAggregator { * return the BLS "message" for the given UserOp. * the account checks the signature over this value using its public key */ - function userOpToMessage(UserOperation memory userOp) public view returns (uint256[2] memory) { + function userOpToMessage(PackedUserOperation memory userOp) public view returns (uint256[2] memory) { bytes32 publicKeyHash = _getPublicKeyHash(getUserOpPublicKey(userOp)); return _userOpToMessage(userOp, publicKeyHash); } - function _userOpToMessage(UserOperation memory userOp, bytes32 publicKeyHash) internal view returns (uint256[2] memory) { + function _userOpToMessage(PackedUserOperation memory userOp, bytes32 publicKeyHash) internal view returns (uint256[2] memory) { bytes32 userOpHash = _getUserOpHash(userOp, publicKeyHash); return BLSOpen.hashToPoint(BLS_DOMAIN, abi.encodePacked(userOpHash)); } - function getUserOpHash(UserOperation memory userOp) public view returns (bytes32) { + function getUserOpHash(PackedUserOperation memory userOp) public view returns (bytes32) { bytes32 publicKeyHash = _getPublicKeyHash(getUserOpPublicKey(userOp)); return _getUserOpHash(userOp, publicKeyHash); } - function _getUserOpHash(UserOperation memory userOp, bytes32 publicKeyHash) internal view returns (bytes32) { + function _getUserOpHash(PackedUserOperation memory userOp, bytes32 publicKeyHash) internal view returns (bytes32) { return keccak256(abi.encode(internalUserOpHash(userOp), publicKeyHash, address(this), block.chainid, entryPoint)); } @@ -130,7 +130,7 @@ contract BLSSignatureAggregator is IAggregator { * @return sigForUserOp the value to put into the signature field of the userOp when calling handleOps. * (usually empty, unless account and aggregator support some kind of "multisig" */ - function validateUserOpSignature(UserOperation calldata userOp) + function validateUserOpSignature(PackedUserOperation calldata userOp) external view returns (bytes memory sigForUserOp) { uint256[2] memory signature = abi.decode(userOp.signature, (uint256[2])); uint256[4] memory pubkey = getUserOpPublicKey(userOp); @@ -148,7 +148,7 @@ contract BLSSignatureAggregator is IAggregator { * @param userOps array of UserOperations to collect the signatures from. * @return aggregatedSignature the aggregated signature */ - function aggregateSignatures(UserOperation[] calldata userOps) external pure returns (bytes memory aggregatedSignature) { + function aggregateSignatures(PackedUserOperation[] calldata userOps) external pure returns (bytes memory aggregatedSignature) { BLSHelper.XY[] memory points = new BLSHelper.XY[](userOps.length); for (uint i = 0; i < points.length; i++) { (uint256 x, uint256 y) = abi.decode(userOps[i].signature, (uint256, uint256)); diff --git a/contracts/samples/gnosis/EIP4337Fallback.sol b/contracts/samples/gnosis/EIP4337Fallback.sol index 4cc0978d..124f0aa7 100644 --- a/contracts/samples/gnosis/EIP4337Fallback.sol +++ b/contracts/samples/gnosis/EIP4337Fallback.sol @@ -45,7 +45,7 @@ contract EIP4337Fallback is DefaultCallbackHandler, IAccount, IERC1271 { /** * called from the Safe. delegate actual work to EIP4337Manager */ - function validateUserOp(UserOperation calldata, bytes32, uint256) override external returns (uint256 deadline){ + function validateUserOp(PackedUserOperation calldata, bytes32, uint256) override external returns (uint256 deadline){ bytes memory ret = delegateToManager(); return abi.decode(ret, (uint256)); } diff --git a/contracts/samples/gnosis/EIP4337Manager.sol b/contracts/samples/gnosis/EIP4337Manager.sol index a89a943d..951688eb 100644 --- a/contracts/samples/gnosis/EIP4337Manager.sol +++ b/contracts/samples/gnosis/EIP4337Manager.sol @@ -42,7 +42,7 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor { /** * delegate-called (using execFromModule) through the fallback, so "real" msg.sender is attached as last 20 bytes */ - function validateUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 missingAccountFunds) + function validateUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 missingAccountFunds) external override returns (uint256 validationData) { address msgSender = address(bytes20(msg.data[msg.data.length - 20 :])); require(msgSender == entryPoint, "account: not from entrypoint"); @@ -163,8 +163,8 @@ contract EIP4337Manager is IAccount, GnosisSafeStorage, Executor { sig[2] = bytes1(uint8(1)); sig[35] = bytes1(uint8(1)); uint256 nonce = uint256(IEntryPoint(manager.entryPoint()).getNonce(address(safe), 0)); - UserOperation memory userOp = UserOperation(address(safe), nonce, "", "", bytes32(bytes16(uint128(0x0f4240))), 0, 0, 0, "", sig); - UserOperation[] memory userOps = new UserOperation[](1); + PackedUserOperation memory userOp = PackedUserOperation(address(safe), nonce, "", "", bytes32(bytes16(uint128(0x0f4240))), 0, 0, 0, "", sig); + PackedUserOperation[] memory userOps = new PackedUserOperation[](1); userOps[0] = userOp; IEntryPoint _entryPoint = IEntryPoint(payable(manager.entryPoint())); try _entryPoint.handleOps(userOps, payable(msg.sender)) { diff --git a/contracts/test/BrokenBlsAccount.sol b/contracts/test/BrokenBlsAccount.sol index 5690c696..dfd7381f 100644 --- a/contracts/test/BrokenBlsAccount.sol +++ b/contracts/test/BrokenBlsAccount.sol @@ -27,7 +27,7 @@ contract BrokenBLSAccount is SimpleAccount, IBLSAccount { super._initialize(address(0)); } - function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash) + function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash) internal override view returns (uint256 validationData) { (userOp, userOpHash); diff --git a/contracts/test/MaliciousAccount.sol b/contracts/test/MaliciousAccount.sol index a4f53825..704e44f9 100644 --- a/contracts/test/MaliciousAccount.sol +++ b/contracts/test/MaliciousAccount.sol @@ -9,7 +9,7 @@ contract MaliciousAccount is IAccount { constructor(IEntryPoint _ep) payable { ep = _ep; } - function validateUserOp(UserOperation calldata userOp, bytes32, uint256 missingAccountFunds) + function validateUserOp(PackedUserOperation calldata userOp, bytes32, uint256 missingAccountFunds) external returns (uint256 validationData) { ep.depositTo{value : missingAccountFunds}(address(this)); // Now calculate basefee per EntryPoint.getUserOpGasPrice() and compare it to the basefe we pass off-chain in the signature diff --git a/contracts/test/TestAggregatedAccount.sol b/contracts/test/TestAggregatedAccount.sol index 23bf2954..1d5d93b1 100644 --- a/contracts/test/TestAggregatedAccount.sol +++ b/contracts/test/TestAggregatedAccount.sol @@ -22,7 +22,7 @@ contract TestAggregatedAccount is SimpleAccount { super._initialize(address(0)); } - function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash) + function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash) internal override view returns (uint256 validationData) { (userOp, userOpHash); return _packValidationData(ValidationData(aggregator, 0, 0)); diff --git a/contracts/test/TestExpirePaymaster.sol b/contracts/test/TestExpirePaymaster.sol index abf9897b..dda059c0 100644 --- a/contracts/test/TestExpirePaymaster.sol +++ b/contracts/test/TestExpirePaymaster.sol @@ -11,7 +11,7 @@ contract TestExpirePaymaster is BasePaymaster { constructor(IEntryPoint _entryPoint) BasePaymaster(_entryPoint) {} - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) internal virtual override view returns (bytes memory context, uint256 validationData) { (userOp, userOpHash, maxCost); diff --git a/contracts/test/TestExpiryAccount.sol b/contracts/test/TestExpiryAccount.sol index 294f4aaf..5c9e21a0 100644 --- a/contracts/test/TestExpiryAccount.sol +++ b/contracts/test/TestExpiryAccount.sol @@ -35,7 +35,7 @@ contract TestExpiryAccount is SimpleAccount { } /// implement template method of BaseAccount - function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash) + function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash) internal override view returns (uint256 validationData) { bytes32 hash = userOpHash.toEthSignedMessageHash(); address signer = hash.recover(userOp.signature); diff --git a/contracts/test/TestPaymasterAcceptAll.sol b/contracts/test/TestPaymasterAcceptAll.sol index 6445717d..1d602150 100644 --- a/contracts/test/TestPaymasterAcceptAll.sol +++ b/contracts/test/TestPaymasterAcceptAll.sol @@ -17,7 +17,7 @@ contract TestPaymasterAcceptAll is BasePaymaster { } - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) internal virtual override view returns (bytes memory context, uint256 validationData) { (userOp, userOpHash, maxCost); diff --git a/contracts/test/TestPaymasterRevertCustomError.sol b/contracts/test/TestPaymasterRevertCustomError.sol index 1ceac5e0..da56c66d 100644 --- a/contracts/test/TestPaymasterRevertCustomError.sol +++ b/contracts/test/TestPaymasterRevertCustomError.sol @@ -13,7 +13,7 @@ contract TestPaymasterRevertCustomError is BasePaymaster { constructor(IEntryPoint _entryPoint) BasePaymaster(_entryPoint) {} - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32, uint256) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32, uint256) internal virtual override view returns (bytes memory context, uint256 validationData) { validationData = 0; diff --git a/contracts/test/TestRevertAccount.sol b/contracts/test/TestRevertAccount.sol index d7c376c4..6b0523de 100644 --- a/contracts/test/TestRevertAccount.sol +++ b/contracts/test/TestRevertAccount.sol @@ -9,7 +9,7 @@ contract TestRevertAccount is IAccount { ep = _ep; } - function validateUserOp(UserOperation calldata, bytes32, uint256 missingAccountFunds) + function validateUserOp(PackedUserOperation calldata, bytes32, uint256 missingAccountFunds) external override returns (uint256 validationData) { ep.depositTo{value : missingAccountFunds}(address(this)); return 0; diff --git a/contracts/test/TestSignatureAggregator.sol b/contracts/test/TestSignatureAggregator.sol index 97b9eb5f..1eeb68fd 100644 --- a/contracts/test/TestSignatureAggregator.sol +++ b/contracts/test/TestSignatureAggregator.sol @@ -14,7 +14,7 @@ import "../samples/SimpleAccount.sol"; contract TestSignatureAggregator is IAggregator { /// @inheritdoc IAggregator - function validateSignatures(UserOperation[] calldata userOps, bytes calldata signature) external pure override { + function validateSignatures(PackedUserOperation[] calldata userOps, bytes calldata signature) external pure override { uint sum = 0; for (uint i = 0; i < userOps.length; i++) { uint nonce = userOps[i].nonce; @@ -26,7 +26,7 @@ contract TestSignatureAggregator is IAggregator { } /// @inheritdoc IAggregator - function validateUserOpSignature(UserOperation calldata) + function validateUserOpSignature(PackedUserOperation calldata) external pure returns (bytes memory) { return ""; } @@ -34,7 +34,7 @@ contract TestSignatureAggregator is IAggregator { /** * dummy test aggregator: sum all nonce values of UserOps. */ - function aggregateSignatures(UserOperation[] calldata userOps) external pure returns (bytes memory aggregatedSignature) { + function aggregateSignatures(PackedUserOperation[] calldata userOps) external pure returns (bytes memory aggregatedSignature) { uint sum = 0; for (uint i = 0; i < userOps.length; i++) { sum += userOps[i].nonce; diff --git a/contracts/test/TestUtil.sol b/contracts/test/TestUtil.sol index 594868b2..11f081ba 100644 --- a/contracts/test/TestUtil.sol +++ b/contracts/test/TestUtil.sol @@ -5,9 +5,9 @@ import "../interfaces/UserOperation.sol"; import "../core/UserOperationLib.sol"; contract TestUtil { - using UserOperationLib for UserOperation; + using UserOperationLib for PackedUserOperation; - function encodeUserOp(UserOperation calldata op) external pure returns (bytes memory){ + function encodeUserOp(PackedUserOperation calldata op) external pure returns (bytes memory){ return op.encode(); } diff --git a/contracts/test/TestWarmColdAccount.sol b/contracts/test/TestWarmColdAccount.sol index 76f35bc8..4e69cd6f 100644 --- a/contracts/test/TestWarmColdAccount.sol +++ b/contracts/test/TestWarmColdAccount.sol @@ -14,7 +14,7 @@ contract TestWarmColdAccount is IAccount { ep = _ep; } - function validateUserOp(UserOperation calldata userOp, bytes32, uint256 missingAccountFunds) + function validateUserOp(PackedUserOperation calldata userOp, bytes32, uint256 missingAccountFunds) external override returns (uint256 validationData) { ep.depositTo{value : missingAccountFunds}(address(this)); if (userOp.nonce == 1) { From 415ef698ff1ab5a25a44acda2fcab93576405be6 Mon Sep 17 00:00:00 2001 From: shahafn Date: Thu, 21 Dec 2023 00:44:47 +0200 Subject: [PATCH 15/24] Gas calc --- 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 ee826227..efb1b60a 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 │ 82855 │ │ ║ +║ simple │ 1 │ 82836 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 45041 │ 16027 ║ +║ simple - diff from previous │ 2 │ │ 45022 │ 16008 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 488438 │ │ ║ +║ simple │ 10 │ 488236 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 45041 │ 16027 ║ +║ simple - diff from previous │ 11 │ │ 45046 │ 16032 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 89734 │ │ ║ +║ simple paymaster │ 1 │ 89242 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 44633 │ 15619 ║ +║ simple paymaster with diff │ 2 │ │ 44189 │ 15175 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 491588 │ │ ║ +║ simple paymaster │ 10 │ 487100 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 44630 │ 15616 ║ +║ simple paymaster with diff │ 11 │ │ 44198 │ 15184 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 183900 │ │ ║ +║ big tx 5k │ 1 │ 183893 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 145553 │ 20293 ║ +║ big tx - diff from previous │ 2 │ │ 145522 │ 20262 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1494090 │ │ ║ +║ big tx 5k │ 10 │ 1493876 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 145600 │ 20340 ║ +║ big tx - diff from previous │ 11 │ │ 145641 │ 20381 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 47a57fa590f3965f493a3c62001b0fafaa0057b8 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jan 2024 22:09:01 +0200 Subject: [PATCH 16/24] remove debug comments --- test/samples/TokenPaymaster.test.ts | 1 - test/verifying_paymaster.test.ts | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/test/samples/TokenPaymaster.test.ts b/test/samples/TokenPaymaster.test.ts index ffcdf833..a254d506 100644 --- a/test/samples/TokenPaymaster.test.ts +++ b/test/samples/TokenPaymaster.test.ts @@ -265,7 +265,6 @@ describe('TokenPaymaster', function () { }, entryPoint) op = signUserOp(op, accountOwner, entryPoint.address, chainId) const opPacked = packUserOp(op) - console.log('op', opPacked, op) // for simpler 'gasPrice()' calculation await ethers.provider.send('hardhat_setNextBlockBaseFeePerGas', [utils.hexlify(op.maxFeePerGas)]) diff --git a/test/verifying_paymaster.test.ts b/test/verifying_paymaster.test.ts index c48375a7..fa15ebb3 100644 --- a/test/verifying_paymaster.test.ts +++ b/test/verifying_paymaster.test.ts @@ -50,10 +50,10 @@ describe('EntryPoint with VerifyingPaymaster', function () { ]) console.log(paymasterAndData) const res = await paymaster.parsePaymasterAndData(paymasterAndData) - console.log('MOCK_VALID_UNTIL, MOCK_VALID_AFTER', MOCK_VALID_UNTIL, MOCK_VALID_AFTER) - console.log('validUntil after', res.validUntil, res.validAfter) - console.log('MOCK SIG', MOCK_SIG) - console.log('sig', res.signature) + // console.log('MOCK_VALID_UNTIL, MOCK_VALID_AFTER', MOCK_VALID_UNTIL, MOCK_VALID_AFTER) + // console.log('validUntil after', res.validUntil, res.validAfter) + // console.log('MOCK SIG', MOCK_SIG) + // console.log('sig', res.signature) expect(res.validUntil).to.be.equal(ethers.BigNumber.from(MOCK_VALID_UNTIL)) expect(res.validAfter).to.be.equal(ethers.BigNumber.from(MOCK_VALID_AFTER)) expect(res.signature).equal(MOCK_SIG) From 08ef38ea553ffbcb2dc8e6b42f597912f4e7399f Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Wed, 3 Jan 2024 17:21:43 +0200 Subject: [PATCH 17/24] changes --- contracts/core/UserOperationLib.sol | 2 +- contracts/interfaces/IAccount.sol | 2 +- contracts/interfaces/IAccountExecute.sol | 4 +- contracts/interfaces/IAggregator.sol | 2 +- contracts/interfaces/IEntryPoint.sol | 2 +- .../interfaces/IEntryPointSimulations.sol | 2 +- contracts/interfaces/IPaymaster.sol | 2 +- contracts/test/TestExecAccount.sol | 4 +- contracts/test/TestPaymasterWithPostOp.sol | 2 +- contracts/test/TestUtil.sol | 2 +- reports/gas-checker.txt | 40 +++++++++---------- test/testExecAccount.test.ts | 4 +- 12 files changed, 34 insertions(+), 34 deletions(-) diff --git a/contracts/core/UserOperationLib.sol b/contracts/core/UserOperationLib.sol index 7cb01bdf..3ecef7dc 100644 --- a/contracts/core/UserOperationLib.sol +++ b/contracts/core/UserOperationLib.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.12; /* solhint-disable no-inline-assembly */ -import "../interfaces/UserOperation.sol"; +import "../interfaces/PackedUserOperation.sol"; import {calldataKeccak} from "./Helpers.sol"; /** diff --git a/contracts/interfaces/IAccount.sol b/contracts/interfaces/IAccount.sol index 86d96412..7d4f04e5 100644 --- a/contracts/interfaces/IAccount.sol +++ b/contracts/interfaces/IAccount.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.12; -import "./UserOperation.sol"; +import "./PackedUserOperation.sol"; interface IAccount { /** diff --git a/contracts/interfaces/IAccountExecute.sol b/contracts/interfaces/IAccountExecute.sol index 440bd4b0..825ab90e 100644 --- a/contracts/interfaces/IAccountExecute.sol +++ b/contracts/interfaces/IAccountExecute.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.12; -import "./UserOperation.sol"; +import "./PackedUserOperation.sol"; interface IAccountExecute { /** @@ -14,7 +14,7 @@ interface IAccountExecute { * @param userOpHash - Hash of the user's request data. */ function executeUserOp( - UserOperation calldata userOp, + PackedUserOperation calldata userOp, bytes32 userOpHash ) external; } diff --git a/contracts/interfaces/IAggregator.sol b/contracts/interfaces/IAggregator.sol index d8b447f8..ad5efe1d 100644 --- a/contracts/interfaces/IAggregator.sol +++ b/contracts/interfaces/IAggregator.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.12; -import "./UserOperation.sol"; +import "./PackedUserOperation.sol"; /** * Aggregated Signatures validator. diff --git a/contracts/interfaces/IEntryPoint.sol b/contracts/interfaces/IEntryPoint.sol index e1300066..01e7c483 100644 --- a/contracts/interfaces/IEntryPoint.sol +++ b/contracts/interfaces/IEntryPoint.sol @@ -9,7 +9,7 @@ pragma solidity ^0.8.12; /* solhint-disable no-inline-assembly */ /* solhint-disable reason-string */ -import "./UserOperation.sol"; +import "./PackedUserOperation.sol"; import "./IStakeManager.sol"; import "./IAggregator.sol"; import "./INonceManager.sol"; diff --git a/contracts/interfaces/IEntryPointSimulations.sol b/contracts/interfaces/IEntryPointSimulations.sol index dcd039fb..9e2c5d26 100644 --- a/contracts/interfaces/IEntryPointSimulations.sol +++ b/contracts/interfaces/IEntryPointSimulations.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.12; -import "./UserOperation.sol"; +import "./PackedUserOperation.sol"; import "./IEntryPoint.sol"; interface IEntryPointSimulations is IEntryPoint { diff --git a/contracts/interfaces/IPaymaster.sol b/contracts/interfaces/IPaymaster.sol index 63c8a00d..65f69093 100644 --- a/contracts/interfaces/IPaymaster.sol +++ b/contracts/interfaces/IPaymaster.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.12; -import "./UserOperation.sol"; +import "./PackedUserOperation.sol"; /** * The interface exposed by a paymaster contract, who agrees to pay the gas for user's operations. diff --git a/contracts/test/TestExecAccount.sol b/contracts/test/TestExecAccount.sol index 9a624f03..44df4040 100644 --- a/contracts/test/TestExecAccount.sol +++ b/contracts/test/TestExecAccount.sol @@ -20,9 +20,9 @@ contract TestExecAccount is SimpleAccount, IAccountExecute { constructor(IEntryPoint anEntryPoint) SimpleAccount(anEntryPoint){ } - event Executed(UserOperation userOp, bytes innerCallRet); + event Executed(PackedUserOperation userOp, bytes innerCallRet); - function executeUserOp(UserOperation calldata userOp, bytes32 /*userOpHash*/) external { + function executeUserOp(PackedUserOperation calldata userOp, bytes32 /*userOpHash*/) external { _requireFromEntryPointOrOwner(); // read from the userOp.callData, but skip the "magic" prefix (executeUserOp sig), diff --git a/contracts/test/TestPaymasterWithPostOp.sol b/contracts/test/TestPaymasterWithPostOp.sol index 694dde51..9590d513 100644 --- a/contracts/test/TestPaymasterWithPostOp.sol +++ b/contracts/test/TestPaymasterWithPostOp.sol @@ -13,7 +13,7 @@ contract TestPaymasterWithPostOp is TestPaymasterAcceptAll { constructor(IEntryPoint _entryPoint) TestPaymasterAcceptAll(_entryPoint) { } - function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) + function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost) internal virtual override view returns (bytes memory context, uint256 validationData) { (userOp, userOpHash, maxCost); diff --git a/contracts/test/TestUtil.sol b/contracts/test/TestUtil.sol index 11f081ba..a1e10f53 100644 --- a/contracts/test/TestUtil.sol +++ b/contracts/test/TestUtil.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.12; -import "../interfaces/UserOperation.sol"; +import "../interfaces/PackedUserOperation.sol"; import "../core/UserOperationLib.sol"; contract TestUtil { diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 3fbf000b..85dcdf10 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -12,44 +12,44 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 81570 │ │ ║ +║ simple │ 1 │ 82677 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 43842 │ 14863 ║ +║ simple - diff from previous │ 2 │ │ 44872 │ 15893 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 476227 │ │ ║ +║ simple │ 10 │ 486716 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 43873 │ 14894 ║ +║ simple - diff from previous │ 11 │ │ 44917 │ 15938 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 1 │ 87750 │ │ ║ +║ simple paymaster │ 1 │ 89079 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 42716 │ 13737 ║ +║ simple paymaster with diff │ 2 │ │ 43963 │ 14984 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 472423 │ │ ║ +║ simple paymaster │ 10 │ 485035 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 42781 │ 13802 ║ +║ simple paymaster with diff │ 11 │ │ 44041 │ 15062 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 1 │ 182627 │ │ ║ +║ big tx 5k │ 1 │ 183735 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 2 │ │ 144341 │ 19117 ║ +║ big tx - diff from previous │ 2 │ │ 145383 │ 20159 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1481819 │ │ ║ +║ big tx 5k │ 10 │ 1492399 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 144445 │ 19221 ║ +║ big tx - diff from previous │ 11 │ │ 145428 │ 20204 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 89483 │ │ ║ +║ paymaster+postOp │ 1 │ 90895 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 44463 │ 15484 ║ +║ paymaster+postOp with diff │ 2 │ │ 45802 │ 16823 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 489783 │ │ ║ +║ paymaster+postOp │ 10 │ 503359 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 44508 │ 15529 ║ +║ paymaster+postOp with diff │ 11 │ │ 45899 │ 16920 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster │ 1 │ 147976 │ │ ║ +║ token paymaster │ 1 │ 149447 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 2 │ │ 72659 │ 43680 ║ +║ token paymaster with diff │ 2 │ │ 74010 │ 45031 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster │ 10 │ 802242 │ │ ║ +║ token paymaster │ 10 │ 816138 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 11 │ │ 72727 │ 43748 ║ +║ token paymaster with diff │ 11 │ │ 74178 │ 45199 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ diff --git a/test/testExecAccount.test.ts b/test/testExecAccount.test.ts index 4a1e5035..312a270f 100644 --- a/test/testExecAccount.test.ts +++ b/test/testExecAccount.test.ts @@ -6,7 +6,7 @@ import { TestExecAccountFactory__factory } from '../typechain' import { createAccountOwner, deployEntryPoint, fund, objdump } from './testutils' -import { fillAndSign } from './UserOp' +import { fillSignAndPack } from './UserOp' import { Signer, Wallet } from 'ethers' import { ethers } from 'hardhat' import { defaultAbiCoder, hexConcat, hexStripZeros } from 'ethers/lib/utils' @@ -37,7 +37,7 @@ describe('IAccountExecute', () => { account.interface.encodeFunctionData('entryPoint') ]) - const userOp = await fillAndSign({ + const userOp = await fillSignAndPack({ sender: account.address, callGasLimit: 100000, // normal estimate also chokes on this callData callData: hexConcat([execSig, innerCall]) From 052a8ad5cc0b66b4696b03a84a1abbdbcc722aff Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Wed, 3 Jan 2024 17:25:59 +0200 Subject: [PATCH 18/24] fix tokenpaymaster test --- test/samples/TokenPaymaster.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/samples/TokenPaymaster.test.ts b/test/samples/TokenPaymaster.test.ts index 2ee230c7..1ef3bf4c 100644 --- a/test/samples/TokenPaymaster.test.ts +++ b/test/samples/TokenPaymaster.test.ts @@ -301,6 +301,7 @@ describe('TokenPaymaster', function () { const overrideTokenPrice = priceDenominator.mul(50) let op = await fillUserOp({ sender: account.address, + maxFeePerGas: 1000000000, paymaster: paymasterAddress, paymasterVerificationGasLimit: 3e5, paymasterPostOpGasLimit: 3e5, From 277c37901be8bcde1e6c81f789b80f9de4549843 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Wed, 3 Jan 2024 18:54:06 +0200 Subject: [PATCH 19/24] fixed pr comments --- contracts/core/EntryPoint.sol | 2 +- contracts/core/UserOperationLib.sol | 10 +++-- contracts/samples/TokenPaymaster.sol | 2 +- contracts/samples/VerifyingPaymaster.sol | 47 ++++++++++++------------ contracts/test/TestExpirePaymaster.sol | 3 +- reports/gas-checker.txt | 26 ++++++------- test/UserOp.ts | 8 ++-- test/UserOperation.ts | 2 +- test/entrypoint.test.ts | 2 +- test/testutils.ts | 2 +- test/verifying_paymaster.test.ts | 17 +++++---- 11 files changed, 64 insertions(+), 57 deletions(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 501fe4df..877e922f 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -354,7 +354,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, bytes calldata paymasterAndData = userOp.paymasterAndData; if (paymasterAndData.length > 0) { require( - paymasterAndData.length >= 20, + paymasterAndData.length >= UserOperationLib.PAYMASTER_DATA_OFFSET, "AA93 invalid paymasterAndData" ); (mUserOp.paymaster, mUserOp.paymasterVerificationGasLimit, mUserOp.paymasterPostOpGasLimit) = UserOperationLib.unpackPaymasterStaticFields(paymasterAndData); diff --git a/contracts/core/UserOperationLib.sol b/contracts/core/UserOperationLib.sol index 3ecef7dc..511b3a55 100644 --- a/contracts/core/UserOperationLib.sol +++ b/contracts/core/UserOperationLib.sol @@ -76,14 +76,18 @@ library UserOperationLib { function unpackAccountGasLimits( bytes32 accountGasLimits - ) internal pure returns(uint128 validationGasLimit, uint128 callGasLimit) { + ) internal pure returns (uint128 validationGasLimit, uint128 callGasLimit) { return (uint128(bytes16(accountGasLimits)), uint128(uint256(accountGasLimits))); } function unpackPaymasterStaticFields( bytes calldata paymasterAndData - ) internal pure returns(address paymaster, uint128 validationGasLimit, uint128 postOp) { - return (address(bytes20(paymasterAndData[:PAYMASTER_VALIDATION_GAS_OFFSET])), uint128(bytes16(paymasterAndData[PAYMASTER_VALIDATION_GAS_OFFSET:PAYMASTER_POSTOP_GAS_OFFSET])), uint128(bytes16(paymasterAndData[PAYMASTER_POSTOP_GAS_OFFSET:PAYMASTER_DATA_OFFSET]))); + ) internal pure returns (address paymaster, uint128 validationGasLimit, uint128 postOp) { + return ( + address(bytes20(paymasterAndData[: PAYMASTER_VALIDATION_GAS_OFFSET])), + uint128(bytes16(paymasterAndData[PAYMASTER_VALIDATION_GAS_OFFSET : PAYMASTER_POSTOP_GAS_OFFSET])), + uint128(bytes16(paymasterAndData[PAYMASTER_POSTOP_GAS_OFFSET : PAYMASTER_DATA_OFFSET])) + ); } /** diff --git a/contracts/samples/TokenPaymaster.sol b/contracts/samples/TokenPaymaster.sol index 1360025c..2b40c6cc 100644 --- a/contracts/samples/TokenPaymaster.sol +++ b/contracts/samples/TokenPaymaster.sol @@ -129,7 +129,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { // note: as price is in ether-per-token and we want more tokens increasing it means dividing it by markup uint256 cachedPriceWithMarkup = cachedPrice * PRICE_DENOMINATOR / priceMarkup; if (paymasterAndDataLength == 32) { - uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[UserOperationLib.PAYMASTER_DATA_OFFSET : 84])); + uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[UserOperationLib.PAYMASTER_DATA_OFFSET : UserOperationLib.PAYMASTER_DATA_OFFSET + 32])); if (clientSuppliedPrice < cachedPriceWithMarkup) { // note: smaller number means 'more ether per token' cachedPriceWithMarkup = clientSuppliedPrice; diff --git a/contracts/samples/VerifyingPaymaster.sol b/contracts/samples/VerifyingPaymaster.sol index 2be66a33..8bad94c8 100644 --- a/contracts/samples/VerifyingPaymaster.sol +++ b/contracts/samples/VerifyingPaymaster.sol @@ -23,15 +23,14 @@ contract VerifyingPaymaster is BasePaymaster { address public immutable verifyingSigner; - uint256 private constant VALID_TIMESTAMP_OFFSET = 52; + uint256 private constant VALID_TIMESTAMP_OFFSET = UserOperationLib.PAYMASTER_DATA_OFFSET; - uint256 private constant SIGNATURE_OFFSET = 116; + uint256 private constant SIGNATURE_OFFSET = VALID_TIMESTAMP_OFFSET + 64; constructor(IEntryPoint _entryPoint, address _verifyingSigner) BasePaymaster(_entryPoint) { verifyingSigner = _verifyingSigner; } - /** * return the hash we're going to sign off-chain (and validate on-chain) * this method is called by the off-chain service, to sign the request. @@ -45,22 +44,22 @@ contract VerifyingPaymaster is BasePaymaster { address sender = userOp.getSender(); return keccak256( - abi.encode( - sender, - userOp.nonce, - keccak256(userOp.initCode), - keccak256(userOp.callData), - userOp.accountGasLimits, - uint256(bytes32(userOp.paymasterAndData[20:52])), - userOp.preVerificationGas, - userOp.maxFeePerGas, - userOp.maxPriorityFeePerGas, - block.chainid, - address(this), - validUntil, - validAfter - ) - ); + abi.encode( + sender, + userOp.nonce, + keccak256(userOp.initCode), + keccak256(userOp.callData), + userOp.accountGasLimits, + uint256(bytes32(userOp.paymasterAndData[20 : 52])), + userOp.preVerificationGas, + userOp.maxFeePerGas, + userOp.maxPriorityFeePerGas, + block.chainid, + address(this), + validUntil, + validAfter + ) + ); } /** @@ -82,16 +81,16 @@ contract VerifyingPaymaster is BasePaymaster { //don't revert on signature failure: return SIG_VALIDATION_FAILED if (verifyingSigner != ECDSA.recover(hash, signature)) { - return ("",_packValidationData(true,validUntil,validAfter)); + return ("", _packValidationData(true, validUntil, validAfter)); } //no need for other on-chain validation: entire UserOp should have been checked // by the external service prior to signing it. - return ("",_packValidationData(false,validUntil,validAfter)); + return ("", _packValidationData(false, validUntil, validAfter)); } - function parsePaymasterAndData(bytes calldata paymasterAndData) public pure returns(uint48 validUntil, uint48 validAfter, bytes calldata signature) { - (validUntil, validAfter) = abi.decode(paymasterAndData[VALID_TIMESTAMP_OFFSET:SIGNATURE_OFFSET],(uint48, uint48)); - signature = paymasterAndData[SIGNATURE_OFFSET:]; + function parsePaymasterAndData(bytes calldata paymasterAndData) public pure returns (uint48 validUntil, uint48 validAfter, bytes calldata signature) { + (validUntil, validAfter) = abi.decode(paymasterAndData[VALID_TIMESTAMP_OFFSET :], (uint48, uint48)); + signature = paymasterAndData[SIGNATURE_OFFSET :]; } } diff --git a/contracts/test/TestExpirePaymaster.sol b/contracts/test/TestExpirePaymaster.sol index dda059c0..de7a374e 100644 --- a/contracts/test/TestExpirePaymaster.sol +++ b/contracts/test/TestExpirePaymaster.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.12; import "../core/BasePaymaster.sol"; +import "../core/UserOperationLib.sol"; /** * test expiry mechanism: paymasterData encodes the "validUntil" and validAfter" times @@ -15,7 +16,7 @@ contract TestExpirePaymaster is BasePaymaster { internal virtual override view returns (bytes memory context, uint256 validationData) { (userOp, userOpHash, maxCost); - (uint48 validAfter, uint48 validUntil) = abi.decode(userOp.paymasterAndData[52 :], (uint48, uint48)); + (uint48 validAfter, uint48 validUntil) = abi.decode(userOp.paymasterAndData[UserOperationLib.PAYMASTER_DATA_OFFSET :], (uint48, uint48)); validationData = _packValidationData(false, validUntil, validAfter); context = ""; } diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 85dcdf10..2b3b6a5f 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -12,11 +12,11 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 82677 │ │ ║ +║ simple │ 1 │ 82653 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple - diff from previous │ 2 │ │ 44872 │ 15893 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 486716 │ │ ║ +║ simple │ 10 │ 486704 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple - diff from previous │ 11 │ │ 44917 │ 15938 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ @@ -24,32 +24,32 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple paymaster with diff │ 2 │ │ 43963 │ 14984 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 485035 │ │ ║ +║ simple paymaster │ 10 │ 485071 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 44041 │ 15062 ║ +║ simple paymaster with diff │ 11 │ │ 44017 │ 15038 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx 5k │ 1 │ 183735 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx - diff from previous │ 2 │ │ 145383 │ 20159 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1492399 │ │ ║ +║ big tx 5k │ 10 │ 1492339 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 145428 │ 20204 ║ +║ big tx - diff from previous │ 11 │ │ 145440 │ 20216 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 90895 │ │ ║ +║ paymaster+postOp │ 1 │ 90907 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 45802 │ 16823 ║ +║ paymaster+postOp with diff │ 2 │ │ 45814 │ 16835 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 503359 │ │ ║ +║ paymaster+postOp │ 10 │ 503383 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 45899 │ 16920 ║ +║ paymaster+postOp with diff │ 11 │ │ 45851 │ 16872 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ token paymaster │ 1 │ 149447 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 2 │ │ 74010 │ 45031 ║ +║ token paymaster with diff │ 2 │ │ 74022 │ 45043 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster │ 10 │ 816138 │ │ ║ +║ token paymaster │ 10 │ 816150 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 11 │ │ 74178 │ 45199 ║ +║ token paymaster with diff │ 11 │ │ 74094 │ 45115 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ diff --git a/test/UserOp.ts b/test/UserOp.ts index 67afcb85..fac8540a 100644 --- a/test/UserOp.ts +++ b/test/UserOp.ts @@ -21,7 +21,7 @@ import { IEntryPointSimulations } from '../typechain/contracts/core/EntryPointSi export function packUserOp (userOp: UserOperation): PackedUserOperation { const accountGasLimits = packAccountGasLimits(userOp.verificationGasLimit, userOp.callGasLimit) let paymasterAndData = '0x' - if (userOp.paymaster.length >= 20) { + if (userOp.paymaster.length >= 20 && userOp.paymaster !== AddressZero) { paymasterAndData = packPaymasterData(userOp.paymaster as string, userOp.paymasterVerificationGasLimit, userOp.paymasterPostOpGasLimit, userOp.paymasterData as string) } return { @@ -42,7 +42,7 @@ export function encodeUserOp (userOp: UserOperation, forSignature = true): strin if (forSignature) { return defaultAbiCoder.encode( ['address', 'uint256', 'bytes32', 'bytes32', - 'uint256', 'uint256', 'uint256', 'uint256', + 'bytes32', 'uint256', 'uint256', 'uint256', 'bytes32'], [packedUserOp.sender, packedUserOp.nonce, keccak256(packedUserOp.initCode), keccak256(packedUserOp.callData), packedUserOp.accountGasLimits, packedUserOp.preVerificationGas, packedUserOp.maxFeePerGas, packedUserOp.maxPriorityFeePerGas, @@ -51,7 +51,7 @@ export function encodeUserOp (userOp: UserOperation, forSignature = true): strin // for the purpose of calculating gas cost encode also signature (and no keccak of bytes) return defaultAbiCoder.encode( ['address', 'uint256', 'bytes', 'bytes', - 'uint256', 'uint256', 'uint256', 'uint256', + 'bytes32', 'uint256', 'uint256', 'uint256', 'bytes', 'bytes'], [packedUserOp.sender, packedUserOp.nonce, packedUserOp.initCode, packedUserOp.callData, packedUserOp.accountGasLimits, packedUserOp.preVerificationGas, packedUserOp.maxFeePerGas, packedUserOp.maxPriorityFeePerGas, @@ -77,7 +77,7 @@ export const DefaultsForUserOp: UserOperation = { preVerificationGas: 21000, // should also cover calldata cost. maxFeePerGas: 0, maxPriorityFeePerGas: 1e9, - paymaster: '0x', + paymaster: AddressZero, paymasterData: '0x', paymasterVerificationGasLimit: 3e5, paymasterPostOpGasLimit: 0, diff --git a/test/UserOperation.ts b/test/UserOperation.ts index c1b2d327..778f69b3 100644 --- a/test/UserOperation.ts +++ b/test/UserOperation.ts @@ -11,7 +11,7 @@ export interface UserOperation { preVerificationGas: typ.uint256 maxFeePerGas: typ.uint256 maxPriorityFeePerGas: typ.uint256 - paymaster: typ.bytes + paymaster: typ.address paymasterVerificationGasLimit: typ.uint128 paymasterPostOpGasLimit: typ.uint128 paymasterData: typ.bytes diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts index 6415b3ef..d4ca06ca 100644 --- a/test/entrypoint.test.ts +++ b/test/entrypoint.test.ts @@ -275,7 +275,7 @@ describe('EntryPoint', function () { // we need maxFeeperGas > block.basefee + maxPriorityFeePerGas so requiredPrefund onchain is basefee + maxPriorityFeePerGas maxFeePerGas: block.baseFeePerGas.mul(3), maxPriorityFeePerGas: block.baseFeePerGas, - paymaster: '0x', + paymaster: AddressZero, paymasterData: '0x', paymasterVerificationGasLimit: 0, paymasterPostOpGasLimit: 0 diff --git a/test/testutils.ts b/test/testutils.ts index 0e1c92fe..a19b58b8 100644 --- a/test/testutils.ts +++ b/test/testutils.ts @@ -304,7 +304,7 @@ export async function createAccount ( } } -export function packAccountGasLimits (validationGasLimit: BytesLike | Hexable | number | bigint, callGasLimit: BytesLike | Hexable | number | bigint): string { +export function packAccountGasLimits (validationGasLimit: BigNumberish, callGasLimit: BigNumberish): string { return ethers.utils.hexConcat([ hexZeroPad(hexlify(validationGasLimit, { hexPad: 'left' }), 16), hexZeroPad(hexlify(callGasLimit, { hexPad: 'left' }), 16) ]) diff --git a/test/verifying_paymaster.test.ts b/test/verifying_paymaster.test.ts index c1ea55d5..4db471d1 100644 --- a/test/verifying_paymaster.test.ts +++ b/test/verifying_paymaster.test.ts @@ -10,10 +10,10 @@ import { import { createAccount, createAccountOwner, createAddress, decodeRevertReason, - deployEntryPoint + deployEntryPoint, packPaymasterData } from './testutils' import { DefaultsForUserOp, fillAndSign, fillSignAndPack, packUserOp, simulateValidation } from './UserOp' -import { arrayify, defaultAbiCoder, hexConcat, hexlify, hexZeroPad, parseEther } from 'ethers/lib/utils' +import { arrayify, defaultAbiCoder, hexConcat, parseEther } from 'ethers/lib/utils' import { PackedUserOperation } from './UserOperation' const MOCK_VALID_UNTIL = '0x00000000deadbeef' @@ -43,11 +43,14 @@ describe('EntryPoint with VerifyingPaymaster', function () { describe('#parsePaymasterAndData', () => { it('should parse data properly', async () => { - const paymasterAndData = hexConcat([ - paymaster.address, hexZeroPad(hexlify(DefaultsForUserOp.paymasterVerificationGasLimit, { hexPad: 'left' }), 16), - hexZeroPad(hexlify(DefaultsForUserOp.paymasterPostOpGasLimit, { hexPad: 'left' }), 16), - defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), MOCK_SIG - ]) + const paymasterAndData = packPaymasterData( + paymaster.address, + DefaultsForUserOp.paymasterVerificationGasLimit, + DefaultsForUserOp.paymasterPostOpGasLimit, + hexConcat([ + defaultAbiCoder.encode(['uint48', 'uint48'], [MOCK_VALID_UNTIL, MOCK_VALID_AFTER]), MOCK_SIG + ]) + ) console.log(paymasterAndData) const res = await paymaster.parsePaymasterAndData(paymasterAndData) // console.log('MOCK_VALID_UNTIL, MOCK_VALID_AFTER', MOCK_VALID_UNTIL, MOCK_VALID_AFTER) From effa64fc6bb314d9768cd48e93f0f1ca19961328 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Thu, 4 Jan 2024 11:10:21 +0200 Subject: [PATCH 20/24] add paymaster constants to BasePaymaster --- contracts/core/BasePaymaster.sol | 6 +++++- contracts/samples/LegacyTokenPaymaster.sol | 2 +- contracts/samples/TokenPaymaster.sol | 4 ++-- contracts/samples/VerifyingPaymaster.sol | 4 ++-- contracts/test/TestExpirePaymaster.sol | 2 +- reports/gas-checker.txt | 14 +++++++------- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/contracts/core/BasePaymaster.sol b/contracts/core/BasePaymaster.sol index f8aec9d3..efb8b9cb 100644 --- a/contracts/core/BasePaymaster.sol +++ b/contracts/core/BasePaymaster.sol @@ -8,7 +8,7 @@ import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import "../interfaces/IPaymaster.sol"; import "../interfaces/IEntryPoint.sol"; import "./Helpers.sol"; - +import "./UserOperationLib.sol"; /** * Helper class for creating a paymaster. * provides helper methods for staking. @@ -17,6 +17,10 @@ import "./Helpers.sol"; abstract contract BasePaymaster is IPaymaster, Ownable { IEntryPoint public immutable entryPoint; + uint256 internal constant PAYMASTER_VALIDATION_GAS_OFFSET = UserOperationLib.PAYMASTER_VALIDATION_GAS_OFFSET; + uint256 internal constant PAYMASTER_POSTOP_GAS_OFFSET = UserOperationLib.PAYMASTER_POSTOP_GAS_OFFSET; + uint256 internal constant PAYMASTER_DATA_OFFSET = UserOperationLib.PAYMASTER_DATA_OFFSET; + constructor(IEntryPoint _entryPoint) Ownable(msg.sender) { _validateEntryPointInterface(_entryPoint); entryPoint = _entryPoint; diff --git a/contracts/samples/LegacyTokenPaymaster.sol b/contracts/samples/LegacyTokenPaymaster.sol index bc962b95..d673aefc 100644 --- a/contracts/samples/LegacyTokenPaymaster.sol +++ b/contracts/samples/LegacyTokenPaymaster.sol @@ -76,7 +76,7 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 { // verificationGasLimit is dual-purposed, as gas limit for postOp. make sure it is high enough // make sure that verificationGasLimit is high enough to handle postOp - require(uint128(bytes16(userOp.paymasterAndData[UserOperationLib.PAYMASTER_POSTOP_GAS_OFFSET:UserOperationLib.PAYMASTER_DATA_OFFSET])) > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); + require(uint128(bytes16(userOp.paymasterAndData[PAYMASTER_POSTOP_GAS_OFFSET : PAYMASTER_DATA_OFFSET])) > COST_OF_POST, "TokenPaymaster: gas too low for postOp"); if (userOp.initCode.length != 0) { _validateConstructor(userOp); diff --git a/contracts/samples/TokenPaymaster.sol b/contracts/samples/TokenPaymaster.sol index 2b40c6cc..00238e1e 100644 --- a/contracts/samples/TokenPaymaster.sol +++ b/contracts/samples/TokenPaymaster.sol @@ -121,7 +121,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { override returns (bytes memory context, uint256 validationResult) {unchecked { uint256 priceMarkup = tokenPaymasterConfig.priceMarkup; - uint256 paymasterAndDataLength = userOp.paymasterAndData.length - UserOperationLib.PAYMASTER_DATA_OFFSET; + uint256 paymasterAndDataLength = userOp.paymasterAndData.length - PAYMASTER_DATA_OFFSET; require(paymasterAndDataLength == 0 || paymasterAndDataLength == 32, "TPM: invalid data length" ); @@ -129,7 +129,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { // note: as price is in ether-per-token and we want more tokens increasing it means dividing it by markup uint256 cachedPriceWithMarkup = cachedPrice * PRICE_DENOMINATOR / priceMarkup; if (paymasterAndDataLength == 32) { - uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[UserOperationLib.PAYMASTER_DATA_OFFSET : UserOperationLib.PAYMASTER_DATA_OFFSET + 32])); + uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[PAYMASTER_DATA_OFFSET : PAYMASTER_DATA_OFFSET + 32])); if (clientSuppliedPrice < cachedPriceWithMarkup) { // note: smaller number means 'more ether per token' cachedPriceWithMarkup = clientSuppliedPrice; diff --git a/contracts/samples/VerifyingPaymaster.sol b/contracts/samples/VerifyingPaymaster.sol index 8bad94c8..6fb13ccc 100644 --- a/contracts/samples/VerifyingPaymaster.sol +++ b/contracts/samples/VerifyingPaymaster.sol @@ -23,7 +23,7 @@ contract VerifyingPaymaster is BasePaymaster { address public immutable verifyingSigner; - uint256 private constant VALID_TIMESTAMP_OFFSET = UserOperationLib.PAYMASTER_DATA_OFFSET; + uint256 private constant VALID_TIMESTAMP_OFFSET = PAYMASTER_DATA_OFFSET; uint256 private constant SIGNATURE_OFFSET = VALID_TIMESTAMP_OFFSET + 64; @@ -50,7 +50,7 @@ contract VerifyingPaymaster is BasePaymaster { keccak256(userOp.initCode), keccak256(userOp.callData), userOp.accountGasLimits, - uint256(bytes32(userOp.paymasterAndData[20 : 52])), + uint256(bytes32(userOp.paymasterAndData[PAYMASTER_VALIDATION_GAS_OFFSET : PAYMASTER_DATA_OFFSET])), userOp.preVerificationGas, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, diff --git a/contracts/test/TestExpirePaymaster.sol b/contracts/test/TestExpirePaymaster.sol index de7a374e..a92e3ff2 100644 --- a/contracts/test/TestExpirePaymaster.sol +++ b/contracts/test/TestExpirePaymaster.sol @@ -16,7 +16,7 @@ contract TestExpirePaymaster is BasePaymaster { internal virtual override view returns (bytes memory context, uint256 validationData) { (userOp, userOpHash, maxCost); - (uint48 validAfter, uint48 validUntil) = abi.decode(userOp.paymasterAndData[UserOperationLib.PAYMASTER_DATA_OFFSET :], (uint48, uint48)); + (uint48 validAfter, uint48 validUntil) = abi.decode(userOp.paymasterAndData[PAYMASTER_DATA_OFFSET :], (uint48, uint48)); validationData = _packValidationData(false, validUntil, validAfter); context = ""; } diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 2b3b6a5f..2af08a79 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -22,11 +22,11 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple paymaster │ 1 │ 89079 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 2 │ │ 43963 │ 14984 ║ +║ simple paymaster with diff │ 2 │ │ 43987 │ 15008 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster │ 10 │ 485071 │ │ ║ +║ simple paymaster │ 10 │ 485083 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 44017 │ 15038 ║ +║ simple paymaster with diff │ 11 │ │ 44005 │ 15026 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx 5k │ 1 │ 183735 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ @@ -36,11 +36,11 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx - diff from previous │ 11 │ │ 145440 │ 20216 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 90907 │ │ ║ +║ paymaster+postOp │ 1 │ 90895 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 45814 │ 16835 ║ +║ paymaster+postOp with diff │ 2 │ │ 45850 │ 16871 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 503383 │ │ ║ +║ paymaster+postOp │ 10 │ 503503 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ paymaster+postOp with diff │ 11 │ │ 45851 │ 16872 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ @@ -50,6 +50,6 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ token paymaster │ 10 │ 816150 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 11 │ │ 74094 │ 45115 ║ +║ token paymaster with diff │ 11 │ │ 74106 │ 45127 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 8ff7490d0482b44d080543003c2f384439c56e1e Mon Sep 17 00:00:00 2001 From: shahafn Date: Sun, 7 Jan 2024 20:14:15 +0200 Subject: [PATCH 21/24] PR fix --- contracts/core/EntryPoint.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/core/EntryPoint.sol b/contracts/core/EntryPoint.sol index 877e922f..0a3994bb 100644 --- a/contracts/core/EntryPoint.sol +++ b/contracts/core/EntryPoint.sol @@ -681,7 +681,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard, 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; + executionGasLimit += mUserOp.paymasterPostOpGasLimit; } uint256 executionGasUsed = actualGas - opInfo.preOpGas; // this check is required for the gas used within EntryPoint and not covered by explicit gas limits From 24cf39ed6ed69098b864f998fefb4020a6cb0d6a Mon Sep 17 00:00:00 2001 From: shahafn Date: Sun, 7 Jan 2024 20:54:04 +0200 Subject: [PATCH 22/24] Removing redundant initializer --- test/entrypoint.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/entrypoint.test.ts b/test/entrypoint.test.ts index d4ca06ca..2bf18a03 100644 --- a/test/entrypoint.test.ts +++ b/test/entrypoint.test.ts @@ -1073,7 +1073,6 @@ describe('EntryPoint', function () { const pm = createAddress() const op = await fillSignAndPack({ paymaster: pm, - paymasterData: '0x', paymasterVerificationGasLimit: 3e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(account2Owner.address, simpleAccountFactory), @@ -1086,7 +1085,6 @@ describe('EntryPoint', function () { it('should fail if paymaster has no deposit', async function () { const op = await fillSignAndPack({ paymaster: paymaster.address, - paymasterData: '0x', paymasterVerificationGasLimit: 3e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(account2Owner.address, simpleAccountFactory), @@ -1107,7 +1105,6 @@ describe('EntryPoint', function () { const op = await fillSignAndPack({ paymaster: errorPostOp.address, - paymasterData: '0x', paymasterPostOpGasLimit: 1e5, paymasterVerificationGasLimit: 3e6, callData: accountExecFromEntryPoint.data, @@ -1150,7 +1147,6 @@ describe('EntryPoint', function () { await paymaster.deposit({ value: ONE_ETH }) const op = await fillSignAndPack({ paymaster: paymaster.address, - paymasterData: '0x', paymasterVerificationGasLimit: 1e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(account2Owner.address, simpleAccountFactory) @@ -1169,7 +1165,6 @@ describe('EntryPoint', function () { const op = await fillSignAndPack({ paymaster: paymaster.address, - paymasterData: '0x', paymasterVerificationGasLimit: 1e6, callData: accountExecFromEntryPoint.data, initCode: getAccountInitCode(anOwner.address, simpleAccountFactory) From b9504f485a8b0a9839924cfa9f3cb806f7882235 Mon Sep 17 00:00:00 2001 From: shahafn Date: Sun, 7 Jan 2024 21:25:00 +0200 Subject: [PATCH 23/24] gas calc --- reports/gas-checker.txt | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/reports/gas-checker.txt b/reports/gas-checker.txt index 2af08a79..e09f02d9 100644 --- a/reports/gas-checker.txt +++ b/reports/gas-checker.txt @@ -12,13 +12,13 @@ ║ │ │ │ (delta for │ (compared to ║ ║ │ │ │ one UserOp) │ account.exec()) ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 1 │ 82653 │ │ ║ +║ simple │ 1 │ 82677 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 2 │ │ 44872 │ 15893 ║ +║ simple - diff from previous │ 2 │ │ 44884 │ 15905 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple │ 10 │ 486704 │ │ ║ +║ simple │ 10 │ 486740 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple - diff from previous │ 11 │ │ 44917 │ 15938 ║ +║ simple - diff from previous │ 11 │ │ 44929 │ 15950 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple paymaster │ 1 │ 89079 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ @@ -26,29 +26,29 @@ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ simple paymaster │ 10 │ 485083 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ simple paymaster with diff │ 11 │ │ 44005 │ 15026 ║ +║ simple paymaster with diff │ 11 │ │ 43981 │ 15002 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx 5k │ 1 │ 183735 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ big tx - diff from previous │ 2 │ │ 145383 │ 20159 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx 5k │ 10 │ 1492339 │ │ ║ +║ big tx 5k │ 10 │ 1492351 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ big tx - diff from previous │ 11 │ │ 145440 │ 20216 ║ +║ big tx - diff from previous │ 11 │ │ 145452 │ 20228 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 1 │ 90895 │ │ ║ +║ paymaster+postOp │ 1 │ 90919 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 2 │ │ 45850 │ 16871 ║ +║ paymaster+postOp with diff │ 2 │ │ 45802 │ 16823 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp │ 10 │ 503503 │ │ ║ +║ paymaster+postOp │ 10 │ 503479 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ paymaster+postOp with diff │ 11 │ │ 45851 │ 16872 ║ +║ paymaster+postOp with diff │ 11 │ │ 45875 │ 16896 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ token paymaster │ 1 │ 149447 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster with diff │ 2 │ │ 74022 │ 45043 ║ +║ token paymaster with diff │ 2 │ │ 74046 │ 45067 ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ -║ token paymaster │ 10 │ 816150 │ │ ║ +║ token paymaster │ 10 │ 816174 │ │ ║ ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢ ║ token paymaster with diff │ 11 │ │ 74106 │ 45127 ║ ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝ From 62523870378e8cc328d97d6f71ac674d11d0b9e3 Mon Sep 17 00:00:00 2001 From: shahafn Date: Sun, 7 Jan 2024 21:44:58 +0200 Subject: [PATCH 24/24] Fix test --- test/samples/TokenPaymaster.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/samples/TokenPaymaster.test.ts b/test/samples/TokenPaymaster.test.ts index 1ef3bf4c..f59e785a 100644 --- a/test/samples/TokenPaymaster.test.ts +++ b/test/samples/TokenPaymaster.test.ts @@ -204,7 +204,7 @@ describe('TokenPaymaster', function () { assert.equal(actualTokenChargeEvents.toString(), actualTokenCharge.toString()) assert.equal(actualTokenChargeEvents.toString(), expectedTokenCharge.toString()) assert.equal(actualTokenPrice / (priceDenominator as any), expectedTokenPrice) - assert.closeTo(postOpGasCost.div(tx.effectiveGasPrice).toNumber(), 40000, 20000) + assert.closeTo(postOpGasCost.div(tx.effectiveGasPrice).toNumber(), 50000, 20000) await ethers.provider.send('evm_revert', [snapshot]) })