From 19c4141dfec546b83b468aad55a70c7c55f8af4d Mon Sep 17 00:00:00 2001 From: Ryan Hall Date: Thu, 10 Oct 2024 17:02:58 -0400 Subject: [PATCH] make gas staleness configurable per chain in FeeQuoter --- contracts/src/v0.8/ccip/FeeQuoter.sol | 55 +++++++++++++------ .../v0.8/ccip/test/feeQuoter/FeeQuoter.t.sol | 43 ++++++++++++--- .../ccip/test/feeQuoter/FeeQuoterSetup.t.sol | 3 +- .../ccip/deployment_test/deployment_test.go | 6 +- 4 files changed, 77 insertions(+), 30 deletions(-) diff --git a/contracts/src/v0.8/ccip/FeeQuoter.sol b/contracts/src/v0.8/ccip/FeeQuoter.sol index d2a6b77eff..9c6d6a5331 100644 --- a/contracts/src/v0.8/ccip/FeeQuoter.sol +++ b/contracts/src/v0.8/ccip/FeeQuoter.sol @@ -29,7 +29,6 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, error TokenNotSupported(address token); error FeeTokenNotSupported(address token); - error ChainNotSupported(uint64 chain); error StaleGasPrice(uint64 destChainSelector, uint256 threshold, uint256 timePassed); error StaleKeystoneUpdate(address token, uint256 feedTimestamp, uint256 storedTimeStamp); error DataFeedValueOutOfUint224Range(); @@ -76,7 +75,8 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, struct StaticConfig { uint96 maxFeeJuelsPerMsg; // ─╮ Maximum fee that can be charged for a message address linkToken; // ────────╯ LINK token address - uint32 stalenessThreshold; // The amount of time a gas price can be stale before it is considered invalid. + // The amount of time a token price can be stale before it is considered invalid (gas price staleness is configured per dest chain) + uint32 tokenPriceStalenessThreshold; } /// @dev The struct representing the received CCIP feed report from keystone IReceiver.onReport() @@ -103,6 +103,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, uint32 defaultTxGasLimit; //─────────────────╮ Default gas limit for a tx uint64 gasMultiplierWeiPerEth; // │ Multiplier for gas costs, 1e18 based so 11e17 = 10% extra cost. uint32 networkFeeUSDCents; // │ Flat network fee to charge for messages, multiples of 0.01 USD + uint32 gasPriceStalenessThreshold; // │ The amount of time a gas price can be stale before it is considered invalid (0 means disabled) bool enforceOutOfOrder; // │ Whether to enforce the allowOutOfOrderExecution extraArg value to be true. bytes4 chainFamilySelector; // ──────────────╯ Selector that identifies the destination chain's family. Used to determine the correct validations to perform for the dest chain. } @@ -202,8 +203,8 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, /// @dev Subset of tokens which prices tracked by this registry which are fee tokens. EnumerableSet.AddressSet private s_feeTokens; - /// @dev The amount of time a gas price can be stale before it is considered invalid. - uint32 private immutable i_stalenessThreshold; + /// @dev The amount of time a token price can be stale before it is considered invalid. + uint32 private immutable i_tokenPriceStalenessThreshold; constructor( StaticConfig memory staticConfig, @@ -216,14 +217,14 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, ) AuthorizedCallers(priceUpdaters) { if ( staticConfig.linkToken == address(0) || staticConfig.maxFeeJuelsPerMsg == 0 - || staticConfig.stalenessThreshold == 0 + || staticConfig.tokenPriceStalenessThreshold == 0 ) { revert InvalidStaticConfig(); } i_linkToken = staticConfig.linkToken; i_maxFeeJuelsPerMsg = staticConfig.maxFeeJuelsPerMsg; - i_stalenessThreshold = staticConfig.stalenessThreshold; + i_tokenPriceStalenessThreshold = staticConfig.tokenPriceStalenessThreshold; _applyFeeTokensUpdates(feeTokens, new address[](0)); _updateTokenPriceFeeds(tokenPriceFeeds); @@ -241,7 +242,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, Internal.TimestampedPackedUint224 memory tokenPrice = s_usdPerToken[token]; // If the token price is not stale, return it - if (block.timestamp - tokenPrice.timestamp < i_stalenessThreshold) { + if (block.timestamp - tokenPrice.timestamp < i_tokenPriceStalenessThreshold) { return tokenPrice; } @@ -305,14 +306,12 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, function getTokenAndGasPrices( address token, uint64 destChainSelector - ) public view returns (uint224 tokenPrice, uint224 gasPriceValue) { - Internal.TimestampedPackedUint224 memory gasPrice = s_usdPerUnitGasByDestChainSelector[destChainSelector]; - // We do allow a gas price of 0, but no stale or unset gas prices - if (gasPrice.timestamp == 0) revert ChainNotSupported(destChainSelector); - uint256 timePassed = block.timestamp - gasPrice.timestamp; - if (timePassed > i_stalenessThreshold) revert StaleGasPrice(destChainSelector, i_stalenessThreshold, timePassed); - - return (_getValidatedTokenPrice(token), gasPrice.value); + ) external view returns (uint224 tokenPrice, uint224 gasPriceValue) { + if (!s_destChainConfigs[destChainSelector].isEnabled) revert DestinationChainNotEnabled(destChainSelector); + return ( + _getValidatedTokenPrice(token), + _getValidatedGasPrice(destChainSelector, s_destChainConfigs[destChainSelector].gasPriceStalenessThreshold) + ); } /// @notice Convert a given token amount to target token amount. @@ -374,6 +373,27 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, return Internal.TimestampedPackedUint224({value: rebasedValue, timestamp: uint32(block.timestamp)}); } + /// @dev Gets the fee token price and the gas price, both denominated in dollars. + /// @param destChainSelector The destination chain to get the gas price for. + /// @param gasPriceStalenessThreshold The amount of time a gas price can be stale before it is considered invalid. + /// @return gasPriceValue The price of gas in 1e18 dollars per base unit. + function _getValidatedGasPrice( + uint64 destChainSelector, + uint32 gasPriceStalenessThreshold + ) private view returns (uint224 gasPriceValue) { + Internal.TimestampedPackedUint224 memory gasPrice = s_usdPerUnitGasByDestChainSelector[destChainSelector]; + // If the staleness threshold is 0, we consider the gas price to be always valid + if (gasPriceStalenessThreshold != 0) { + // We do allow a gas price of 0, but no stale or unset gas prices + uint256 timePassed = block.timestamp - gasPrice.timestamp; + if (timePassed > gasPriceStalenessThreshold) { + revert StaleGasPrice(destChainSelector, gasPriceStalenessThreshold, timePassed); + } + } + + return gasPrice.value; + } + // ================================================================ // │ Fee tokens │ // ================================================================ @@ -507,7 +527,8 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, _validateMessage(destChainConfig, message.data.length, numberOfTokens, message.receiver); // The below call asserts that feeToken is a supported token - (uint224 feeTokenPrice, uint224 packedGasPrice) = getTokenAndGasPrices(message.feeToken, destChainSelector); + uint224 feeTokenPrice = _getValidatedTokenPrice(message.feeToken); + uint224 packedGasPrice = _getValidatedGasPrice(destChainSelector, destChainConfig.gasPriceStalenessThreshold); // Calculate premiumFee in USD with 18 decimals precision first. // If message-only and no token transfers, a flat network fee is charged. @@ -990,7 +1011,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, return StaticConfig({ maxFeeJuelsPerMsg: i_maxFeeJuelsPerMsg, linkToken: i_linkToken, - stalenessThreshold: i_stalenessThreshold + tokenPriceStalenessThreshold: i_tokenPriceStalenessThreshold }); } } diff --git a/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoter.t.sol b/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoter.t.sol index 41720898dd..4417a54e9c 100644 --- a/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoter.t.sol +++ b/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoter.t.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity 0.8.24; -import {IFeeQuoter} from "../../interfaces/IFeeQuoter.sol"; - import {KeystoneFeedsPermissionHandler} from "../../../keystone/KeystoneFeedsPermissionHandler.sol"; import {AuthorizedCallers} from "../../../shared/access/AuthorizedCallers.sol"; import {MockV3Aggregator} from "../../../tests/MockV3Aggregator.sol"; @@ -35,7 +33,7 @@ contract FeeQuoter_constructor is FeeQuoterSetup { FeeQuoter.StaticConfig memory staticConfig = FeeQuoter.StaticConfig({ linkToken: s_sourceTokens[0], maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS, - stalenessThreshold: uint32(TWELVE_HOURS) + tokenPriceStalenessThreshold: uint32(TWELVE_HOURS) }); s_feeQuoter = new FeeQuoterHelper( staticConfig, @@ -93,7 +91,7 @@ contract FeeQuoter_constructor is FeeQuoterSetup { FeeQuoter.StaticConfig memory staticConfig = FeeQuoter.StaticConfig({ linkToken: s_sourceTokens[0], maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS, - stalenessThreshold: 0 + tokenPriceStalenessThreshold: 0 }); vm.expectRevert(FeeQuoter.InvalidStaticConfig.selector); @@ -113,7 +111,7 @@ contract FeeQuoter_constructor is FeeQuoterSetup { FeeQuoter.StaticConfig memory staticConfig = FeeQuoter.StaticConfig({ linkToken: address(0), maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS, - stalenessThreshold: uint32(TWELVE_HOURS) + tokenPriceStalenessThreshold: uint32(TWELVE_HOURS) }); vm.expectRevert(FeeQuoter.InvalidStaticConfig.selector); @@ -133,7 +131,7 @@ contract FeeQuoter_constructor is FeeQuoterSetup { FeeQuoter.StaticConfig memory staticConfig = FeeQuoter.StaticConfig({ linkToken: s_sourceTokens[0], maxFeeJuelsPerMsg: 0, - stalenessThreshold: uint32(TWELVE_HOURS) + tokenPriceStalenessThreshold: uint32(TWELVE_HOURS) }); vm.expectRevert(FeeQuoter.InvalidStaticConfig.selector); @@ -173,7 +171,7 @@ contract FeeQuoter_getTokenPrice is FeeQuoterSetup { uint256 originalTimestampValue = block.timestamp; // Above staleness threshold - vm.warp(originalTimestampValue + s_feeQuoter.getStaticConfig().stalenessThreshold + 1); + vm.warp(originalTimestampValue + s_feeQuoter.getStaticConfig().tokenPriceStalenessThreshold + 1); address sourceToken = _initialiseSingleTokenPriceFeed(); Internal.TimestampedPackedUint224 memory tokenPriceAnswer = s_feeQuoter.getTokenPrice(sourceToken); @@ -596,8 +594,35 @@ contract FeeQuoter_getTokenAndGasPrices is FeeQuoterSetup { assertEq(gasPrice, priceUpdates.gasPriceUpdates[0].usdPerUnitGas); } + function test_StalenessCheckDisabled_Success() public { + uint64 neverStaleChainSelector = 345678; + FeeQuoter.DestChainConfigArgs[] memory destChainConfigArgs = _generateFeeQuoterDestChainConfigArgs(); + destChainConfigArgs[0].destChainSelector = neverStaleChainSelector; + destChainConfigArgs[0].destChainConfig.gasPriceStalenessThreshold = 0; // disables the staleness check + + s_feeQuoter.applyDestChainConfigUpdates(destChainConfigArgs); + + Internal.GasPriceUpdate[] memory gasPriceUpdates = new Internal.GasPriceUpdate[](1); + gasPriceUpdates[0] = Internal.GasPriceUpdate({destChainSelector: neverStaleChainSelector, usdPerUnitGas: 999}); + + Internal.PriceUpdates memory priceUpdates = + Internal.PriceUpdates({tokenPriceUpdates: new Internal.TokenPriceUpdate[](0), gasPriceUpdates: gasPriceUpdates}); + s_feeQuoter.updatePrices(priceUpdates); + + // this should have no affect! But we do it anyway to make sure the staleness check is disabled + vm.warp(block.timestamp + 52_000_000 weeks); // 1M-ish years + + (, uint224 gasPrice) = s_feeQuoter.getTokenAndGasPrices(s_sourceFeeToken, neverStaleChainSelector); + + assertEq(gasPrice, 999); + } + function test_ZeroGasPrice_Success() public { uint64 zeroGasDestChainSelector = 345678; + FeeQuoter.DestChainConfigArgs[] memory destChainConfigArgs = _generateFeeQuoterDestChainConfigArgs(); + destChainConfigArgs[0].destChainSelector = zeroGasDestChainSelector; + + s_feeQuoter.applyDestChainConfigUpdates(destChainConfigArgs); Internal.GasPriceUpdate[] memory gasPriceUpdates = new Internal.GasPriceUpdate[](1); gasPriceUpdates[0] = Internal.GasPriceUpdate({destChainSelector: zeroGasDestChainSelector, usdPerUnitGas: 0}); @@ -607,11 +632,11 @@ contract FeeQuoter_getTokenAndGasPrices is FeeQuoterSetup { (, uint224 gasPrice) = s_feeQuoter.getTokenAndGasPrices(s_sourceFeeToken, zeroGasDestChainSelector); - assertEq(gasPrice, priceUpdates.gasPriceUpdates[0].usdPerUnitGas); + assertEq(gasPrice, 0); } function test_UnsupportedChain_Revert() public { - vm.expectRevert(abi.encodeWithSelector(FeeQuoter.ChainNotSupported.selector, DEST_CHAIN_SELECTOR + 1)); + vm.expectRevert(abi.encodeWithSelector(FeeQuoter.DestinationChainNotEnabled.selector, DEST_CHAIN_SELECTOR + 1)); s_feeQuoter.getTokenAndGasPrices(s_sourceTokens[0], DEST_CHAIN_SELECTOR + 1); } diff --git a/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoterSetup.t.sol b/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoterSetup.t.sol index d630c55c0d..9d98fa7e77 100644 --- a/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoterSetup.t.sol +++ b/contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoterSetup.t.sol @@ -162,7 +162,7 @@ contract FeeQuoterSetup is TokenSetup { FeeQuoter.StaticConfig({ linkToken: s_sourceTokens[0], maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS, - stalenessThreshold: uint32(TWELVE_HOURS) + tokenPriceStalenessThreshold: uint32(TWELVE_HOURS) }), priceUpdaters, feeTokens, @@ -254,6 +254,7 @@ contract FeeQuoterSetup is TokenSetup { defaultTxGasLimit: GAS_LIMIT, gasMultiplierWeiPerEth: 5e17, networkFeeUSDCents: 1_00, + gasPriceStalenessThreshold: uint32(TWELVE_HOURS), enforceOutOfOrder: false, chainFamilySelector: Internal.CHAIN_FAMILY_SELECTOR_EVM }) diff --git a/core/gethwrappers/ccip/deployment_test/deployment_test.go b/core/gethwrappers/ccip/deployment_test/deployment_test.go index a9c9dc1dd4..698b30c206 100644 --- a/core/gethwrappers/ccip/deployment_test/deployment_test.go +++ b/core/gethwrappers/ccip/deployment_test/deployment_test.go @@ -73,9 +73,9 @@ func TestDeployAllV1_6(t *testing.T) { owner, chain, fee_quoter.FeeQuoterStaticConfig{ - MaxFeeJuelsPerMsg: big.NewInt(1e18), - LinkToken: common.HexToAddress("0x1"), - StalenessThreshold: 10, + MaxFeeJuelsPerMsg: big.NewInt(1e18), + LinkToken: common.HexToAddress("0x1"), + TokenPriceStalenessThreshold: 10, }, []common.Address{common.HexToAddress("0x1")}, []common.Address{common.HexToAddress("0x2")},