Skip to content

Commit

Permalink
add decimal & overflow checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@RensR
RensR committed Nov 26, 2024
1 parent 0c233b7 commit 0a29b6e
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 350 deletions.
52 changes: 26 additions & 26 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,11 @@ HybridLockReleaseUSDCTokenPool_lockOrBurn:test_onLockReleaseMechanism_thenSwitch
HybridLockReleaseUSDCTokenPool_releaseOrMint:test_OnLockReleaseMechanism_Success() (gas: 213127)
HybridLockReleaseUSDCTokenPool_releaseOrMint:test_WhileMigrationPause_Revert() (gas: 109646)
HybridLockReleaseUSDCTokenPool_releaseOrMint:test_incomingMessageWithPrimaryMechanism() (gas: 265910)
LockReleaseTokenPool_canAcceptLiquidity:test_CanAcceptLiquidity_Success() (gas: 3053405)
LockReleaseTokenPool_canAcceptLiquidity:test_CanAcceptLiquidity_Success() (gas: 3123780)
LockReleaseTokenPool_lockOrBurn:test_LockOrBurnWithAllowList_Revert() (gas: 29734)
LockReleaseTokenPool_lockOrBurn:test_LockOrBurnWithAllowList_Success() (gas: 80647)
LockReleaseTokenPool_lockOrBurn:test_PoolBurnRevertNotHealthy_Revert() (gas: 59227)
LockReleaseTokenPool_provideLiquidity:test_LiquidityNotAccepted_Revert() (gas: 3049734)
LockReleaseTokenPool_provideLiquidity:test_LiquidityNotAccepted_Revert() (gas: 3120109)
LockReleaseTokenPool_provideLiquidity:test_Unauthorized_Revert() (gas: 11511)
LockReleaseTokenPool_releaseOrMint:test_ChainNotAllowed_Revert() (gas: 74108)
LockReleaseTokenPool_releaseOrMint:test_PoolMintNotHealthy_Revert() (gas: 54745)
Expand Down Expand Up @@ -535,7 +535,7 @@ OnRamp_forwardFromRouter:test_ShouldIncrementNonceOnlyOnOrdered_Success() (gas:
OnRamp_forwardFromRouter:test_ShouldIncrementSeqNumAndNonce_Success() (gas: 213012)
OnRamp_forwardFromRouter:test_ShouldStoreLinkFees() (gas: 147026)
OnRamp_forwardFromRouter:test_ShouldStoreNonLinkFees() (gas: 161215)
OnRamp_forwardFromRouter:test_SourceTokenDataTooLarge_Revert() (gas: 3919758)
OnRamp_forwardFromRouter:test_SourceTokenDataTooLarge_Revert() (gas: 3987624)
OnRamp_forwardFromRouter:test_UnAllowedOriginalSender_Revert() (gas: 24015)
OnRamp_forwardFromRouter:test_UnsupportedToken_Revert() (gas: 75832)
OnRamp_forwardFromRouter:test_forwardFromRouter_UnsupportedToken_Revert() (gas: 38588)
Expand Down Expand Up @@ -681,14 +681,14 @@ TokenAdminRegistry_setPool:test_setPool_ZeroAddressRemovesPool_Success() (gas: 3
TokenAdminRegistry_transferAdminRole:test_transferAdminRole_OnlyAdministrator_Revert() (gas: 18202)
TokenAdminRegistry_transferAdminRole:test_transferAdminRole_Success() (gas: 49592)
TokenPoolFactory_constructor:test_constructor_Revert() (gas: 1121653)
TokenPoolFactory_createTokenPool:test_createTokenPoolLockRelease_ExistingToken_predict_Success() (gas: 12293466)
TokenPoolFactory_createTokenPool:test_createTokenPool_BurnFromMintTokenPool_Success() (gas: 6290905)
TokenPoolFactory_createTokenPool:test_createTokenPool_ExistingRemoteToken_AndPredictPool_Success() (gas: 13043750)
TokenPoolFactory_createTokenPool:test_createTokenPool_RemoteTokenHasDifferentDecimals_Success() (gas: 13051052)
TokenPoolFactory_createTokenPool:test_createTokenPool_WithNoExistingRemoteContracts_predict_Success() (gas: 13380456)
TokenPoolFactory_createTokenPool:test_createTokenPool_WithNoExistingTokenOnRemoteChain_Success() (gas: 6075765)
TokenPoolFactory_createTokenPool:test_createTokenPool_WithRemoteTokenAndRemotePool_Success() (gas: 6287377)
TokenPoolWithAllowList_applyAllowListUpdates:test_AllowListNotEnabled_Revert() (gas: 2670523)
TokenPoolFactory_createTokenPool:test_createTokenPoolLockRelease_ExistingToken_predict_Success() (gas: 12436024)
TokenPoolFactory_createTokenPool:test_createTokenPool_BurnFromMintTokenPool_Success() (gas: 6400000)
TokenPoolFactory_createTokenPool:test_createTokenPool_ExistingRemoteToken_AndPredictPool_Success() (gas: 13228940)
TokenPoolFactory_createTokenPool:test_createTokenPool_RemoteTokenHasDifferentDecimals_Success() (gas: 13236242)
TokenPoolFactory_createTokenPool:test_createTokenPool_WithNoExistingRemoteContracts_predict_Success() (gas: 13565664)
TokenPoolFactory_createTokenPool:test_createTokenPool_WithNoExistingTokenOnRemoteChain_Success() (gas: 6186359)
TokenPoolFactory_createTokenPool:test_createTokenPool_WithRemoteTokenAndRemotePool_Success() (gas: 6396544)
TokenPoolWithAllowList_applyAllowListUpdates:test_AllowListNotEnabled_Revert() (gas: 2740884)
TokenPoolWithAllowList_applyAllowListUpdates:test_OnlyOwner_Revert() (gas: 12119)
TokenPoolWithAllowList_applyAllowListUpdates:test_SetAllowListSkipsZero_Success() (gas: 23567)
TokenPoolWithAllowList_applyAllowListUpdates:test_SetAllowList_Success() (gas: 178398)
Expand All @@ -707,8 +707,8 @@ TokenPool_applyChainUpdates:test_applyChainUpdates_OnlyCallableByOwner_Revert()
TokenPool_applyChainUpdates:test_applyChainUpdates_Success() (gas: 592089)
TokenPool_applyChainUpdates:test_applyChainUpdates_UpdatesRemotePoolHashes() (gas: 1077776)
TokenPool_applyChainUpdates:test_applyChainUpdates_ZeroAddressNotAllowed_Revert() (gas: 226472)
TokenPool_calculateLocalAmount:test_calculateLocalAmount() (gas: 84708)
TokenPool_constructor:test_ZeroAddressNotAllowed_Revert() (gas: 71386)
TokenPool_calculateLocalAmount:test_calculateLocalAmount() (gas: 98017)
TokenPool_constructor:test_ZeroAddressNotAllowed_Revert() (gas: 71558)
TokenPool_constructor:test_immutableFields_Success() (gas: 21902)
TokenPool_getRemotePool:test_getRemotePools() (gas: 330500)
TokenPool_onlyOffRamp:test_CallerIsNotARampOnRouter_Revert() (gas: 21504)
Expand All @@ -726,20 +726,20 @@ TokenPool_setChainRateLimiterConfig:test_NonExistentChain_Revert() (gas: 17214)
TokenPool_setChainRateLimiterConfig:test_OnlyOwnerOrRateLimitAdmin_Revert() (gas: 15307)
TokenPool_setRateLimitAdmin:test_SetRateLimitAdmin_Revert() (gas: 11002)
TokenPool_setRateLimitAdmin:test_SetRateLimitAdmin_Success() (gas: 37606)
USDCBridgeMigrator_BurnLockedUSDC:test_PrimaryMechanism_Success() (gas: 136004)
USDCBridgeMigrator_BurnLockedUSDC:test_WhileMigrationPause_Revert() (gas: 109849)
USDCBridgeMigrator_BurnLockedUSDC:test_PrimaryMechanism_Success() (gas: 135869)
USDCBridgeMigrator_BurnLockedUSDC:test_WhileMigrationPause_Revert() (gas: 109729)
USDCBridgeMigrator_BurnLockedUSDC:test_invalidPermissions_Revert() (gas: 39493)
USDCBridgeMigrator_BurnLockedUSDC:test_lockOrBurn_then_BurnInCCTPMigration_Success() (gas: 310057)
USDCBridgeMigrator_BurnLockedUSDC:test_onLockReleaseMechanism_Success() (gas: 147035)
USDCBridgeMigrator_BurnLockedUSDC:test_onLockReleaseMechanism_thenSwitchToPrimary_Success() (gas: 209377)
USDCBridgeMigrator_BurnLockedUSDC:test_lockOrBurn_then_BurnInCCTPMigration_Success() (gas: 309798)
USDCBridgeMigrator_BurnLockedUSDC:test_onLockReleaseMechanism_Success() (gas: 146939)
USDCBridgeMigrator_BurnLockedUSDC:test_onLockReleaseMechanism_thenSwitchToPrimary_Success() (gas: 209089)
USDCBridgeMigrator_cancelMigrationProposal:test_cancelExistingCCTPMigrationProposal_Success() (gas: 56155)
USDCBridgeMigrator_cancelMigrationProposal:test_cannotCancelANonExistentMigrationProposal_Revert() (gas: 12669)
USDCBridgeMigrator_excludeTokensFromBurn:test_excludeTokensWhenNoMigrationProposalPending_Revert() (gas: 13579)
USDCBridgeMigrator_proposeMigration:test_ChainNotUsingLockRelease_Revert() (gas: 15765)
USDCBridgeMigrator_provideLiquidity:test_PrimaryMechanism_Success() (gas: 135986)
USDCBridgeMigrator_provideLiquidity:test_WhileMigrationPause_Revert() (gas: 109871)
USDCBridgeMigrator_provideLiquidity:test_cannotModifyLiquidityWithoutPermissions_Revert() (gas: 13379)
USDCBridgeMigrator_provideLiquidity:test_cannotProvideLiquidityWhenMigrationProposalPending_Revert() (gas: 67417)
USDCBridgeMigrator_provideLiquidity:test_cannotModifyLiquidityWithoutPermissions_Revert() (gas: 13390)
USDCBridgeMigrator_provideLiquidity:test_cannotProvideLiquidityWhenMigrationProposalPending_Revert() (gas: 67428)
USDCBridgeMigrator_provideLiquidity:test_cannotProvideLiquidity_AfterMigration_Revert() (gas: 313898)
USDCBridgeMigrator_provideLiquidity:test_invalidPermissions_Revert() (gas: 39493)
USDCBridgeMigrator_provideLiquidity:test_lockOrBurn_then_BurnInCCTPMigration_Success() (gas: 310057)
Expand All @@ -758,13 +758,13 @@ USDCBridgeMigrator_updateChainSelectorMechanism:test_lockOrBurn_then_BurnInCCTPM
USDCBridgeMigrator_updateChainSelectorMechanism:test_onLockReleaseMechanism_Success() (gas: 147035)
USDCBridgeMigrator_updateChainSelectorMechanism:test_onLockReleaseMechanism_thenSwitchToPrimary_Success() (gas: 209448)
USDCTokenPool__validateMessage:test_ValidateInvalidMessage_Revert() (gas: 26049)
USDCTokenPool_lockOrBurn:test_CallerIsNotARampOnRouter_Revert() (gas: 35369)
USDCTokenPool_lockOrBurn:test_LockOrBurnWithAllowList_Revert() (gas: 29947)
USDCTokenPool_lockOrBurn:test_LockOrBurn_Success() (gas: 133581)
USDCTokenPool_lockOrBurn:test_UnknownDomain_Revert() (gas: 433813)
USDCTokenPool_releaseOrMint:test_ReleaseOrMintRealTx_Success() (gas: 265825)
USDCTokenPool_lockOrBurn:test_CallerIsNotARampOnRouter_Revert() (gas: 35297)
USDCTokenPool_lockOrBurn:test_LockOrBurnWithAllowList_Revert() (gas: 29875)
USDCTokenPool_lockOrBurn:test_LockOrBurn_Success() (gas: 133408)
USDCTokenPool_lockOrBurn:test_UnknownDomain_Revert() (gas: 433408)
USDCTokenPool_releaseOrMint:test_ReleaseOrMintRealTx_Success() (gas: 265695)
USDCTokenPool_releaseOrMint:test_TokenMaxCapacityExceeded_Revert() (gas: 47231)
USDCTokenPool_releaseOrMint:test_UnlockingUSDCFailed_Revert() (gas: 95315)
USDCTokenPool_releaseOrMint:test_UnlockingUSDCFailed_Revert() (gas: 95262)
USDCTokenPool_setDomains:test_InvalidDomain_Revert() (gas: 66437)
USDCTokenPool_setDomains:test_OnlyOwner_Revert() (gas: 11314)
USDCTokenPool_supportsInterface:test_SupportsInterface_Success() (gas: 10107)
36 changes: 33 additions & 3 deletions contracts/src/v0.8/ccip/pools/TokenPool.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {Pool} from "../libraries/Pool.sol";
import {RateLimiter} from "../libraries/RateLimiter.sol";

import {IERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol";
import {IERC20Metadata} from
"../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import {IERC165} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/introspection/IERC165.sol";
import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/structs/EnumerableSet.sol";

Expand Down Expand Up @@ -49,6 +51,8 @@ abstract contract TokenPool is IPoolV1, Ownable2StepMsgSender {
error PoolAlreadyAdded(uint64 remoteChainSelector, bytes remotePoolAddress);
error InvalidRemotePoolForChain(uint64 remoteChainSelector, bytes remotePoolAddress);
error InvalidRemoteChainDecimals(bytes sourcePoolData);
error OverOrUnderflowDetected(uint8 sourceDecimals, uint8 localDecimals, uint256 localAmount);
error InvalidDecimalArgs(uint8 expected, uint8 actual);

event Locked(address indexed sender, uint256 amount);
event Burned(address indexed sender, uint256 amount);
Expand Down Expand Up @@ -119,7 +123,17 @@ abstract contract TokenPool is IPoolV1, Ownable2StepMsgSender {
if (address(token) == address(0) || router == address(0) || rmnProxy == address(0)) revert ZeroAddressNotAllowed();
i_token = token;
i_rmnProxy = rmnProxy;

try IERC20Metadata(address(token)).decimals() returns (uint8 actualTokenDecimals) {
if (localTokenDecimals != actualTokenDecimals) {
revert InvalidDecimalArgs(localTokenDecimals, actualTokenDecimals);
}
} catch {
// The decimals function doesn't exist, which is possible since it's optional in the ERC20 spec. We skip the check and
// assume the supplied token decimals are correct.
}
i_tokenDecimals = localTokenDecimals;

s_router = IRouter(router);

// Pool can be set as permissioned or permissionless at deployment time only to save hot-path gas.
Expand Down Expand Up @@ -256,17 +270,33 @@ abstract contract TokenPool is IPoolV1, Ownable2StepMsgSender {
/// @param remoteAmount The amount on the remote chain.
/// @param remoteDecimals The decimals of the token on the remote chain.
/// @return The local amount.
/// @dev This function assumes the inputs don't overflow and does no checks to avoid this. For any normal inputs, this
/// should not be a problem. The only way to overflow is when the given arguments cannot be represented in the uint256
/// type, which means the inputs are invalid.
/// @dev This function protects against overflows and underflows. If there is a transaction that hits the overflow
/// or underflow check, it is probably incorrect as that means the amount cannot be represented on this chain. If the
/// local decimals have been wrongly configured, the token issuer could redeploy the pool with the correct decimals
/// and manually re-execute the CCIP tx to fix the issue.
function _calculateLocalAmount(uint256 remoteAmount, uint8 remoteDecimals) internal view virtual returns (uint256) {
if (remoteDecimals == i_tokenDecimals) {
return remoteAmount;
}
if (remoteDecimals > i_tokenDecimals) {
if (remoteDecimals - i_tokenDecimals > 77) {
// This is a safety check to prevent overflow in the next calculation.
revert OverOrUnderflowDetected(remoteDecimals, i_tokenDecimals, remoteAmount);
}
// Solidity rounds down so there is no risk of minting more tokens than the remote chain sent.
return remoteAmount / (10 ** (remoteDecimals - i_tokenDecimals));
}

// This is a safety check to prevent overflow in the next calculation.
// More than 77 would never fit in a uint256 and would cause an overflow. We also check if the resulting amount
// would overflow.
if (
i_tokenDecimals - remoteDecimals > 77
|| remoteAmount > type(uint256).max / (10 ** (i_tokenDecimals - remoteDecimals))
) {
revert OverOrUnderflowDetected(remoteDecimals, i_tokenDecimals, remoteAmount);
}

return remoteAmount * (10 ** (i_tokenDecimals - remoteDecimals));
}

Expand Down
108 changes: 6 additions & 102 deletions .../test/pools/USDC/HybridLockReleaseUSDCTokenPool/HybridLockReleaseUSDCTokenPoolSetup.t.sol
View file
Original file line number Diff line number Diff line change
@@ -1,94 +1,29 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.24;

import {IBurnMintERC20} from "../../../../../shared/token/ERC20/IBurnMintERC20.sol";

import {BurnMintERC677} from "../../../../../shared/token/ERC677/BurnMintERC677.sol";
import {Router} from "../../../../Router.sol";
import {TokenPool} from "../../../../pools/TokenPool.sol";

import {HybridLockReleaseUSDCTokenPool} from "../../../../pools/USDC/HybridLockReleaseUSDCTokenPool.sol";
import {USDCTokenPool} from "../../../../pools/USDC/USDCTokenPool.sol";
import {BaseTest} from "../../../BaseTest.t.sol";
import {MockE2EUSDCTransmitter} from "../../../mocks/MockE2EUSDCTransmitter.sol";
import {MockUSDCTokenMessenger} from "../../../mocks/MockUSDCTokenMessenger.sol";

contract HybridLockReleaseUSDCTokenPoolSetup is BaseTest {
IBurnMintERC20 internal s_token;
MockUSDCTokenMessenger internal s_mockUSDC;
MockE2EUSDCTransmitter internal s_mockUSDCTransmitter;
uint32 internal constant USDC_DEST_TOKEN_GAS = 150_000;

struct USDCMessage {
uint32 version;
uint32 sourceDomain;
uint32 destinationDomain;
uint64 nonce;
bytes32 sender;
bytes32 recipient;
bytes32 destinationCaller;
bytes messageBody;
}

uint32 internal constant SOURCE_DOMAIN_IDENTIFIER = 0x02020202;
uint32 internal constant DEST_DOMAIN_IDENTIFIER = 0;

bytes32 internal constant SOURCE_CHAIN_TOKEN_SENDER = bytes32(uint256(uint160(0x01111111221)));
address internal constant SOURCE_CHAIN_USDC_POOL = address(0x23789765456789);
address internal constant DEST_CHAIN_USDC_POOL = address(0x987384873458734);
address internal constant DEST_CHAIN_USDC_TOKEN = address(0x23598918358198766);

address internal s_routerAllowedOnRamp = address(3456);
address internal s_routerAllowedOffRamp = address(234);
Router internal s_router;
import {USDCSetup} from "../USDCSetup.t.sol";

contract HybridLockReleaseUSDCTokenPoolSetup is USDCSetup {
HybridLockReleaseUSDCTokenPool internal s_usdcTokenPool;
HybridLockReleaseUSDCTokenPool internal s_usdcTokenPoolTransferLiquidity;
address[] internal s_allowedList;

function setUp() public virtual override {
BaseTest.setUp();
BurnMintERC677 usdcToken = new BurnMintERC677("LINK", "LNK", 18, 0);
s_token = usdcToken;
deal(address(s_token), OWNER, type(uint256).max);
_setUpRamps();

s_mockUSDCTransmitter = new MockE2EUSDCTransmitter(0, DEST_DOMAIN_IDENTIFIER, address(s_token));
s_mockUSDC = new MockUSDCTokenMessenger(0, address(s_mockUSDCTransmitter));

usdcToken.grantMintAndBurnRoles(address(s_mockUSDCTransmitter));
super.setUp();

s_usdcTokenPool =
new HybridLockReleaseUSDCTokenPool(s_mockUSDC, s_token, new address[](0), address(s_mockRMN), address(s_router));

s_usdcTokenPoolTransferLiquidity =
new HybridLockReleaseUSDCTokenPool(s_mockUSDC, s_token, new address[](0), address(s_mockRMN), address(s_router));

usdcToken.grantMintAndBurnRoles(address(s_mockUSDC));
usdcToken.grantMintAndBurnRoles(address(s_usdcTokenPool));

bytes[] memory sourcePoolAddresses = new bytes[](1);
sourcePoolAddresses[0] = abi.encode(SOURCE_CHAIN_USDC_POOL);
BurnMintERC677(address(s_token)).grantMintAndBurnRoles(address(s_usdcTokenPool));

bytes[] memory destPoolAddresses = new bytes[](1);
destPoolAddresses[0] = abi.encode(DEST_CHAIN_USDC_POOL);

TokenPool.ChainUpdate[] memory chainUpdates = new TokenPool.ChainUpdate[](2);
chainUpdates[0] = TokenPool.ChainUpdate({
remoteChainSelector: SOURCE_CHAIN_SELECTOR,
remotePoolAddresses: sourcePoolAddresses,
remoteTokenAddress: abi.encode(address(s_token)),
outboundRateLimiterConfig: _getOutboundRateLimiterConfig(),
inboundRateLimiterConfig: _getInboundRateLimiterConfig()
});
chainUpdates[1] = TokenPool.ChainUpdate({
remoteChainSelector: DEST_CHAIN_SELECTOR,
remotePoolAddresses: destPoolAddresses,
remoteTokenAddress: abi.encode(DEST_CHAIN_USDC_TOKEN),
outboundRateLimiterConfig: _getOutboundRateLimiterConfig(),
inboundRateLimiterConfig: _getInboundRateLimiterConfig()
});

s_usdcTokenPool.applyChainUpdates(new uint64[](0), chainUpdates);
_poolApplyChainUpdates(address(s_usdcTokenPool));

USDCTokenPool.DomainUpdate[] memory domains = new USDCTokenPool.DomainUpdate[](1);
domains[0] = USDCTokenPool.DomainUpdate({
Expand All @@ -100,38 +35,7 @@ contract HybridLockReleaseUSDCTokenPoolSetup is BaseTest {

s_usdcTokenPool.setDomains(domains);

vm.expectEmit();
emit HybridLockReleaseUSDCTokenPool.LiquidityProviderSet(address(0), OWNER, DEST_CHAIN_SELECTOR);

s_usdcTokenPool.setLiquidityProvider(DEST_CHAIN_SELECTOR, OWNER);
s_usdcTokenPool.setLiquidityProvider(SOURCE_CHAIN_SELECTOR, OWNER);
}

function _setUpRamps() internal {
s_router = new Router(address(s_token), address(s_mockRMN));

Router.OnRamp[] memory onRampUpdates = new Router.OnRamp[](1);
onRampUpdates[0] = Router.OnRamp({destChainSelector: DEST_CHAIN_SELECTOR, onRamp: s_routerAllowedOnRamp});
Router.OffRamp[] memory offRampUpdates = new Router.OffRamp[](1);
address[] memory offRamps = new address[](1);
offRamps[0] = s_routerAllowedOffRamp;
offRampUpdates[0] = Router.OffRamp({sourceChainSelector: SOURCE_CHAIN_SELECTOR, offRamp: offRamps[0]});

s_router.applyRampUpdates(onRampUpdates, new Router.OffRamp[](0), offRampUpdates);
}

function _generateUSDCMessage(
USDCMessage memory usdcMessage
) internal pure returns (bytes memory) {
return abi.encodePacked(
usdcMessage.version,
usdcMessage.sourceDomain,
usdcMessage.destinationDomain,
usdcMessage.nonce,
usdcMessage.sender,
usdcMessage.recipient,
usdcMessage.destinationCaller,
usdcMessage.messageBody
);
}
}
Loading

0 comments on commit 0a29b6e

Please sign in to comment.