Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Charge l1 data availability fee #86

Merged
merged 24 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
279 changes: 142 additions & 137 deletions contracts/gas-snapshots/ccip.gas-snapshot

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions contracts/src/v0.8/ccip/AggregateRateLimiter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {USDPriceWith18Decimals} from "./libraries/USDPriceWith18Decimals.sol";
/// token transfers, using a price registry to convert to a numeraire asset (e.g. USD).
contract AggregateRateLimiter is OwnerIsCreator {
using RateLimiter for RateLimiter.TokenBucket;
using USDPriceWith18Decimals for uint192;
using USDPriceWith18Decimals for uint224;

error PriceNotFoundForToken(address token);
event AdminSet(address newAdmin);
Expand Down Expand Up @@ -45,7 +45,7 @@ contract AggregateRateLimiter is OwnerIsCreator {
for (uint256 i = 0; i < numberOfTokens; ++i) {
// not fetching validated price, as price staleness is not important for value-based rate limiting
// we only need to verify price is not 0
uint192 pricePerToken = priceRegistry.getTokenPrice(tokenAmounts[i].token).value;
uint224 pricePerToken = priceRegistry.getTokenPrice(tokenAmounts[i].token).value;
if (pricePerToken == 0) revert PriceNotFoundForToken(tokenAmounts[i].token);
value += pricePerToken._calcUSDValueFromTokenAmount(tokenAmounts[i].amount);
}
Expand Down
32 changes: 16 additions & 16 deletions contracts/src/v0.8/ccip/PriceRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {EnumerableSet} from "../vendor/openzeppelin-solidity/v4.8.0/contracts/ut
/// and the price of a token in USD allowing the owner or priceUpdater to update this value.
contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
using EnumerableSet for EnumerableSet.AddressSet;
using USDPriceWith18Decimals for uint192;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great opportunity to add a type and version to the price reg

using USDPriceWith18Decimals for uint224;

error TokenNotSupported(address token);
error ChainNotSupported(uint64 chain);
Expand All @@ -34,15 +34,15 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
/// Very Expensive: 1 unit of gas costs 1 USD -> 1e18
/// Expensive: 1 unit of gas costs 0.1 USD -> 1e17
/// Cheap: 1 unit of gas costs 0.000001 USD -> 1e12
mapping(uint64 destChainSelector => Internal.TimestampedUint192Value price)
mapping(uint64 destChainSelector => Internal.TimestampedPackedUint224 price)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments need updating

private s_usdPerUnitGasByDestChainSelector;

/// @dev The price, in USD with 18 decimals, per 1e18 of the smallest token denomination.
/// @dev Price of 1e18 represents 1 USD per 1e18 token amount.
/// 1 USDC = 1.00 USD per full token, each full token is 1e6 units -> 1 * 1e18 * 1e18 / 1e6 = 1e30
/// 1 ETH = 2,000 USD per full token, each full token is 1e18 units -> 2000 * 1e18 * 1e18 / 1e18 = 2_000e18
/// 1 LINK = 5.00 USD per full token, each full token is 1e18 units -> 5 * 1e18 * 1e18 / 1e18 = 5e18
mapping(address token => Internal.TimestampedUint192Value price) private s_usdPerToken;
mapping(address token => Internal.TimestampedPackedUint224 price) private s_usdPerToken;

// Price updaters are allowed to update the prices.
EnumerableSet.AddressSet private s_priceUpdaters;
Expand All @@ -63,21 +63,21 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
// ================================================================

// @inheritdoc IPriceRegistry
function getTokenPrice(address token) public view override returns (Internal.TimestampedUint192Value memory) {
function getTokenPrice(address token) public view override returns (Internal.TimestampedPackedUint224 memory) {
return s_usdPerToken[token];
}

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

// @inheritdoc IPriceRegistry
function getTokenPrices(
address[] calldata tokens
) external view override returns (Internal.TimestampedUint192Value[] memory) {
) external view override returns (Internal.TimestampedPackedUint224[] memory) {
uint256 length = tokens.length;
Internal.TimestampedUint192Value[] memory tokenPrices = new Internal.TimestampedUint192Value[](length);
Internal.TimestampedPackedUint224[] memory tokenPrices = new Internal.TimestampedPackedUint224[](length);
for (uint256 i = 0; i < length; ++i) {
tokenPrices[i] = getTokenPrice(tokens[i]);
}
Expand All @@ -93,15 +93,15 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
// @inheritdoc IPriceRegistry
function getDestinationChainGasPrice(
uint64 destChainSelector
) external view override returns (Internal.TimestampedUint192Value memory) {
) external view override returns (Internal.TimestampedPackedUint224 memory) {
return s_usdPerUnitGasByDestChainSelector[destChainSelector];
}

function getTokenAndGasPrices(
address token,
uint64 destChainSelector
) external view override returns (uint192 tokenPrice, uint192 gasPriceValue) {
Internal.TimestampedUint192Value memory gasPrice = s_usdPerUnitGasByDestChainSelector[destChainSelector];
) external view override returns (uint224 tokenPrice, uint224 gasPriceValue) {
Internal.TimestampedPackedUint224 memory gasPrice = s_usdPerUnitGasByDestChainSelector[destChainSelector];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thoughts on an internal _getValidatedExecutionPrices analogous to the _getValidatedTokenPrice? Yields some nice symmetry validatedXX means timestamp checked for both gas and tokens

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will include in a follow up PR that includes _getValidatedGasPrices and getValidatedExecutionPrices

// 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;
Expand Down Expand Up @@ -131,8 +131,8 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
/// not supported or the price is stale.
/// @param token The address of the token to get the price for
/// @return the token price
function _getValidatedTokenPrice(address token) internal view returns (uint192) {
Internal.TimestampedUint192Value memory tokenPrice = s_usdPerToken[token];
function _getValidatedTokenPrice(address token) internal view returns (uint224) {
Internal.TimestampedPackedUint224 memory tokenPrice = s_usdPerToken[token];
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);
Expand Down Expand Up @@ -187,17 +187,17 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator {

for (uint256 i = 0; i < priceUpdatesLength; ++i) {
Internal.TokenPriceUpdate memory update = priceUpdates.tokenPriceUpdates[i];
s_usdPerToken[update.sourceToken] = Internal.TimestampedUint192Value({
s_usdPerToken[update.sourceToken] = Internal.TimestampedPackedUint224({
value: update.usdPerToken,
timestamp: uint64(block.timestamp)
timestamp: uint32(block.timestamp)
});
emit UsdPerTokenUpdated(update.sourceToken, update.usdPerToken, block.timestamp);
}

if (priceUpdates.destChainSelector != 0) {
s_usdPerUnitGasByDestChainSelector[priceUpdates.destChainSelector] = Internal.TimestampedUint192Value({
s_usdPerUnitGasByDestChainSelector[priceUpdates.destChainSelector] = Internal.TimestampedPackedUint224({
value: priceUpdates.usdPerUnitGas,
timestamp: uint64(block.timestamp)
timestamp: uint32(block.timestamp)
});
emit UsdPerUnitGasUpdated(priceUpdates.destChainSelector, priceUpdates.usdPerUnitGas, block.timestamp);
}
Expand Down
10 changes: 5 additions & 5 deletions contracts/src/v0.8/ccip/interfaces/IPriceRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,24 @@ interface IPriceRegistry {
/// @notice Get the `tokenPrice` for a given token.
/// @param token The token to get the price for.
/// @return tokenPrice The tokenPrice for the given token.
function getTokenPrice(address token) external view returns (Internal.TimestampedUint192Value memory);
function getTokenPrice(address token) external view returns (Internal.TimestampedPackedUint224 memory);

/// @notice Get the `tokenPrice` for a given token, checks if the price is valid.
/// @param token The token to get the price for.
/// @return tokenPrice The tokenPrice for the given token if it exists and is valid.
function getValidatedTokenPrice(address token) external view returns (uint192);
function getValidatedTokenPrice(address token) external view returns (uint224);

/// @notice Get the `tokenPrice` for an array of tokens.
/// @param tokens The tokens to get prices for.
/// @return tokenPrices The tokenPrices for the given tokens.
function getTokenPrices(address[] calldata tokens) external view returns (Internal.TimestampedUint192Value[] memory);
function getTokenPrices(address[] calldata tokens) external view returns (Internal.TimestampedPackedUint224[] memory);

/// @notice Get the `gasPrice` for a given destination chain ID.
/// @param destChainSelector The destination chain to get the price for.
/// @return gasPrice The gasPrice for the given destination chain ID.
function getDestinationChainGasPrice(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably update the comment here? it now returns a multi-dimensional gas price

uint64 destChainSelector
) external view returns (Internal.TimestampedUint192Value memory);
) external view returns (Internal.TimestampedPackedUint224 memory);

/// @notice Gets the fee token price and the gas price, both denominated in dollars.
/// @param token The source token to get the price for.
Expand All @@ -38,7 +38,7 @@ interface IPriceRegistry {
function getTokenAndGasPrices(
address token,
uint64 destChainSelector
) external view returns (uint192 tokenPrice, uint192 gasPrice);
) external view returns (uint224 tokenPrice, uint224 gasPrice);

/// @notice Convert a given token amount to target token amount.
/// @param fromToken The given token address.
Expand Down
25 changes: 20 additions & 5 deletions contracts/src/v0.8/ccip/libraries/Internal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,24 @@ library Internal {
struct PriceUpdates {
TokenPriceUpdate[] tokenPriceUpdates;
uint64 destChainSelector; // --┐ Destination chain selector
uint192 usdPerUnitGas; // -----┘ 1e18 USD per smallest unit (e.g. wei) of destination chain gas
uint224 usdPerUnitGas; // -----┘ 1e18 USD per smallest unit (e.g. wei) of destination chain gas
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we're changing this interface, we should open the door for batched gas price updates i.e. have a GasPriceUpdate[] where GasPriceUpdate{DestChainSelector, GasPrice}. Can be a follow up if painful to add now

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as it is in 1.2 and doesn't bleed into 1.3 I don't mind doing it in follow up

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll post it in a followup PR, this is PR is already quite big.

}

struct TokenPriceUpdate {
address sourceToken; // Source token
uint192 usdPerToken; // 1e18 USD per smallest unit of token
uint224 usdPerToken; // 1e18 USD per smallest unit of token
}

struct TimestampedUint192Value {
uint192 value; // -------┐ The price, in 1e18 USD.
uint64 timestamp; // ----┘ Timestamp of the most recent price update.
struct TimestampedPackedUint224 {
uint224 value; // -------┐ Value in uint224, packed.
uint32 timestamp; // ----┘ Timestamp of the most recent update.
}

/// @dev Gas price is stored in 112-bit unsigned int. uint224 can pack 2 prices.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we abstract this from price reg clients all together? Since the price reg API is breaking anyways, something like getValidatedExecutionPrices(destinationSelector)->(gasPrice, daPrice) where the 2 return values are already parsed for the client. Also that way I think we don't even need the multiplier check (if daPrice == 0, then DA is free/covered by gasPrice)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spoke offline - I'm cool with opaque gas price and ramps interpreting

/// When packing L1 and L2 gas prices, L1 gas price is left-shifted to the higher-order bits.
/// Using uint8, which is strictly lower than 1st shift operand, to avoid shift operand type warning.
uint8 public constant GAS_PRICE_BITS = 112;

struct PoolUpdate {
address token; // The IERC20 token address
address pool; // The token pool address
Expand Down Expand Up @@ -53,6 +58,16 @@ library Internal {
bytes32 messageId;
}

/// @dev EVM2EVMMessage struct has 12 fields, including 2 variable arrays.
/// Each variable array takes 1 more slot to store its length.
/// Hence, when abiEncoded, excluding the dynamic contents of variable arrays,
/// EVM2EVMMessage takes up a fixed number of 14 lots, 32 bytes each.
uint256 public constant MESSAGE_FIXED_BYTES = 32 * 14;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values are only used in the onramp and should live in the onramp

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I'm ok with it here as its a tightly coupled property of EVM2EVMMessage

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are essentially attributes of EVM2EVMMessage, think it makes sense to put it close to the definition of the struct, also less likely to forget to update


/// @dev Each token transfer adds 1 instance of EVMTokenAmount struct to EVM2EVMMessage.
/// EVMTokenAmount struct contains 2 fields, 32 bytes each when abiEncoded.
uint256 public constant MESSAGE_BYTES_PER_TOKEN = 32 * 2;

function _toAny2EVMMessage(
EVM2EVMMessage memory original,
Client.EVMTokenAmount[] memory destTokenAmounts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ library USDPriceWith18Decimals {
/// @dev this function assumes that no more than 1e59 US dollar worth of token is passed in.
/// If more is sent, this function will overflow and revert.
/// Since there isn't even close to 1e59 dollars, this is ok for all legit tokens.
function _calcUSDValueFromTokenAmount(uint192 tokenPrice, uint256 tokenAmount) internal pure returns (uint256) {
function _calcUSDValueFromTokenAmount(uint224 tokenPrice, uint256 tokenAmount) internal pure returns (uint256) {
/// LINK Example:
/// tokenPrice: 8e18 -> $8/LINK, as 1e18 token amount is 1 LINK, worth 8 USD, or 8e18 with 18 decimals
/// tokenAmount: 2e18 -> 2 LINK
Expand All @@ -30,7 +30,7 @@ library USDPriceWith18Decimals {
/// @param tokenPrice The USD price of the token.
/// @param usdValue USD value with 18 decimals.
/// @return Amount of the smallest token denomination.
function _calcTokenAmountFromUSDValue(uint192 tokenPrice, uint256 usdValue) internal pure returns (uint256) {
function _calcTokenAmountFromUSDValue(uint224 tokenPrice, uint256 usdValue) internal pure returns (uint256) {
/// LINK Example:
/// tokenPrice: 8e18 -> $8/LINK, as 1e18 token amount is 1 LINK, worth 8 USD, or 8e18 with 18 decimals
/// usdValue: 16e18 -> $16
Expand Down
Loading