Connect EVM with EVM: Add support for skipping FQ v2 config if already configured during migration#21665
Connect EVM with EVM: Add support for skipping FQ v2 config if already configured during migration#21665HelloKashif wants to merge 1 commit intodevelopfrom
Conversation
HelloKashif
requested review from
AmrMohamedRezk,
agusaldasoro,
b-gopalswami,
Copilot,
makramkd,
ogtownsend,
tt-cll and
winder
|
👋 HelloKashif, 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! |
|
✅ No conflicts with other open PRs targeting |
HelloKashif
force-pushed
the
feat/skip-existing-fq-dest-configs
branch
from
0da1c07 to
aa34bbe
Compare
There was a problem hiding this comment.
Pull request overview
Risk Rating: HIGH
This PR changes how v2 FeeQuoter destination config updates are generated during lane updates, aiming to avoid overwriting already-configured v2 FeeQuoter destinations during migration.
Changes:
- Add
FilterOutExistingDestChainConfigsto query v2 FeeQuoter on-chain state and skip dest configs that are already enabled. - Invoke this filter when building v2 FeeQuoter lane-update operations.
- Add a unit test covering the “already enabled dest config is filtered out and not overwritten” behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
deployment/ccip/changeset/v1_6/cs_update_bidirectional_lanes.go |
Adds and wires a new on-chain filtering step for v2 FeeQuoter dest config updates. |
deployment/ccip/changeset/v1_6/cs_update_bidirectional_lanes_test.go |
Adds a focused test ensuring enabled destinations are filtered and existing config remains unchanged. |
Requires scrupulous human review (behavioral / breaking-change risk):
FilterOutExistingDestChainConfigs+ its unconditional use inUpdateLanesLogicfor v2 FeeQuoter chains (it can change the semantics of “update” by skipping updates to already-enabled destinations).
Suggested reviewers (per .github/CODEOWNERS for /deployment/ccip):
@smartcontractkit/ccip-tooling,@smartcontractkit/ccip-offchain,@smartcontractkit/operations-platform(and@smartcontractkit/coreas needed)
| for _, destCfg := range destCfgs { | ||
| existing, err := fqContract.GetDestChainConfig(&bind.CallOpts{Context: e.GetContext()}, destCfg.DestChainSelector) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to query existing dest chain config on chain %d for dest %d: %w", | ||
| chainSel, destCfg.DestChainSelector, err) | ||
| } | ||
| if existing.IsEnabled { | ||
| e.Logger.Infow("skipping dest chain config already present on v2 FeeQuoter", | ||
| "sourceChain", chainSel, | ||
| "destChain", destCfg.DestChainSelector, | ||
| ) | ||
| continue | ||
| } | ||
| filtered = append(filtered, destCfg) |
There was a problem hiding this comment.
FilterOutExistingDestChainConfigs currently drops all updates for destinations that are already enabled on-chain. That means legitimate config changes (or an intended disable) for an already-enabled destination will be silently skipped, which changes the semantics of running UpdateLanesLogic against an existing v2 FeeQuoter. Consider narrowing the filter to the migration/idempotency case (e.g., only skip when the requested config is an enable and the on-chain config already matches, or gate behind an explicit flag) so intentional updates to enabled destinations still apply.
HelloKashif
force-pushed
the
feat/skip-existing-fq-dest-configs
branch
from
aa34bbe to
fc270eb
Compare
HelloKashif
force-pushed
the
feat/skip-existing-fq-dest-configs
branch
from
fc270eb to
b4914f5
Compare
…y configured during migration
HelloKashif
force-pushed
the
feat/skip-existing-fq-dest-configs
branch
from
b4914f5 to
134adea
Compare
|
Connect EVM with EVM: Add support for skipping FQ v2 config if already configured during migration