Skip to content

Commit

Permalink
add additional safety checks and add new token pool type
Browse files Browse the repository at this point in the history
  • Loading branch information
jhweintraub committed Sep 23, 2024
1 parent 2a08e17 commit 72a0c6f
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -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)
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 9 additions & 5 deletions contracts/src/v0.8/ccip/pools/USDC/USDCBridgeMigrator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -373,19 +373,19 @@ 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);

assertEq(s_usdcTokenPool.getCurrentProposedCCTPChainMigration(), DEST_CHAIN_SELECTOR);

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"
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand All @@ -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();
}

Expand All @@ -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();
}

Expand All @@ -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();
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand All @@ -825,8 +839,6 @@ contract HybridUSDCTokenPoolMigrationTests is HybridUSDCTokenPoolTests {

s_usdcTokenPool.setCircleMigratorAddress(CIRCLE);

s_usdcTokenPool.proposeCCTPMigration(SOURCE_CHAIN_SELECTOR);

vm.startPrank(CIRCLE);

s_usdcTokenPool.burnLockedUSDC();
Expand Down Expand Up @@ -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);
}
}

0 comments on commit 72a0c6f

Please sign in to comment.