fix: add threshold change timelock and harden USDC migration accounting

[alva-p]

Summary

This PR addresses four issues found during a security review of the CCIP token pool contracts: one design gap in AdvancedPoolHooks and three accounting/ordering issues in the SiloedUSDCTokenPool CCTP migration flow.

Reviewed commit: d601419f87a1dbf6ff7b28c1cdba1655b9f0d643. Verified that the affected contracts are unchanged at current main (6c8ce3b).

---

AdvancedPoolHooks: retroactive threshold decrease can permanently block in-flight messages

setThresholdAmount takes effect immediately with no delay. The OffRamp evaluates CCV requirements at execution time by calling pool.getRequiredCCVs() live, not at send time. A message committed when the threshold was high enough to require only base CCVs can land in a stricter configuration that demands additional CCVs that were never collected. The OffRamp then enters FAILURE state and subsequent retries revert with NoStateProgressMade, permanently blocking the message until the owner manually restores a compatible threshold.

Fix: setThresholdAmount now schedules a pending change instead of applying immediately. The change becomes active only after THRESHOLD_CHANGE_DELAY (2 days) via a separate applyThresholdAmount call. Added getPendingThreshold for observability.

---

SiloedUSDCTokenPool: burnLockedUSDC can over-withdraw if excluded messages execute before the burn

burnLockedUSDC computes tokensToBurn = s_lockedUSDCToBurn - s_tokensExcludedFromBurn. When an excluded in-flight message is delivered via the permissionless OffRamp, releaseOrMint decrements s_tokensExcludedFromBurn and withdraws from the lockbox. If all excluded messages execute before Circle calls burnLockedUSDC, the exclusion counter reaches zero while the snapshot stays at its original value, causing the function to attempt a withdrawal larger than the remaining lockbox balance. The migration is blocked until the owner cancels and reproposes.

The comment at line 133 (burnableUSDC = lockboxBalance - excludedInFlight) already describes the correct formula. The implementation did not match.

Fix: burnLockedUSDC now reads the actual lockbox balance via IERC20(address(i_token)).balanceOf(address(lockBox)). The snapshot is kept as an upper bound to prevent burning USDC that was deposited directly to the lockbox without going through the bridge.

---

SiloedUSDCTokenPool: proposeCCTPMigration instantly blocks in-flight messages with no grace period

proposeCCTPMigration sets s_proposedUSDCMigrationChain immediately. Any releaseOrMint for that chain then requires the amount to be covered by s_tokensExcludedFromBurn, which is zero until the owner calls excludeTokensFromBurn. Messages committed on the remote chain before the proposal but not yet executed revert with InsufficientLiquidity(0, amount) and enter FAILURE state. Finality windows of 5 to 15 minutes mean this race window cannot be eliminated purely through operational care.

Fix: proposeCCTPMigration sets s_migrationActiveAt = block.timestamp + MIGRATION_PROPOSAL_GRACE_PERIOD (1 hour). The migration guard in releaseOrMint only activates after this timestamp, giving the owner time to pause the lane before committed messages start reverting.

---

SiloedUSDCTokenPool: setLockedUSDCToBurn allows snapshot below current exclusion amount, causing underflow in burnLockedUSDC

setLockedUSDCToBurn sets s_lockedUSDCToBurn without checking that the new value is at least as large as the current s_tokensExcludedFromBurn[remoteChainSelector]. Setting a snapshot below the exclusion amount causes s_lockedUSDCToBurn - s_tokensExcludedFromBurn in burnLockedUSDC to underflow under Solidity 0.8 checked arithmetic, reverting with a panic and blocking the migration until the owner cancels and reproposes.

Fix: setLockedUSDCToBurn now validates lockedUSDCToBurn >= s_tokensExcludedFromBurn[remoteChainSelector] and reverts with LockedUSDCBelowExcluded if not.

---

Test plan

The existing Foundry suite compiles and the non-security baseline tests pass. The PoC tests in the audit suite now correctly fail to reproduce the vulnerable behavior, confirming the fixes are effective.

forge test --match-contract 'Audit_(MEDIUM1|MEDIUM2|LOW1|LOW2)' -vv

Recommendation: add regression tests covering the new applyThresholdAmount, the grace-period guard in releaseOrMint, the LockedUSDCBelowExcluded revert path, and the lockbox-balance-capped burn amount in burnLockedUSDC.

[Copilot]

Pull request overview

This PR hardens CCIP token pool behavior in two areas: (1) it introduces a timelocked threshold change mechanism in AdvancedPoolHooks to prevent retroactive CCV requirement changes from permanently blocking in-flight messages, and (2) it fixes ordering/accounting pitfalls in the SiloedUSDCTokenPool USDC CCTP migration flow (grace period for migration guard, safer burn accounting, and snapshot validation).

Changes:

• AdvancedPoolHooks: replace immediate setThresholdAmount with a schedule/apply flow using a 2-day delay and add pending-threshold observability.
• SiloedUSDCTokenPool: add a 1-hour grace period before the migration guard activates, validate burn snapshot vs excluded amount, and cap burn amount using the lockbox’s actual balance.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
chains/evm/contracts/pools/USDC/SiloedUSDCTokenPool.sol	Adds migration grace-period gating to avoid immediate failures for in-flight messages; hardens burnLockedUSDC accounting using lockbox balance and snapshot validation.
chains/evm/contracts/pools/AdvancedPoolHooks.sol	Introduces timelocked threshold changes with pending state + apply function to avoid retroactive CCV requirement tightening.

Comments suppressed due to low confidence (2)

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

• The comment says the lane-level supply lock is required "once migration is proposed/completed", but the new grace-period logic intentionally does not activate the guard immediately on proposal (it only activates after s_migrationActiveAt). The comment should be updated to reflect that the guard becomes active only after the grace period (or when excludedTokens != 0 / migrated).

      // Circle's migration procedure requires a lane-level supply lock once migration is proposed/completed.
      // For those lanes, releaseOrMint must only consume the explicitly excluded in-flight reserve, and this
      // guard must still apply when excludedTokens == 0 to prevent releasing newly deposited lockbox liquidity.
      if (releaseOrMintIn.sourceDenominatedAmount > excludedTokens) {

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

• burnLockedUSDC() clears s_proposedUSDCMigrationChain and s_lockedUSDCToBurn but leaves s_migrationActiveAt set. While it’s currently harmless (since the guard checks the proposed chain), deleting s_migrationActiveAt here as well would keep migration state consistent and avoid stale values lingering across proposals.

    // Apply migration state updates before external calls to preserve CEI.
    delete s_proposedUSDCMigrationChain;
    delete s_lockedUSDCToBurn;
    s_migratedChains.add(burnChainSelector);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.