CCTP V2 Token Pool Bug Fixes (#1331)

* bug fixes initial

* better formatting, comments, and wrappers

* comment fix

* modify ITokenMessenger getMinFeeAmount() function

* nit fixes

* change abi.encode() to abi.encodePacked()

* remove comment

* formatting fix

* fix errant test

---------

Co-authored-by: Suryansh <[email protected]>

Author: jhweintraub

chains/evm/.changeset/large-places-pick.md

@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts-ccip": patch
+---
+
+Bugfixes for CCTP V2 Contracts

chains/evm/.gas-snapshot

@@ -14,7 +14,7 @@ BurnMintTokenPool_lockOrBurn:test_lockOrBurn_() (gas: 243286)
 BurnMintTokenPool_releaseOrMint:test_PoolMint() (gas: 105278)
 BurnMintWithLockReleaseFlagTokenPool_lockOrBurn:test_LockOrBurn_CorrectReturnData() (gas: 243807)
 BurnMintWithLockReleaseFlagTokenPool_releaseOrMint:test_releaseOrMint_EmptySourcePoolData() (gas: 104725)
-BurnMintWithLockReleaseFlagTokenPool_releaseOrMint:test_releaseOrMint_LockReleaseFlagInSourcePoolData() (gas: 104702)
+BurnMintWithLockReleaseFlagTokenPool_releaseOrMint:test_releaseOrMint_LockReleaseFlagInSourcePoolData() (gas: 104699)
 BurnToAddressMintTokenPool_lockOrBurn:test_LockOrBurn() (gas: 241000)
 BurnWithFromMintTokenPool_lockOrBurn:test_constructor() (gas: 23404)
 BurnWithFromMintTokenPool_lockOrBurn:test_lockOrBurn() (gas: 245593)
@@ -488,16 +488,16 @@ SiloedLockReleaseTokenPool_setRebalancer:test_setSiloRebalancer() (gas: 28778)
 SiloedLockReleaseTokenPool_updateSiloDesignations:test_updateSiloDesignations() (gas: 183879)
 SiloedLockReleaseTokenPool_withdrawLiqudity:test_withdrawLiquidity_SiloedFunds() (gas: 110516)
 SiloedLockReleaseTokenPool_withdrawLiqudity:test_withdrawLiquidity_UnsiloedFunds_LegacyFunctionSelector() (gas: 112193)
-SiloedUSDCTokenPool_burnLockedUSDC:test_burnLockedUSDC() (gas: 414564)
+SiloedUSDCTokenPool_burnLockedUSDC:test_burnLockedUSDC() (gas: 414741)
 SiloedUSDCTokenPool_cancelExistingCCTPMigrationProposal:test_cancelExistingCCTPMigrationProposal() (gas: 30532)
 SiloedUSDCTokenPool_cancelExistingCCTPMigrationProposal:test_cancelExistingCCTPMigrationProposal_EmitsEvent() (gas: 29852)
 SiloedUSDCTokenPool_cancelExistingCCTPMigrationProposal:test_cancelExistingCCTPMigrationProposal_ResetsExcludedTokens() (gas: 185948)
 SiloedUSDCTokenPool_excludeTokensFromBurn:test_excludeTokensFromBurn() (gas: 192396)
 SiloedUSDCTokenPool_excludeTokensFromBurn:test_excludeTokensFromBurn_EmitsEvent() (gas: 192244)
-SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_MultipleLocks() (gas: 161173)
-SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_Success() (gas: 135564)
-SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_UpdatesLockedTokensAccounting() (gas: 132654)
-SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_UpdatesSiloedTokensAccounting() (gas: 134596)
+SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_MultipleLocks() (gas: 161615)
+SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_Success() (gas: 135785)
+SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_UpdatesLockedTokensAccounting() (gas: 132875)
+SiloedUSDCTokenPool_lockOrBurn:test_lockOrBurn_UpdatesSiloedTokensAccounting() (gas: 134817)
 SiloedUSDCTokenPool_proposeCCTPMigration:test_proposeCCTPMigration() (gas: 37970)
 SiloedUSDCTokenPool_proposeCCTPMigration:test_proposeCCTPMigration_AfterCancellation() (gas: 50568)
 SiloedUSDCTokenPool_releaseOrMint:test_releaseOrMint_SubtractsFromExcludedTokens() (gas: 352087)
@@ -550,11 +550,12 @@ TokenPool_validateReleaseOrMint:test_validateReleaseOrMint_Success() (gas: 39621
 USDCSourcePoolDataCodec__calculateDepositHash:test__calculateDepositHash() (gas: 9917)
 USDCSourcePoolDataCodec__decodeSourceTokenDataPayloadV2:test__decodeSourceTokenDataPayloadV2_CCTPV2() (gas: 5161)
 USDCSourcePoolDataCodec__decodeSourceTokenDataPayloadV2:test__decodeSourceTokenDataPayloadV2_CCTPV2CCV() (gas: 4916)
-USDCTokenPoolCCTPV2_constructor:test_constructor() (gas: 3429212)
-USDCTokenPoolCCTPV2_lockOrBurn:test_lockOrBurn() (gas: 129326)
-USDCTokenPoolCCTPV2_lockOrBurn:test_lockOrBurn_MintRecipientOverride() (gas: 156828)
+USDCTokenPoolCCTPV2_constructor:test_constructor() (gas: 3470365)
+USDCTokenPoolCCTPV2_lockOrBurn:test_lockOrBurn() (gas: 129947)
+USDCTokenPoolCCTPV2_lockOrBurn:test_lockOrBurn_MinFeeNotSupported() (gas: 114123)
+USDCTokenPoolCCTPV2_lockOrBurn:test_lockOrBurn_MintRecipientOverride() (gas: 157604)
 USDCTokenPoolCCTPV2_releaseOrMint:test_releaseOrMint_RealTx() (gas: 265533)
-USDCTokenPoolProxy_constructor:test_constructor() (gas: 1816306)
+USDCTokenPoolProxy_constructor:test_constructor() (gas: 1823082)
 USDCTokenPoolProxy_generateNewReleaseOrMintIn:test_generateNewReleaseOrMintIn() (gas: 35218)
 USDCTokenPoolProxy_lockOrBurn:test_lockOrBurn_CCTPV1() (gas: 104790)
 USDCTokenPoolProxy_lockOrBurn:test_lockOrBurn_CCTPV2() (gas: 94306)
@@ -571,9 +572,9 @@ USDCTokenPoolProxy_updateLockReleasePoolAddresses:test_updateLockReleasePoolAddr
 USDCTokenPoolProxy_updateLockReleasePoolAddresses:test_updateLockReleasePoolAddresses_Single() (gas: 52529)
 USDCTokenPoolProxy_updateLockReleasePoolAddresses:test_updateLockReleasePoolAddresses_ZeroAddress() (gas: 25431)
 USDCTokenPoolProxy_updatePoolAddresses:test_updatePoolAddresses() (gas: 55499)
-USDCTokenPool_constructor:test_constructor() (gas: 3377431)
-USDCTokenPool_lockOrBurn:test_LockOrBurn() (gas: 130827)
-USDCTokenPool_lockOrBurn:test_LockOrBurn_LegacySourcePoolDataFormat() (gas: 143734)
-USDCTokenPool_lockOrBurn:test_LockOrBurn_MintRecipientOverride() (gas: 159276)
+USDCTokenPool_constructor:test_constructor() (gas: 3377453)
+USDCTokenPool_lockOrBurn:test_LockOrBurn() (gas: 130897)
+USDCTokenPool_lockOrBurn:test_LockOrBurn_LegacySourcePoolDataFormat() (gas: 143822)
+USDCTokenPool_lockOrBurn:test_LockOrBurn_MintRecipientOverride() (gas: 159364)
 USDCTokenPool_releaseOrMint:test_ReleaseOrMintRealTx() (gas: 265775)
 USDCTokenPool_supportsInterface:test_SupportsInterface() (gas: 9990)

chains/evm/contracts/pools/USDC/SiloedUSDCTokenPool.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.24;
 
 import {Pool} from "../../libraries/Pool.sol";
+import {USDCSourcePoolDataCodec} from "../../libraries/USDCSourcePoolDataCodec.sol";
 import {SiloedLockReleaseTokenPool} from "../SiloedLockReleaseTokenPool.sol";
 
 import {AuthorizedCallers} from "@chainlink/contracts/src/v0.8/shared/access/AuthorizedCallers.sol";
@@ -90,7 +91,9 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers {
     // No excluded tokens is the common path, as it means no migration has occured yet, and any released
     // tokens should come from the stored token balance of previously deposited tokens.
     if (excludedTokens == 0) {
-      if (localAmount > remoteConfig.tokenBalance) revert InsufficientLiquidity(remoteConfig.tokenBalance, localAmount);
+      if (localAmount > remoteConfig.tokenBalance) {
+        revert InsufficientLiquidity(remoteConfig.tokenBalance, localAmount);
+      }
 
       remoteConfig.tokenBalance -= localAmount;
 
@@ -115,6 +118,22 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers {
     return Pool.ReleaseOrMintOutV1({destinationAmount: localAmount});
   }
 
+  /// @inheritdoc SiloedLockReleaseTokenPool
+  /// @dev This function is overridden to encode the LOCK_RELEASE_FLAG into the destPoolData, as the destination pool.
+  /// will be a BurnMintWithLockReleaseFlagTokenPool and may need to be processed by a proxy first.
+  function lockOrBurn(
+    Pool.LockOrBurnInV1 calldata lockOrBurnIn
+  ) public virtual override returns (Pool.LockOrBurnOutV1 memory) {
+    // Call the parent contract's lockOrBurn function to get the base output. All functionality of this child
+    // is inherited from the parent contract except for the overwritten destPoolData.
+    Pool.LockOrBurnOutV1 memory baseOutput = super.lockOrBurn(lockOrBurnIn);
+
+    // Encode the LOCK_RELEASE_FLAG into the destPoolData using encodePacked to save space.
+    baseOutput.destPoolData = abi.encodePacked(USDCSourcePoolDataCodec.LOCK_RELEASE_FLAG);
+
+    return baseOutput;
+  }
+
   /// @dev This function is overridden to prevent providing liquidity to a chain that has already been migrated, and thus should use CCTP-proper instead of a Lock/Release mechanism.
   function _provideLiquidity(uint64 remoteChainSelector, uint256 amount) internal override {
     if (s_migratedChains.contains(remoteChainSelector)) {

chains/evm/contracts/pools/USDC/USDCTokenPoolCCTPV2.sol

@@ -22,6 +22,7 @@ contract USDCTokenPoolCCTPV2 is USDCTokenPool {
   error InvalidExecutionFinalityThreshold(uint32 expected, uint32 got);
   error InvalidDepositHash(bytes32 expected, bytes32 got);
   error InvalidBurnToken(address expected, address got);
+  error InvalidMinFee(uint256 maxAcceptableFee, uint256 actualFee);
 
   /// @dev CCTP's max fee is based on the use of fast-burn. Since this pool does not utilize that feature, max fee should be 0.
   uint32 public constant MAX_FEE = 0;
@@ -64,6 +65,18 @@ contract USDCTokenPoolCCTPV2 is USDCTokenPool {
       revert InvalidReceiver(lockOrBurnIn.receiver);
     }
 
+    // Some CCTP-V2 chains support a configurable fee switch, but not all. It is therefore
+    // necessary to check via a try-catch block. If the call reverts, then the fee switch is not supported and the
+    // standard transfer fee will be zero, and no further action is required.
+    try i_tokenMessenger.getMinFeeAmount(lockOrBurnIn.amount) returns (uint256 minFee) {
+      // This token pool only supports zero-fee standard transfers. If the minFee is non-zero
+      // then the function should revert as the message may not be able to be successfully
+      // delivered on destination due to unexpected minting fees.
+      if (minFee > MAX_FEE) {
+        revert InvalidMinFee(MAX_FEE, minFee);
+      }
+    } catch {}
+
     bytes32 decodedReceiver;
     // For EVM chains, the mintRecipient is not used, but is needed for Solana, where the mintRecipient will
     // be a PDA owned by the pool, and will forward the tokens to its final destination after minting.
@@ -166,8 +179,8 @@ contract USDCTokenPoolCCTPV2 is USDCTokenPool {
   ///     * sender                     32         bytes32   44
   ///     * recipient                  32         bytes32   76
   ///     * destinationCaller          32         bytes32   108
-  ///     * minFinalityThreshold       32         uint32    140
-  ///     * finalityThresholdExecuted  32         uint32    144
+  ///     * minFinalityThreshold       4         uint32     140
+  ///     * finalityThresholdExecuted  4         uint32     144
   ///     * messageBody                dynamic    bytes     148
 
   /// @dev Message Body for USDC.

chains/evm/contracts/pools/USDC/USDCTokenPoolProxy.sol

@@ -146,7 +146,8 @@ contract USDCTokenPoolProxy is Ownable2StepMsgSender, IPoolV1, ITypeAndVersion {
   function supportsInterface(
     bytes4 interfaceId
   ) public pure override returns (bool) {
-    return interfaceId == type(IPoolV1).interfaceId || interfaceId == type(IERC165).interfaceId;
+    return interfaceId == type(IPoolV1).interfaceId || interfaceId == Pool.CCIP_POOL_V1
+      || interfaceId == type(IERC165).interfaceId;
   }
 
   /// @inheritdoc IPoolV1

chains/evm/contracts/pools/USDC/interfaces/ITokenMessenger.sol

@@ -108,4 +108,11 @@ interface ITokenMessenger {
   /// to/from remote domainsmessage transmitter for this token messenger.
   /// @dev immutable
   function localMessageTransmitter() external view returns (address);
+
+  /// Returns the minimum fee required for a deposit for burn message.
+  /// @dev This function is only available for CCTP V2, and not every CCTP-V2 compatible
+  /// chain supports this configurable fee switch.
+  function getMinFeeAmount(
+    uint256 amount
+  ) external view returns (uint256);
 }

chains/evm/contracts/test/mocks/MockE2EUSDCTokenMessenger.sol

@@ -123,6 +123,13 @@ contract MockE2EUSDCTokenMessenger is ITokenMessenger {
     return i_transmitter;
   }
 
+  /// @dev This function is only available for CCTP V2
+  function getMinFeeAmount(
+    uint256
+  ) external pure returns (uint256) {
+    return 0;
+  }
+
   /**
    * @notice Sends a BurnMessage through the local message transmitter
    * @dev calls local message transmitter's sendMessage() function if `_destinationCaller` == bytes32(0),

chains/evm/contracts/test/mocks/MockUSDCTokenMessenger.sol

@@ -75,4 +75,11 @@ contract MockUSDCTokenMessenger is ITokenMessenger {
   function localMessageTransmitter() external view returns (address) {
     return i_transmitter;
   }
+
+  /// @dev This function is only available for CCTP V2
+  function getMinFeeAmount(
+    uint256
+  ) external pure returns (uint256) {
+    return 0;
+  }
 }

chains/evm/contracts/test/pools/BurnMintWithLockReleaseFlagTokenPool/BurnMintWithLockReleaseFlagTokenPool.releaseOrMint.t.sol

@@ -25,7 +25,7 @@ contract BurnMintWithLockReleaseFlagTokenPool_releaseOrMint is BurnMintWithLockR
         localToken: address(s_token),
         remoteChainSelector: DEST_CHAIN_SELECTOR,
         sourcePoolAddress: abi.encode(s_initialRemotePool),
-        sourcePoolData: abi.encode(LOCK_RELEASE_FLAG),
+        sourcePoolData: abi.encodePacked(LOCK_RELEASE_FLAG),
         offchainTokenData: ""
       })
     );

chains/evm/contracts/test/pools/USDC/SiloedUSDCTokenPool/SiloedUSDCTokenPool.lockOrBurn.t.sol

@@ -47,8 +47,7 @@ contract SiloedUSDCTokenPool_lockOrBurn is SiloedUSDCTokenPoolSetup {
     // Assert: Verify the result
     assertEq(s_usdcTokenPool.getAvailableTokens(DEST_CHAIN_SELECTOR), s_amount);
 
-    // destPoolData is the local token decimals abi-encoded to 32 bytes
-    assertEq(result.destPoolData.length, 32);
+    assertEq(result.destPoolData.length, 4);
     vm.stopPrank();
   }
 
@@ -71,8 +70,7 @@ contract SiloedUSDCTokenPool_lockOrBurn is SiloedUSDCTokenPoolSetup {
     // Assert: Verify the locked tokens accounting is updated
     assertEq(s_usdcTokenPool.getAvailableTokens(DEST_CHAIN_SELECTOR), s_amount);
 
-    // destPoolData is the local token decimals abi-encoded to 32 bytes
-    assertEq(result.destPoolData.length, 32);
+    assertEq(result.destPoolData.length, 4);
     vm.stopPrank();
   }
 
@@ -106,9 +104,8 @@ contract SiloedUSDCTokenPool_lockOrBurn is SiloedUSDCTokenPoolSetup {
     // Assert: Verify the locked tokens accounting is updated correctly
     assertEq(s_usdcTokenPool.getAvailableTokens(DEST_CHAIN_SELECTOR), s_amount + amount2);
 
-    // destPoolData is the local token decimals abi-encoded to 32 bytes
-    assertEq(result1.destPoolData.length, 32);
-    assertEq(result2.destPoolData.length, 32);
+    assertEq(result1.destPoolData.length, 4);
+    assertEq(result2.destPoolData.length, 4);
     vm.stopPrank();
   }
 
@@ -128,7 +125,7 @@ contract SiloedUSDCTokenPool_lockOrBurn is SiloedUSDCTokenPoolSetup {
     assertEq(s_usdcTokenPool.getAvailableTokens(DEST_CHAIN_SELECTOR), s_amount);
     assertTrue(s_usdcTokenPool.isSiloed(DEST_CHAIN_SELECTOR));
 
-    assertEq(result.destPoolData.length, 32);
+    assertEq(result.destPoolData.length, 4);
     vm.stopPrank();
   }