CCIP-4748 CCTP Upgradeability Fix #16140
Conversation
# Conflicts: # contracts/gas-snapshots/ccip.gas-snapshot
Static analysis results are availableHey @0xsuryansh, you can view Slither reports in the job summary here or download them as artifact here. |
0xsuryansh
changed the title
AER Report: CI Coreaer_workflow , commit , Clean Go Tidy & Generate , Detect Changes , Scheduled Run Frequency , Core Tests (go_core_tests) , Core Tests (go_core_tests_integration) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_fuzz) , GolangCI Lint (.) , Core Tests (go_core_race_tests) , GolangCI Lint (integration-tests) , GolangCI Lint (deployment) , test-scripts , lint , SonarQube Scan 1. Test failure in
|
# Conflicts: # core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt # deployment/ccip/changeset/cs_prerequisites.go # integration-tests/ccip-tests/contracts/contract_deployer.go
0xsuryansh
requested review from
AnieeG,
kylesmartin and
a team
as code owners
0xsuryansh
requested review from
winder,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara,
0xAustinWang and
0xnogo
Solidity Review Jira issueHey! We have taken the liberty to link this PR to a Jira issue for Solidity Review. This is a new feature, that's currently in the pilot phase, so please make sure that the linkage is correct. In a contrary case, please update it manually in JIRA and replace Solidity Review issue key in the changeset file with the correct one. Any changes to the Solidity Review Jira issue should be reflected in the changeset file. If you need to update the issue key, please do so manually in the following changeset file: This PR has been linked to Solidity Review Jira issue: CCIP-3966 |
…x' into CCIP-4748_cctp-upgradeability-fix
| @@ -0,0 +1,46 @@ | |||
| // SPDX-License-Identifier: BUSL-1.1 | |||
| pragma solidity ^0.8.24; | |||
|
|
|||
There was a problem hiding this comment.
Not: missing natspec on contact and functions
| @@ -146,7 +151,7 @@ contract USDCTokenPool is TokenPool, ITypeAndVersion { | |||
|
|
|||
| _validateMessage(msgAndAttestation.message, sourceTokenDataPayload); | |||
|
|
|||
| if (!i_messageTransmitter.receiveMessage(msgAndAttestation.message, msgAndAttestation.attestation)) { | |||
| if (!i_messageTransmitterProxy.receiveMessage(msgAndAttestation.message, msgAndAttestation.attestation)) { | |||
There was a problem hiding this comment.
Can you include a comment about why changes don't need to be made for outgoing messages and if the domain config needs to be modified to support incoming messages being received by the proxy and not the token pool?
There was a problem hiding this comment.
|
Problem
When upgrading the pool contract on the destination chain, any CCTP messages created on the source chain during the transition still reference the old destination pool address in their
allowedCaller. Since theCCTPMessageTransmitterenforces a match with theallowedCallerin the message, these messages fail verification and revert when processed on the destination chain.Solution
To prevent this issue, we introduce a permanent forwarding contract (
CCTPMessageTransmitterProxy) that acts as the stableallowedCaller. Instead of setting the pool contract directly as theallowedCaller, we register this proxy, ensuring that:destinationCallerin all CCTP messages always points to the proxy, which does not change.Changes
CCTPMessageTransmitterProxyallowedCaller, preventing mismatches during upgrades.USDCTokenPoolContractDeployment & Upgrade Steps
CCTPMessageTransmitterProxy.Domain.allowedCalleron the source chain pool.USDCTokenPool, passing the proxy address.updatePoolinCCTPMessageTransmitterProxyto register the new pool contract.