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 0000000000..3e33fccc63 --- /dev/null +++ b/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.8.0; + +import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol"; + +import {Pool} from "../../libraries/Pool.sol"; +import {BurnMintTokenPool} from "../BurnMintTokenPool.sol"; + +contract BurnMintWithLockReleaseFlagTokenPool is BurnMintTokenPool { + /// bytes4(keccak256("NO_CCTP_USE_LOCK_RELEASE")) + bytes4 public constant LOCK_RELEASE_FLAG = 0xfa7c07de; + + 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 + function lockOrBurn( + Pool.LockOrBurnInV1 calldata lockOrBurnIn + ) external virtual override returns (Pool.LockOrBurnOutV1 memory) { + _validateLockOrBurn(lockOrBurnIn); + + _burn(lockOrBurnIn.amount); + + emit Burned(msg.sender, lockOrBurnIn.amount); + + 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 5fbd71bb2c..8eaa2463cd 100644 --- a/contracts/src/v0.8/ccip/pools/USDC/HybridLockReleaseUSDCTokenPool.sol +++ b/contracts/src/v0.8/ccip/pools/USDC/HybridLockReleaseUSDCTokenPool.sol @@ -34,8 +34,8 @@ 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; + /// bytes4(keccak256("NO_CCTP_USE_LOCK_RELEASE")) + bytes4 public constant LOCK_RELEASE_FLAG = 0xfa7c07de; /// @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, @@ -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); @@ -171,6 +170,10 @@ contract HybridLockReleaseUSDCTokenPool is USDCTokenPool, USDCBridgeMigrator { function provideLiquidity(uint64 remoteChainSelector, uint256 amount) external { if (s_liquidityProvider[remoteChainSelector] != msg.sender) revert TokenPool.Unauthorized(msg.sender); + if (remoteChainSelector == s_proposedUSDCMigrationChain) { + revert LanePausedForCCTPMigration(remoteChainSelector); + } + s_lockedTokensByChainSelector[remoteChainSelector] += amount; i_token.safeTransferFrom(msg.sender, address(this), amount); @@ -211,13 +214,6 @@ 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. diff --git a/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol b/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol index 98616cd79f..41077eb4ec 100644 --- a/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol +++ b/contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol @@ -23,9 +23,8 @@ 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; @@ -49,9 +48,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; @@ -84,6 +83,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); @@ -92,7 +94,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; @@ -136,6 +138,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 = 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 0000000000..88f2075425 --- /dev/null +++ b/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol @@ -0,0 +1,70 @@ +// 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 {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_Setup_Success() public view { + assertEq(address(s_burnMintERC677), address(s_pool.getToken())); + assertEq(address(s_mockRMN), s_pool.getRmnProxy()); + assertEq(false, s_pool.getAllowListEnabled()); + assertEq("BurnMintTokenPool 1.5.0", s_pool.typeAndVersion()); + } + + function test_PoolBurn_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), s_pool.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 ddb8c29dc2..906ff2b113 100644 --- a/contracts/src/v0.8/ccip/test/pools/HybridLockReleaseUSDCTokenPool.t.sol +++ b/contracts/src/v0.8/ccip/test/pools/HybridLockReleaseUSDCTokenPool.t.sol @@ -373,6 +373,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); @@ -380,12 +386,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" @@ -527,6 +527,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); @@ -643,6 +649,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); @@ -665,7 +677,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(); } @@ -684,7 +696,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(); } @@ -700,7 +712,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(); @@ -784,7 +796,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); @@ -805,6 +817,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); @@ -825,8 +839,6 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests { s_usdcTokenPool.setCircleMigratorAddress(CIRCLE); - s_usdcTokenPool.proposeCCTPMigration(SOURCE_CHAIN_SELECTOR); - vm.startPrank(CIRCLE); s_usdcTokenPool.burnLockedUSDC(); @@ -881,4 +893,37 @@ 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); + } }