From 42db9fd17ca32d554f8bc9d0c6aab01e5c0e8c81 Mon Sep 17 00:00:00 2001 From: Josh Weintraub <26035072+jhweintraub@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:27:03 -0400 Subject: [PATCH] Misc. Fixes to HybridUSDCTokenPool (#14922) * copy files from PR on ccip repo and re-open * forgot to include changeset file * [Bot] Update changeset file with jira issues --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> --- contracts/.changeset/moody-humans-judge.md | 10 ++ contracts/gas-snapshots/ccip.gas-snapshot | 60 +++++----- .../BurnMintWithLockReleaseFlagTokenPool.sol | 42 +++++++ .../USDC/HybridLockReleaseUSDCTokenPool.sol | 44 ++++--- .../ccip/pools/USDC/USDCBridgeMigrator.sol | 33 +++--- ...BurnMintWithLockReleaseFlagTokenPool.t.sol | 64 ++++++++++ .../HybridLockReleaseUSDCTokenPool.t.sol | 110 +++++++++++++++--- 7 files changed, 289 insertions(+), 74 deletions(-) create mode 100644 contracts/.changeset/moody-humans-judge.md create mode 100644 contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol create mode 100644 contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol diff --git a/contracts/.changeset/moody-humans-judge.md b/contracts/.changeset/moody-humans-judge.md new file mode 100644 index 00000000000..ef562264a30 --- /dev/null +++ b/contracts/.changeset/moody-humans-judge.md @@ -0,0 +1,10 @@ +--- +'@chainlink/contracts': patch +--- + +Minor fixes to formatting, pragma, imports, etc. for Hybrid USDC Token Pools #bugfix + + +PR issue: CCIP-3014 + +Solidity Review issue: CCIP-3966 \ No newline at end of file diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot index 11d348134c4..cec42f95841 100644 --- a/contracts/gas-snapshots/ccip.gas-snapshot +++ b/contracts/gas-snapshots/ccip.gas-snapshot @@ -15,6 +15,7 @@ BurnMintTokenPool_lockOrBurn:test_Setup_Success() (gas: 17851) BurnMintTokenPool_releaseOrMint:test_ChainNotAllowed_Revert() (gas: 28783) BurnMintTokenPool_releaseOrMint:test_PoolMintNotHealthy_Revert() (gas: 56259) BurnMintTokenPool_releaseOrMint:test_PoolMint_Success() (gas: 112372) +BurnMintWithLockReleaseFlagTokenPool_lockOrBurn:test_LockOrBurn_CorrectReturnData_Success() (gas: 242288) BurnWithFromMintTokenPool_lockOrBurn:test_ChainNotAllowed_Revert() (gas: 28842) BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurnRevertNotHealthy_Revert() (gas: 55271) BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurn_Success() (gas: 244092) @@ -197,33 +198,38 @@ FeeQuoter_validateDestFamilyAddress:test_InvalidEVMAddressPrecompiles_Revert() ( FeeQuoter_validateDestFamilyAddress:test_InvalidEVMAddress_Revert() (gas: 10840) FeeQuoter_validateDestFamilyAddress:test_ValidEVMAddress_Success() (gas: 6775) FeeQuoter_validateDestFamilyAddress:test_ValidNonEVMAddress_Success() (gas: 6489) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209230) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135879) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_WhileMigrationPause_Revert() (gas: 107090) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_onLockReleaseMechanism_Success() (gas: 144653) -HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_OnLockReleaseMechanism_Success() (gas: 214817) -HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_OnLockReleaseMechanism_then_switchToPrimary_Success() (gas: 423690) -HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268967) -HybridUSDCTokenPoolMigrationTests:test_ReleaseOrMint_WhileMigrationPause_Revert() (gas: 111484) -HybridUSDCTokenPoolMigrationTests:test_burnLockedUSDC_invalidPermissions_Revert() (gas: 39362) -HybridUSDCTokenPoolMigrationTests:test_cancelExistingCCTPMigrationProposal() (gas: 33172) -HybridUSDCTokenPoolMigrationTests:test_cannotCancelANonExistentMigrationProposal() (gas: 12714) -HybridUSDCTokenPoolMigrationTests:test_cannotModifyLiquidityWithoutPermissions_Revert() (gas: 13329) -HybridUSDCTokenPoolMigrationTests:test_cannotTransferLiquidityDuringPendingMigration_Revert() (gas: 160900) -HybridUSDCTokenPoolMigrationTests:test_lockOrBurn_then_BurnInCCTPMigration_Success() (gas: 255982) -HybridUSDCTokenPoolMigrationTests:test_transferLiquidity_Success() (gas: 165921) -HybridUSDCTokenPoolMigrationTests:test_unstickManualTxAfterMigration_destChain_Success() (gas: 154307) -HybridUSDCTokenPoolMigrationTests:test_unstickManualTxAfterMigration_homeChain_Success() (gas: 463780) -HybridUSDCTokenPoolTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209230) -HybridUSDCTokenPoolTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135897) -HybridUSDCTokenPoolTests:test_LockOrBurn_WhileMigrationPause_Revert() (gas: 107135) -HybridUSDCTokenPoolTests:test_LockOrBurn_onLockReleaseMechanism_Success() (gas: 144607) -HybridUSDCTokenPoolTests:test_MintOrRelease_OnLockReleaseMechanism_Success() (gas: 214795) -HybridUSDCTokenPoolTests:test_MintOrRelease_OnLockReleaseMechanism_then_switchToPrimary_Success() (gas: 423668) -HybridUSDCTokenPoolTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268949) -HybridUSDCTokenPoolTests:test_ReleaseOrMint_WhileMigrationPause_Revert() (gas: 111528) -HybridUSDCTokenPoolTests:test_cannotTransferLiquidityDuringPendingMigration_Revert() (gas: 160845) -HybridUSDCTokenPoolTests:test_transferLiquidity_Success() (gas: 165904) +HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209283) +HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135915) +HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_WhileMigrationPause_Revert() (gas: 109794) +HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_onLockReleaseMechanism_Success() (gas: 147082) +HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_OnLockReleaseMechanism_Success() (gas: 217024) +HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_OnLockReleaseMechanism_then_switchToPrimary_Success() (gas: 425962) +HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268984) +HybridUSDCTokenPoolMigrationTests:test_ProposeMigration_ChainNotUsingLockRelease_Revert() (gas: 15843) +HybridUSDCTokenPoolMigrationTests:test_ReleaseOrMint_WhileMigrationPause_Revert() (gas: 113503) +HybridUSDCTokenPoolMigrationTests:test_burnLockedUSDC_invalidPermissions_Revert() (gas: 39300) +HybridUSDCTokenPoolMigrationTests:test_cancelExistingCCTPMigrationProposal() (gas: 56208) +HybridUSDCTokenPoolMigrationTests:test_cannotCancelANonExistentMigrationProposal() (gas: 12758) +HybridUSDCTokenPoolMigrationTests:test_cannotModifyLiquidityWithoutPermissions_Revert() (gas: 13395) +HybridUSDCTokenPoolMigrationTests:test_cannotProvideLiquidityWhenMigrationProposalPending_Revert() (gas: 67370) +HybridUSDCTokenPoolMigrationTests:test_cannotRevertChainMechanism_afterMigration_Revert() (gas: 313252) +HybridUSDCTokenPoolMigrationTests:test_cannotTransferLiquidityDuringPendingMigration_Revert() (gas: 177053) +HybridUSDCTokenPoolMigrationTests:test_cnanotProvideLiquidity_AfterMigration_Revert() (gas: 313637) +HybridUSDCTokenPoolMigrationTests:test_excludeTokensWhenNoMigrationProposalPending_Revert() (gas: 13657) +HybridUSDCTokenPoolMigrationTests:test_lockOrBurn_then_BurnInCCTPMigration_Success() (gas: 309797) +HybridUSDCTokenPoolMigrationTests:test_transferLiquidity_Success() (gas: 167051) +HybridUSDCTokenPoolMigrationTests:test_unstickManualTxAfterMigration_destChain_Success() (gas: 156096) +HybridUSDCTokenPoolMigrationTests:test_unstickManualTxAfterMigration_homeChain_Success() (gas: 516027) +HybridUSDCTokenPoolTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209283) +HybridUSDCTokenPoolTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135915) +HybridUSDCTokenPoolTests:test_LockOrBurn_WhileMigrationPause_Revert() (gas: 109794) +HybridUSDCTokenPoolTests:test_LockOrBurn_onLockReleaseMechanism_Success() (gas: 147080) +HybridUSDCTokenPoolTests:test_MintOrRelease_OnLockReleaseMechanism_Success() (gas: 217002) +HybridUSDCTokenPoolTests:test_MintOrRelease_OnLockReleaseMechanism_then_switchToPrimary_Success() (gas: 425918) +HybridUSDCTokenPoolTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268967) +HybridUSDCTokenPoolTests:test_ReleaseOrMint_WhileMigrationPause_Revert() (gas: 113547) +HybridUSDCTokenPoolTests:test_cannotTransferLiquidityDuringPendingMigration_Revert() (gas: 177009) +HybridUSDCTokenPoolTests:test_transferLiquidity_Success() (gas: 167033) LockReleaseTokenPool_canAcceptLiquidity:test_CanAcceptLiquidity_Success() (gas: 2836138) LockReleaseTokenPool_lockOrBurn:test_LockOrBurnWithAllowList_Revert() (gas: 30062) LockReleaseTokenPool_lockOrBurn:test_LockOrBurnWithAllowList_Success() (gas: 79965) diff --git a/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol b/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol new file mode 100644 index 00000000000..fd54387620f --- /dev/null +++ b/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity 0.8.24; + +import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol"; + +import {Pool} from "../../libraries/Pool.sol"; +import {BurnMintTokenPool} from "../BurnMintTokenPool.sol"; +import {LOCK_RELEASE_FLAG} from "./HybridLockReleaseUSDCTokenPool.sol"; + +/// @notice A standard BurnMintTokenPool with modified destPoolData so that the remote pool knows to release tokens +/// instead of minting. This enables interoperability with HybridLockReleaseUSDCTokenPool which uses +// the destPoolData to determine whether to mint or release tokens. +/// @dev The only difference between this contract and BurnMintTokenPool is the destPoolData returns the +/// abi-encoded LOCK_RELEASE_FLAG instead of an empty string. +contract BurnMintWithLockReleaseFlagTokenPool is BurnMintTokenPool { + constructor( + IBurnMintERC20 token, + address[] memory allowlist, + address rmnProxy, + address router + ) BurnMintTokenPool(token, allowlist, rmnProxy, router) {} + + /// @notice Burn the token in the pool + /// @dev The _validateLockOrBurn check is an essential security check + /// @dev Performs the exact same functionality as BurnMintTokenPool, but returns the LOCK_RELEASE_FLAG + /// as the destPoolData to signal to the remote pool to release tokens instead of minting them. + function lockOrBurn( + Pool.LockOrBurnInV1 calldata lockOrBurnIn + ) external override returns (Pool.LockOrBurnOutV1 memory) { + _validateLockOrBurn(lockOrBurnIn); + + _burn(lockOrBurnIn.amount); + + emit Burned(msg.sender, lockOrBurnIn.amount); + + // LOCK_RELEASE_FLAG = bytes4(keccak256("NO_CCTP_USE_LOCK_RELEASE")) + return Pool.LockOrBurnOutV1({ + destTokenAddress: getRemoteToken(lockOrBurnIn.remoteChainSelector), + destPoolData: abi.encode(LOCK_RELEASE_FLAG) + }); + } +} diff --git a/contracts/src/v0.8/ccip/pools/USDC/HybridLockReleaseUSDCTokenPool.sol b/contracts/src/v0.8/ccip/pools/USDC/HybridLockReleaseUSDCTokenPool.sol index 3691ef1c7b6..ea3e8ce70f5 100644 --- a/contracts/src/v0.8/ccip/pools/USDC/HybridLockReleaseUSDCTokenPool.sol +++ b/contracts/src/v0.8/ccip/pools/USDC/HybridLockReleaseUSDCTokenPool.sol @@ -14,6 +14,9 @@ import {IERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/tok import {SafeERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/utils/SafeERC20.sol"; import {EnumerableSet} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableSet.sol"; +// bytes4(keccak256("NO_CCTP_USE_LOCK_RELEASE")) +bytes4 constant LOCK_RELEASE_FLAG = 0xfa7c07de; + /// @notice A token pool for USDC which uses CCTP for supported chains and Lock/Release for all others /// @dev The functionality from LockReleaseTokenPool.sol has been duplicated due to lack of compiler support for shared /// constructors between parents @@ -34,9 +37,6 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { error LanePausedForCCTPMigration(uint64 remoteChainSelector); error TokenLockingNotAllowedAfterMigration(uint64 remoteChainSelector); - /// bytes4(keccak256("NO_CTTP_USE_LOCK_RELEASE")) - bytes4 public constant LOCK_RELEASE_FLAG = 0xd43c7897; - /// @notice The address of the liquidity provider for a specific chain. /// 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 @@ -49,7 +49,7 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { address[] memory allowlist, address rmnProxy, address router - ) USDCTokenPool(tokenMessenger, token, allowlist, rmnProxy, router) USDCBridgeMigrator(address(token), router) {} + ) USDCTokenPool(tokenMessenger, token, allowlist, rmnProxy, router) USDCBridgeMigrator(address(token)) {} // ================================================================ // │ Incoming/Outgoing Mechanisms | @@ -99,7 +99,7 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { _validateReleaseOrMint(releaseOrMintIn); // Circle requires a supply-lock to prevent incoming messages once the migration process begins. - // This prevents new outgoing messages once the migration has begun to ensure any the procedure runs as expected + // This prevents new incoming messages once the migration has begun to ensure any the procedure runs as expected if (s_proposedUSDCMigrationChain == releaseOrMintIn.remoteChainSelector) { revert LanePausedForCCTPMigration(s_proposedUSDCMigrationChain); } @@ -114,7 +114,6 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { s_lockedTokensByChainSelector[releaseOrMintIn.remoteChainSelector] -= releaseOrMintIn.amount; } - // Release to the offRamp, which forwards it to the recipient getToken().safeTransfer(releaseOrMintIn.receiver, releaseOrMintIn.amount); emit Released(msg.sender, releaseOrMintIn.receiver, releaseOrMintIn.amount); @@ -173,6 +172,16 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { function provideLiquidity(uint64 remoteChainSelector, uint256 amount) external { if (s_liquidityProvider[remoteChainSelector] != msg.sender) revert TokenPool.Unauthorized(msg.sender); + // Prevent adding liquidity to a chain which has already been migrated + if (s_migratedChains.contains(remoteChainSelector)) { + revert TokenLockingNotAllowedAfterMigration(remoteChainSelector); + } + + // prevent adding liquidity to a chain which has been proposed for migration + if (remoteChainSelector == s_proposedUSDCMigrationChain) { + revert LanePausedForCCTPMigration(remoteChainSelector); + } + s_lockedTokensByChainSelector[remoteChainSelector] += amount; i_token.safeTransferFrom(msg.sender, address(this), amount); @@ -187,7 +196,7 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { /// withdrawn on this chain, otherwise a mismatch may occur between locked token balance and remote circulating supply /// which may block a potential future migration of the chain to CCTP. function withdrawLiquidity(uint64 remoteChainSelector, uint256 amount) external onlyOwner { - // Circle requires a supply-lock to prevent outgoing messages once the migration process begins. + // A supply-lock is required to prevent outgoing messages once the migration process begins. // This prevents new outgoing messages once the migration has begun to ensure any the procedure runs as expected if (remoteChainSelector == s_proposedUSDCMigrationChain) { revert LanePausedForCCTPMigration(remoteChainSelector); @@ -213,16 +222,10 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { /// @param from The address of the old pool. /// @param remoteChainSelector The chain for which liquidity is being transferred. function transferLiquidity(address from, uint64 remoteChainSelector) external onlyOwner { - // Prevent Liquidity Transfers when a migration is pending. This prevents requiring the new pool to manage - // token exclusions for edge-case messages and ensures that the migration is completed before any new liquidity - // is added to the pool. - if (HybridLockReleaseUSDCTokenPool(from).getCurrentProposedCCTPChainMigration() == remoteChainSelector) { - revert LanePausedForCCTPMigration(remoteChainSelector); - } - OwnerIsCreator(from).acceptOwnership(); - // Withdraw all available liquidity from the old pool. + // Withdraw all available liquidity from the old pool. No check is needed for pending migrations, as the old pool + // will revert if the migration has begun. uint256 withdrawAmount = HybridLockReleaseUSDCTokenPool(from).getLockedTokensForChain(remoteChainSelector); HybridLockReleaseUSDCTokenPool(from).withdrawLiquidity(remoteChainSelector, withdrawAmount); @@ -237,16 +240,17 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { /// @notice Return whether a lane should use the alternative L/R mechanism in the token pool. /// @param remoteChainSelector the remote chain the lane is interacting with - /// @return bool Return true if the alternative L/R mechanism should be used + /// @return bool Return true if the alternative L/R mechanism should be used, and is decided by the Owner function shouldUseLockRelease( uint64 remoteChainSelector ) public view virtual returns (bool) { return s_shouldUseLockRelease[remoteChainSelector]; } - /// @notice Updates Updates designations for chains on whether to use primary or alt mechanism on CCIP messages + /// @notice Updates designations for chains on whether to use primary or alt mechanism on CCIP messages /// @param removes A list of chain selectors to disable Lock-Release, and enforce BM - /// @param adds A list of chain selectors to enable LR instead of BM + /// @param adds A list of chain selectors to enable LR instead of BM. These chains must not have been migrated + /// to CCTP yet or the transaction will revert function updateChainSelectorMechanisms(uint64[] calldata removes, uint64[] calldata adds) external onlyOwner { for (uint256 i = 0; i < removes.length; ++i) { delete s_shouldUseLockRelease[removes[i]]; @@ -254,6 +258,10 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { } for (uint256 i = 0; i < adds.length; ++i) { + // Prevent enabling lock release on chains which have already been migrated + if (s_migratedChains.contains(adds[i])) { + revert TokenLockingNotAllowedAfterMigration(adds[i]); + } s_shouldUseLockRelease[adds[i]] = true; emit LockReleaseEnabled(adds[i]); } diff --git a/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol b/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol index 4737a833445..738d6972f3d 100644 --- a/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol +++ b/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol @@ -1,12 +1,11 @@ -pragma solidity ^0.8.24; +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity 0.8.24; import {OwnerIsCreator} from "../../../shared/access/OwnerIsCreator.sol"; import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol"; import {EnumerableSet} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils/structs/EnumerableSet.sol"; -import {Router} from "../../Router.sol"; - /// @notice Allows migration of a lane in a token pool from Lock/Release to CCTP supported Burn/Mint. Contract /// functionality is based on hard requirements defined by Circle to allow for future CCTP compatibility /// https://github.com/circlefin/stablecoin-evm/blob/master/doc/bridged_USDC_standard.md @@ -23,12 +22,10 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { error onlyCircle(); error ExistingMigrationProposal(); - error NoExistingMigrationProposal(); error NoMigrationProposalPending(); - error InvalidChainSelector(uint64 remoteChainSelector); + error InvalidChainSelector(); - IBurnMintERC20 internal immutable i_USDC; - Router internal immutable i_router; + IBurnMintERC20 private immutable i_USDC; address internal s_circleUSDCMigrator; uint64 internal s_proposedUSDCMigrationChain; @@ -38,9 +35,12 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { mapping(uint64 chainSelector => bool shouldUseLockRelease) internal s_shouldUseLockRelease; - constructor(address token, address router) { + EnumerableSet.UintSet internal s_migratedChains; + + constructor( + address token + ) { i_USDC = IBurnMintERC20(token); - i_router = Router(router); } /// @notice Burn USDC locked for a specific lane so that destination USDC can be converted from @@ -49,9 +49,9 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { /// @dev proposeCCTPMigration must be called first on an approved lane to execute properly. /// @dev This function signature should NEVER be overwritten, otherwise it will be unable to be called by /// circle to properly migrate USDC over to CCTP. - function burnLockedUSDC() public { + function burnLockedUSDC() external { if (msg.sender != s_circleUSDCMigrator) revert onlyCircle(); - if (s_proposedUSDCMigrationChain == 0) revert ExistingMigrationProposal(); + if (s_proposedUSDCMigrationChain == 0) revert NoMigrationProposalPending(); uint64 burnChainSelector = s_proposedUSDCMigrationChain; @@ -71,6 +71,8 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { // Disable L/R automatically on burned chain and enable CCTP delete s_shouldUseLockRelease[burnChainSelector]; + s_migratedChains.add(burnChainSelector); + emit CCTPMigrationExecuted(burnChainSelector, tokensToBurn); } @@ -86,6 +88,9 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { // Prevent overwriting existing migration proposals until the current one is finished if (s_proposedUSDCMigrationChain != 0) revert ExistingMigrationProposal(); + // Ensure that the chain is currently using lock/release and not CCTP + if (!s_shouldUseLockRelease[remoteChainSelector]) revert InvalidChainSelector(); + s_proposedUSDCMigrationChain = remoteChainSelector; emit CCTPMigrationProposed(remoteChainSelector); @@ -94,7 +99,7 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { /// @notice Cancel an existing proposal to migrate a lane to CCTP. /// @notice This function will revert if no proposal is currently in progress. function cancelExistingCCTPMigrationProposal() external onlyOwner { - if (s_proposedUSDCMigrationChain == 0) revert NoExistingMigrationProposal(); + if (s_proposedUSDCMigrationChain == 0) revert NoMigrationProposalPending(); uint64 currentProposalChainSelector = s_proposedUSDCMigrationChain; delete s_proposedUSDCMigrationChain; @@ -142,6 +147,8 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { /// @dev This function should ONLY be called on the home chain, where tokens are locked, NOT on the remote chain /// and strict scrutiny should be applied to ensure that the amount of tokens excluded is accurate. function excludeTokensFromBurn(uint64 remoteChainSelector, uint256 amount) external onlyOwner { + if (s_proposedUSDCMigrationChain != remoteChainSelector) revert NoMigrationProposalPending(); + s_tokensExcludedFromBurn[remoteChainSelector] += amount; uint256 burnableAmountAfterExclusion = @@ -156,7 +163,7 @@ abstract contract USDCBridgeMigrator is OwnerIsCreator { /// @return uint256 amount of tokens excluded from being burned in a CCTP-migration function getExcludedTokensByChain( uint64 remoteChainSelector - ) public view returns (uint256) { + ) external view returns (uint256) { return s_tokensExcludedFromBurn[remoteChainSelector]; } } diff --git a/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol b/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol new file mode 100644 index 00000000000..c9080a0e145 --- /dev/null +++ b/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity 0.8.24; + +import {Pool} from "../../libraries/Pool.sol"; +import {RateLimiter} from "../../libraries/RateLimiter.sol"; +import {TokenPool} from "../../pools/TokenPool.sol"; +import {BurnMintWithLockReleaseFlagTokenPool} from "../../pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol"; + +import {LOCK_RELEASE_FLAG} from "../../pools/USDC/HybridLockReleaseUSDCTokenPool.sol"; +import {BurnMintSetup} from "./BurnMintSetup.t.sol"; + +import {IERC20} from "../../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol"; + +contract BurnMintWithLockReleaseFlagTokenPoolSetup is BurnMintSetup { + BurnMintWithLockReleaseFlagTokenPool internal s_pool; + + function setUp() public virtual override { + BurnMintSetup.setUp(); + + s_pool = new BurnMintWithLockReleaseFlagTokenPool( + s_burnMintERC677, new address[](0), address(s_mockRMN), address(s_sourceRouter) + ); + s_burnMintERC677.grantMintAndBurnRoles(address(s_pool)); + + _applyChainUpdates(address(s_pool)); + } +} + +contract BurnMintWithLockReleaseFlagTokenPool_lockOrBurn is BurnMintWithLockReleaseFlagTokenPoolSetup { + function test_LockOrBurn_CorrectReturnData_Success() public { + uint256 burnAmount = 20_000e18; + + deal(address(s_burnMintERC677), address(s_pool), burnAmount); + assertEq(s_burnMintERC677.balanceOf(address(s_pool)), burnAmount); + + vm.startPrank(s_burnMintOnRamp); + + vm.expectEmit(); + emit RateLimiter.TokensConsumed(burnAmount); + + vm.expectEmit(); + emit IERC20.Transfer(address(s_pool), address(0), burnAmount); + + vm.expectEmit(); + emit TokenPool.Burned(address(s_burnMintOnRamp), burnAmount); + + bytes4 expectedSignature = bytes4(keccak256("burn(uint256)")); + vm.expectCall(address(s_burnMintERC677), abi.encodeWithSelector(expectedSignature, burnAmount)); + + Pool.LockOrBurnOutV1 memory lockOrBurnOut = s_pool.lockOrBurn( + Pool.LockOrBurnInV1({ + originalSender: OWNER, + receiver: bytes(""), + amount: burnAmount, + remoteChainSelector: DEST_CHAIN_SELECTOR, + localToken: address(s_burnMintERC677) + }) + ); + + assertEq(s_burnMintERC677.balanceOf(address(s_pool)), 0); + + assertEq(bytes4(lockOrBurnOut.destPoolData), LOCK_RELEASE_FLAG); + } +} diff --git a/contracts/src/v0.8/ccip/test/pools/HybridLockReleaseUSDCTokenPool.t.sol b/contracts/src/v0.8/ccip/test/pools/HybridLockReleaseUSDCTokenPool.t.sol index 73426129efd..6f0d447f4d5 100644 --- a/contracts/src/v0.8/ccip/test/pools/HybridLockReleaseUSDCTokenPool.t.sol +++ b/contracts/src/v0.8/ccip/test/pools/HybridLockReleaseUSDCTokenPool.t.sol @@ -14,6 +14,7 @@ import {RateLimiter} from "../../libraries/RateLimiter.sol"; import {TokenPool} from "../../pools/TokenPool.sol"; import {HybridLockReleaseUSDCTokenPool} from "../../pools/USDC/HybridLockReleaseUSDCTokenPool.sol"; +import {LOCK_RELEASE_FLAG} from "../../pools/USDC/HybridLockReleaseUSDCTokenPool.sol"; import {USDCBridgeMigrator} from "../../pools/USDC/USDCBridgeMigrator.sol"; import {USDCTokenPool} from "../../pools/USDC/USDCTokenPool.sol"; import {BaseTest} from "../BaseTest.t.sol"; @@ -226,7 +227,7 @@ contract HybridUSDCTokenPoolTests is USDCTokenPoolSetup { localToken: address(s_token), remoteChainSelector: SOURCE_CHAIN_SELECTOR, sourcePoolAddress: sourceTokenData.sourcePoolAddress, - sourcePoolData: abi.encode(s_usdcTokenPool.LOCK_RELEASE_FLAG()), + sourcePoolData: abi.encode(LOCK_RELEASE_FLAG), offchainTokenData: "" }) ); @@ -375,6 +376,12 @@ contract HybridUSDCTokenPoolTests is USDCTokenPoolSetup { } function test_LockOrBurn_WhileMigrationPause_Revert() public { + // Mark the destination chain as supporting CCTP, so use L/R instead. + uint64[] memory destChainAdds = new uint64[](1); + destChainAdds[0] = DEST_CHAIN_SELECTOR; + + s_usdcTokenPool.updateChainSelectorMechanisms(new uint64[](0), destChainAdds); + // Create a fake migration proposal s_usdcTokenPool.proposeCCTPMigration(DEST_CHAIN_SELECTOR); @@ -382,12 +389,6 @@ contract HybridUSDCTokenPoolTests is USDCTokenPoolSetup { bytes32 receiver = bytes32(uint256(uint160(STRANGER))); - // Mark the destination chain as supporting CCTP, so use L/R instead. - uint64[] memory destChainAdds = new uint64[](1); - destChainAdds[0] = DEST_CHAIN_SELECTOR; - - s_usdcTokenPool.updateChainSelectorMechanisms(new uint64[](0), destChainAdds); - assertTrue( s_usdcTokenPool.shouldUseLockRelease(DEST_CHAIN_SELECTOR), "Lock Release mech not configured for outgoing message to DEST_CHAIN_SELECTOR" @@ -444,7 +445,7 @@ contract HybridUSDCTokenPoolTests is USDCTokenPoolSetup { destGasAmount: USDC_DEST_TOKEN_GAS }); - bytes memory sourcePoolDataLockRelease = abi.encode(s_usdcTokenPool.LOCK_RELEASE_FLAG()); + bytes memory sourcePoolDataLockRelease = abi.encode(LOCK_RELEASE_FLAG); uint256 amount = 1e6; @@ -529,6 +530,12 @@ contract HybridUSDCTokenPoolTests is USDCTokenPoolSetup { // Set as the OWNER so we can provide liquidity vm.startPrank(OWNER); + // Mark the destination chain as supporting CCTP, so use L/R instead. + uint64[] memory destChainAdds = new uint64[](1); + destChainAdds[0] = DEST_CHAIN_SELECTOR; + + s_usdcTokenPool.updateChainSelectorMechanisms(new uint64[](0), destChainAdds); + s_usdcTokenPool.setLiquidityProvider(DEST_CHAIN_SELECTOR, OWNER); s_token.approve(address(s_usdcTokenPool), type(uint256).max); @@ -645,6 +652,12 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { function test_cancelExistingCCTPMigrationProposal() public { vm.startPrank(OWNER); + // Mark the destination chain as supporting CCTP, so use L/R instead. + uint64[] memory destChainAdds = new uint64[](1); + destChainAdds[0] = DEST_CHAIN_SELECTOR; + + s_usdcTokenPool.updateChainSelectorMechanisms(new uint64[](0), destChainAdds); + vm.expectEmit(); emit USDCBridgeMigrator.CCTPMigrationProposed(DEST_CHAIN_SELECTOR); @@ -667,7 +680,7 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { "migration proposal exists, but shouldn't after being cancelled" ); - vm.expectRevert(USDCBridgeMigrator.NoExistingMigrationProposal.selector); + vm.expectRevert(USDCBridgeMigrator.NoMigrationProposalPending.selector); s_usdcTokenPool.cancelExistingCCTPMigrationProposal(); } @@ -686,7 +699,7 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { vm.startPrank(CIRCLE); - vm.expectRevert(abi.encodeWithSelector(USDCBridgeMigrator.ExistingMigrationProposal.selector)); + vm.expectRevert(abi.encodeWithSelector(USDCBridgeMigrator.NoMigrationProposalPending.selector)); s_usdcTokenPool.burnLockedUSDC(); } @@ -702,7 +715,7 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { } function test_cannotCancelANonExistentMigrationProposal() public { - vm.expectRevert(USDCBridgeMigrator.NoExistingMigrationProposal.selector); + vm.expectRevert(USDCBridgeMigrator.NoMigrationProposalPending.selector); // Proposal to migrate doesn't exist, and so the chain selector is zero, and therefore should revert s_usdcTokenPool.cancelExistingCCTPMigrationProposal(); @@ -747,7 +760,7 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { localToken: address(s_token), remoteChainSelector: SOURCE_CHAIN_SELECTOR, sourcePoolAddress: sourceTokenData.sourcePoolAddress, - sourcePoolData: abi.encode(s_usdcTokenPool.LOCK_RELEASE_FLAG()), + sourcePoolData: abi.encode(LOCK_RELEASE_FLAG), offchainTokenData: "" }) ); @@ -786,7 +799,7 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { // Mark the destination chain as supporting CCTP, so use L/R instead. uint64[] memory destChainAdds = new uint64[](1); - destChainAdds[0] = DEST_CHAIN_SELECTOR; + destChainAdds[0] = SOURCE_CHAIN_SELECTOR; s_usdcTokenPool.updateChainSelectorMechanisms(new uint64[](0), destChainAdds); @@ -807,6 +820,8 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { // since there's no corresponding attestation to use for minting. vm.startPrank(OWNER); + s_usdcTokenPool.proposeCCTPMigration(SOURCE_CHAIN_SELECTOR); + // Exclude the tokens from being burned and check for the event vm.expectEmit(); emit USDCBridgeMigrator.TokensExcludedFromBurn(SOURCE_CHAIN_SELECTOR, amount, (amount * 3) - amount); @@ -827,8 +842,6 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { s_usdcTokenPool.setCircleMigratorAddress(CIRCLE); - s_usdcTokenPool.proposeCCTPMigration(SOURCE_CHAIN_SELECTOR); - vm.startPrank(CIRCLE); s_usdcTokenPool.burnLockedUSDC(); @@ -866,7 +879,7 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { localToken: address(s_token), remoteChainSelector: SOURCE_CHAIN_SELECTOR, sourcePoolAddress: sourceTokenData.sourcePoolAddress, - sourcePoolData: abi.encode(s_usdcTokenPool.LOCK_RELEASE_FLAG()), + sourcePoolData: abi.encode(LOCK_RELEASE_FLAG), offchainTokenData: "" }) ); @@ -883,4 +896,69 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { // We also want to check that the system uses CCTP Burn/Mint for all other messages that don't have that flag test_MintOrRelease_incomingMessageWithPrimaryMechanism(); } + + function test_ProposeMigration_ChainNotUsingLockRelease_Revert() public { + vm.expectRevert(abi.encodeWithSelector(USDCBridgeMigrator.InvalidChainSelector.selector)); + + vm.startPrank(OWNER); + + s_usdcTokenPool.proposeCCTPMigration(0x98765); + } + + function test_excludeTokensWhenNoMigrationProposalPending_Revert() public { + vm.expectRevert(abi.encodeWithSelector(USDCBridgeMigrator.NoMigrationProposalPending.selector)); + + vm.startPrank(OWNER); + + s_usdcTokenPool.excludeTokensFromBurn(SOURCE_CHAIN_SELECTOR, 1e6); + } + + function test_cannotProvideLiquidityWhenMigrationProposalPending_Revert() public { + vm.startPrank(OWNER); + + // Mark the destination chain as supporting CCTP, so use L/R instead. + uint64[] memory destChainAdds = new uint64[](1); + destChainAdds[0] = DEST_CHAIN_SELECTOR; + + s_usdcTokenPool.updateChainSelectorMechanisms(new uint64[](0), destChainAdds); + + s_usdcTokenPool.proposeCCTPMigration(DEST_CHAIN_SELECTOR); + + vm.expectRevert( + abi.encodeWithSelector(HybridLockReleaseUSDCTokenPool.LanePausedForCCTPMigration.selector, DEST_CHAIN_SELECTOR) + ); + s_usdcTokenPool.provideLiquidity(DEST_CHAIN_SELECTOR, 1e6); + } + + function test_cannotRevertChainMechanism_afterMigration_Revert() public { + test_lockOrBurn_then_BurnInCCTPMigration_Success(); + + vm.startPrank(OWNER); + + // Mark the destination chain as supporting CCTP, so use L/R instead. + uint64[] memory destChainAdds = new uint64[](1); + destChainAdds[0] = DEST_CHAIN_SELECTOR; + + vm.expectRevert( + abi.encodeWithSelector( + HybridLockReleaseUSDCTokenPool.TokenLockingNotAllowedAfterMigration.selector, DEST_CHAIN_SELECTOR + ) + ); + + s_usdcTokenPool.updateChainSelectorMechanisms(new uint64[](0), destChainAdds); + } + + function test_cnanotProvideLiquidity_AfterMigration_Revert() public { + test_lockOrBurn_then_BurnInCCTPMigration_Success(); + + vm.startPrank(OWNER); + + vm.expectRevert( + abi.encodeWithSelector( + HybridLockReleaseUSDCTokenPool.TokenLockingNotAllowedAfterMigration.selector, DEST_CHAIN_SELECTOR + ) + ); + + s_usdcTokenPool.provideLiquidity(DEST_CHAIN_SELECTOR, 1e6); + } }