CCIP-6698 New USDC Token Pool Proxy for routing USDC messages to the proper token pool #1134
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jhweintraub
changed the title
jhweintraub
changed the title
0xsuryansh
reviewed
Aug 11, 2025
0xsuryansh
reviewed
Aug 11, 2025
0xsuryansh
reviewed
Aug 13, 2025
0xsuryansh
reviewed
Aug 13, 2025
0xsuryansh
reviewed
Aug 13, 2025
0xsuryansh
reviewed
Aug 13, 2025
chains/evm/contracts/test/pools/USDC/USDCTokenPoolProxy/USDCTokenPoolProxy.lockOrBurn.t.sol
Show resolved
Hide resolved
0xsuryansh
reviewed
Aug 13, 2025
matYang
reviewed
Oct 1, 2025
| bytes4 public constant CCTP_VERSION_1_TAG = 0xf3567d18; | ||
|
|
||
| /// @dev There are two different tags for CCTP V2 to allow for CCIP V1.7 Compatibility which will enable fast transfers. | ||
| /// Both tags will route to the same CCTP V2 pool, but will allow for pools to identify the type of transfer (slow or fast). |
There was a problem hiding this comment.
@RensR correct me if I'm wrong, the 2_TAG + 2_CCV_TAG enables the pool to identify the expected caller right, as opposed to necessarily the transfer type (tho feels possible if faster transfer uses a dedicated CCV)
There was a problem hiding this comment.
so as far as I am aware we're not doing anything with this additional tag at the moment but it is supposed to give us extra flexibility as we go into 1.7 development
matYang
reviewed
Oct 1, 2025
matYang
reviewed
Oct 1, 2025
matYang
reviewed
Oct 2, 2025
matYang
reviewed
Oct 2, 2025
matYang
reviewed
Oct 2, 2025
matYang
reviewed
Oct 2, 2025
matYang
reviewed
Oct 2, 2025
.../evm/contracts/test/pools/USDC/USDCTokenPoolCCTPV2/USDCTokenPoolCCTPV2.validateMessage.t.sol
Outdated
Show resolved
Hide resolved
matYang
reviewed
Oct 6, 2025
| /// @param maxFee The max fee of the message. | ||
| /// @param minFinalityThreshold The min finality threshold of the message. | ||
| /// @return depositHash The deposit hash of the source pool data which will be matched off-chain to its CCTP attestation. | ||
| function _calculateDepositHash( |
There was a problem hiding this comment.
there is value to make a library helper dedicated to this lib and individually test the functions, for example there are plenty of tests that rely on _calculateDepositHash but none dedicated to verifying this function is implemented correctly
happy to make this a followup and not blocker of this PR
There was a problem hiding this comment.
matYang
reviewed
Oct 6, 2025
| uint32 minFinalityThreshold | ||
| ) internal pure returns (bytes32) { | ||
| return keccak256( | ||
| abi.encodePacked( |
There was a problem hiding this comment.
any reason to not use abi.encode?
There was a problem hiding this comment.
gas savings and preventing unpredictable encoding but in terms of actual functionality no
There was a problem hiding this comment.
abi.encode prevents encoding (and thus hash) collisions, my understanding is we should use that unless there is a good reason not to
There was a problem hiding this comment.
matYang
reviewed
Oct 6, 2025
| // This branch must come before a version check, because the first field would be a uint64 and thus if a version | ||
| // was attempted to be extracted from the first 4-bytes of a uint64, it would be 0, and thus the message would be | ||
| // routed to the CCTP V1 pool without first sanitizing the source pool data for proper formatting. | ||
| if (releaseOrMintIn.sourcePoolData.length == 64) { |
There was a problem hiding this comment.
is it always impossible for the new version of sourcePoolData to be 64, feels like so given math is 4 + 8 + 32? Let's document that clearly here; + not sure if there's a more reliable way to detect legacy source data
There was a problem hiding this comment.
so it's not technically impossible but the idea is that this branch should only be reached in the event that the first 4 bytes of the source pool data do not match an existing known versioning tag, so even if there is a source token data payload that is 64 bytes long and created in the future it would not trigger this anyways. There are other ways we could potentially try and test this but it would involve weird try-catch logic that I think is overkill
There was a problem hiding this comment.
|
matYang
approved these changes
Oct 7, 2025
RensR
approved these changes
Oct 7, 2025
RensR
added a commit
that referenced
this pull request
Oct 8, 2025
* add owners for deployments (#1239) * owners * owners * add zksync gobinding (#1238) Co-authored-by: kylesmartin <[email protected]> * add warning gobindings (#1235) * Add factory param for LOOPP enabled families (#1231) * Add factory param for LOOPP enabled families * add tests for feature flag map for USDC reader * [CCIP-7090] Move chain family agnostic utilities to deployment module (#1237) * [CCIP-7090] Move chain family agnostic utilities to deployment module * Update top level deploy pkg * Fix, + add test workflows * Checkout * Fix * Fix again * Workflow * Add checkout * OpCount -> GetChainMetadata * Fix tests * Use mcms.Input * Move contract writes back to EVM * Rm global registry * update npm release action (#1252) * CCIP-6698 New USDC Token Pool Proxy for routing USDC messages to the proper token pool (#1134) * begin migrating files from other PR for phase 2 * initial compilation and tests passing. * cleanup and wrapper updates * remove previousPool from the USDC Token Pool as it will be moved to the future proxy * more cleanup * checkpointing * cleanup * productionize * comments and proxy updates * additional comments * forge fmt * bump type and version for future release cuts * wrappers regen * changeset * review comment fixes and test cleanup * formatting and gas optimization * add expectCall to lockOrBurn tests * fix CI from merge conflicts * fix CI from merge conflicts * code review fixes * feedback fixes * comment fix * resolve CI from merge conflicts in previous commit * typo fix * comment fixes and gas optimizations * add additional branches for direct non-transmitter-proxy upgrades * better comments and gas optimization * gobindings and comment fix * simplify proxy by removing tokenPool dependency * linter fix * constructor input sanitization check * modify lock release pools to be dynamic based on remote chain selector * fix dependency pathing * fix comment feedback * Change source pool data codec * update encoding schemes for more specific versioning * cleanup * fill in coverage test gaps * comment fixes and PoolV1 interface support for proxy * productionize codec library * add cctp fast transfer version tag * snapshot fix * remove superfluous constructor checks * forge fmt * comment feedback fixes * fill in coverage gaps and add new tokenPoolProxy helper contract. * *spiderman voice* let's try this one last time * deposit hash validation on destination * more fuzz tests and better message formatting comments * wrapper and snapshot fix from merge conflict resolution * add support for legacy source pool data format to prevent breaking changes * additional library test files, hash collision prevention, and better commenting * cld ccip api e2e (#1227) * wip * flatten * flatten * iterate * iterate * iterate * iterate * iterate * iterate * iterate * seed solana * move ref * [CCIP-7268-pt-1] Add ConfigureTokensForTransfers to product spec (#1242) * [CCIP-7090] Move chain family agnostic utilities to deployment module * Update top level deploy pkg * Fix, + add test workflows * Checkout * Fix * Fix again * Workflow * Add checkout * OpCount -> GetChainMetadata * Fix tests * Use mcms.Input * Move contract writes back to EVM * [CCIP-7268-pt-1] Add ConfigureTokensForTransfers to product spec * Very WIP * Rm global registry * Test * Comments * prevent breaking o11y with idl changes (#1247) * prevent breaking o11y with idl changes * test change idl * fix action * fix * change in idl * only ones we need to track * Revert "change in idl" This reverts commit 8378c29. * shouldn't trigger * should detect multiple * fix actin * Revert "should detect multiple" This reverts commit 1a8fd3d. * Revert "shouldn't trigger" This reverts commit c4e8ef2. * fix some comms * add unlabeled * should trigger * Revert "add unlabeled" This reverts commit 79a1d6c. * unlabeled * Revert "should trigger" This reverts commit b47dd70. * fix missing permissions for ID token (#1254) * fix missing permissions for ID token * move to job level * Validate dest pool in FTF pool (#1251) * add validation for destination pool * add tests * update wrapper * update comment + refactor * lint fix * update changesets setup --------- Co-authored-by: tt-cll <[email protected]> Co-authored-by: Matthew Romage <[email protected]> Co-authored-by: kylesmartin <[email protected]> Co-authored-by: Oliver Townsend <[email protected]> Co-authored-by: Josh Weintraub <[email protected]> Co-authored-by: Juan Farber <[email protected]> Co-authored-by: Suryansh <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CCIP-6698