Adding changes needed for the SetTokenTransferFee Update#1836
Adding changes needed for the SetTokenTransferFee Update#1836sowgandhi11 wants to merge 14 commits intomainfrom
Conversation
sowgandhi11
requested review from
0xAustinWang,
KodeyThomas,
chris-de-leon-cll,
Copilot,
matYang,
mateusz-sekara,
tt-cll and
winder
|
👋 sowgandhi11, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR updates the fee deployment flow to support SetTokenTransferFee across newer CCIP FeeQuoter versions by (a) discovering the deployed fee contract/version from the datastore, (b) returning AddressRef for fee contracts, and (c) adding an EVM CCIP 2.0 fee adapter implementation.
Changes:
- Extend the
FeeAdapterinterface withGetFeeContractRef(...) (datastore.AddressRef, error)and update EVM adapters to returnAddressRefdata. - Update the SetTokenTransferFee changeset to select an adapter based on the discovered on-chain fee contract version.
- Add a CCIP 2.0 EVM fees adapter and adjust v1.6 FeeQuoter “latest address” selection logic.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
deployment/fees/set_token_transfer_fee.go |
Selects fee adapter based on discovered fee contract ref/version before executing the update sequence. |
deployment/fees/product.go |
Extends FeeAdapter interface to expose fee contract AddressRef. |
chains/evm/deployment/v2_0_0/adapters/fees.go |
New CCIP 2.0 EVM fees adapter implementation (FeeQuoter-based). |
chains/evm/deployment/v1_6_0/sequences/adapter.go |
Updates FeeQuoter “latest version” selection to remove the previous upper bound. |
chains/evm/deployment/v1_5_0/adapters/fees.go |
Updates v1.5 fee adapter to return a fee contract AddressRef (OnRamp-based). |
Comments suppressed due to low confidence (1)
deployment/fees/set_token_transfer_fee.go:132
inferTokenTransferFeeArgsis called withadapter(resolved fromcfg.Version), but the actual sequence execution usesupdater(resolved from the on-chain fee contract ref version). Ifcfg.Versiondiffers from the deployed contract version, inference may query the wrong ABI/contract and fail or infer incorrect defaults. Consider selecting a single adapter based on the discovered contract ref version and using it consistently for inference and updates (or error if versions disagree).
settings[dst.Selector] = map[string]*TokenTransferFeeArgs{}
for _, feeCfg := range dst.Settings {
if args, err := inferTokenTransferFeeArgs(adapter, e, src.Selector, dst.Selector, feeCfg); err != nil {
return cldf.ChangesetOutput{}, fmt.Errorf("failed to infer token transfer fee args for token %s: %w", feeCfg.Address, err)
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| version *semver.Version | ||
| adapter FeeAdapter | ||
| settings map[uint64]map[string]*TokenTransferFeeArgs | ||
| }{} |
There was a problem hiding this comment.
mind if we add a type for this under line 102?
batchOps := make([]mcms_types.BatchOperation, 0)
reports := make([]cldf_ops.Report[any, any], 0)
type FeeGroup struct {
version *semver.Version
adapter FeeAdapter
settings map[uint64]map[string]*TokenTransferFeeArgs
}
// ...
versionGroups := map[string]FeeGroup{}| for dst, dstCfg := range input.Settings { | ||
|
|
There was a problem hiding this comment.
small nit: is it possible to revert the formatting changes on this so the diff is smaller? For context, sometimes when debugging we need to look at past PRs to see where the issue got introduced and having a smaller diff always helps!
|
|
||
| feeContractRef, err := adapter.GetFeeContractRef(e, src.Selector, dst.Selector) | ||
| if err != nil { | ||
| return cldf.ChangesetOutput{}, fmt.Errorf("failed to get fee contract ref for src %d and dst %d: %w", src.Selector, src.Settings[0].Selector, err) |
There was a problem hiding this comment.
| return cldf.ChangesetOutput{}, fmt.Errorf("failed to get fee contract ref for src %d and dst %d: %w", src.Selector, src.Settings[0].Selector, err) | |
| return cldf.ChangesetOutput{}, fmt.Errorf("failed to get fee contract ref for src %d and dst %d: %w", src.Selector, dst.Selector, err) |
| adapter FeeAdapter | ||
| settings map[uint64]map[string]*TokenTransferFeeArgs | ||
| }{ | ||
| version: feeContractRef.Version, |
There was a problem hiding this comment.
Is this field used/needed? If so, should we be storing lookupVersion here?
| ) | ||
|
|
||
| if err != nil { | ||
| return datastore.AddressRef{}, fmt.Errorf("failed to find FeeQuoter address ref for chain selector %d: %w", src, err) |
There was a problem hiding this comment.
| return datastore.AddressRef{}, fmt.Errorf("failed to find FeeQuoter address ref for chain selector %d: %w", src, err) | |
| return datastore.AddressRef{}, fmt.Errorf("failed to find EVM2EVMOnRamp address ref for chain selector %d: %w", src, err) |
| ChainSelector: input.Selector, | ||
| ExistingAddresses: []datastore.AddressRef{fqRef}, |
There was a problem hiding this comment.
I think we need to pass more things here right? i.e. updatesByChain?
| feecontractref := e.DataStore.Addresses().Filter( | ||
| datastore.AddressRefByAddress(solana.PublicKeyFromBytes(fqAddr).String()), | ||
| datastore.AddressRefByType(datastore.ContractType(fee_quoter_operations.ContractType)), | ||
| datastore.AddressRefByChainSelector(src), | ||
| ) | ||
|
|
||
| if len(feecontractref) == 0 { | ||
| return datastore.AddressRef{}, fmt.Errorf("no address ref found for FeeQuoter contract at address %s on chain selector %d", fqAddr, src) | ||
| } | ||
|
|
||
| return feecontractref[0], nil |
There was a problem hiding this comment.
Probably meant to use FindAndFormatRef?
| Address: common.BytesToAddress(fqAddr).Hex(), | ||
| ChainSelector: src, | ||
| } | ||
| feecontractref, err := datastore_utils.FindAndFormatRef( |
There was a problem hiding this comment.
nit: camel case
Removed the upper bound for identifying the latest FQ address
Returning the
AddressRefdata for the FeeQuoterAdded a new method
GetFeeContractRef(e cldf.Environment, src uint64, dst uint64) (AddressRef, error)Added a fees.go adapter implementation for CCIP 2.0