Skip to content

Commit

Permalink
Changes from review
Browse files Browse the repository at this point in the history
  • Loading branch information
justinkaseman committed Apr 11, 2024
1 parent c9de21c commit 970d31d
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 43 deletions.
18 changes: 18 additions & 0 deletions contracts/src/v0.8/ccip/AggregateRateLimiter.sol
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.19;

import {IPriceRegistry} from "./interfaces/IPriceRegistry.sol";

import {OwnerIsCreator} from "./../shared/access/OwnerIsCreator.sol";
import {Client} from "./libraries/Client.sol";
import {RateLimiter} from "./libraries/RateLimiter.sol";
import {USDPriceWith18Decimals} from "./libraries/USDPriceWith18Decimals.sol";

/// @notice The aggregate rate limiter is a wrapper of the token bucket rate limiter
/// which permits rate limiting based on the aggregate value of a group of
/// 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 uint224;

error PriceNotFoundForToken(address token);

event AdminSet(address newAdmin);

Expand All @@ -35,6 +42,17 @@ contract AggregateRateLimiter is OwnerIsCreator {
s_rateLimiter._consume(value, address(0));
}

function _getTokenValue(
Client.EVMTokenAmount memory tokenAmount,
IPriceRegistry priceRegistry
) internal view returns (uint256) {
// not fetching validated price, as price staleness is not important for value-based rate limiting
// we only need to verify the price is not 0
uint224 pricePerToken = priceRegistry.getTokenPrice(tokenAmount.token).value;
if (pricePerToken == 0) revert PriceNotFoundForToken(tokenAmount.token);
return pricePerToken._calcUSDValueFromTokenAmount(tokenAmount.amount);
}

/// @notice Gets the token bucket with its values for the block it was requested at.
/// @return The token bucket.
function currentRateLimiterState() external view returns (RateLimiter.TokenBucket memory) {
Expand Down
43 changes: 13 additions & 30 deletions contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {IRouter} from "../interfaces/IRouter.sol";
import {IPool} from "../interfaces/pools/IPool.sol";

import {CallWithExactGas} from "../../shared/call/CallWithExactGas.sol";
import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableSet.sol";
import {AggregateRateLimiter} from "../AggregateRateLimiter.sol";
import {Client} from "../libraries/Client.sol";
import {Internal} from "../libraries/Internal.sol";
Expand All @@ -20,6 +19,7 @@ import {USDPriceWith18Decimals} from "../libraries/USDPriceWith18Decimals.sol";
import {OCR2BaseNoChecks} from "../ocr/OCR2BaseNoChecks.sol";

import {ERC165Checker} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/introspection/ERC165Checker.sol";
import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableSet.sol";

/// @notice EVM2EVMOffRamp enables OCR networks to execute multiple messages
/// in an OffRamp in a single transaction.
Expand Down Expand Up @@ -55,7 +55,6 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
error InvalidMessageId();
error InvalidAddress(bytes encodedAddress);
error InvalidNewState(uint64 sequenceNumber, Internal.MessageExecutionState newState);
error PriceNotFoundForToken(address token);

/// @dev Atlas depends on this event, if changing, please notify Atlas.
event ConfigSet(StaticConfig staticConfig, DynamicConfig dynamicConfig);
Expand Down Expand Up @@ -146,7 +145,10 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio

i_metadataHash = _metadataHash(Internal.EVM_2_EVM_MESSAGE_HASH);

_addRateLimitedTokens(rateLimitedTokens);
for (uint256 i = 0; i < rateLimitedTokens.length; ++i) {
bool success = s_rateLimitedTokens.add(rateLimitedTokens[i]);
if (success) emit TokenAggregateRateLimitAdded(rateLimitedTokens[i]);
}
}

// ================================================================
Expand Down Expand Up @@ -489,29 +491,21 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio

