Skip to content

Commit

Permalink
⛳ Golf & comments, move tokenAdminRegistry to static config (#1009)
Browse files Browse the repository at this point in the history
## Motivation
Some minor golfing and adding comments

Main change is moving the token admin registry to the static config in
the onRamp
  • Loading branch information
RensR authored Jun 14, 2024
1 parent 574f164 commit 54ee4f1
Show file tree
Hide file tree
Showing 50 changed files with 433 additions and 515 deletions.
374 changes: 187 additions & 187 deletions contracts/gas-snapshots/ccip.gas-snapshot

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions contracts/gas-snapshots/liquiditymanager.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ LiquidityManager_addLiquidity:test_addLiquiditySuccess() (gas: 279154)
LiquidityManager_rebalanceLiquidity:test_InsufficientLiquidityReverts() (gas: 19585)
LiquidityManager_rebalanceLiquidity:test_InvalidRemoteChainReverts() (gas: 190304)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess() (gas: 8809026)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8757871)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8753079)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8679893)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8731220)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8726428)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8653242)
LiquidityManager_rebalanceLiquidity:test_rebalanceLiquiditySuccess() (gas: 379308)
LiquidityManager_removeLiquidity:test_InsufficientLiquidityReverts() (gas: 184481)
LiquidityManager_removeLiquidity:test_OnlyOwnerReverts() (gas: 10967)
Expand All @@ -16,7 +16,7 @@ LiquidityManager_setCrossChainRebalancer:test_ZeroChainSelectorReverts() (gas: 1
LiquidityManager_setCrossChainRebalancer:test_setCrossChainRebalancerSuccess() (gas: 162208)
LiquidityManager_setLocalLiquidityContainer:test_OnlyOwnerReverts() (gas: 11030)
LiquidityManager_setLocalLiquidityContainer:test_ReverstWhen_CalledWithTheZeroAddress() (gas: 10621)
LiquidityManager_setLocalLiquidityContainer:test_setLocalLiquidityContainerSuccess() (gas: 3464367)
LiquidityManager_setLocalLiquidityContainer:test_setLocalLiquidityContainerSuccess() (gas: 3437694)
LiquidityManager_setMinimumLiquidity:test_OnlyOwnerReverts() (gas: 10925)
LiquidityManager_setMinimumLiquidity:test_setMinimumLiquiditySuccess() (gas: 36434)
LiquidityManager_withdrawNative:test_OnlyOwnerReverts() (gas: 13115)
Expand Down
8 changes: 7 additions & 1 deletion contracts/src/v0.8/ccip/libraries/Internal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,17 @@ library Internal {
return _validateEVMAddressFromUint256(abi.decode(encodedAddress, (uint256)));
}

/// @dev We disallow the first 1024 addresses to never allow calling precompiles. It is extremely unlikely that
/// anyone would ever be able to generate an address in this range.
uint256 public constant PRECOMPILE_SPACE = 1024;

/// @notice This method provides a safe way to convert a uint256 to an address.
/// It will revert if the uint256 is not a valid EVM address, or a precompile address.
/// @return The address if it is valid, the function will revert otherwise.
function _validateEVMAddressFromUint256(uint256 encodedAddress) internal pure returns (address) {
if (encodedAddress > type(uint160).max || encodedAddress < 10) revert InvalidEVMAddress(abi.encode(encodedAddress));
if (encodedAddress > type(uint160).max || encodedAddress < PRECOMPILE_SPACE) {
revert InvalidEVMAddress(abi.encode(encodedAddress));
}
return address(uint160(encodedAddress));
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/src/v0.8/ccip/libraries/RateLimiter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ library RateLimiter {
error TokenRateLimitReached(uint256 minWaitInSeconds, uint256 available, address tokenAddress);
error AggregateValueMaxCapacityExceeded(uint256 capacity, uint256 requested);
error AggregateValueRateLimitReached(uint256 minWaitInSeconds, uint256 available);
error InvalidRatelimitRate(Config rateLimiterConfig);
error InvalidRateLimitRate(Config rateLimiterConfig);
error DisabledNonZeroRateLimit(Config config);
error RateLimitMustBeDisabled();

Expand Down Expand Up @@ -120,7 +120,7 @@ library RateLimiter {
function _validateTokenBucketConfig(Config memory config, bool mustBeDisabled) internal pure {
if (config.isEnabled) {
if (config.rate >= config.capacity || config.rate == 0) {
revert InvalidRatelimitRate(config);
revert InvalidRateLimitRate(config);
}
if (mustBeDisabled) {
revert RateLimitMustBeDisabled();
Expand Down
24 changes: 17 additions & 7 deletions contracts/src/v0.8/ccip/ocr/OCR2Base.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {OCR2Abstract} from "./OCR2Abstract.sol";
/// @dev For details on its operation, see the offchain reporting protocol design
/// doc, which refers to this contract as simply the "contract".
abstract contract OCR2Base is OwnerIsCreator, OCR2Abstract {
error InvalidConfig(string message);
error InvalidConfig(InvalidConfigErrorType errorType);
error WrongMessageLength(uint256 expected, uint256 actual);
error ConfigDigestMismatch(bytes32 expected, bytes32 actual);
error ForkedChain(uint256 expected, uint256 actual);
Expand All @@ -19,6 +19,14 @@ abstract contract OCR2Base is OwnerIsCreator, OCR2Abstract {
error NonUniqueSignatures();
error OracleCannotBeZeroAddress();

enum InvalidConfigErrorType {
F_MUST_BE_POSITIVE,
TOO_MANY_SIGNERS,
F_TOO_HIGH,
REPEATED_ORACLE_ADDRESS,
NUM_SIGNERS_NOT_NUM_TRANSMITTERS
}

// Packing these fields used on the hot path in a ConfigInfo variable reduces the
// retrieval of all of them to a minimum number of SLOADs.
struct ConfigInfo {
Expand Down Expand Up @@ -88,10 +96,10 @@ abstract contract OCR2Base is OwnerIsCreator, OCR2Abstract {

// Reverts transaction if config args are invalid
modifier checkConfigValid(uint256 numSigners, uint256 numTransmitters, uint256 f) {
if (numSigners > MAX_NUM_ORACLES) revert InvalidConfig("too many signers");
if (f == 0) revert InvalidConfig("f must be positive");
if (numSigners != numTransmitters) revert InvalidConfig("oracle addresses out of registration");
if (numSigners <= 3 * f) revert InvalidConfig("faulty-oracle f too high");
if (numSigners > MAX_NUM_ORACLES) revert InvalidConfig(InvalidConfigErrorType.TOO_MANY_SIGNERS);
if (f == 0) revert InvalidConfig(InvalidConfigErrorType.F_MUST_BE_POSITIVE);
if (numSigners != numTransmitters) revert InvalidConfig(InvalidConfigErrorType.NUM_SIGNERS_NOT_NUM_TRANSMITTERS);
if (numSigners <= 3 * f) revert InvalidConfig(InvalidConfigErrorType.F_TOO_HIGH);
_;
}

Expand Down Expand Up @@ -121,12 +129,14 @@ abstract contract OCR2Base is OwnerIsCreator, OCR2Abstract {
for (uint256 i = 0; i < newSignersLength; ++i) {
// add new signer/transmitter addresses
address signer = signers[i];
if (s_oracles[signer].role != Role.Unset) revert InvalidConfig("repeated signer address");
if (s_oracles[signer].role != Role.Unset) revert InvalidConfig(InvalidConfigErrorType.REPEATED_ORACLE_ADDRESS);
if (signer == address(0)) revert OracleCannotBeZeroAddress();
s_oracles[signer] = Oracle(uint8(i), Role.Signer);

address transmitter = transmitters[i];
if (s_oracles[transmitter].role != Role.Unset) revert InvalidConfig("repeated transmitter address");
if (s_oracles[transmitter].role != Role.Unset) {
revert InvalidConfig(InvalidConfigErrorType.REPEATED_ORACLE_ADDRESS);
}
if (transmitter == address(0)) revert OracleCannotBeZeroAddress();
s_oracles[transmitter] = Oracle(uint8(i), Role.Transmitter);
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
// wrap and rethrow the error so we can catch it lower in the stack
if (!success) revert TokenHandlingError(returnData);

// If the call was successful, the returnData should be the local token address.
// If the call was successful, the returnData should contain only the local token amount.
if (returnData.length != Pool.CCIP_POOL_V1_RET_BYTES) {
revert InvalidDataLength(Pool.CCIP_POOL_V1_RET_BYTES, returnData.length);
}
Expand Down
20 changes: 13 additions & 7 deletions contracts/src/v0.8/ccip/onRamp/EVM2EVMMultiOnRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyMultiOnRamp, ILinkAvailable, AggregateRat
error MustBeCalledByRouter();
error RouterMustSetOriginalSender();
error InvalidConfig();
error InvalidAddress(bytes encodedAddress);
error CursedByRMN(uint64 sourceChainSelector);
error LinkBalanceNotSettled();
error InvalidNopAddress(address nop);
Expand Down Expand Up @@ -81,14 +80,14 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyMultiOnRamp, ILinkAvailable, AggregateRat
uint64 chainSelector; // ─────╯ Source chainSelector
uint96 maxNopFeesJuels; // ───╮ Max nop fee balance onramp can have
address rmnProxy; // ─────────╯ Address of RMN proxy
address tokenAdminRegistry; // Token admin registry address
}

/// @dev Struct to contains the dynamic configuration
// solhint-disable-next-line gas-struct-packing
struct DynamicConfig {
address router; // Router address
address priceRegistry; // Price registry address
address tokenAdminRegistry; // Token admin registry address
}

/// @dev Struct to hold the fee token configuration for a token, same as the s_premiumMultiplierWeiPerEth but with
Expand Down Expand Up @@ -192,6 +191,8 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyMultiOnRamp, ILinkAvailable, AggregateRat
uint64 internal immutable i_chainSelector;
/// @dev The address of the rmn proxy
address internal immutable i_rmnProxy;
/// @dev The address of the token admin registry
address internal immutable i_tokenAdminRegistry;
/// @dev the maximum number of nops that can be configured at the same time.
/// Used to bound gas for loops over nops.
uint256 private constant MAX_NUMBER_OF_NOPS = 64;
Expand Down Expand Up @@ -230,15 +231,18 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyMultiOnRamp, ILinkAvailable, AggregateRat
TokenTransferFeeConfigArgs[] memory tokenTransferFeeConfigArgs,
NopAndWeight[] memory nopsAndWeights
) AggregateRateLimiter(rateLimiterConfig) {
if (staticConfig.linkToken == address(0) || staticConfig.chainSelector == 0 || staticConfig.rmnProxy == address(0))
{
if (
staticConfig.linkToken == address(0) || staticConfig.chainSelector == 0 || staticConfig.rmnProxy == address(0)
|| staticConfig.tokenAdminRegistry == address(0)
) {
revert InvalidConfig();
}

i_linkToken = staticConfig.linkToken;
i_chainSelector = staticConfig.chainSelector;
i_maxNopFeesJuels = staticConfig.maxNopFeesJuels;
i_rmnProxy = staticConfig.rmnProxy;
i_tokenAdminRegistry = staticConfig.tokenAdminRegistry;

_setDynamicConfig(dynamicConfig);
_applyDestChainConfigUpdates(destChainConfigArgs);
Expand Down Expand Up @@ -455,7 +459,8 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyMultiOnRamp, ILinkAvailable, AggregateRat
linkToken: i_linkToken,
chainSelector: i_chainSelector,
maxNopFeesJuels: i_maxNopFeesJuels,
rmnProxy: i_rmnProxy
rmnProxy: i_rmnProxy,
tokenAdminRegistry: i_tokenAdminRegistry
});
}

Expand Down Expand Up @@ -483,7 +488,8 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyMultiOnRamp, ILinkAvailable, AggregateRat
linkToken: i_linkToken,
chainSelector: i_chainSelector,
maxNopFeesJuels: i_maxNopFeesJuels,
rmnProxy: i_rmnProxy
rmnProxy: i_rmnProxy,
tokenAdminRegistry: i_tokenAdminRegistry
}),
dynamicConfig
);
Expand All @@ -495,7 +501,7 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyMultiOnRamp, ILinkAvailable, AggregateRat

/// @inheritdoc IEVM2AnyOnRampClient
function getPoolBySourceToken(uint64, /*destChainSelector*/ IERC20 sourceToken) public view returns (IPool) {
return IPool(ITokenAdminRegistry(s_dynamicConfig.tokenAdminRegistry).getPool(address(sourceToken)));
return IPool(ITokenAdminRegistry(i_tokenAdminRegistry).getPool(address(sourceToken)));
}

/// @inheritdoc IEVM2AnyOnRampClient
Expand Down
17 changes: 11 additions & 6 deletions contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
error MustBeCalledByRouter();
error RouterMustSetOriginalSender();
error InvalidConfig();
error InvalidAddress(bytes encodedAddress);
error CursedByRMN();
error LinkBalanceNotSettled();
error InvalidNopAddress(address nop);
Expand Down Expand Up @@ -78,6 +77,7 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
uint96 maxNopFeesJuels; // ───╯ Max nop fee balance onramp can have
address prevOnRamp; // Address of previous-version OnRamp
address rmnProxy; // Address of RMN proxy
address tokenAdminRegistry; // Address of the token admin registry
}

/// @dev Struct to contains the dynamic configuration
Expand All @@ -92,10 +92,9 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
address priceRegistry; // │ Price registry address
uint32 maxDataBytes; // │ Maximum payload data size in bytes
uint32 maxPerMsgGasLimit; // ────────────────╯ Maximum gas limit for messages targeting EVMs
address tokenAdminRegistry; // ──────────────╮ Token admin registry address
// │
// The following three properties are defaults, they can be overridden by setting the TokenTransferFeeConfig for a token
uint16 defaultTokenFeeUSDCents; // Default token fee charged per token transfer
uint16 defaultTokenFeeUSDCents; // ──────────╮ Default token fee charged per token transfer
uint32 defaultTokenDestGasOverhead; // │ Default gas charged to execute the token transfer on the destination chain
// │ Default data availability bytes that are returned from the source pool and sent
uint32 defaultTokenDestBytesOverhead; // | to the destination pool. Must be >= Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES
Expand Down Expand Up @@ -174,6 +173,8 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
address internal immutable i_prevOnRamp;
/// @dev The address of the RMN proxy
address internal immutable i_rmnProxy;
/// @dev The address of the token admin registry
address internal immutable i_tokenAdminRegistry;
/// @dev the maximum number of nops that can be configured at the same time.
/// Used to bound gas for loops over nops.
uint256 private constant MAX_NUMBER_OF_NOPS = 64;
Expand Down Expand Up @@ -214,6 +215,7 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
if (
staticConfig.linkToken == address(0) || staticConfig.chainSelector == 0 || staticConfig.destChainSelector == 0
|| staticConfig.defaultTxGasLimit == 0 || staticConfig.rmnProxy == address(0)
|| staticConfig.tokenAdminRegistry == address(0)
) revert InvalidConfig();

i_metadataHash = keccak256(
Expand All @@ -228,6 +230,7 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
i_maxNopFeesJuels = staticConfig.maxNopFeesJuels;
i_prevOnRamp = staticConfig.prevOnRamp;
i_rmnProxy = staticConfig.rmnProxy;
i_tokenAdminRegistry = staticConfig.tokenAdminRegistry;

_setDynamicConfig(dynamicConfig);
_setFeeTokenConfig(feeTokenConfigs);
Expand Down Expand Up @@ -433,7 +436,8 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
defaultTxGasLimit: i_defaultTxGasLimit,
maxNopFeesJuels: i_maxNopFeesJuels,
prevOnRamp: i_prevOnRamp,
rmnProxy: i_rmnProxy
rmnProxy: i_rmnProxy,
tokenAdminRegistry: i_tokenAdminRegistry
});
}

Expand Down Expand Up @@ -467,7 +471,8 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
defaultTxGasLimit: i_defaultTxGasLimit,
maxNopFeesJuels: i_maxNopFeesJuels,
prevOnRamp: i_prevOnRamp,
rmnProxy: i_rmnProxy
rmnProxy: i_rmnProxy,
tokenAdminRegistry: i_tokenAdminRegistry
}),
dynamicConfig
);
Expand All @@ -479,7 +484,7 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,

/// @inheritdoc IEVM2AnyOnRampClient
function getPoolBySourceToken(uint64, /*destChainSelector*/ IERC20 sourceToken) public view returns (IPool) {
return IPool(ITokenAdminRegistry(s_dynamicConfig.tokenAdminRegistry).getPool(address(sourceToken)));
return IPool(ITokenAdminRegistry(i_tokenAdminRegistry).getPool(address(sourceToken)));
}

/// @inheritdoc IEVM2AnyOnRampClient
Expand Down
1 change: 1 addition & 0 deletions contracts/src/v0.8/ccip/pools/BurnFromMintTokenPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {SafeERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/tok
/// It either accepts any address as originalSender, or only accepts whitelisted originalSender.
/// The only way to change whitelisting mode is to deploy a new pool.
/// If that is expected, please make sure the token's burner/minter roles are adjustable.
/// @dev This contract is a variant of BurnMintTokenPool that uses `burnFrom(from, amount)`.
contract BurnFromMintTokenPool is BurnMintTokenPoolAbstract, ITypeAndVersion {
using SafeERC20 for IBurnMintERC20;

Expand Down
1 change: 1 addition & 0 deletions contracts/src/v0.8/ccip/pools/BurnMintTokenPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {TokenPool} from "./TokenPool.sol";
/// It either accepts any address as originalSender, or only accepts whitelisted originalSender.
/// The only way to change whitelisting mode is to deploy a new pool.
/// If that is expected, please make sure the token's burner/minter roles are adjustable.
/// @dev This contract is a variant of BurnMintTokenPool that uses `burn(amount)`.
contract BurnMintTokenPool is BurnMintTokenPoolAbstract, ITypeAndVersion {
string public constant override typeAndVersion = "BurnMintTokenPool 1.5.0-dev";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {SafeERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/tok
/// It either accepts any address as originalSender, or only accepts whitelisted originalSender.
/// The only way to change whitelisting mode is to deploy a new pool.
/// If that is expected, please make sure the token's burner/minter roles are adjustable.
/// @dev This contract is a variant of BurnMintTokenPool that uses `burn(from, amount)`.
contract BurnWithFromMintTokenPool is BurnMintTokenPoolAbstract, ITypeAndVersion {
using SafeERC20 for IBurnMintERC20;

Expand Down
11 changes: 1 addition & 10 deletions contracts/src/v0.8/ccip/pools/LockReleaseTokenPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ contract LockReleaseTokenPool is TokenPool, ILiquidityContainer, ITypeAndVersion

string public constant override typeAndVersion = "LockReleaseTokenPool 1.5.0-dev";

/// @dev The unique lock release pool flag to signal through EIP 165.
bytes4 private constant LOCK_RELEASE_INTERFACE_ID = bytes4(keccak256("LockReleaseTokenPool"));

/// @dev Whether or not the pool accepts liquidity.
/// External liquidity is not required when there is one canonical token deployed to a chain,
/// and CCIP is facilitating mint/burn on all the other chains, in which case the invariant
Expand Down Expand Up @@ -83,15 +80,9 @@ contract LockReleaseTokenPool is TokenPool, ILiquidityContainer, ITypeAndVersion
return Pool.ReleaseOrMintOutV1({destinationAmount: releaseOrMintIn.amount});
}

/// @notice returns the lock release interface flag used for EIP165 identification.
function getLockReleaseInterfaceId() public pure returns (bytes4) {
return LOCK_RELEASE_INTERFACE_ID;
}

// @inheritdoc IERC165
function supportsInterface(bytes4 interfaceId) public pure virtual override returns (bool) {
return interfaceId == LOCK_RELEASE_INTERFACE_ID || interfaceId == type(ILiquidityContainer).interfaceId
|| super.supportsInterface(interfaceId);
return interfaceId == type(ILiquidityContainer).interfaceId || super.supportsInterface(interfaceId);
}

/// @notice Gets LiquidityManager, can be address(0) if none is configured.
Expand Down
Loading

0 comments on commit 54ee4f1

Please sign in to comment.