Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

add postOp param (gasPrice) #395

Merged
merged 4 commits into from
Jan 9, 2024
Merged

add postOp param (gasPrice) #395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a18ece
add postOp param (gasPrice)
drortirosh Jan 6, 2024
49e9917
update erc
drortirosh Jan 8, 2024
b7902f0
PR review rename field
drortirosh Jan 8, 2024
8faf84a
Merge branch 'develop' into postOp-gas-price
drortirosh Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions contracts/core/BasePaymaster.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@ abstract contract BasePaymaster is IPaymaster, Ownable {
function postOp(
PostOpMode mode,
bytes calldata context,
uint256 actualGasCost
uint256 actualGasCost,
uint actualUserOpFeePerGas
) external override {
_requireFromEntryPoint();
_postOp(mode, context, actualGasCost);
_postOp(mode, context, actualGasCost, actualUserOpFeePerGas);
}

/**
Expand All @@ -75,14 +76,18 @@ abstract contract BasePaymaster is IPaymaster, Ownable {
* postOpReverted - User op succeeded, but caused postOp (in mode=opSucceeded) to revert.
* Now this is the 2nd call, after user's op was deliberately reverted.
* @param context - The context value returned by validatePaymasterUserOp
* @param actualGasCost - Actual gas used so far (without this postOp call).
* @param actualUserOpFeePerGas - the gas price this UserOp pays. This value is based on the UserOp's maxFeePerGas
* and maxPriorityFee (and basefee)
* It is not the same as tx.gasprice, which is what the bundler pays.

*/
function _postOp(
PostOpMode mode,
bytes calldata context,
uint256 actualGasCost
uint256 actualGasCost,
uint actualUserOpFeePerGas
) internal virtual {
(mode, context, actualGasCost); // unused params
(mode, context, actualGasCost, actualUserOpFeePerGas); // unused params
// subclass must override this method if validatePaymasterUserOp returns a context
revert("must override");
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/core/EntryPoint.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
if (mode != IPaymaster.PostOpMode.postOpReverted) {
try IPaymaster(paymaster).postOp{
gas: mUserOp.paymasterPostOpGasLimit
}(mode, context, actualGasCost)
}(mode, context, actualGasCost, gasPrice)
// solhint-disable-next-line no-empty-blocks
{} catch {
bytes memory reason = Exec.getReturnData(REVERT_REASON_MAX_LEN);
Expand Down
6 changes: 5 additions & 1 deletion contracts/interfaces/IPaymaster.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,14 @@ interface IPaymaster {
* Now this is the 2nd call, after user's op was deliberately reverted.
* @param context - The context value returned by validatePaymasterUserOp
* @param actualGasCost - Actual gas used so far (without this postOp call).
* @param actualUserOpFeePerGas - the gas price this UserOp pays. This value is based on the UserOp's maxFeePerGas
* and maxPriorityFee (and basefee)
* It is not the same as tx.gasprice, which is what the bundler pays.
*/
function postOp(
PostOpMode mode,
bytes calldata context,
uint256 actualGasCost
uint256 actualGasCost,
uint256 actualUserOpFeePerGas
) external;
}
4 changes: 2 additions & 2 deletions contracts/samples/LegacyTokenPaymaster.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ contract LegacyTokenPaymaster is BasePaymaster, ERC20 {
* the user's TX , back to the state it was before the transaction started (before the validatePaymasterUserOp),
* and the transaction should succeed there.
*/
function _postOp(PostOpMode mode, bytes calldata context, uint256 actualGasCost) internal override {
function _postOp(PostOpMode mode, bytes calldata context, uint256 actualGasCost, uint actualUserOpFeePerGas) internal override {
//we don't really care about the mode, we just pay the gas with the user's tokens.
(mode);
address sender = abi.decode(context, (address));
uint256 charge = getTokenValueOfEth(actualGasCost + COST_OF_POST);
uint256 charge = getTokenValueOfEth(actualGasCost + COST_OF_POST * actualUserOpFeePerGas);
//actualGasCost is known to be no larger than the above requiredPreFund, so the transfer should succeed.
_transfer(sender, address(this), charge);
}
Expand Down
14 changes: 7 additions & 7 deletions contracts/samples/TokenPaymaster.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper {
}
uint256 tokenAmount = weiToToken(preChargeNative, cachedPriceWithMarkup);
SafeERC20.safeTransferFrom(token, userOp.sender, address(this), tokenAmount);
context = abi.encode(tokenAmount, userOp.maxFeePerGas, userOp.maxPriorityFeePerGas, userOp.sender);
context = abi.encode(tokenAmount, userOp.sender);
validationResult = _packValidationData(
false,
uint48(cachedPriceTimestamp + tokenPaymasterConfig.priceMaxAge),
Expand All @@ -150,21 +150,21 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper {
/// @dev This function is called after a user operation has been executed or reverted.
/// @param context The context containing the token amount and user sender address.
/// @param actualGasCost The actual gas cost of the transaction.
function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost) internal override {
/// @param actualUserOpFeePerGas - the gas price this UserOp pays. This value is based on the UserOp's maxFeePerGas
// and maxPriorityFee (and basefee)
// It is not the same as tx.gasprice, which is what the bundler pays.
function _postOp(PostOpMode, bytes calldata context, uint256 actualGasCost, uint actualUserOpFeePerGas) internal override {
unchecked {
uint256 priceMarkup = tokenPaymasterConfig.priceMarkup;
(
uint256 preCharge,
uint256 maxFeePerGas,
uint256 maxPriorityFeePerGas,
address userOpSender
) = abi.decode(context, (uint256, uint256, uint256, address));
uint256 gasPrice = getGasPrice(maxFeePerGas, maxPriorityFeePerGas);
) = abi.decode(context, (uint256, address));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This means the entire getGasPrice function is no longer needed - please remove.

uint256 _cachedPrice = updateCachedPrice(false);
// 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;
// Refund tokens based on actual gas cost
uint256 actualChargeNative = actualGasCost + tokenPaymasterConfig.refundPostopCost * gasPrice;
uint256 actualChargeNative = actualGasCost + tokenPaymasterConfig.refundPostopCost * actualUserOpFeePerGas;
uint256 actualTokenNeeded = weiToToken(actualChargeNative, cachedPriceWithMarkup);
if (preCharge > actualTokenNeeded) {
// If the initially provided token amount is greater than the actual amount needed, refund the difference
Expand Down
2 changes: 1 addition & 1 deletion contracts/test/TestPaymasterRevertCustomError.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ contract TestPaymasterRevertCustomError is BasePaymaster {
revertType = _revertType;
}

function _postOp(PostOpMode, bytes calldata, uint256) internal view override {
function _postOp(PostOpMode, bytes calldata, uint256, uint256) internal view override {
if (revertType == RevertType.customError){
revert CustomError("this is a long revert reason string we are looking for");
}
Expand Down
10 changes: 3 additions & 7 deletions contracts/test/TestPaymasterWithPostOp.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,14 @@ contract TestPaymasterWithPostOp is TestPaymasterAcceptAll {
constructor(IEntryPoint _entryPoint) TestPaymasterAcceptAll(_entryPoint) {
}

function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost)
function _validatePaymasterUserOp(PackedUserOperation calldata, bytes32, uint256)
internal virtual override view
returns (bytes memory context, uint256 validationData) {
(userOp, userOpHash, maxCost);
// return a context, to force a call for postOp.
return ("1", 0);
}

function _postOp(
PostOpMode mode,
bytes calldata context,
uint256 actualGasCost
) internal override {
function _postOp(PostOpMode, bytes calldata, uint256, uint256)
internal override {
}
}
2 changes: 1 addition & 1 deletion erc/ERCS/erc-4337.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ The paymaster interface is as follows:
external returns (bytes memory context, uint256 validationData);

function postOp
(PostOpMode mode, bytes calldata context, uint256 actualGasCost)
(PostOpMode mode, bytes calldata context, uint256 actualGasCost, uint256 actualUserOpFeePerGas)
external;

enum PostOpMode {
Expand Down
36 changes: 18 additions & 18 deletions reports/gas-checker.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,40 +16,40 @@
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 2 │ │ 44884 │ 15905 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple │ 10 │ 486740 │ │ ║
║ simple │ 10 │ 486728 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple - diff from previous │ 11 │ │ 44929 │ 15950 ║
║ simple - diff from previous │ 11 │ │ 44881 │ 15902 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 1 │ 89079 │ │ ║
║ simple paymaster │ 1 │ 89067 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 2 │ │ 43987 │ 15008 ║
║ simple paymaster with diff │ 2 │ │ 43975 │ 14996 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster │ 10 │ 485083 │ │ ║
║ simple paymaster │ 10 │ 484963 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ simple paymaster with diff │ 11 │ │ 43981 │ 15002 ║
║ simple paymaster with diff │ 11 │ │ 44029 │ 15050 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 1 │ 183735 │ │ ║
║ big tx 5k │ 1 │ 183723 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 2 │ │ 145383 │ 20159 ║
║ big tx - diff from previous │ 2 │ │ 145395 │ 20171 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx 5k │ 10 │ 1492351 │ │ ║
║ big tx 5k │ 10 │ 1492327 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ big tx - diff from previous │ 11 │ │ 145452 │ 20228 ║
║ big tx - diff from previous │ 11 │ │ 145512 │ 20288 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp │ 1 │ 90919 │ │ ║
║ paymaster+postOp │ 1 │ 90943 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp with diff │ 2 │ │ 45802 │ 16823 ║
║ paymaster+postOp with diff │ 2 │ │ 45838 │ 16859 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp │ 10 │ 503479 │ │ ║
║ paymaster+postOp │ 10 │ 503743 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ paymaster+postOp with diff │ 11 │ │ 45875 │ 16896 ║
║ paymaster+postOp with diff │ 11 │ │ 45887 │ 16908 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster │ 1 │ 149447 │ │ ║
║ token paymaster │ 1 │ 148703 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster with diff │ 2 │ │ 74046 │ 45067 ║
║ token paymaster with diff │ 2 │ │ 73307 │ 44328 ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster │ 10 │ 816174 │ │ ║
║ token paymaster │ 10 │ 808665 │ │ ║
╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
║ token paymaster with diff │ 11 │ │ 74106 │ 45127 ║
║ token paymaster with diff │ 11 │ │ 73378 │ 44399 ║
╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝

Loading