Skip to content

Commit

Permalink
PriceRegistry - remove token price staleness validations (#993)
Browse files Browse the repository at this point in the history 
## Motivation
The token price & gas price staleness validations in the PriceRegistry
serve as an update staleness validation. However, since this is already
captured by the gas price updates, the token price staleness checks can
be removed completely, especially since most tokens will be priced
through data feeds (where the staleness check should be skipped for
decoupling)

## Solution
* Removes the staleness threshold check for all token price flows,
keeping the check only on `getTokenAndGasPrices`
* Refactors PriceRegistry price updaters to use the `AuthorizedCallers`
shared contract
  • Loading branch information
@elatoskinas
elatoskinas authored Jun 25, 2024
1 parent df2225c commit ffc4a2f
Show file tree
Hide file tree
Showing 13 changed files with 748 additions and 421 deletions.
1 change: 0 additions & 1 deletion contracts/.husky/pre-push
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ if ! [[ -n $changes_root ]]; then
exit 0
fi

./contracts/.husky/verify-changeset.sh
cd contracts

FOUNDRY_PROFILE=$FOUNDRY_PROFILE forge build
Expand Down
218 changes: 106 additions & 112 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file

Large diffs are not rendered by default.

88 changes: 24 additions & 64 deletions contracts/src/v0.8/ccip/PriceRegistry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity 0.8.24;
import {ITypeAndVersion} from "../shared/interfaces/ITypeAndVersion.sol";
import {IPriceRegistry} from "./interfaces/IPriceRegistry.sol";

import {OwnerIsCreator} from "./../shared/access/OwnerIsCreator.sol";
import {AuthorizedCallers} from "../shared/access/AuthorizedCallers.sol";
import {AggregatorV3Interface} from "./../shared/interfaces/AggregatorV3Interface.sol";
import {Internal} from "./libraries/Internal.sol";
import {USDPriceWith18Decimals} from "./libraries/USDPriceWith18Decimals.sol";
Expand All @@ -13,7 +13,8 @@ import {EnumerableSet} from "../vendor/openzeppelin-solidity/v4.8.3/contracts/ut

/// @notice The PriceRegistry contract responsibility is to store the current gas price in USD for a given destination chain,
/// and the price of a token in USD allowing the owner or priceUpdater to update this value.
contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
/// The authorized callers in the contract represent the fee price updaters.
contract PriceRegistry is AuthorizedCallers, IPriceRegistry, ITypeAndVersion {
using EnumerableSet for EnumerableSet.AddressSet;
using USDPriceWith18Decimals for uint224;

Expand All @@ -25,9 +26,7 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {

error TokenNotSupported(address token);
error ChainNotSupported(uint64 chain);
error OnlyCallableByUpdaterOrOwner();
error StaleGasPrice(uint64 destChainSelector, uint256 threshold, uint256 timePassed);
error StaleTokenPrice(address token, uint256 threshold, uint256 timePassed);
error InvalidStalenessThreshold();
error DataFeedValueOutOfUint224Range();

Expand Down Expand Up @@ -66,16 +65,15 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
EnumerableSet.AddressSet private s_priceUpdaters;
// Subset of tokens which prices tracked by this registry which are fee tokens.
EnumerableSet.AddressSet private s_feeTokens;
// The amount of time a price can be stale before it is considered invalid.
// The amount of time a gas price can be stale before it is considered invalid.
uint32 private immutable i_stalenessThreshold;

constructor(
address[] memory priceUpdaters,
address[] memory feeTokens,
uint32 stalenessThreshold,
TokenPriceFeedUpdate[] memory tokenPriceFeeds
) {
_applyPriceUpdatersUpdates(priceUpdaters, new address[](0));
) AuthorizedCallers(priceUpdaters) {
_applyFeeTokensUpdates(feeTokens, new address[](0));
_updateTokenPriceFeeds(tokenPriceFeeds);
if (stalenessThreshold == 0) revert InvalidStalenessThreshold();
Expand All @@ -86,23 +84,22 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
// │ Price calculations │
// ================================================================

// @inheritdoc IPriceRegistry
/// @inheritdoc IPriceRegistry
function getTokenPrice(address token) public view override returns (Internal.TimestampedPackedUint224 memory) {
IPriceRegistry.TokenPriceFeedConfig memory priceFeedConfig = s_usdPriceFeedsPerToken[token];
if (priceFeedConfig.dataFeedAddress == address(0)) {
return s_usdPerToken[token];
}

(uint224 price, uint32 timestamp) = _getTokenPriceFromDataFeed(priceFeedConfig);
return Internal.TimestampedPackedUint224({value: price, timestamp: timestamp});
return _getTokenPriceFromDataFeed(priceFeedConfig);
}

// @inheritdoc IPriceRegistry
/// @inheritdoc IPriceRegistry
function getValidatedTokenPrice(address token) external view override returns (uint224) {
return _getValidatedTokenPrice(token);
}

// @inheritdoc IPriceRegistry
/// @inheritdoc IPriceRegistry
function getTokenPrices(address[] calldata tokens)
external
view
Expand All @@ -117,7 +114,7 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
return tokenPrices;
}

// @inheritdoc IPriceRegistry
/// @inheritdoc IPriceRegistry
function getTokenPriceFeedConfig(address token)
external
view
Expand All @@ -133,7 +130,7 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
return i_stalenessThreshold;
}

// @inheritdoc IPriceRegistry
/// @inheritdoc IPriceRegistry
function getDestinationChainGasPrice(uint64 destChainSelector)
external
view
Expand All @@ -143,6 +140,7 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
return s_usdPerUnitGasByDestChainSelector[destChainSelector];
}

/// @inheritdoc IPriceRegistry
function getTokenAndGasPrices(
address token,
uint64 destChainSelector
Expand Down Expand Up @@ -173,26 +171,23 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
return (fromTokenAmount * _getValidatedTokenPrice(fromToken)) / _getValidatedTokenPrice(toToken);
}

/// @notice Gets the token price for a given token and revert if the token is either
/// not supported or the price is stale.
/// @notice Gets the token price for a given token and revert if the token is not supported
/// @param token The address of the token to get the price for
/// @return the token price
function _getValidatedTokenPrice(address token) internal view returns (uint224) {
Internal.TimestampedPackedUint224 memory tokenPrice = getTokenPrice(token);
// Token price must be set at least once
if (tokenPrice.timestamp == 0 || tokenPrice.value == 0) revert TokenNotSupported(token);
uint256 timePassed = block.timestamp - tokenPrice.timestamp;
if (timePassed > i_stalenessThreshold) revert StaleTokenPrice(token, i_stalenessThreshold, timePassed);
return tokenPrice.value;
}

/// @notice Gets the token price from a data feed address, rebased to the same units as s_usdPerToken
/// @param priceFeedConfig token data feed configuration with valid data feed address (used to retrieve price & timestamp)
/// @return value data feed answer value (rebased to s_usdPerToken units)
/// @return timestamp data feed last updated timestamp
/// @return tokenPrice data feed price answer rebased to s_usdPerToken units, with latest block timestamp
function _getTokenPriceFromDataFeed(IPriceRegistry.TokenPriceFeedConfig memory priceFeedConfig)
internal
view
returns (uint224 value, uint32 timestamp)
returns (Internal.TimestampedPackedUint224 memory tokenPrice)
{
AggregatorV3Interface dataFeedContract = AggregatorV3Interface(priceFeedConfig.dataFeedAddress);
(
Expand Down Expand Up @@ -230,7 +225,9 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
if (rebasedValue > type(uint224).max) {
revert DataFeedValueOutOfUint224Range();
}
return (uint224(rebasedValue), uint32(block.timestamp));

// Data feed staleness is unchecked to decouple the PriceRegistry from data feed delay issues
return Internal.TimestampedPackedUint224({value: uint224(rebasedValue), timestamp: uint32(block.timestamp)});
}

// ================================================================
Expand Down Expand Up @@ -274,7 +271,7 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
// │ Price updates │
// ================================================================

// @inheritdoc IPriceRegistry
/// @inheritdoc IPriceRegistry
function updatePrices(Internal.PriceUpdates calldata priceUpdates) external override requireUpdaterOrOwner {
uint256 tokenUpdatesLength = priceUpdates.tokenPriceUpdates.length;

Expand Down Expand Up @@ -317,49 +314,12 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
// ================================================================
// │ Access │
// ================================================================

/// @notice Get the list of price updaters.
/// @return The price updaters.
function getPriceUpdaters() external view returns (address[] memory) {
return s_priceUpdaters.values();
}

/// @notice Adds new priceUpdaters and remove existing ones.
/// @param priceUpdatersToAdd The addresses of the priceUpdaters that are now allowed
/// to send fee updates.
/// @param priceUpdatersToRemove The addresses of the priceUpdaters that are no longer allowed
/// to send fee updates.
function applyPriceUpdatersUpdates(
address[] memory priceUpdatersToAdd,
address[] memory priceUpdatersToRemove
) external onlyOwner {
_applyPriceUpdatersUpdates(priceUpdatersToAdd, priceUpdatersToRemove);
}

/// @notice Adds new priceUpdaters and remove existing ones.
/// @param priceUpdatersToAdd The addresses of the priceUpdaters that are now allowed
/// to send fee updates.
/// @param priceUpdatersToRemove The addresses of the priceUpdaters that are no longer allowed
/// to send fee updates.
function _applyPriceUpdatersUpdates(
address[] memory priceUpdatersToAdd,
address[] memory priceUpdatersToRemove
) private {
for (uint256 i = 0; i < priceUpdatersToAdd.length; ++i) {
if (s_priceUpdaters.add(priceUpdatersToAdd[i])) {
emit PriceUpdaterSet(priceUpdatersToAdd[i]);
}
}
for (uint256 i = 0; i < priceUpdatersToRemove.length; ++i) {
if (s_priceUpdaters.remove(priceUpdatersToRemove[i])) {
emit PriceUpdaterRemoved(priceUpdatersToRemove[i]);
}
}
}

/// @notice Require that the caller is the owner or a fee updater.
modifier requireUpdaterOrOwner() {
if (msg.sender != owner() && !s_priceUpdaters.contains(msg.sender)) revert OnlyCallableByUpdaterOrOwner();
if (msg.sender != owner()) {
// if not owner - check if the caller is a fee updater
_validateCaller();
}
_;
}
}
5 changes: 4 additions & 1 deletion contracts/src/v0.8/ccip/test/commitStore/CommitStore.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity 0.8.24;
import {IPriceRegistry} from "../../interfaces/IPriceRegistry.sol";
import {IRMN} from "../../interfaces/IRMN.sol";

import {AuthorizedCallers} from "../../../shared/access/AuthorizedCallers.sol";
import {CommitStore} from "../../CommitStore.sol";
import {PriceRegistry} from "../../PriceRegistry.sol";
import {RMN} from "../../RMN.sol";
Expand Down Expand Up @@ -36,7 +37,9 @@ contract CommitStoreSetup is PriceRegistrySetup, OCR2BaseSetup {

address[] memory priceUpdaters = new address[](1);
priceUpdaters[0] = address(s_commitStore);
s_priceRegistry.applyPriceUpdatersUpdates(priceUpdaters, new address[](0));
s_priceRegistry.applyAuthorizedCallerUpdates(
AuthorizedCallers.AuthorizedCallerArgs({addedCallers: priceUpdaters, removedCallers: new address[](0)})
);
}
}

Expand Down
9 changes: 7 additions & 2 deletions contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRampSetup.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {IAny2EVMOffRamp} from "../../interfaces/IAny2EVMOffRamp.sol";
import {ICommitStore} from "../../interfaces/ICommitStore.sol";
import {IRMN} from "../../interfaces/IRMN.sol";

import {AuthorizedCallers} from "../../../shared/access/AuthorizedCallers.sol";
import {RMN} from "../../RMN.sol";
import {Router} from "../../Router.sol";
import {Client} from "../../libraries/Client.sol";
Expand Down Expand Up @@ -113,7 +114,9 @@ contract EVM2EVMMultiOffRampSetup is TokenSetup, PriceRegistrySetup, MultiOCR3Ba

address[] memory priceUpdaters = new address[](1);
priceUpdaters[0] = address(s_offRamp);
s_priceRegistry.applyPriceUpdatersUpdates(priceUpdaters, new address[](0));
s_priceRegistry.applyAuthorizedCallerUpdates(
AuthorizedCallers.AuthorizedCallerArgs({addedCallers: priceUpdaters, removedCallers: new address[](0)})
);
}

