From 713d31d038e7f80565ab160381d2b08801a55c34 Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Wed, 4 Dec 2024 12:51:47 +0300 Subject: [PATCH 1/5] compiles --- .../common/BiconomyTokenPaymasterErrors.sol | 10 +- .../interfaces/IBiconomyTokenPaymaster.sol | 12 +- .../libraries/TokenPaymasterParserLib.sol | 8 +- contracts/token/BiconomyTokenPaymaster.sol | 123 +++++++++--------- test/base/TestBase.sol | 11 +- test/mocks/PaymasterParserLibExposed.sol | 3 +- test/mocks/PaymasterParserLibWrapper.sol | 5 +- .../concrete/TestTokenPaymaster.Base.t.sol | 2 +- test/unit/concrete/TestTokenPaymaster.t.sol | 15 +-- .../TestTokenPaymasterParserLib.t.sol | 12 +- 10 files changed, 102 insertions(+), 99 deletions(-) diff --git a/contracts/common/BiconomyTokenPaymasterErrors.sol b/contracts/common/BiconomyTokenPaymasterErrors.sol index 274999b..2929bb7 100644 --- a/contracts/common/BiconomyTokenPaymasterErrors.sol +++ b/contracts/common/BiconomyTokenPaymasterErrors.sol @@ -81,8 +81,14 @@ contract BiconomyTokenPaymasterErrors { */ error WithdrawalFailed(); + + /** + * @notice Throws when PM was not able to charge user + */ + error FailedToChargeTokens(address account, address token, uint256 amount, bytes32 userOpHash); + /** - * @notice Emitted when ETH is withdrawn from the paymaster + * Throws when account has insufficient token balance to pay for gas */ - event EthWithdrawn(address indexed recipient, uint256 indexed amount); + error InsufficientTokenBalance(address account, address token, uint256 amount, bytes32 userOpHash); } diff --git a/contracts/interfaces/IBiconomyTokenPaymaster.sol b/contracts/interfaces/IBiconomyTokenPaymaster.sol index 9fc43e8..e7e9fec 100644 --- a/contracts/interfaces/IBiconomyTokenPaymaster.sol +++ b/contracts/interfaces/IBiconomyTokenPaymaster.sol @@ -8,7 +8,6 @@ interface IBiconomyTokenPaymaster { enum PaymasterMode { EXTERNAL, // Price provided by external service. Authenticated using signature from verifyingSigner INDEPENDENT // Price queried from oracle. No signature needed from external service. - } // Struct for storing information about the token @@ -26,7 +25,7 @@ interface IBiconomyTokenPaymaster { event TokensRefunded( address indexed userOpSender, address indexed token, uint256 refundAmount, bytes32 indexed userOpHash ); - event PaidGasInTokens( + event PaidGasInTokensIndependent( address indexed userOpSender, address indexed token, uint256 nativeCharge, @@ -35,6 +34,15 @@ interface IBiconomyTokenPaymaster { uint256 tokenPrice, bytes32 indexed userOpHash ); + event PaidGasInTokensExternal( + address indexed userOpSender, + address indexed token, + uint256 tokenAmount, + bytes32 indexed userOpHash + ); + + event EthWithdrawn(address indexed recipient, uint256 indexed amount); + event Received(address indexed sender, uint256 value); event TokensWithdrawn(address indexed token, address indexed to, uint256 indexed amount, address actor); event AddedToTokenDirectory(address indexed tokenAddress, IOracle indexed oracle, uint8 decimals); diff --git a/contracts/libraries/TokenPaymasterParserLib.sol b/contracts/libraries/TokenPaymasterParserLib.sol index aaae02a..0cb2e72 100644 --- a/contracts/libraries/TokenPaymasterParserLib.sol +++ b/contracts/libraries/TokenPaymasterParserLib.sol @@ -31,17 +31,15 @@ library TokenPaymasterParserLib { uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 tokenPrice, - uint32 externalPriceMarkup, + uint256 estimatedTokenAmount, bytes calldata signature ) { validUntil = uint48(bytes6(modeSpecificData[:6])); validAfter = uint48(bytes6(modeSpecificData[6:12])); tokenAddress = address(bytes20(modeSpecificData[12:32])); - tokenPrice = uint256(bytes32(modeSpecificData[32:64])); - externalPriceMarkup = uint32(bytes4(modeSpecificData[64:68])); - signature = modeSpecificData[68:]; + estimatedTokenAmount = uint256(bytes32(modeSpecificData[32:64])); + signature = modeSpecificData[64:]; } function parseIndependentModeSpecificData( diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index eb868ad..fbd0417 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -391,8 +391,7 @@ contract BiconomyTokenPaymaster is uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 tokenPrice, - uint32 externalPriceMarkup + uint256 estimatedTokenAmount ) public view @@ -415,8 +414,7 @@ contract BiconomyTokenPaymaster is validUntil, validAfter, tokenAddress, - tokenPrice, - externalPriceMarkup + estimatedTokenAmount ) ); } @@ -479,14 +477,6 @@ contract BiconomyTokenPaymaster is revert InvalidPaymasterMode(); } - // callGasLimit + paymasterPostOpGas - uint256 maxPenalty = ( - ( - uint128(uint256(userOp.accountGasLimits)) - + uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET:_PAYMASTER_DATA_OFFSET])) - ) * 10 * userOp.unpackMaxFeePerGas() - ) / 100; - if (mode == PaymasterMode.EXTERNAL) { // Use the price and other params specified in modeSpecificData by the verifyingSigner // Useful for supporting tokens which don't have oracle support @@ -495,8 +485,7 @@ contract BiconomyTokenPaymaster is uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 tokenPrice, // Note: what backend should pass is nativeTokenPriceInUsd/tokenPriceInUsd * 10^token decimals - uint32 externalPriceMarkup, + uint256 estimatedTokenAmount, bytes memory signature ) = modeSpecificData.parseExternalModeSpecificData(); @@ -506,7 +495,7 @@ contract BiconomyTokenPaymaster is bool validSig = verifyingSigner.isValidSignatureNow( ECDSA_solady.toEthSignedMessageHash( - getHash(userOp, validUntil, validAfter, tokenAddress, tokenPrice, externalPriceMarkup) + getHash(userOp, validUntil, validAfter, tokenAddress, estimatedTokenAmount) ), signature ); @@ -516,38 +505,33 @@ contract BiconomyTokenPaymaster is return ("", _packValidationData(true, validUntil, validAfter)); } - if (externalPriceMarkup > _MAX_PRICE_MARKUP || externalPriceMarkup < _PRICE_DENOMINATOR) { - revert InvalidPriceMarkup(); - } - - - uint256 tokenAmount; - { - uint256 maxFeePerGas = UserOperationLib.unpackMaxFeePerGas(userOp); - tokenAmount = ((maxCost + maxPenalty + (unaccountedGas * maxFeePerGas)) * externalPriceMarkup * tokenPrice) - / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); + if(IERC20(tokenAddress).balanceOf(userOp.sender) < estimatedTokenAmount) { + revert InsufficientTokenBalance(userOp.sender, tokenAddress, estimatedTokenAmount, userOpHash); } - // Transfer full amount to this address. Unused amount will be refunded in postOP - SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount); - - // deduct max penalty from the token amount we pass to the postOp - // so we don't refund it at postOp context = abi.encode( + mode, userOp.sender, tokenAddress, - tokenAmount-((maxPenalty*tokenPrice*externalPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)), - tokenPrice, - externalPriceMarkup, + estimatedTokenAmount, userOpHash ); validationData = _packValidationData(false, validUntil, validAfter); + + /// INDEPENDENT MODE } else if (mode == PaymasterMode.INDEPENDENT) { // Use only oracles for the token specified in modeSpecificData if (modeSpecificData.length != 20) { revert InvalidTokenAddress(); } + uint256 maxPenalty = ( + ( + uint128(uint256(userOp.accountGasLimits)) + + uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET:_PAYMASTER_DATA_OFFSET])) + ) * 10 * userOp.unpackMaxFeePerGas() + ) / 100; + // Get address for token used to pay address tokenAddress = modeSpecificData.parseIndependentModeSpecificData(); uint256 tokenPrice = _getPrice(tokenAddress); @@ -566,13 +550,17 @@ contract BiconomyTokenPaymaster is } // Transfer full amount to this address. Unused amount will be refunded in postOP - SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount); + // SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount); + if(IERC20(tokenAddress).balanceOf(userOp.sender) < tokenAmount) { + revert InsufficientTokenBalance(userOp.sender, tokenAddress, tokenAmount, userOpHash); + } context = abi.encode( + mode, userOp.sender, tokenAddress, - tokenAmount-((maxPenalty*tokenPrice*priceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)), + maxPenalty, tokenPrice, priceMarkup, userOpHash @@ -596,31 +584,48 @@ contract BiconomyTokenPaymaster is ) internal override - { - // Decode context data - ( - address userOpSender, - address tokenAddress, - uint256 prechargedAmount, - uint256 tokenPrice, - uint32 appliedPriceMarkup, - bytes32 userOpHash - ) = abi.decode(context, (address, address, uint256, uint256, uint32, bytes32)); - - // Calculate the actual cost in tokens based on the actual gas cost and the token price - uint256 actualTokenAmount = ( - (actualGasCost + (unaccountedGas * actualUserOpFeePerGas)) * appliedPriceMarkup * tokenPrice - ) / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); - if (prechargedAmount > actualTokenAmount) { - // If the user was overcharged, refund the excess tokens - uint256 refundAmount = prechargedAmount - actualTokenAmount; - SafeTransferLib.safeTransfer(tokenAddress, userOpSender, refundAmount); - emit TokensRefunded(userOpSender, tokenAddress, refundAmount, userOpHash); - } + { + + PaymasterMode mode = PaymasterMode(uint8(context[0])); - emit PaidGasInTokens( - userOpSender, tokenAddress, actualGasCost, actualTokenAmount, appliedPriceMarkup, tokenPrice, userOpHash - ); + if (mode == PaymasterMode.EXTERNAL) { + // Decode context data + ( + address userOpSender, + address tokenAddress, + uint256 estimatedTokenAmount, + bytes32 userOpHash + ) = abi.decode(context[1:], (address, address, uint256, bytes32)); + + if (SafeTransferLib.trySafeTransferFrom(tokenAddress, userOpSender, address(this), estimatedTokenAmount)) { + emit PaidGasInTokensExternal(userOpSender, tokenAddress, estimatedTokenAmount, userOpHash); + } else { + revert FailedToChargeTokens(userOpSender, tokenAddress, estimatedTokenAmount, userOpHash); + } + + } else if (mode == PaymasterMode.INDEPENDENT) { + ( + address userOpSender, + address tokenAddress, + uint256 maxPenalty, + uint256 tokenPrice, + uint32 appliedPriceMarkup, + bytes32 userOpHash + ) = abi.decode(context[1:], (address, address, uint256, uint256, uint32, bytes32)); + // Calculate the amount to charge. unaccountedGas and maxPenalty are used, as we do not know the exact gas spent for postop and actual penalty at this point + // this is obviously overcharge, however, the excess amount can be refunded by backend, when we know the exact gas spent (emitted by EP after executing UserOp) + uint256 tokenAmount = ( + (actualGasCost + ((unaccountedGas + maxPenalty)) * actualUserOpFeePerGas)) * appliedPriceMarkup * tokenPrice + / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); + + if (SafeTransferLib.trySafeTransferFrom(tokenAddress, userOpSender, address(this), tokenAmount)) { + emit PaidGasInTokensIndependent( + userOpSender, tokenAddress, actualGasCost, tokenAmount, appliedPriceMarkup, tokenPrice, userOpHash + ); + } else { + revert FailedToChargeTokens(userOpSender, tokenAddress, tokenAmount, userOpHash); + } + } } function _validateTokenInfo(TokenInfo memory tokenInfo) internal view { diff --git a/test/base/TestBase.sol b/test/base/TestBase.sol index efe7333..bb2519f 100644 --- a/test/base/TestBase.sol +++ b/test/base/TestBase.sol @@ -63,8 +63,7 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { uint48 validUntil; uint48 validAfter; address tokenAddress; - uint256 tokenPrice; - uint32 externalPriceMarkup; + uint256 estimatedTokenAmount; } // Used to buffer user op gas limits @@ -342,8 +341,7 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { pmData.validUntil, pmData.validAfter, pmData.tokenAddress, - pmData.tokenPrice, - pmData.externalPriceMarkup, + pmData.estimatedTokenAmount, new bytes(65) // Zero signature ); @@ -352,7 +350,7 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { // Generate hash to be signed bytes32 paymasterHash = - paymaster.getHash(userOp, pmData.validUntil, pmData.validAfter, pmData.tokenAddress, pmData.tokenPrice, pmData.externalPriceMarkup); + paymaster.getHash(userOp, pmData.validUntil, pmData.validAfter, pmData.tokenAddress, pmData.estimatedTokenAmount); // Sign the hash signature = signMessage(signer, paymasterHash); @@ -367,8 +365,7 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { pmData.validUntil, pmData.validAfter, pmData.tokenAddress, - pmData.tokenPrice, - pmData.externalPriceMarkup, + pmData.estimatedTokenAmount, signature ); } diff --git a/test/mocks/PaymasterParserLibExposed.sol b/test/mocks/PaymasterParserLibExposed.sol index 41207ce..083ba03 100644 --- a/test/mocks/PaymasterParserLibExposed.sol +++ b/test/mocks/PaymasterParserLibExposed.sol @@ -15,8 +15,7 @@ library PaymasterParserLibExposed { uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 tokenPrice, - uint32 externalPriceMarkup, + uint256 estimatedTokenAmount, bytes calldata signature ) { return modeSpecificData.parseExternalModeSpecificData(); diff --git a/test/mocks/PaymasterParserLibWrapper.sol b/test/mocks/PaymasterParserLibWrapper.sol index 72c3206..648ea6c 100644 --- a/test/mocks/PaymasterParserLibWrapper.sol +++ b/test/mocks/PaymasterParserLibWrapper.sol @@ -15,11 +15,10 @@ contract PaymasterParserLibWrapper { uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 tokenPrice, - uint32 externalPriceMarkup, + uint256 estimatedTokenAmount, bytes memory signature ) { - (validUntil, validAfter, tokenAddress, tokenPrice, externalPriceMarkup, signature) = modeSpecificData.parseExternalModeSpecificData(); + (validUntil, validAfter, tokenAddress, estimatedTokenAmount, signature) = modeSpecificData.parseExternalModeSpecificData(); } function parseIndependentModeSpecificData(bytes calldata modeSpecificData) external pure returns (address tokenAddress) { diff --git a/test/unit/concrete/TestTokenPaymaster.Base.t.sol b/test/unit/concrete/TestTokenPaymaster.Base.t.sol index e5e6347..87b9357 100644 --- a/test/unit/concrete/TestTokenPaymaster.Base.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.Base.t.sol @@ -115,7 +115,7 @@ contract TestTokenPaymasterBase is TestBase { emit IBiconomyTokenPaymaster.TokensRefunded(address(ALICE_ACCOUNT), address(usdc), 0, bytes32(0)); vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.PaidGasInTokens(address(ALICE_ACCOUNT), address(usdc), 0, 0, 1e6, 0, bytes32(0)); + emit IBiconomyTokenPaymaster.PaidGasInTokensIndependent(address(ALICE_ACCOUNT), address(usdc), 0, 0, 1e6, 0, bytes32(0)); uint256 customGasPrice = 3e6; startPrank(BUNDLER.addr); diff --git a/test/unit/concrete/TestTokenPaymaster.t.sol b/test/unit/concrete/TestTokenPaymaster.t.sol index 44a8140..27df054 100644 --- a/test/unit/concrete/TestTokenPaymaster.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.t.sol @@ -309,7 +309,6 @@ contract TestTokenPaymaster is TestBase { vm.stopPrank(); PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); - uint128 tokenPrice = 1e18; // Assume 1 token = 1 native token = 1 native? TokenPaymasterData memory pmData = TokenPaymasterData({ paymasterValGasLimit: 3e6, @@ -318,8 +317,7 @@ contract TestTokenPaymaster is TestBase { validUntil: uint48(block.timestamp + 1 days), validAfter: uint48(block.timestamp), tokenAddress: address(testToken), - tokenPrice: tokenPrice, - externalPriceMarkup: 1e6 + estimatedTokenAmount: 999*1e18 }); (bytes memory paymasterAndData,) = generateAndSignTokenPaymasterData( @@ -354,7 +352,6 @@ contract TestTokenPaymaster is TestBase { vm.stopPrank(); PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); - uint128 tokenPrice = 1e18; TokenPaymasterData memory pmData = TokenPaymasterData({ paymasterValGasLimit: 3e6, @@ -363,8 +360,7 @@ contract TestTokenPaymaster is TestBase { validUntil: uint48(block.timestamp + 1 days), validAfter: uint48(block.timestamp), tokenAddress: address(testToken), - tokenPrice: tokenPrice, - externalPriceMarkup: 1e6 + estimatedTokenAmount: 999*1e18 }); // Create a valid paymasterAndData @@ -424,8 +420,7 @@ contract TestTokenPaymaster is TestBase { validUntil: validUntil, validAfter: validAfter, tokenAddress: address(testToken), - tokenPrice: tokenPrice, - externalPriceMarkup: externalPriceMarkup + estimatedTokenAmount: 1e18 }); // Generate and sign the token paymaster data @@ -446,7 +441,7 @@ contract TestTokenPaymaster is TestBase { emit IBiconomyTokenPaymaster.TokensRefunded(address(ALICE_ACCOUNT), address(testToken), 0, bytes32(0)); vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.PaidGasInTokens(address(ALICE_ACCOUNT), address(testToken), 0, 0, 1e6, 0, bytes32(0)); + emit IBiconomyTokenPaymaster.PaidGasInTokensExternal(address(ALICE_ACCOUNT), address(testToken), 0, bytes32(0)); // Execute the operation startPrank(BUNDLER.addr); @@ -503,7 +498,7 @@ contract TestTokenPaymaster is TestBase { 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.PaidGasInTokens(address(ALICE_ACCOUNT), address(testToken), 0, 0, 1e6, 0, bytes32(0)); + 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)); diff --git a/test/unit/concrete/TestTokenPaymasterParserLib.t.sol b/test/unit/concrete/TestTokenPaymasterParserLib.t.sol index 97aa904..123eeed 100644 --- a/test/unit/concrete/TestTokenPaymasterParserLib.t.sol +++ b/test/unit/concrete/TestTokenPaymasterParserLib.t.sol @@ -70,8 +70,7 @@ contract TestTokenPaymasterParserLib is Test { uint48 expectedValidUntil = uint48(block.timestamp + 1 days); uint48 expectedValidAfter = uint48(block.timestamp); address expectedTokenAddress = address(0x1234567890AbcdEF1234567890aBcdef12345678); - uint256 expectedTokenPrice = 1e8; - uint32 expectedExternalPriceMarkup = 1e6; + uint256 expectedEstimatedTokenAmount = 256*1e8; bytes memory expectedSignature = hex"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef"; // Construct external mode specific data @@ -79,8 +78,7 @@ contract TestTokenPaymasterParserLib is Test { bytes6(abi.encodePacked(expectedValidUntil)), bytes6(abi.encodePacked(expectedValidAfter)), bytes20(expectedTokenAddress), - bytes32(abi.encodePacked(expectedTokenPrice)), - bytes4(abi.encodePacked(expectedExternalPriceMarkup)), + bytes32(abi.encodePacked(expectedEstimatedTokenAmount)), expectedSignature ); @@ -89,8 +87,7 @@ contract TestTokenPaymasterParserLib is Test { uint48 parsedValidUntil, uint48 parsedValidAfter, address parsedTokenAddress, - uint256 parsedTokenPrice, - uint32 parsedExternalPriceMarkup, + uint256 parsedEstimatedTokenAmount, bytes memory parsedSignature ) = parser.parseExternalModeSpecificData(externalModeSpecificData); @@ -98,8 +95,7 @@ contract TestTokenPaymasterParserLib is Test { assertEq(parsedValidUntil, expectedValidUntil, "ValidUntil should match"); assertEq(parsedValidAfter, expectedValidAfter, "ValidAfter should match"); assertEq(parsedTokenAddress, expectedTokenAddress, "Token address should match"); - assertEq(parsedTokenPrice, expectedTokenPrice, "Token price should match"); - assertEq(parsedExternalPriceMarkup, expectedExternalPriceMarkup, "Dynamic adjustment should match"); + assertEq(parsedEstimatedTokenAmount, expectedEstimatedTokenAmount, "Estimated Token amount should match"); assertEq(parsedSignature, expectedSignature, "Signature should match"); } From 3c27ca657bcb1fb4d04fd6067bbb00c7b0e6815a Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Wed, 4 Dec 2024 13:47:12 +0300 Subject: [PATCH 2/5] 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)); From abfc70a8b2fb2e47ebb9f10dbbe2dba5ae0e486a Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Wed, 4 Dec 2024 13:58:39 +0300 Subject: [PATCH 3/5] approve in exec --- test/unit/concrete/TestTokenPaymaster.Base.t.sol | 9 +++++---- test/unit/concrete/TestTokenPaymaster.t.sol | 12 +++++------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/test/unit/concrete/TestTokenPaymaster.Base.t.sol b/test/unit/concrete/TestTokenPaymaster.Base.t.sol index 08df0ec..3707027 100644 --- a/test/unit/concrete/TestTokenPaymaster.Base.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.Base.t.sol @@ -81,9 +81,6 @@ contract TestTokenPaymasterBase is TestBase { function test_BaseFork_Success_TokenPaymaster_IndependentMode_WithoutPremium() external { tokenPaymaster.deposit{ value: 10 ether }(); deal(address(usdc), address(ALICE_ACCOUNT), 100e6); - vm.startPrank(address(ALICE_ACCOUNT)); - usdc.approve(address(tokenPaymaster), usdc.balanceOf(address(ALICE_ACCOUNT))); - vm.stopPrank(); vm.startPrank(PAYMASTER_OWNER.addr); tokenPaymaster.setUnaccountedGas(40_000); @@ -94,7 +91,11 @@ contract TestTokenPaymasterBase is TestBase { uint256 initialUserTokenBalance = usdc.balanceOf(address(ALICE_ACCOUNT)); uint256 initialPaymasterTokenBalance = usdc.balanceOf(address(tokenPaymaster)); - PackedUserOperation memory userOp = buildUserOpWithCalldata(ALICE, "", address(VALIDATOR_MODULE)); + PackedUserOperation memory userOp = buildUserOpWithCalldata( + ALICE, + abi.encodeWithSelector(IExecutionHelper.execute.selector, bytes32(0), abi.encodePacked(address(usdc), uint256(0), abi.encodeWithSelector(IERC20.approve.selector, address(tokenPaymaster), 1_000*1e18))), + address(VALIDATOR_MODULE) + ); // Encode paymasterAndData for independent mode bytes memory paymasterAndData = abi.encodePacked( diff --git a/test/unit/concrete/TestTokenPaymaster.t.sol b/test/unit/concrete/TestTokenPaymaster.t.sol index ac42598..6d3bb10 100644 --- a/test/unit/concrete/TestTokenPaymaster.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.t.sol @@ -389,9 +389,6 @@ contract TestTokenPaymaster is TestBase { function test_Success_TokenPaymaster_ExternalMode_WithoutPremium() external { tokenPaymaster.deposit{ value: 10 ether }(); testToken.mint(address(ALICE_ACCOUNT), 100_000 * (10 ** testToken.decimals())); - vm.startPrank(address(ALICE_ACCOUNT)); - testToken.approve(address(tokenPaymaster), testToken.balanceOf(address(ALICE_ACCOUNT))); - vm.stopPrank(); vm.startPrank(PAYMASTER_OWNER.addr); tokenPaymaster.setUnaccountedGas(22_000); @@ -467,9 +464,6 @@ contract TestTokenPaymaster is TestBase { function test_Success_TokenPaymaster_IndependentMode_WithoutPremium() external { tokenPaymaster.deposit{ value: 10 ether }(); testToken.mint(address(ALICE_ACCOUNT), 100_000 * (10 ** testToken.decimals())); - vm.startPrank(address(ALICE_ACCOUNT)); - testToken.approve(address(tokenPaymaster), testToken.balanceOf(address(ALICE_ACCOUNT))); - vm.stopPrank(); vm.startPrank(PAYMASTER_OWNER.addr); tokenPaymaster.setUnaccountedGas(40_000); @@ -480,7 +474,11 @@ contract TestTokenPaymaster is TestBase { uint256 initialUserTokenBalance = testToken.balanceOf(address(ALICE_ACCOUNT)); uint256 initialPaymasterTokenBalance = testToken.balanceOf(address(tokenPaymaster)); - 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) + ); // Encode paymasterAndData for independent mode bytes memory paymasterAndData = abi.encodePacked( From b45fc808ff0d4c5ad28bbca3a82dd4d7d1e6a701 Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Wed, 4 Dec 2024 18:34:45 +0300 Subject: [PATCH 4/5] rm indexed --- contracts/interfaces/IBiconomyTokenPaymaster.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/interfaces/IBiconomyTokenPaymaster.sol b/contracts/interfaces/IBiconomyTokenPaymaster.sol index 6a354c8..0d973b2 100644 --- a/contracts/interfaces/IBiconomyTokenPaymaster.sol +++ b/contracts/interfaces/IBiconomyTokenPaymaster.sol @@ -30,13 +30,13 @@ interface IBiconomyTokenPaymaster { uint256 tokenCharge, uint32 priceMarkup, uint256 tokenPrice, - bytes32 indexed userOpHash + bytes32 userOpHash ); event PaidGasInTokensExternal( address indexed userOpSender, address indexed token, uint256 tokenAmount, - bytes32 indexed userOpHash + bytes32 userOpHash ); event EthWithdrawn(address indexed recipient, uint256 indexed amount); From 0d72fa46a44f44929c3679c6952c8b6c4f2de29e Mon Sep 17 00:00:00 2001 From: Filipp Makarov Date: Thu, 5 Dec 2024 11:00:50 +0300 Subject: [PATCH 5/5] price+markup --- .../common/BiconomyTokenPaymasterErrors.sol | 5 - .../interfaces/IBiconomyTokenPaymaster.sol | 10 +- .../libraries/TokenPaymasterParserLib.sol | 8 +- contracts/token/BiconomyTokenPaymaster.sol | 156 +++++++----------- test/base/TestBase.sol | 11 +- test/mocks/PaymasterParserLibExposed.sol | 3 +- test/mocks/PaymasterParserLibWrapper.sol | 5 +- .../concrete/TestTokenPaymaster.Base.t.sol | 4 +- test/unit/concrete/TestTokenPaymaster.t.sol | 15 +- .../TestTokenPaymasterParserLib.t.sol | 12 +- 10 files changed, 99 insertions(+), 130 deletions(-) diff --git a/contracts/common/BiconomyTokenPaymasterErrors.sol b/contracts/common/BiconomyTokenPaymasterErrors.sol index 2929bb7..37602f5 100644 --- a/contracts/common/BiconomyTokenPaymasterErrors.sol +++ b/contracts/common/BiconomyTokenPaymasterErrors.sol @@ -86,9 +86,4 @@ contract BiconomyTokenPaymasterErrors { * @notice Throws when PM was not able to charge user */ error FailedToChargeTokens(address account, address token, uint256 amount, bytes32 userOpHash); - - /** - * Throws when account has insufficient token balance to pay for gas - */ - error InsufficientTokenBalance(address account, address token, uint256 amount, bytes32 userOpHash); } diff --git a/contracts/interfaces/IBiconomyTokenPaymaster.sol b/contracts/interfaces/IBiconomyTokenPaymaster.sol index 0d973b2..841d794 100644 --- a/contracts/interfaces/IBiconomyTokenPaymaster.sol +++ b/contracts/interfaces/IBiconomyTokenPaymaster.sol @@ -23,7 +23,7 @@ interface IBiconomyTokenPaymaster { event UpdatedFeeCollector(address indexed oldFeeCollector, address indexed newFeeCollector, address indexed actor); event UpdatedPriceExpiryDuration(uint256 indexed oldValue, uint256 indexed newValue); - event PaidGasInTokensIndependent( + event PaidGasInTokens( address indexed userOpSender, address indexed token, uint256 gasCostBeforePostOpAndPenalty, @@ -32,13 +32,7 @@ interface IBiconomyTokenPaymaster { uint256 tokenPrice, bytes32 userOpHash ); - event PaidGasInTokensExternal( - address indexed userOpSender, - address indexed token, - uint256 tokenAmount, - bytes32 userOpHash - ); - + event EthWithdrawn(address indexed recipient, uint256 indexed amount); event Received(address indexed sender, uint256 value); diff --git a/contracts/libraries/TokenPaymasterParserLib.sol b/contracts/libraries/TokenPaymasterParserLib.sol index 0cb2e72..31a1ab0 100644 --- a/contracts/libraries/TokenPaymasterParserLib.sol +++ b/contracts/libraries/TokenPaymasterParserLib.sol @@ -31,15 +31,17 @@ library TokenPaymasterParserLib { uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 estimatedTokenAmount, + uint256 tokenPrice, + uint32 appliedPriceMarkup, bytes calldata signature ) { validUntil = uint48(bytes6(modeSpecificData[:6])); validAfter = uint48(bytes6(modeSpecificData[6:12])); tokenAddress = address(bytes20(modeSpecificData[12:32])); - estimatedTokenAmount = uint256(bytes32(modeSpecificData[32:64])); - signature = modeSpecificData[64:]; + tokenPrice = uint256(bytes32(modeSpecificData[32:64])); + appliedPriceMarkup = uint32(bytes4(modeSpecificData[64:68])); + signature = modeSpecificData[68:]; } function parseIndependentModeSpecificData( diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index 87dd8d1..6a37e0e 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -15,8 +15,8 @@ import { IOracle } from "../interfaces/oracles/IOracle.sol"; import { TokenPaymasterParserLib } from "../libraries/TokenPaymasterParserLib.sol"; import { SignatureCheckerLib } from "solady/utils/SignatureCheckerLib.sol"; import { ECDSA as ECDSA_solady } from "solady/utils/ECDSA.sol"; -import "account-abstraction/core/Helpers.sol"; import { Uniswapper, IV3SwapRouter } from "./swaps/Uniswapper.sol"; +import "account-abstraction/core/Helpers.sol"; /** * @title BiconomyTokenPaymaster @@ -391,7 +391,8 @@ contract BiconomyTokenPaymaster is uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 estimatedTokenAmount + uint256 tokenPrice, + uint32 appliedPriceMarkup ) public view @@ -414,7 +415,8 @@ contract BiconomyTokenPaymaster is validUntil, validAfter, tokenAddress, - estimatedTokenAmount + tokenPrice, + appliedPriceMarkup ) ); } @@ -477,6 +479,11 @@ contract BiconomyTokenPaymaster is revert InvalidPaymasterMode(); } + uint256 maxPenalty = ( + ( uint128(uint256(userOp.accountGasLimits)) + + uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET:_PAYMASTER_DATA_OFFSET])) + ) * 10 ) / 100; + if (mode == PaymasterMode.EXTERNAL) { // Use the price and other params specified in modeSpecificData by the verifyingSigner // Useful for supporting tokens which don't have oracle support @@ -485,7 +492,8 @@ contract BiconomyTokenPaymaster is uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 estimatedTokenAmount, + uint256 tokenPrice, + uint32 externalPriceMarkup, bytes memory signature ) = modeSpecificData.parseExternalModeSpecificData(); @@ -495,7 +503,7 @@ contract BiconomyTokenPaymaster is bool validSig = verifyingSigner.isValidSignatureNow( ECDSA_solady.toEthSignedMessageHash( - getHash(userOp, validUntil, validAfter, tokenAddress, estimatedTokenAmount) + getHash(userOp, validUntil, validAfter, tokenAddress, tokenPrice, externalPriceMarkup) ), signature ); @@ -505,69 +513,34 @@ contract BiconomyTokenPaymaster is return ("", _packValidationData(true, validUntil, validAfter)); } - if(IERC20(tokenAddress).balanceOf(userOp.sender) < estimatedTokenAmount) { - revert InsufficientTokenBalance(userOp.sender, tokenAddress, estimatedTokenAmount, userOpHash); - } - - context = abi.encodePacked( - PaymasterMode.EXTERNAL, - abi.encode( - userOp.sender, - tokenAddress, - estimatedTokenAmount, - userOpHash - ) + context = abi.encode( + userOp.sender, + tokenAddress, + maxPenalty, + tokenPrice, + externalPriceMarkup, + userOpHash ); validationData = _packValidationData(false, validUntil, validAfter); /// INDEPENDENT MODE } else if (mode == PaymasterMode.INDEPENDENT) { + + address tokenAddress = modeSpecificData.parseIndependentModeSpecificData(); + // Use only oracles for the token specified in modeSpecificData if (modeSpecificData.length != 20) { revert InvalidTokenAddress(); } - uint256 maxPenalty = ( - ( - uint128(uint256(userOp.accountGasLimits)) - + uint128(bytes16(userOp.paymasterAndData[_PAYMASTER_POSTOP_GAS_OFFSET:_PAYMASTER_DATA_OFFSET])) - ) * 10 //* userOp.unpackMaxFeePerGas() - ) / 100; - - // Get address for token used to pay - address tokenAddress = modeSpecificData.parseIndependentModeSpecificData(); - uint256 tokenPrice = _getPrice(tokenAddress); - - if(tokenPrice == 0) { - revert TokenNotSupported(); - } - uint256 tokenAmount; - uint32 priceMarkup = independentTokenDirectory[tokenAddress].priceMarkup; - - { - // Calculate token amount to precharge - uint256 maxFeePerGas = UserOperationLib.unpackMaxFeePerGas(userOp); - tokenAmount = ((maxCost + maxPenalty + (unaccountedGas * maxFeePerGas)) * priceMarkup * tokenPrice) - / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); - } - - // Transfer full amount to this address. Unused amount will be refunded in postOP - // SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount); - if(IERC20(tokenAddress).balanceOf(userOp.sender) < tokenAmount) { - revert InsufficientTokenBalance(userOp.sender, tokenAddress, tokenAmount, userOpHash); - } - - context = abi.encodePacked( - PaymasterMode.INDEPENDENT, - abi.encode( - userOp.sender, - tokenAddress, - maxPenalty, - tokenPrice, - priceMarkup, - userOpHash - ) - ); + context = abi.encode( + userOp.sender, + tokenAddress, + maxPenalty, + uint256(0), // pass 0, so we can check the price in _postOp and be 4337 compliant + independentTokenDirectory[tokenAddress].priceMarkup, + userOpHash + ); validationData = 0; // Validation success and price is valid indefinetly } } @@ -588,45 +561,38 @@ contract BiconomyTokenPaymaster is internal override { - PaymasterMode pmMode = PaymasterMode(uint8(context[0])); - if (pmMode == PaymasterMode.EXTERNAL) { - // Decode context data - ( - address userOpSender, - address tokenAddress, - uint256 estimatedTokenAmount, - bytes32 userOpHash - ) = abi.decode(context[1:], (address, address, uint256, bytes32)); - - if (SafeTransferLib.trySafeTransferFrom(tokenAddress, userOpSender, address(this), estimatedTokenAmount)) { - emit PaidGasInTokensExternal(userOpSender, tokenAddress, estimatedTokenAmount, userOpHash); - } else { - revert FailedToChargeTokens(userOpSender, tokenAddress, estimatedTokenAmount, userOpHash); - } - - } else if (pmMode == PaymasterMode.INDEPENDENT) { - ( - address userOpSender, - address tokenAddress, - uint256 maxPenalty, - uint256 tokenPrice, - uint32 appliedPriceMarkup, - bytes32 userOpHash - ) = abi.decode(context[1:], (address, address, uint256, uint256, uint32, bytes32)); - // Calculate the amount to charge. unaccountedGas and maxPenalty are used, as we do not know the exact gas spent for postop and actual penalty at this point - // this is obviously overcharge, however, the excess amount can be refunded by backend, when we know the exact gas spent (emitted by EP after executing UserOp) - uint256 tokenAmount = ( - (actualGasCost + ((unaccountedGas + maxPenalty)) * actualUserOpFeePerGas)) * appliedPriceMarkup * tokenPrice - / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); - - if (SafeTransferLib.trySafeTransferFrom(tokenAddress, userOpSender, address(this), tokenAmount)) { - emit PaidGasInTokensIndependent( - userOpSender, tokenAddress, actualGasCost, tokenAmount, appliedPriceMarkup, tokenPrice, userOpHash - ); - } else { - revert FailedToChargeTokens(userOpSender, tokenAddress, tokenAmount, userOpHash); + ( + address userOpSender, + address tokenAddress, + uint256 maxPenalty, + uint256 tokenPrice, + uint32 appliedPriceMarkup, + bytes32 userOpHash + ) = abi.decode(context, (address, address, uint256, uint256, uint32, bytes32)); + + // If tokenPrice is 0, it means it was not set in the validatePaymasterUserOp => independent mode + // So we need to get the price of the token from the oracle now + if(tokenPrice == 0) { + tokenPrice = _getPrice(tokenAddress); + // if tokenPrice is still 0, it means the token is not supported + if(tokenPrice == 0) { + revert TokenNotSupported(); } } + + // Calculate the amount to charge. unaccountedGas and maxPenalty are used, + // as we do not know the exact gas spent for postop and actual penalty at this point + // this is obviously overcharge, however, the excess amount can be refunded by backend, + // when we know the exact gas spent (emitted by EP after executing UserOp) + uint256 tokenAmount = ( + (actualGasCost + ((unaccountedGas + maxPenalty)) * actualUserOpFeePerGas)) * appliedPriceMarkup * tokenPrice + / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); + + if (SafeTransferLib.trySafeTransferFrom(tokenAddress, userOpSender, address(this), tokenAmount)) { + emit PaidGasInTokens(userOpSender, tokenAddress, actualGasCost, tokenAmount, appliedPriceMarkup, tokenPrice, userOpHash); + } else { + revert FailedToChargeTokens(userOpSender, tokenAddress, tokenAmount, userOpHash); + } } function _validateTokenInfo(TokenInfo memory tokenInfo) internal view { diff --git a/test/base/TestBase.sol b/test/base/TestBase.sol index c835559..671083e 100644 --- a/test/base/TestBase.sol +++ b/test/base/TestBase.sol @@ -66,7 +66,8 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { uint48 validUntil; uint48 validAfter; address tokenAddress; - uint256 estimatedTokenAmount; + uint256 tokenPrice; + uint32 appliedPriceMarkup; } // Used to buffer user op gas limits @@ -344,7 +345,8 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { pmData.validUntil, pmData.validAfter, pmData.tokenAddress, - pmData.estimatedTokenAmount, + pmData.tokenPrice, + pmData.appliedPriceMarkup, new bytes(65) // Zero signature ); @@ -353,7 +355,7 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { // Generate hash to be signed bytes32 paymasterHash = - paymaster.getHash(userOp, pmData.validUntil, pmData.validAfter, pmData.tokenAddress, pmData.estimatedTokenAmount); + paymaster.getHash(userOp, pmData.validUntil, pmData.validAfter, pmData.tokenAddress, pmData.tokenPrice, pmData.appliedPriceMarkup); // Sign the hash signature = signMessage(signer, paymasterHash); @@ -368,7 +370,8 @@ abstract contract TestBase is CheatCodes, TestHelper, BaseEventsAndErrors { pmData.validUntil, pmData.validAfter, pmData.tokenAddress, - pmData.estimatedTokenAmount, + pmData.tokenPrice, + pmData.appliedPriceMarkup, signature ); } diff --git a/test/mocks/PaymasterParserLibExposed.sol b/test/mocks/PaymasterParserLibExposed.sol index 083ba03..aa29a92 100644 --- a/test/mocks/PaymasterParserLibExposed.sol +++ b/test/mocks/PaymasterParserLibExposed.sol @@ -15,7 +15,8 @@ library PaymasterParserLibExposed { uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 estimatedTokenAmount, + uint256 tokenPrice, + uint32 appliedPriceMarkup, bytes calldata signature ) { return modeSpecificData.parseExternalModeSpecificData(); diff --git a/test/mocks/PaymasterParserLibWrapper.sol b/test/mocks/PaymasterParserLibWrapper.sol index 648ea6c..e882e86 100644 --- a/test/mocks/PaymasterParserLibWrapper.sol +++ b/test/mocks/PaymasterParserLibWrapper.sol @@ -15,10 +15,11 @@ contract PaymasterParserLibWrapper { uint48 validUntil, uint48 validAfter, address tokenAddress, - uint256 estimatedTokenAmount, + uint256 tokenPrice, + uint32 appliedPriceMarkup, bytes memory signature ) { - (validUntil, validAfter, tokenAddress, estimatedTokenAmount, signature) = modeSpecificData.parseExternalModeSpecificData(); + (validUntil, validAfter, tokenAddress, tokenPrice, appliedPriceMarkup, signature) = modeSpecificData.parseExternalModeSpecificData(); } function parseIndependentModeSpecificData(bytes calldata modeSpecificData) external pure returns (address tokenAddress) { diff --git a/test/unit/concrete/TestTokenPaymaster.Base.t.sol b/test/unit/concrete/TestTokenPaymaster.Base.t.sol index 3707027..95de2af 100644 --- a/test/unit/concrete/TestTokenPaymaster.Base.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.Base.t.sol @@ -83,7 +83,7 @@ contract TestTokenPaymasterBase is TestBase { deal(address(usdc), address(ALICE_ACCOUNT), 100e6); vm.startPrank(PAYMASTER_OWNER.addr); - tokenPaymaster.setUnaccountedGas(40_000); + tokenPaymaster.setUnaccountedGas(80_000); vm.stopPrank(); uint256 initialBundlerBalance = BUNDLER.addr.balance; @@ -113,7 +113,7 @@ contract TestTokenPaymasterBase is TestBase { ops[0] = userOp; vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.PaidGasInTokensIndependent(address(ALICE_ACCOUNT), address(usdc), 0, 0, 1e6, 0, bytes32(0)); + emit IBiconomyTokenPaymaster.PaidGasInTokens(address(ALICE_ACCOUNT), address(usdc), 0, 0, 1e6, 0, bytes32(0)); uint256 customGasPrice = 3e6; startPrank(BUNDLER.addr); diff --git a/test/unit/concrete/TestTokenPaymaster.t.sol b/test/unit/concrete/TestTokenPaymaster.t.sol index 6d3bb10..0e25bc1 100644 --- a/test/unit/concrete/TestTokenPaymaster.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.t.sol @@ -317,7 +317,8 @@ contract TestTokenPaymaster is TestBase { validUntil: uint48(block.timestamp + 1 days), validAfter: uint48(block.timestamp), tokenAddress: address(testToken), - estimatedTokenAmount: 999*1e18 + tokenPrice: 1e18, + appliedPriceMarkup: 1e6 }); (bytes memory paymasterAndData,) = generateAndSignTokenPaymasterData( @@ -360,7 +361,8 @@ contract TestTokenPaymaster is TestBase { validUntil: uint48(block.timestamp + 1 days), validAfter: uint48(block.timestamp), tokenAddress: address(testToken), - estimatedTokenAmount: 999*1e18 + tokenPrice: 1e18, + appliedPriceMarkup: 1e6 }); // Create a valid paymasterAndData @@ -421,7 +423,8 @@ contract TestTokenPaymaster is TestBase { validUntil: validUntil, validAfter: validAfter, tokenAddress: address(testToken), - estimatedTokenAmount: 3_000_000_000_000 + tokenPrice: 1e18, + appliedPriceMarkup: 1e6 }); // Generate and sign the token paymaster data @@ -439,7 +442,7 @@ contract TestTokenPaymaster is TestBase { ops[0] = userOp; vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.PaidGasInTokensExternal(address(ALICE_ACCOUNT), address(testToken), 0, bytes32(0)); + emit IBiconomyTokenPaymaster.PaidGasInTokens(address(ALICE_ACCOUNT), address(testToken), 0, 0, 1e6, 0, bytes32(0)); // Execute the operation startPrank(BUNDLER.addr); @@ -466,7 +469,7 @@ contract TestTokenPaymaster is TestBase { testToken.mint(address(ALICE_ACCOUNT), 100_000 * (10 ** testToken.decimals())); vm.startPrank(PAYMASTER_OWNER.addr); - tokenPaymaster.setUnaccountedGas(40_000); + tokenPaymaster.setUnaccountedGas(55_000); vm.stopPrank(); uint256 initialBundlerBalance = BUNDLER.addr.balance; @@ -496,7 +499,7 @@ contract TestTokenPaymaster is TestBase { ops[0] = userOp; vm.expectEmit(true, true, false, false, address(tokenPaymaster)); - emit IBiconomyTokenPaymaster.PaidGasInTokensIndependent(address(ALICE_ACCOUNT), address(testToken), 0, 0, 1e6, 0, bytes32(0)); + emit IBiconomyTokenPaymaster.PaidGasInTokens(address(ALICE_ACCOUNT), address(testToken), 0, 0, 1e6, 0, bytes32(0)); startPrank(BUNDLER.addr); uint256 gasValue = gasleft(); diff --git a/test/unit/concrete/TestTokenPaymasterParserLib.t.sol b/test/unit/concrete/TestTokenPaymasterParserLib.t.sol index 123eeed..7263b04 100644 --- a/test/unit/concrete/TestTokenPaymasterParserLib.t.sol +++ b/test/unit/concrete/TestTokenPaymasterParserLib.t.sol @@ -70,7 +70,8 @@ contract TestTokenPaymasterParserLib is Test { uint48 expectedValidUntil = uint48(block.timestamp + 1 days); uint48 expectedValidAfter = uint48(block.timestamp); address expectedTokenAddress = address(0x1234567890AbcdEF1234567890aBcdef12345678); - uint256 expectedEstimatedTokenAmount = 256*1e8; + uint256 expectedTokenPrice = 1e18; + uint32 expectedAppliedPriceMarkup = 1e6; bytes memory expectedSignature = hex"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef"; // Construct external mode specific data @@ -78,7 +79,8 @@ contract TestTokenPaymasterParserLib is Test { bytes6(abi.encodePacked(expectedValidUntil)), bytes6(abi.encodePacked(expectedValidAfter)), bytes20(expectedTokenAddress), - bytes32(abi.encodePacked(expectedEstimatedTokenAmount)), + bytes32(abi.encodePacked(expectedTokenPrice)), + bytes4(abi.encodePacked(expectedAppliedPriceMarkup)), expectedSignature ); @@ -87,7 +89,8 @@ contract TestTokenPaymasterParserLib is Test { uint48 parsedValidUntil, uint48 parsedValidAfter, address parsedTokenAddress, - uint256 parsedEstimatedTokenAmount, + uint256 parsedTokenPrice, + uint32 parsedAppliedPriceMarkup, bytes memory parsedSignature ) = parser.parseExternalModeSpecificData(externalModeSpecificData); @@ -95,7 +98,8 @@ contract TestTokenPaymasterParserLib is Test { assertEq(parsedValidUntil, expectedValidUntil, "ValidUntil should match"); assertEq(parsedValidAfter, expectedValidAfter, "ValidAfter should match"); assertEq(parsedTokenAddress, expectedTokenAddress, "Token address should match"); - assertEq(parsedEstimatedTokenAmount, expectedEstimatedTokenAmount, "Estimated Token amount should match"); + assertEq(parsedTokenPrice, expectedTokenPrice, "Token price should match"); + assertEq(parsedAppliedPriceMarkup, expectedAppliedPriceMarkup, "Applied price markup should match"); assertEq(parsedSignature, expectedSignature, "Signature should match"); }