Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USDC token pool changesets (+ token pool patches) #16062

Open
wants to merge 66 commits into
base: develop
Choose a base branch
from

Conversation

kylesmartin
Copy link
Contributor

@kylesmartin kylesmartin commented Jan 24, 2025

https://smartcontract-it.atlassian.net/browse/CCIP-4507

This PR adds two new changesets:

  • DeployUSDCTokenPoolContractsChangeset: Deploys USDC token pools on given chains
  • SyncUSDCDomainsWithChainsChangeset: Syncs USDC domain support with supported chains

It also makes a few patches to token pool utilities and changesets:

  • Replaces references to deployment.Version1_5_1 with CurrentTokenPoolVersion to make updating the token pool version easier (This assumes that the API won't break though, a better approach to versioning is still needed long term)
  • Fixes a bug in ConfigureTokenPoolContractsChangeset where we were validating ownership of the token admin registry rather than the relevant token pool
  • Always check for existence of timelock, required for ownership check even if MCMS is not used
  • Add check in deployer group, specifically enactMCMS, for 0 transactions (expected if there is nothing to do)

Requires

#16140

@kylesmartin kylesmartin changed the title USDC token pool configuration changeset USDC token pool configuration changeset (+ token pool patches) Jan 28, 2025
@kylesmartin kylesmartin marked this pull request as ready for review January 28, 2025 14:52
@kylesmartin kylesmartin requested review from AnieeG and a team as code owners January 28, 2025 14:52
@kylesmartin kylesmartin changed the title USDC token pool configuration changeset (+ token pool patches) USDC token pool changesets (+ token pool patches) Jan 28, 2025
@kylesmartin kylesmartin requested a review from bukata-sa January 28, 2025 18:48
deployment/ccip/changeset/cs_deploy_usdc_token_pools.go Outdated Show resolved Hide resolved
}
if rmnProxy := chainState.RMNProxy; rmnProxy == nil {
return fmt.Errorf("missing rmnProxy on %s", chain)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check if token address in present in the state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't store the token address in the address book, only the token pool

}

for chainSelector, poolConfig := range c.NewUSDCPools {
chain := env.Chains[chainSelector]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can there be a check whether token pool is already deployed with same version for the token address?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That check exists in DeployUSDCTokenPoolInput.Validate

Comment on lines 92 to 93
if _, ok := c.USDCConfigsByChain[chainSelector]; !ok && chainState.USDCTokenPools != nil {
return fmt.Errorf("no USDC chain config defined for %s, which does support USDC", env.Chains[chainSelector])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if we already have the chain config set up for few USDC chains?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then there will just be no net action to take for those chains. I think it is best to enforce defining every chain that supports USDC any time we run this command. They all point at each other, so it is best to have the user specify every which USDC versions should be active so that each chain can reference the correct remote pool.

}

if len(domainUpdates) > 0 {
_, err := usdcTokenPool.SetDomains(writeOpts, domainUpdates)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validating token pool ownership anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea that check is present in SyncUSDCDomainsWithChainsConfig.Validate

"github.com/smartcontractkit/chainlink/v2/core/logger"
)

func deployChainWithUSDCTokenPool(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not already have this in NewEnvironmentWithJobsAndContracts with Testconfig as USDC Deployment set to true?

deployment/ccip/changeset/token_pools.go Outdated Show resolved Hide resolved
@kylesmartin
Copy link
Contributor Author

Will need to rebase this PR against #16140

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants