From 3c27ca657bcb1fb4d04fd6067bbb00c7b0e6815a Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Wed, 4 Dec 2024 13:47:12 +0300 Subject: [PATCH] charge in postop, external mode uses off-chain estimations --- .../interfaces/IBiconomyTokenPaymaster.sol | 6 ++-- contracts/token/BiconomyTokenPaymaster.sol | 31 ++++++++++--------- test/base/TestBase.sol | 7 +++-- .../concrete/TestTokenPaymaster.Base.t.sol | 3 -- test/unit/concrete/TestTokenPaymaster.t.sol | 17 +++++----- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/contracts/interfaces/IBiconomyTokenPaymaster.sol b/contracts/interfaces/IBiconomyTokenPaymaster.sol index e7e9fec..6a354c8 100644 --- a/contracts/interfaces/IBiconomyTokenPaymaster.sol +++ b/contracts/interfaces/IBiconomyTokenPaymaster.sol @@ -22,13 +22,11 @@ interface IBiconomyTokenPaymaster { event UpdatedVerifyingSigner(address indexed oldSigner, address indexed newSigner, address indexed actor); event UpdatedFeeCollector(address indexed oldFeeCollector, address indexed newFeeCollector, address indexed actor); event UpdatedPriceExpiryDuration(uint256 indexed oldValue, uint256 indexed newValue); - event TokensRefunded( - address indexed userOpSender, address indexed token, uint256 refundAmount, bytes32 indexed userOpHash - ); + event PaidGasInTokensIndependent( address indexed userOpSender, address indexed token, - uint256 nativeCharge, + uint256 gasCostBeforePostOpAndPenalty, uint256 tokenCharge, uint32 priceMarkup, uint256 tokenPrice, diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index fbd0417..87dd8d1 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -509,12 +509,14 @@ contract BiconomyTokenPaymaster is revert InsufficientTokenBalance(userOp.sender, tokenAddress, estimatedTokenAmount, userOpHash); } - context = abi.encode( - mode, - userOp.sender, - tokenAddress, - estimatedTokenAmount, - userOpHash + context = abi.encodePacked( + PaymasterMode.EXTERNAL, + abi.encode( + userOp.sender, + tokenAddress, + estimatedTokenAmount, + userOpHash + ) ); validationData = _packValidationData(false, validUntil, validAfter); @@ -529,7 +531,7 @@ contract BiconomyTokenPaymaster is ( uint128(uint256(userOp.accountGasLimits)) + uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET:_PAYMASTER_DATA_OFFSET])) - ) * 10 * userOp.unpackMaxFeePerGas() + ) * 10 //* userOp.unpackMaxFeePerGas() ) / 100; // Get address for token used to pay @@ -555,16 +557,17 @@ contract BiconomyTokenPaymaster is revert InsufficientTokenBalance(userOp.sender, tokenAddress, tokenAmount, userOpHash); } - context = + context = abi.encodePacked( + PaymasterMode.INDEPENDENT, abi.encode( - mode, userOp.sender, tokenAddress, maxPenalty, tokenPrice, priceMarkup, userOpHash - ); + ) + ); validationData = 0; // Validation success and price is valid indefinetly } } @@ -585,10 +588,8 @@ contract BiconomyTokenPaymaster is internal override { - - PaymasterMode mode = PaymasterMode(uint8(context[0])); - - if (mode == PaymasterMode.EXTERNAL) { + PaymasterMode pmMode = PaymasterMode(uint8(context[0])); + if (pmMode == PaymasterMode.EXTERNAL) { // Decode context data ( address userOpSender, @@ -603,7 +604,7 @@ contract BiconomyTokenPaymaster is revert FailedToChargeTokens(userOpSender, tokenAddress, estimatedTokenAmount, userOpHash); } - } else if (mode == PaymasterMode.INDEPENDENT) { + } else if (pmMode == PaymasterMode.INDEPENDENT) { ( address userOpSender, address tokenAddress, diff --git a/test/base/TestBase.sol b/test/base/TestBase.sol index bb2519f..c835559 100644 --- a/test/base/TestBase.sol +++ b/test/base/TestBase.sol @@ -14,6 +14,9 @@ import { Exec } from "account-abstraction/utils/Exec.sol"; import { IPaymaster } from "account-abstraction/interfaces/IPaymaster.sol"; import { Nexus } from "@nexus/contracts/Nexus.sol"; +import { INexus } from "@nexus/contracts/interfaces/INexus.sol"; +import { IERC7579Account } from "@nexus/contracts/interfaces/IERC7579Account.sol"; +import { IExecutionHelper } from "@nexus/contracts/interfaces/base/IExecutionHelper.sol"; import { CheatCodes } from "@nexus/test/foundry/utils/CheatCodes.sol"; import { BaseEventsAndErrors } from "./BaseEventsAndErrors.sol"; import { MockToken } from "@nexus/contracts/mocks/MockToken.sol"; @@ -479,10 +482,10 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { // Assert we never undercharge assertGe(gasPaidBySAInNativeTokens, BUNDLER.addr.balance - initialBundlerBalance); - // Ensure that max 2% difference between what is should have been charged and what was charged + // Ensure that max 3% difference between what should have been charged and what was charged // this difference comes from difference of postop gas and estimated postop gas (paymaster.unaccountedGas) // and from estimation of real penalty which is not emitted by EP :( - assertApproxEqRel((totalGasFeePaid + maxPenalty - realPenalty) * priceMarkup / _PRICE_MARKUP_DENOMINATOR, gasPaidBySAInNativeTokens, 0.02e18, "If this fails, check the test case inline comments"); + assertApproxEqRel((totalGasFeePaid + maxPenalty - realPenalty) * priceMarkup / _PRICE_MARKUP_DENOMINATOR, gasPaidBySAInNativeTokens, 0.03e18, "If this fails, check the test case inline comments"); } function _toSingletonArray(address addr) internal pure returns (address[] memory) { diff --git a/test/unit/concrete/TestTokenPaymaster.Base.t.sol b/test/unit/concrete/TestTokenPaymaster.Base.t.sol index 87b9357..08df0ec 100644 --- a/test/unit/concrete/TestTokenPaymaster.Base.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.Base.t.sol @@ -111,9 +111,6 @@ contract TestTokenPaymasterBase is TestBase { PackedUserOperation[] memory ops = new PackedUserOperation[](1); ops[0] = userOp; - vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.TokensRefunded(address(ALICE_ACCOUNT), address(usdc), 0, bytes32(0)); - vm.expectEmit(true, true, false, false, address(tokenPaymaster)); emit IBiconomyTokenPaymaster.PaidGasInTokensIndependent(address(ALICE_ACCOUNT), address(usdc), 0, 0, 1e6, 0, bytes32(0)); diff --git a/test/unit/concrete/TestTokenPaymaster.t.sol b/test/unit/concrete/TestTokenPaymaster.t.sol index 27df054..ac42598 100644 --- a/test/unit/concrete/TestTokenPaymaster.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.t.sol @@ -407,7 +407,11 @@ contract TestTokenPaymaster is TestBase { uint256 initialPaymasterTokenBalance = testToken.balanceOf(address(tokenPaymaster)); // Build the user operation for external mode - PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); + PackedUserOperation memory userOp = buildUserOpWithCalldata( + ALICE, + abi.encodeWithSelector(IExecutionHelper.execute.selector, bytes32(0), abi.encodePacked(address(testToken), uint256(0), abi.encodeWithSelector(IERC20.approve.selector, address(tokenPaymaster), 1_000*1e18))), + address(VALIDATOR_MODULE) + ); uint48 validUntil = uint48(block.timestamp + 1 days); uint48 validAfter = uint48(block.timestamp); uint256 tokenPrice = 1e18; // Assume 1 token = 1 native token = 1 USD ? @@ -420,7 +424,7 @@ contract TestTokenPaymaster is TestBase { validUntil: validUntil, validAfter: validAfter, tokenAddress: address(testToken), - estimatedTokenAmount: 1e18 + estimatedTokenAmount: 3_000_000_000_000 }); // Generate and sign the token paymaster data @@ -437,9 +441,6 @@ contract TestTokenPaymaster is TestBase { PackedUserOperation[] memory ops = new PackedUserOperation[](1); ops[0] = userOp; - vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.TokensRefunded(address(ALICE_ACCOUNT), address(testToken), 0, bytes32(0)); - vm.expectEmit(true, true, false, false, address(tokenPaymaster)); emit IBiconomyTokenPaymaster.PaidGasInTokensExternal(address(ALICE_ACCOUNT), address(testToken), 0, bytes32(0)); @@ -471,7 +472,7 @@ contract TestTokenPaymaster is TestBase { vm.stopPrank(); vm.startPrank(PAYMASTER_OWNER.addr); - tokenPaymaster.setUnaccountedGas(20_000); + tokenPaymaster.setUnaccountedGas(40_000); vm.stopPrank(); uint256 initialBundlerBalance = BUNDLER.addr.balance; @@ -495,10 +496,10 @@ contract TestTokenPaymaster is TestBase { PackedUserOperation[] memory ops = new PackedUserOperation[](1); ops[0] = userOp; - vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.TokensRefunded(address(ALICE_ACCOUNT), address(testToken), 0, bytes32(0)); + vm.expectEmit(true, true, false, false, address(tokenPaymaster)); emit IBiconomyTokenPaymaster.PaidGasInTokensIndependent(address(ALICE_ACCOUNT), address(testToken), 0, 0, 1e6, 0, bytes32(0)); + startPrank(BUNDLER.addr); uint256 gasValue = gasleft(); ENTRYPOINT.handleOps(ops, payable(BUNDLER.addr));