// TODO: function can be made common across OffRampSetup and MultiOffRampSetup
Expand Down Expand Up @@ -463,7 +466,9 @@ contract EVM2EVMMultiOffRampSetup is TokenSetup, PriceRegistrySetup, MultiOCR3Ba

address[] memory priceUpdaters = new address[](1);
priceUpdaters[0] = address(s_offRamp);
s_priceRegistry.applyPriceUpdatersUpdates(priceUpdaters, new address[](0));
s_priceRegistry.applyAuthorizedCallerUpdates(
AuthorizedCallers.AuthorizedCallerArgs({addedCallers: priceUpdaters, removedCallers: new address[](0)})
);
}

function _setupRealRMN() internal {
Expand Down
18 changes: 0 additions & 18 deletions contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1474,24 +1474,6 @@ contract EVM2EVMMultiOnRamp_getTokenTransferCost is EVM2EVMMultiOnRamp_getFeeSet

s_onRamp.getTokenTransferCost(DEST_CHAIN_SELECTOR, message.feeToken, s_feeTokenPrice, message.tokenAmounts);
}

function test_ValidatedPriceStaleness_Revert() public {
vm.warp(block.timestamp + TWELVE_HOURS + 1);

Client.EVM2AnyMessage memory message = _generateSingleTokenMessage(s_sourceFeeToken, 1e36);
message.tokenAmounts[0].token = s_sourceRouter.getWrappedNative();

vm.expectRevert(
abi.encodeWithSelector(
PriceRegistry.StaleTokenPrice.selector,
s_sourceRouter.getWrappedNative(),
uint128(TWELVE_HOURS),
uint128(TWELVE_HOURS + 1)
)
);

s_onRamp.getTokenTransferCost(DEST_CHAIN_SELECTOR, message.feeToken, s_feeTokenPrice, message.tokenAmounts);
}
}