/// @notice Get all tokens which are included in Aggregate Rate Limiting.
/// @return rateLimitedTokens Array of Aggregate Rate Limited tokens.
function getAllRateLimitedTokens() external view returns (address[] memory) {
function getAllRateLimitTokens() external view returns (address[] memory) {
return s_rateLimitedTokens.values();
}

/// @notice Adds tokens from being used in Aggregate Rate Limiting.
/// @notice Adds or removes tokens from being used in Aggregate Rate Limiting.
/// @param adds A list of one or more tokens to be added.
function addRateLimitedTokens(address[] memory adds) external onlyOwner {
_addRateLimitedTokens(adds);
}

function _addRateLimitedTokens(address[] memory adds) internal {
/// @param removes A list of one or more tokens to be removed.
function updateRateLimitTokens(address[] memory removes, address[] memory adds) external onlyOwner {
for (uint256 i = 0; i < adds.length; ++i) {
s_rateLimitedTokens.add(adds[i]);
emit TokenAggregateRateLimitAdded(adds[i]);
bool success = s_rateLimitedTokens.add(adds[i]);
if (success) emit TokenAggregateRateLimitAdded(adds[i]);
}
}

/// @notice Removes tokens from being used in Aggregate Rate Limiting.
/// @param removes A list of one or more tokens to be removed.
function removeRateLimitedTokens(address[] memory removes) external onlyOwner {
for (uint256 i = 0; i < removes.length; ++i) {
s_rateLimitedTokens.remove(removes[i]);
emit TokenAggregateRateLimitRemoved(removes[i]);
bool success = s_rateLimitedTokens.remove(removes[i]);
if (success) emit TokenAggregateRateLimitRemoved(removes[i]);
}
}

Expand Down Expand Up @@ -591,17 +585,6 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
return address(uint160(decodedAddress));
}

function _getTokenValue(
Client.EVMTokenAmount memory tokenAmount,
IPriceRegistry priceRegistry
) internal view returns (uint256) {
// not fetching validated price, as price staleness is not important for value-based rate limiting
// we only need to verify the price is not 0
uint224 pricePerToken = priceRegistry.getTokenPrice(tokenAmount.token).value;
if (pricePerToken == 0) revert PriceNotFoundForToken(tokenAmount.token);
return pricePerToken._calcUSDValueFromTokenAmount(tokenAmount.amount);
}

// ================================================================
// │ Access and ARM │
// ================================================================
Expand Down
12 changes: 0 additions & 12 deletions contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
error CannotSendZeroTokens();
error SourceTokenDataTooLarge(address token);
error InvalidChainSelector(uint64 chainSelector);
error PriceNotFoundForToken(address token);

event ConfigSet(StaticConfig staticConfig, DynamicConfig dynamicConfig);
event NopPaid(address indexed nop, uint256 amount);
Expand Down Expand Up @@ -459,17 +458,6 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
return ITokenAdminRegistry(s_dynamicConfig.tokenAdminRegistry).getAllConfiguredTokens();
}

function _getTokenValue(
Client.EVMTokenAmount memory tokenAmount,
IPriceRegistry priceRegistry
) internal view returns (uint256) {
// not fetching validated price, as price staleness is not important for value-based rate limiting
// we only need to verify the price is not 0
uint224 pricePerToken = priceRegistry.getTokenPrice(tokenAmount.token).value;
if (pricePerToken == 0) revert PriceNotFoundForToken(tokenAmount.token);
return pricePerToken._calcUSDValueFromTokenAmount(tokenAmount.amount);
}

// ================================================================
// │ Fees │
// ================================================================
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/ccip/test/onRamp/EVM2EVMOnRamp.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ contract EVM2EVMOnRamp_forwardFromRouter is EVM2EVMOnRampSetup {
message.tokenAmounts[0].token = CUSTOM_TOKEN;
message.tokenAmounts[0].amount = 1;

vm.expectRevert(abi.encodeWithSelector(EVM2EVMOnRamp.PriceNotFoundForToken.selector, CUSTOM_TOKEN));
vm.expectRevert(abi.encodeWithSelector(AggregateRateLimiter.PriceNotFoundForToken.selector, CUSTOM_TOKEN));

s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, 0, OWNER);
}
Expand Down

0 comments on commit 970d31d

Please sign in to comment.