-
Notifications
You must be signed in to change notification settings - Fork 1.8k
EVM USDC Token Pool Changesets #18560
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
base: develop
Are you sure you want to change the base?
Conversation
bb458ba
to
91d3526
Compare
if i.PreviousPoolAddress == utils.ZeroAddress { | ||
return errors.New("previous pool address must be defined") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's possible that there isn't a previous token pool. Maybe there should be a "NoPreviousTokenPool" flag to ensure this field isn't skipped by accident.
5f87a79
to
6a77988
Compare
} | ||
|
||
return cldf.ChangesetOutput{ | ||
AddressBook: newAddresses, // TODO: this is deprecated, how do I use the DataStore instead? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we used the DataStore in the solana tooling so this is fairly new to me as well. I tried looking for other changesets which have adopted this but haven't found any yet. However, I did come across an example in the docs here which seems relevant - perhaps this could get you started on the right track?
previousPoolAddress = chainState.USDCTokenPools[deployment.Version1_6_1].Address() | ||
} else { | ||
return cldf.ContractDeploy[*usdc_token_pool.USDCTokenPool]{ | ||
Err: fmt.Errorf("previous USDC pool address not found on %s", previousPoolAddress, chain), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err: fmt.Errorf("previous USDC pool address not found on %s", previousPoolAddress, chain), | |
Err: fmt.Errorf("previous USDC pool address (%s) not found on %s", previousPoolAddress.Hex(), chain.Name()), |
func (i DeployCCTPMessageTransmitterProxyInput) Validate(ctx context.Context, chain cldf_evm.Chain, state evm.CCIPChainState) error { | ||
// The message transmitter consts are defined in the chainlink-deployments project so we can't validate them here. | ||
if i.TokenMessenger == utils.ZeroAddress { | ||
return fmt.Errorf("token messenger must be defined for chain %s", chain.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return fmt.Errorf("token messenger must be defined for chain %s", chain.Name) | |
return fmt.Errorf("token messenger must be defined for chain %s", chain.Name()) |
|
||
func (i ConfigUSDCTokenPoolInput) Validate(ctx context.Context, chain cldf_evm.Chain, state evm.CCIPChainState) error { | ||
if _, ok := state.USDCTokenPools_v1_6[deployment.Version1_6_0]; !ok { | ||
return fmt.Errorf("no USDC token pool with version %s found on %s", deployment.Version1_6_0, chain.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return fmt.Errorf("no USDC token pool with version %s found on %s", deployment.Version1_6_0, chain.Name) | |
return fmt.Errorf("no USDC token pool with version %s found on %s", deployment.Version1_6_0, chain.Name()) |
if _, ok := state.CCTPMessageTransmitterProxies[deployment.Version1_6_0]; !ok { | ||
// TODO: Could this be deployed automatically if it doesn't exist? | ||
return fmt.Errorf("CCTP message transmitter proxy for version %s not found on %s", deployment.Version1_6_0, chain) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be deployed automatically if it doesn't exist?
Yep this is possible - in fact the Solana tooling automatically deploys prerequisite contracts if they are missing from state:
chainlink/deployment/ccip/changeset/solana/cs_deploy_chain.go
Lines 288 to 292 in ccacda4
if chainState.FeeQuoter.IsZero() { | |
feeQuoterAddress, err = DeployAndMaybeSaveToAddressBook(e, chain, ab, shared.FeeQuoter, deployment.Version1_0_0, false, "") | |
if err != nil { | |
return batches, fmt.Errorf("failed to deploy program: %w", err) | |
} |
There's a few routes that you can take here:
- You can do the deployment of the token pool and transmitter from this changeset
- You can create a separate changeset which deploys everything end-to-end using a sequence
- You can leave this as is and write them out separately in CLD
I think the sequence approach is probably what the CLD team would recommend - this would allow you to reduce duplication and have each changeset be more focused on a single responsibility
} | ||
|
||
return cldf.ChangesetOutput{ | ||
AddressBook: newAddresses, // TODO: this is deprecated, how do I use the DataStore instead? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was having trouble figuring out how I was supposed to use it 🤷♂️
// If the previous pool address is not set, we try to find the latest deployed pool address | ||
if _, ok := chainState.USDCTokenPools[deployment.Version1_5_0]; !ok { | ||
previousPoolAddress = chainState.USDCTokenPools[deployment.Version1_5_0].Address() | ||
} else if _, ok := chainState.USDCTokenPools[deployment.Version1_5_1]; !ok { | ||
previousPoolAddress = chainState.USDCTokenPools[deployment.Version1_5_1].Address() | ||
} else if _, ok := chainState.USDCTokenPools_v1_6[deployment.Version1_6_0]; !ok { | ||
previousPoolAddress = chainState.USDCTokenPools[deployment.Version1_6_0].Address() | ||
} else if _, ok := chainState.USDCTokenPools_v1_6[deployment.Version1_6_1]; !ok { | ||
previousPoolAddress = chainState.USDCTokenPools[deployment.Version1_6_1].Address() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the ordering of this should be reversed right? At the moment, this will look for the oldest one first rather than the latest one first
if update.MintRecipient == utils.ZeroAddress { | ||
return fmt.Errorf("mint recipient must be defined for Solana destination chain selector %d", destSelector) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could just be a misunderstanding from my side, but it seems like we're checking the if the chain family is Solana then doing EVM-based address comparisons - is that intentional? Did you instead mean to perform solana-based address comparisons instead? Wanted to ask just to be safe because the zero address is slightly different on Solana and this check might not work as expected
CCTPMessageTransmitterProxyConfigOp = opsutil.NewEVMCallOperation( | ||
"CCTPMessageTransmitterProxyConfigOp", | ||
semver.MustParse("1.0.0"), | ||
"Setting CCTP message transmitter proxy config across multiple EVM chains", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is for a single chain only, not multiple
|
||
// TODO: New token pool contract should be imported from the latest version | ||
|
||
mtp "github.com/smartcontractkit/chainlink-ccip/chains/evm/gobindings/generated/latest/cctp_message_transmitter_proxy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I called this cmtp in the deploy changeset
|
||
// ConfigureCCTPMessageTransmitterProxyContractConfig defines the configuration for configuring the CCTP message transmitter proxy contracts. | ||
type ConfigureCCTPMessageTransmitterProxyContractConfig struct { | ||
USDCProxies map[uint64]ConfigureCCTPMessageTransmitterProxyInput |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: CCTPProxies?
|
Add changesets to deploy and configure the latest USDC Token Pool:
Some updates were also made to the state object to hold references to the new usdc token pool and the cctp proxy.