contract EVM2EVMMultiOnRamp_setDynamicConfig is EVM2EVMMultiOnRampSetup {
Expand Down
18 changes: 0 additions & 18 deletions contracts/src/v0.8/ccip/test/onRamp/EVM2EVMOnRamp.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1337,24 +1337,6 @@ contract EVM2EVMOnRamp_getTokenTransferCost is EVM2EVMOnRamp_getFeeSetup {

s_onRamp.getTokenTransferCost(message.feeToken, s_feeTokenPrice, message.tokenAmounts);
}

function test_ValidatedPriceStaleness_Revert() public {
vm.warp(block.timestamp + TWELVE_HOURS + 1);

Client.EVM2AnyMessage memory message = _generateSingleTokenMessage(s_sourceFeeToken, 1e36);
message.tokenAmounts[0].token = s_sourceRouter.getWrappedNative();

vm.expectRevert(
abi.encodeWithSelector(
PriceRegistry.StaleTokenPrice.selector,
s_sourceRouter.getWrappedNative(),
uint128(TWELVE_HOURS),
uint128(TWELVE_HOURS + 1)
)
);

s_onRamp.getTokenTransferCost(message.feeToken, s_feeTokenPrice, message.tokenAmounts);
}
}

contract EVM2EVMOnRamp_getFee is EVM2EVMOnRamp_getFeeSetup {
Expand Down
Loading

0 comments on commit ffc4a2f

Please sign in to comment.