From 9802bc0052b41269f273cb63a2b05d9aa469331e Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 22 Oct 2024 13:04:40 -0400 Subject: [PATCH] update BurnMintWithLockReleaseFlag to import the flag instead of redefining it for safety --- contracts/gas-snapshots/ccip.gas-snapshot | 64 +++++++++---------- .../BurnMintWithLockReleaseFlagTokenPool.sol | 7 +- .../USDC/HybridLockReleaseUSDCTokenPool.sol | 6 +- ...BurnMintWithLockReleaseFlagTokenPool.t.sol | 4 +- .../HybridLockReleaseUSDCTokenPool.t.sol | 9 +-- 5 files changed, 47 insertions(+), 43 deletions(-) diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot index 371bd6fda2..3695e6d458 100644 --- a/contracts/gas-snapshots/ccip.gas-snapshot +++ b/contracts/gas-snapshots/ccip.gas-snapshot @@ -30,7 +30,7 @@ BurnMintTokenPool_lockOrBurn:test_Setup_Success() (gas: 17851) BurnMintTokenPool_releaseOrMint:test_ChainNotAllowed_Revert() (gas: 28805) BurnMintTokenPool_releaseOrMint:test_PoolMintNotHealthy_Revert() (gas: 56253) BurnMintTokenPool_releaseOrMint:test_PoolMint_Success() (gas: 112391) -BurnMintWithLockReleaseFlagTokenPool_lockOrBurn:test_PoolBurn_CorrectReturnData_Success() (gas: 242931) +BurnMintWithLockReleaseFlagTokenPool_lockOrBurn:test_PoolBurn_CorrectReturnData_Success() (gas: 242252) BurnWithFromMintTokenPool_lockOrBurn:test_ChainNotAllowed_Revert() (gas: 28842) BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurnRevertNotHealthy_Revert() (gas: 55271) BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurn_Success() (gas: 244050) @@ -425,38 +425,38 @@ FeeQuoter_validateDestFamilyAddress:test_InvalidEVMAddressPrecompiles_Revert() ( FeeQuoter_validateDestFamilyAddress:test_InvalidEVMAddress_Revert() (gas: 10839) FeeQuoter_validateDestFamilyAddress:test_ValidEVMAddress_Success() (gas: 6731) FeeQuoter_validateDestFamilyAddress:test_ValidNonEVMAddress_Success() (gas: 6511) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209230) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135861) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_WhileMigrationPause_Revert() (gas: 109814) -HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_onLockReleaseMechanism_Success() (gas: 147081) -HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_OnLockReleaseMechanism_Success() (gas: 217718) -HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_OnLockReleaseMechanism_then_switchToPrimary_Success() (gas: 426564) -HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268928) -HybridUSDCTokenPoolMigrationTests:test_ProposeMigration_ChainNotUsingLockRelease_Revert() (gas: 15821) -HybridUSDCTokenPoolMigrationTests:test_ReleaseOrMint_WhileMigrationPause_Revert() (gas: 114166) -HybridUSDCTokenPoolMigrationTests:test_burnLockedUSDC_invalidPermissions_Revert() (gas: 39362) -HybridUSDCTokenPoolMigrationTests:test_cancelExistingCCTPMigrationProposal() (gas: 56207) -HybridUSDCTokenPoolMigrationTests:test_cannotCancelANonExistentMigrationProposal() (gas: 12736) -HybridUSDCTokenPoolMigrationTests:test_cannotModifyLiquidityWithoutPermissions_Revert() (gas: 13373) -HybridUSDCTokenPoolMigrationTests:test_cannotProvideLiquidityWhenMigrationProposalPending_Revert() (gas: 67304) -HybridUSDCTokenPoolMigrationTests:test_cannotRevertChainMechanism_afterMigration_Revert() (gas: 313390) -HybridUSDCTokenPoolMigrationTests:test_cannotTransferLiquidityDuringPendingMigration_Revert() (gas: 177033) -HybridUSDCTokenPoolMigrationTests:test_cnanotProvideLiquidity_AfterMigration_Revert() (gas: 313775) +HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209283) +HybridUSDCTokenPoolMigrationTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135879) +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: 425913) +HybridUSDCTokenPoolMigrationTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268945) +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: 309952) -HybridUSDCTokenPoolMigrationTests:test_transferLiquidity_Success() (gas: 167124) -HybridUSDCTokenPoolMigrationTests:test_unstickManualTxAfterMigration_destChain_Success() (gas: 156736) -HybridUSDCTokenPoolMigrationTests:test_unstickManualTxAfterMigration_homeChain_Success() (gas: 516552) -HybridUSDCTokenPoolTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209230) -HybridUSDCTokenPoolTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135880) -HybridUSDCTokenPoolTests:test_LockOrBurn_WhileMigrationPause_Revert() (gas: 109814) -HybridUSDCTokenPoolTests:test_LockOrBurn_onLockReleaseMechanism_Success() (gas: 147079) -HybridUSDCTokenPoolTests:test_MintOrRelease_OnLockReleaseMechanism_Success() (gas: 217696) -HybridUSDCTokenPoolTests:test_MintOrRelease_OnLockReleaseMechanism_then_switchToPrimary_Success() (gas: 426520) -HybridUSDCTokenPoolTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268910) -HybridUSDCTokenPoolTests:test_ReleaseOrMint_WhileMigrationPause_Revert() (gas: 114210) -HybridUSDCTokenPoolTests:test_cannotTransferLiquidityDuringPendingMigration_Revert() (gas: 176989) -HybridUSDCTokenPoolTests:test_transferLiquidity_Success() (gas: 167107) +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: 515988) +HybridUSDCTokenPoolTests:test_LockOrBurn_LockReleaseMechanism_then_switchToPrimary_Success() (gas: 209283) +HybridUSDCTokenPoolTests:test_LockOrBurn_PrimaryMechanism_Success() (gas: 135897) +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: 425869) +HybridUSDCTokenPoolTests:test_MintOrRelease_incomingMessageWithPrimaryMechanism() (gas: 268928) +HybridUSDCTokenPoolTests:test_ReleaseOrMint_WhileMigrationPause_Revert() (gas: 113547) +HybridUSDCTokenPoolTests:test_cannotTransferLiquidityDuringPendingMigration_Revert() (gas: 177009) +HybridUSDCTokenPoolTests:test_transferLiquidity_Success() (gas: 167033) LockReleaseTokenPoolAndProxy_setRebalancer:test_SetRebalancer_Revert() (gas: 10989) LockReleaseTokenPoolAndProxy_setRebalancer:test_SetRebalancer_Success() (gas: 18028) LockReleaseTokenPoolPoolAndProxy_canAcceptLiquidity:test_CanAcceptLiquidity_Success() (gas: 3051552) diff --git a/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol b/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol index e260300bd7..fd54387620 100644 --- a/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol +++ b/contracts/src/v0.8/ccip/pools/USDC/BurnMintWithLockReleaseFlagTokenPool.sol @@ -5,6 +5,7 @@ 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 @@ -12,9 +13,6 @@ import {BurnMintTokenPool} from "../BurnMintTokenPool.sol"; /// @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 { - /// bytes4(keccak256("NO_CCTP_USE_LOCK_RELEASE")) - bytes4 public constant LOCK_RELEASE_FLAG = 0xfa7c07de; - constructor( IBurnMintERC20 token, address[] memory allowlist, @@ -24,6 +22,8 @@ contract BurnMintWithLockReleaseFlagTokenPool is BurnMintTokenPool { /// @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) { @@ -33,6 +33,7 @@ contract BurnMintWithLockReleaseFlagTokenPool is BurnMintTokenPool { 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 c034fcad75..e61f0cc7dd 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_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, /// and CCIP is facilitating mint/burn on all the other chains, in which case the invariant diff --git a/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol b/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol index c84fc2efc5..9fc7e160e9 100644 --- a/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol +++ b/contracts/src/v0.8/ccip/test/pools/BurnMintWithLockReleaseFlagTokenPool.t.sol @@ -5,6 +5,8 @@ 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"; @@ -57,6 +59,6 @@ contract BurnMintWithLockReleaseFlagTokenPool_lockOrBurn is BurnMintWithLockRele assertEq(s_burnMintERC677.balanceOf(address(s_pool)), 0); - assertEq(bytes4(lockOrBurnOut.destPoolData), s_pool.LOCK_RELEASE_FLAG()); + 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 de92219435..60f3c6720f 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"; @@ -224,7 +225,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: "" }) ); @@ -442,7 +443,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; @@ -757,7 +758,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: "" }) ); @@ -876,7 +877,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: "" }) );