From 364172c8532aade03a0a39bbf0c70f59056379e4 Mon Sep 17 00:00:00 2001 From: alva-p Date: Mon, 25 May 2026 11:50:08 -0300 Subject: [PATCH] fix: add threshold change timelock and harden USDC migration accounting AdvancedPoolHooks: setThresholdAmount now schedules a pending change instead of applying immediately. The new value takes effect only after THRESHOLD_CHANGE_DELAY (2 days) via a separate applyThresholdAmount call. This prevents a retroactive threshold decrease from blocking in-flight messages that were committed under a higher threshold and therefore never collected the additional CCVs that would be required under the new value. SiloedUSDCTokenPool: three fixes to the CCTP migration flow. burnLockedUSDC now derives tokensToBurn from the actual lockbox balance rather than the owner-set snapshot. If excluded in-flight messages are delivered before Circle calls burnLockedUSDC, both the lockbox balance and the excluded counter are decremented together, so using the lockbox balance avoids attempting to withdraw tokens that are no longer there. The snapshot is kept as an upper bound to prevent burning USDC deposited directly to the lockbox without going through the bridge. proposeCCTPMigration now sets a grace period (MIGRATION_PROPOSAL_GRACE_PERIOD, 1 hour) before the migration guard becomes active in releaseOrMint. This gives the owner time to pause the lane before messages committed on the remote chain but not yet executed start reverting with InsufficientLiquidity. setLockedUSDCToBurn now validates that the new snapshot is not below the current s_tokensExcludedFromBurn value. Setting a snapshot below the exclusion amount would cause an arithmetic underflow in burnLockedUSDC and block the migration until the owner cancels and reproposes. --- .../evm/contracts/pools/AdvancedPoolHooks.sol | 50 ++++++++++++++++--- .../pools/USDC/SiloedUSDCTokenPool.sol | 33 ++++++++++-- 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/chains/evm/contracts/pools/AdvancedPoolHooks.sol b/chains/evm/contracts/pools/AdvancedPoolHooks.sol index 2480653595..710ac097a0 100644 --- a/chains/evm/contracts/pools/AdvancedPoolHooks.sol +++ b/chains/evm/contracts/pools/AdvancedPoolHooks.sol @@ -26,6 +26,7 @@ contract AdvancedPoolHooks is IAdvancedPoolHooks, ITypeAndVersion, AuthorizedCal error SenderNotAllowed(address sender); error MustSpecifyUnderThresholdCCVsForThresholdCCVs(); error PolicyEngineDetachReverted(address oldPolicyEngine, bytes err); + error ThresholdChangeNotReady(uint256 effectiveAt); event AllowListAdd(address sender); event AllowListRemove(address sender); @@ -37,6 +38,7 @@ contract AdvancedPoolHooks is IAdvancedPoolHooks, ITypeAndVersion, AuthorizedCal address[] thresholdInboundCCVs ); event ThresholdAmountSet(uint256 thresholdAmount); + event ThresholdAmountScheduled(uint256 newThreshold, uint256 effectiveAt); event PolicyEngineAttached(address indexed policyEngine); event PolicyEngineDetachFailed(address indexed policyEngine, bytes reason); @@ -67,6 +69,18 @@ contract AdvancedPoolHooks is IAdvancedPoolHooks, ITypeAndVersion, AuthorizedCal /// Value of 0 means that there is no threshold and additional CCVs are not required for any transfer amount. uint256 internal s_thresholdAmountForAdditionalCCVs; + /// @dev Pending threshold value queued via setThresholdAmount, takes effect after s_thresholdEffectiveAt. + uint256 internal s_pendingThreshold; + + /// @dev Timestamp after which s_pendingThreshold becomes the active threshold. + uint256 internal s_thresholdEffectiveAt; + + /// @notice Minimum delay between scheduling and applying a threshold change. + /// @dev In-flight messages collect CCVs based on the threshold at send time. Applying a lower threshold + /// immediately would require additional CCVs that were never collected, permanently blocking those messages. + /// The delay gives in-flight messages enough time to be executed before stricter requirements take effect. + uint256 public constant THRESHOLD_CHANGE_DELAY = 2 days; + /// @dev The policy engine to use. Value of 0 disables policy engine checks. IPolicyEngine internal s_policyEngine; @@ -320,20 +334,44 @@ contract AdvancedPoolHooks is IAdvancedPoolHooks, ITypeAndVersion, AuthorizedCal return _resolveRequiredCCVs(config.outboundCCVs, config.thresholdOutboundCCVs, amount); } - /// @notice Gets the threshold amount above which additional CCVs are required. - /// @return The threshold amount. + /// @notice Gets the active threshold amount above which additional CCVs are required. + /// @return The threshold amount currently enforced at execution time. function getThresholdAmount() public view virtual returns (uint256) { return s_thresholdAmountForAdditionalCCVs; } - /// @notice Sets the threshold amount above which additional CCVs are required. - /// @param thresholdAmount The new threshold amount. + /// @notice Gets the pending threshold and the timestamp when it becomes active. + /// @return pendingThreshold The threshold value queued by the most recent setThresholdAmount call. + /// @return effectiveAt The timestamp after which applyThresholdAmount can be called. + function getPendingThreshold() public view virtual returns (uint256 pendingThreshold, uint256 effectiveAt) { + return (s_pendingThreshold, s_thresholdEffectiveAt); + } + + /// @notice Schedules a threshold change to take effect after THRESHOLD_CHANGE_DELAY. + /// @dev The new threshold is not applied immediately. Call applyThresholdAmount after the delay expires. + /// This prevents in-flight messages from being permanently blocked by a retroactive threshold decrease. + /// @param thresholdAmount The new threshold amount to schedule. function setThresholdAmount( uint256 thresholdAmount ) public virtual onlyOwner { - s_thresholdAmountForAdditionalCCVs = thresholdAmount; + s_pendingThreshold = thresholdAmount; + s_thresholdEffectiveAt = block.timestamp + THRESHOLD_CHANGE_DELAY; + + emit ThresholdAmountScheduled(thresholdAmount, s_thresholdEffectiveAt); + } + + /// @notice Applies the pending threshold change once the delay has elapsed. + /// @dev Reverts if the delay period has not yet passed. + function applyThresholdAmount() public virtual onlyOwner { + uint256 effectiveAt = s_thresholdEffectiveAt; + if (block.timestamp < effectiveAt) revert ThresholdChangeNotReady(effectiveAt); + + uint256 newThreshold = s_pendingThreshold; + s_thresholdAmountForAdditionalCCVs = newThreshold; + delete s_pendingThreshold; + delete s_thresholdEffectiveAt; - emit ThresholdAmountSet(thresholdAmount); + emit ThresholdAmountSet(newThreshold); } function _resolveRequiredCCVs( diff --git a/chains/evm/contracts/pools/USDC/SiloedUSDCTokenPool.sol b/chains/evm/contracts/pools/USDC/SiloedUSDCTokenPool.sol index 2b6ab7f0a6..f65b633f91 100644 --- a/chains/evm/contracts/pools/USDC/SiloedUSDCTokenPool.sol +++ b/chains/evm/contracts/pools/USDC/SiloedUSDCTokenPool.sol @@ -40,6 +40,8 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers { error ChainAlreadyMigrated(uint64 remoteChainSelector); error LockBoxCannotBeShared(uint64 chainSelectorA, uint64 chainSelectorB, address lockBox); error InsufficientLiquidity(uint256 availableLiquidity, uint256 requestedAmount); + error LockedUSDCBelowExcluded(uint256 lockedUSDC, uint256 excludedTokens); + error MigrationGracePeriodActive(uint256 activeAt); /// @notice The address of the circle-controlled wallet which will execute a CCTP lane migration address internal s_circleUSDCMigrator; @@ -55,6 +57,16 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers { /// @notice The chains that have been migrated to CCTP. EnumerableSet.UintSet internal s_migratedChains; + /// @notice Timestamp after which the migration guard becomes active for the proposed lane. + /// @dev Set to block.timestamp + MIGRATION_PROPOSAL_GRACE_PERIOD when a migration is proposed. + /// The grace period gives the owner time to pause the lane before in-flight messages start reverting. + uint256 internal s_migrationActiveAt; + + /// @notice Minimum time between proposing a migration and the guard blocking releases on the proposed lane. + /// @dev Finality windows across supported chains can be 5 to 15 minutes. One hour provides enough margin + /// for already-committed messages to arrive and for the owner to pause the lane without a race. + uint256 public constant MIGRATION_PROPOSAL_GRACE_PERIOD = 1 hours; + /// @dev The authorized callers are set as empty since the USDCTokenPoolProxy is the only authorized caller, /// but cannot be deployed until after this contract. The allowed callers will be set after deployment. /// @param token The token managed by this pool. @@ -119,8 +131,10 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers { uint64 remoteChainSelector = releaseOrMintIn.remoteChainSelector; uint256 excludedTokens = s_tokensExcludedFromBurn[remoteChainSelector]; + bool migrationGuardActive = remoteChainSelector == s_proposedUSDCMigrationChain + && block.timestamp >= s_migrationActiveAt; if ( - excludedTokens != 0 || remoteChainSelector == s_proposedUSDCMigrationChain + excludedTokens != 0 || migrationGuardActive || s_migratedChains.contains(remoteChainSelector) ) { // Circle's migration procedure requires a lane-level supply lock once migration is proposed/completed. @@ -207,6 +221,7 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers { if (s_migratedChains.contains(remoteChainSelector)) revert ChainAlreadyMigrated(remoteChainSelector); delete s_lockedUSDCToBurn; s_proposedUSDCMigrationChain = remoteChainSelector; + s_migrationActiveAt = block.timestamp + MIGRATION_PROPOSAL_GRACE_PERIOD; emit CCTPMigrationProposed(remoteChainSelector); } @@ -223,6 +238,7 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers { // re-excluded if the proposal is re-proposed in the future delete s_tokensExcludedFromBurn[currentProposalChainSelector]; delete s_lockedUSDCToBurn; + delete s_migrationActiveAt; emit CCTPMigrationCancelled(currentProposalChainSelector); } @@ -260,6 +276,8 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers { // Selector 0 is reserved as the "no proposal pending" sentinel in state. if (remoteChainSelector == 0) revert InvalidChainSelector(); if (s_proposedUSDCMigrationChain != remoteChainSelector) revert NoMigrationProposalPending(); + uint256 excluded = s_tokensExcludedFromBurn[remoteChainSelector]; + if (lockedUSDCToBurn < excluded) revert LockedUSDCBelowExcluded(lockedUSDCToBurn, excluded); s_lockedUSDCToBurn = lockedUSDCToBurn; emit LockedUSDCToBurnSet(remoteChainSelector, lockedUSDCToBurn); @@ -316,8 +334,17 @@ contract SiloedUSDCTokenPool is SiloedLockReleaseTokenPool, AuthorizedCallers { if (burnChainSelector == 0) revert NoMigrationProposalPending(); ILockBox lockBox = getLockBox(burnChainSelector); - // Burnable tokens is the owner-set locked snapshot minus the amount excluded from burn. - uint256 tokensToBurn = s_lockedUSDCToBurn - s_tokensExcludedFromBurn[burnChainSelector]; + uint256 excluded = s_tokensExcludedFromBurn[burnChainSelector]; + uint256 snapshot = s_lockedUSDCToBurn; + // The lockbox balance is the authoritative source for how much USDC can actually be withdrawn. + // If excluded in-flight messages were delivered before this call, both the lockbox balance and + // the excluded counter were decremented together, so using the lockbox balance here correctly + // avoids attempting to withdraw tokens that have already been released. + // The snapshot acts as an upper bound to prevent burning USDC that was deposited directly to + // the lockbox without going through the bridge. + uint256 lockboxBalance = IERC20(address(i_token)).balanceOf(address(lockBox)); + uint256 burnableFromLockbox = lockboxBalance < snapshot ? lockboxBalance : snapshot; + uint256 tokensToBurn = burnableFromLockbox - excluded; // Apply migration state updates before external calls to preserve CEI. delete s_proposedUSDCMigrationChain;