diff --git a/contracts/interfaces/IBiconomyTokenPaymaster.sol b/contracts/interfaces/IBiconomyTokenPaymaster.sol index 365411e..0902b7b 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 @@ -40,6 +39,7 @@ interface IBiconomyTokenPaymaster { event RemovedFromTokenDirectory(address indexed tokenAddress); event UpdatedNativeAssetOracle(IOracle indexed oldOracle, IOracle indexed newOracle); event TokensSwappedAndRefilledEntryPoint(address indexed tokenAddress, uint256 indexed tokenAmount, uint256 indexed amountOut, address actor); + event TokenPriceUpdated(uint256 indexed currentPrice, uint256 indexed previousPrice, uint48 indexed cachedPriceTimestamp); event SwappableTokensAdded(address[] indexed tokenAddresses); function setSigner(address newVerifyingSigner) external payable; diff --git a/contracts/references/SampleTokenPaymaster.sol b/contracts/references/SampleTokenPaymaster.sol index e3ea19b..086e592 100644 --- a/contracts/references/SampleTokenPaymaster.sol +++ b/contracts/references/SampleTokenPaymaster.sol @@ -39,7 +39,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { } /// @notice All 'price' variables are multiplied by this value to avoid rounding up - uint256 private constant _PRICE_DENOMINATOR = 1e26; + uint256 private constant _MARKUP_DENOMINATOR = 1e26; TokenPaymasterConfig public tokenPaymasterConfig; @@ -101,8 +101,8 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { /// @notice Updates the configuration for the Token Paymaster. /// @param paymasterConfig The new configuration struct. function setTokenPaymasterConfig(TokenPaymasterConfig memory paymasterConfig) public onlyOwner { - require(paymasterConfig.priceMarkup <= 2 * _PRICE_DENOMINATOR, "TPM: price markup too high"); - require(paymasterConfig.priceMarkup >= _PRICE_DENOMINATOR, "TPM: price markup too low"); + require(paymasterConfig.priceMarkup <= 2 * _MARKUP_DENOMINATOR, "TPM: price markup too high"); + require(paymasterConfig.priceMarkup >= _MARKUP_DENOMINATOR, "TPM: price markup too low"); tokenPaymasterConfig = paymasterConfig; emit ConfigUpdated(paymasterConfig); } @@ -132,7 +132,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { uint256 preChargeNative = requiredPreFund + (refundPostopCost * maxFeePerGas); // note: as price is in native-asset-per-token and we want more tokens increasing it means dividing it by // markup - uint256 cachedPriceWithMarkup = (cachedPrice * _PRICE_DENOMINATOR) / priceMarkup; + uint256 cachedPriceWithMarkup = (cachedPrice * _MARKUP_DENOMINATOR) / priceMarkup; if (dataLength == 32) { uint256 clientSuppliedPrice = uint256(bytes32(userOp.paymasterAndData[PAYMASTER_DATA_OFFSET:PAYMASTER_DATA_OFFSET + 32])); @@ -171,7 +171,7 @@ contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { uint256 cachedPrice = updateCachedPrice(false); // note: as price is in native-asset-per-token and we want more tokens increasing it means dividing it by // markup - uint256 cachedPriceWithMarkup = (cachedPrice * _PRICE_DENOMINATOR) / priceMarkup; + uint256 cachedPriceWithMarkup = (cachedPrice * _MARKUP_DENOMINATOR) / priceMarkup; // Refund tokens based on actual gas cost uint256 actualChargeNative = actualGasCost + tokenPaymasterConfig.refundPostopCost * actualUserOpFeePerGas; uint256 actualTokenNeeded = weiToToken(actualChargeNative, cachedPriceWithMarkup); diff --git a/contracts/sponsorship/BiconomySponsorshipPaymaster.sol b/contracts/sponsorship/BiconomySponsorshipPaymaster.sol index 19a11aa..5325f3a 100644 --- a/contracts/sponsorship/BiconomySponsorshipPaymaster.sol +++ b/contracts/sponsorship/BiconomySponsorshipPaymaster.sol @@ -44,7 +44,7 @@ contract BiconomySponsorshipPaymaster is uint256 public minDeposit; // Denominator to prevent precision errors when applying price markup - uint256 private constant _PRICE_DENOMINATOR = 1e6; + uint256 private constant _MARKUP_DENOMINATOR = 1e6; // Offset in PaymasterAndData to get to PAYMASTER_ID_OFFSET uint256 private constant _PAYMASTER_ID_OFFSET = _PAYMASTER_DATA_OFFSET; // Limit for unaccounted gas cost @@ -348,7 +348,7 @@ contract BiconomySponsorshipPaymaster is // unaccountedGas = postOpGas + EP overhead gas actualGasCost = actualGasCost + (unaccountedGas * actualUserOpFeePerGas); // Apply the price markup - uint256 adjustedGasCost = (actualGasCost * priceMarkup) / _PRICE_DENOMINATOR; + uint256 adjustedGasCost = (actualGasCost * priceMarkup) / _MARKUP_DENOMINATOR; uint256 premium = adjustedGasCost - actualGasCost; @@ -440,7 +440,7 @@ contract BiconomySponsorshipPaymaster is // Deduct the max gas cost. uint256 effectiveCost = - (((requiredPreFund + unaccountedGas * userOp.unpackMaxFeePerGas()) * priceMarkup) / _PRICE_DENOMINATOR); + (((requiredPreFund + unaccountedGas * userOp.unpackMaxFeePerGas()) * priceMarkup) / _MARKUP_DENOMINATOR); if (effectiveCost + maxPenalty > paymasterIdBalances[paymasterId]) { revert InsufficientFundsForPaymasterId(); diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index ff2d048..ae35eff 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -17,7 +17,6 @@ import { SignatureCheckerLib } from "solady/utils/SignatureCheckerLib.sol"; import { ECDSA as ECDSA_solady } from "solady/utils/ECDSA.sol"; import "account-abstraction/core/Helpers.sol"; import "./swaps/Uniswapper.sol"; -// Todo: marked for removal import "forge-std/console2.sol"; /** @@ -57,10 +56,16 @@ contract BiconomyTokenPaymaster is IOracle public nativeAssetToUsdOracle; // ETH -> USD price oracle mapping(address => TokenInfo) public independentTokenDirectory; // mapping of token address => info for tokens // supported in // independent mode + mapping(address => uint256) public cachedPrices; // mapping of token address => cached price + mapping(address => uint48) public cachedPricesTimestamps; // mapping of token address => cached price timestamp + + uint48 cacheTimeToLive; // time to live for cached prices + uint256 priceUpdateThreshold; // threshold for price update uint256 private constant _UNACCOUNTED_GAS_LIMIT = 200_000; // Limit for unaccounted gas cost - uint32 private constant _PRICE_DENOMINATOR = 1e6; // Denominator used when calculating cost with price markup + uint32 private constant _MARKUP_DENOMINATOR = 1e6; // Denominator used when calculating cost with price markup uint32 private constant _MAX_PRICE_MARKUP = 2e6; // 100% premium on price (2e6/PRICE_DENOMINATOR) + uint256 private constant _PRICE_DENOMINATOR = 1e26; uint256 private immutable _NATIVE_TOKEN_DECIMALS; constructor( @@ -78,7 +83,9 @@ contract BiconomyTokenPaymaster is // mode IOracle[] memory oraclesArg, // Array of corresponding oracle addresses for independently supported tokens address[] memory swappableTokens, // Array of tokens that you want swappable by the uniswapper - uint24[] memory swappableTokenPoolFeeTiers // Array of uniswap pool fee tiers for each swappable token + uint24[] memory swappableTokenPoolFeeTiers, // Array of uniswap pool fee tiers for each swappable token, + uint48 cacheTimeToLiveArg, + uint256 priceUpdateThresholdArg ) BasePaymaster(owner, entryPoint) Uniswapper(uniswapRouterArg, wrappedNativeArg, swappableTokens, swappableTokenPoolFeeTiers) @@ -93,7 +100,7 @@ contract BiconomyTokenPaymaster is if (unaccountedGasArg > _UNACCOUNTED_GAS_LIMIT) { revert UnaccountedGasTooHigh(); } - if (independentPriceMarkupArg > _MAX_PRICE_MARKUP || independentPriceMarkupArg < _PRICE_DENOMINATOR) { + if (independentPriceMarkupArg > _MAX_PRICE_MARKUP || independentPriceMarkupArg < _MARKUP_DENOMINATOR) { // Not between 0% and 100% markup revert InvalidPriceMarkup(); } @@ -107,6 +114,8 @@ contract BiconomyTokenPaymaster is if (block.timestamp < priceExpiryDurationArg) { revert InvalidPriceExpiryDuration(); } + cacheTimeToLive = cacheTimeToLiveArg; + priceUpdateThreshold = priceUpdateThresholdArg; // Set state variables assembly ("memory-safe") { @@ -256,7 +265,7 @@ contract BiconomyTokenPaymaster is * @notice only to be called by the owner of the contract. */ function setPriceMarkup(uint32 newIndependentPriceMarkup) external payable onlyOwner { - if (newIndependentPriceMarkup > _MAX_PRICE_MARKUP || newIndependentPriceMarkup < _PRICE_DENOMINATOR) { + if (newIndependentPriceMarkup > _MAX_PRICE_MARKUP || newIndependentPriceMarkup < _MARKUP_DENOMINATOR) { // Not between 0% and 100% markup revert InvalidPriceMarkup(); } @@ -398,6 +407,32 @@ contract BiconomyTokenPaymaster is entryPoint.withdrawTo(withdrawAddress, amount); } + function updateCachedPrice(address tokenAddress, bool force) public returns (uint256 price) { + uint256 cacheAge = block.timestamp - cachedPricesTimestamps[tokenAddress]; + if (!force && cacheAge <= cacheTimeToLive) { + return cachedPrices[tokenAddress]; + } + + uint256 _cachedPrice = cachedPrices[tokenAddress]; + uint256 newPrice = _getPrice(tokenAddress); + if(_cachedPrice == 0) { + _cachedPrice = newPrice; + } + uint256 priceRatio = _PRICE_DENOMINATOR * newPrice / _cachedPrice; + + // Review: Note markup can be used any value. + bool updateRequired = force || + priceRatio > _PRICE_DENOMINATOR + priceUpdateThreshold || + priceRatio < _PRICE_DENOMINATOR - priceUpdateThreshold; + if (!updateRequired) { + return _cachedPrice; + } + cachedPrices[tokenAddress] = newPrice; + cachedPricesTimestamps[tokenAddress] = uint48(block.timestamp); + emit TokenPriceUpdated(newPrice, _cachedPrice, cachedPricesTimestamps[tokenAddress]); + return newPrice; + } + /** * 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. @@ -473,7 +508,6 @@ contract BiconomyTokenPaymaster is 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 - ( uint48 validUntil, uint48 validAfter, @@ -499,7 +533,7 @@ contract BiconomyTokenPaymaster is return ("", _packValidationData(true, validUntil, validAfter)); } - if (externalPriceMarkup > _MAX_PRICE_MARKUP || externalPriceMarkup < _PRICE_DENOMINATOR) { + if (externalPriceMarkup > _MAX_PRICE_MARKUP || externalPriceMarkup < _MARKUP_DENOMINATOR) { revert InvalidPriceMarkup(); } @@ -509,7 +543,7 @@ contract BiconomyTokenPaymaster is { uint256 maxFeePerGas = UserOperationLib.unpackMaxFeePerGas(userOp); tokenAmount = ((maxCost + maxPenalty + (unaccountedGas * maxFeePerGas)) * externalPriceMarkup * tokenPrice) - / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); + / (_NATIVE_TOKEN_DECIMALS * _MARKUP_DENOMINATOR); } // Transfer full amount to this address. Unused amount will be refunded in postOP @@ -517,7 +551,7 @@ contract BiconomyTokenPaymaster is // deduct max penalty from the token amount we pass to the postOp // so we don't refund it at postOp - context = abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice*externalPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)), tokenPrice, externalPriceMarkup, userOpHash); + context = abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice*externalPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_MARKUP_DENOMINATOR)), tokenPrice, externalPriceMarkup, userOpHash); validationData = _packValidationData(false, validUntil, validAfter); } else if (mode == PaymasterMode.INDEPENDENT) { // Use only oracles for the token specified in modeSpecificData @@ -527,25 +561,23 @@ contract BiconomyTokenPaymaster is // Get address for token used to pay address tokenAddress = modeSpecificData.parseIndependentModeSpecificData(); - uint256 tokenPrice = _getPrice(tokenAddress); + uint256 tokenPrice = cachedPrices[tokenAddress]; if(tokenPrice == 0) { revert TokenNotSupported(); } uint256 tokenAmount; - - // TODO: Account for penalties here { // Calculate token amount to precharge uint256 maxFeePerGas = UserOperationLib.unpackMaxFeePerGas(userOp); tokenAmount = ((maxCost + maxPenalty + (unaccountedGas * maxFeePerGas)) * independentPriceMarkup * tokenPrice) - / (_NATIVE_TOKEN_DECIMALS * _PRICE_DENOMINATOR); + / (_NATIVE_TOKEN_DECIMALS * _MARKUP_DENOMINATOR); } // Transfer full amount to this address. Unused amount will be refunded in postOP SafeTransferLib.safeTransferFrom(tokenAddress, userOp.sender, address(this), tokenAmount); context = - abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice*independentPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_PRICE_DENOMINATOR)), tokenPrice, independentPriceMarkup, userOpHash); + abi.encode(userOp.sender, tokenAddress, tokenAmount-((maxPenalty*tokenPrice*independentPriceMarkup)/(_NATIVE_TOKEN_DECIMALS*_MARKUP_DENOMINATOR)), independentPriceMarkup, userOpHash); validationData = 0; // Validation success and price is valid indefinetly } } @@ -576,10 +608,12 @@ contract BiconomyTokenPaymaster is bytes32 userOpHash ) = abi.decode(context, (address, address, uint256, uint256, uint32, bytes32)); + tokenPrice = updateCachedPrice(tokenAddress, false); + // 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); + ) / (_NATIVE_TOKEN_DECIMALS * _MARKUP_DENOMINATOR); if (prechargedAmount > actualTokenAmount) { // If the user was overcharged, refund the excess tokens uint256 refundAmount = prechargedAmount - actualTokenAmount; diff --git a/test/unit/concrete/TestTokenPaymaster.Base.t.sol b/test/unit/concrete/TestTokenPaymaster.Base.t.sol index 210ecd1..94b98de 100644 --- a/test/unit/concrete/TestTokenPaymaster.Base.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.Base.t.sol @@ -49,8 +49,12 @@ contract TestTokenPaymasterBase is TestBase { _toSingletonArray(address(usdc)), _toSingletonArray(IOracle(address(tokenOracle))), _toSingletonArray(address(usdc)), - _toSingletonArray(uint24(500)) // from here: https://basescan.org/address/0xd0b53D9277642d899DF5C87A3966A349A798F224#readContract + _toSingletonArray(uint24(500)), // from here: https://basescan.org/address/0xd0b53D9277642d899DF5C87A3966A349A798F224#readContract + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); + + // tokenPaymaster.updateCachedPrice(address(usdc), true); } function test_Deploy_BaseFork() external { @@ -69,7 +73,9 @@ contract TestTokenPaymasterBase is TestBase { _toSingletonArray(address(usdc)), _toSingletonArray(IOracle(address(tokenOracle))), _toSingletonArray(address(usdc)), - _toSingletonArray(uint24(500)) // from here: https://basescan.org/address/0xd0b53D9277642d899DF5C87A3966A349A798F224#readContract + _toSingletonArray(uint24(500)), // from here: https://basescan.org/address/0xd0b53D9277642d899DF5C87A3966A349A798F224#readContract + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); assertEq(testArtifact.owner(), PAYMASTER_OWNER.addr); @@ -82,6 +88,7 @@ contract TestTokenPaymasterBase is TestBase { function test_BaseFork_Success_TokenPaymaster_IndependentMode_WithoutPremium() external { tokenPaymaster.deposit{ value: 10 ether }(); + tokenPaymaster.updateCachedPrice(address(usdc), true); deal(address(usdc), address(ALICE_ACCOUNT), 100e6); vm.startPrank(address(ALICE_ACCOUNT)); usdc.approve(address(tokenPaymaster), usdc.balanceOf(address(ALICE_ACCOUNT))); diff --git a/test/unit/concrete/TestTokenPaymaster.t.sol b/test/unit/concrete/TestTokenPaymaster.t.sol index 0407bc8..8f0c64b 100644 --- a/test/unit/concrete/TestTokenPaymaster.t.sol +++ b/test/unit/concrete/TestTokenPaymaster.t.sol @@ -49,8 +49,13 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(tokenOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); + + // Review: fails with [FAIL. Reason: setup failed: panic: division or modulo by zero (0x12)] setUp() + // tokenPaymaster.updateCachedPrice(address(testToken), true); } function test_Deploy() external { @@ -69,7 +74,9 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(tokenOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); assertEq(testArtifact.owner(), PAYMASTER_OWNER.addr); @@ -97,7 +104,9 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(tokenOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); } @@ -118,7 +127,9 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(tokenOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); } @@ -138,7 +149,9 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(tokenOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); } @@ -158,7 +171,9 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(tokenOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); } @@ -232,7 +247,9 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(tokenOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); } @@ -253,7 +270,9 @@ contract TestTokenPaymaster is TestBase { _toSingletonArray(address(testToken)), _toSingletonArray(IOracle(address(invalidOracle))), new address[](0), - new uint24[](0) + new uint24[](0), + 1e8, // cache time to live // Review + 1e26 * 12 / 100 // price update threshold ); } @@ -470,6 +489,7 @@ contract TestTokenPaymaster is TestBase { function test_Success_TokenPaymaster_IndependentMode_WithoutPremium() external { tokenPaymaster.deposit{ value: 10 ether }(); + tokenPaymaster.updateCachedPrice(address(testToken), true); testToken.mint(address(ALICE_ACCOUNT), 100_000 * (10 ** testToken.decimals())); vm.startPrank(address(ALICE_ACCOUNT)); testToken.approve(address(tokenPaymaster), testToken.balanceOf(address(ALICE_ACCOUNT)));