[Fix] Strand-free USDC+message offramp: use pool local_amount, not source_pool_data#407
Draft
m-j-e wants to merge 1 commit into
Draft
[Fix] Strand-free USDC+message offramp: use pool local_amount, not source_pool_data#407m-j-e wants to merge 1 commit into
m-j-e wants to merge 1 commit into
Conversation
…urce_pool_data complete_token_transfer re-derived the destination amount by parsing the token's source_pool_data as a 32-byte ABI decimals word (assert!(data_len == 32, EInvalidRemoteChainDecimals)). The USDC/CCTP pool encodes source_pool_data as encode_u64(nonce) ++ encode_u32(domain) = 64 bytes, so every inbound USDC transfer carrying a programmatic message aborted on that assert. The source USDC is already CCTP-burned, so funds were stranded and the whole USDC+message flow was unexecutable per lane. Introduced by #391 (NONEVM-5192): the conversion math is correct, but the "source_pool_data == abi(decimals)" interface assumption is false for USDC. Fix: make the pool the single source of truth. Each pool already computes the exact local_amount it mints/releases (standard pools via calculate_release_or_mint_amount, USDC via the CCTP-attested burn_message::amount) and uses it for the mint and the emitted event. complete_token_transfer now takes that local_amount and writes it straight to the message, with no re-parse of source_pool_data. This also removes the risk of the message amount diverging from the minted amount. - offramp_state_helper: add local_amount: u64 param to complete_token_transfer; drop parse_remote_decimals / calculate_local_amount / get_local_decimals_for_token from the message branch, plus the now-dead helpers, constants, and import. - burn_mint / lock_release / managed_token / usdc pools: pass their computed local amount into complete_token_transfer. - tests: add f01_usdc_message_regression_test (64-byte USDC + message now completes and surfaces the pool's amount); update existing callers for the new signature; remove decimal_parity_test (it only covered the deleted duplicate; the math is unchanged in each pool's token_pool.move). Verified: 487 tests pass across ccip, ccip_offramp, ccip_onramp, the three standard token pools, ccip_router, and managed_token. usdc_token_pool is not built standalone (pre-existing vendored Circle manifest issue); its call-site change mirrors the three pools that pass.
|
Hello, @m-j-e! 👋
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Describe your changes
complete_token_transfer re-derived the destination amount by parsing the token's source_pool_data as a 32-byte ABI decimals word (assert!(data_len == 32, EInvalidRemoteChainDecimals)). The USDC/CCTP pool encodes source_pool_data as encode_u64(nonce) ++ encode_u32(domain) = 64 bytes, so every inbound USDC transfer carrying a programmatic message aborted on that assert. The source USDC is already CCTP-burned, so funds were stranded and the whole USDC+message flow was unexecutable per lane.
Fix: make the pool the single source of truth. Each pool already computes the exact local_amount it mints/releases (standard pools via calculate_release_or_mint_amount, USDC via the CCTP-attested burn_message::amount) and uses it for the mint and the emitted event. complete_token_transfer now takes that local_amount and writes it straight to the message, with no re-parse of source_pool_data. This also removes the risk of the message amount diverging from the minted amount.
Describe highly relevant files or code snippets that are critical in the review
Verified: 487 tests pass across ccip, ccip_offramp, ccip_onramp, the three standard token pools, ccip_router, and managed_token. usdc_token_pool is not built standalone (pre-existing vendored Circle manifest issue); its call-site change mirrors the three pools that pass.