Conversation
sowgandhi11
requested review from
AmrMohamedRezk,
agusaldasoro,
asoliman92,
b-gopalswami,
Copilot,
makramkd,
ogtownsend and
winder
There was a problem hiding this comment.
Pull request overview
Adds optional “dynamic FeeQuoter address resolution” so lane setup can source the FeeQuoter from on-chain OnRamp dynamic config (instead of only from datastore), and updates EVM fee adapters to use the dynamic lookup.
Changes:
- Introduces
lanes.DynamicFeeQuoterinterface and updates lane address population to use it when available. - Implements
GetFQAddressDynamicfor EVM adapters by readingOnRamp.getDynamicConfig().feeQuoter. - Updates fee adapters/tests to rely on the dynamic FeeQuoter path.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| deployment/lanes/product.go | Adds optional DynamicFeeQuoter interface definition. |
| deployment/lanes/connect_chains.go | Updates populateAddresses to optionally use dynamic FeeQuoter resolution. |
| deployment/lanes/disable_lane.go | Updates calls to populateAddresses after signature change. |
| chains/evm/deployment/v1_6_0/sequences/adapter.go | Adds GetFQAddressDynamic implementation for v1.6.0 EVM adapter. |
| chains/evm/deployment/v1_6_0/adapters/fees.go | Switches fee contract lookup to dynamic FeeQuoter resolution. |
| chains/evm/deployment/v2_0_0/adapters/fees.go | Switches fee contract lookup to dynamic FeeQuoter resolution. |
| ccv/chains/evm/deployment/v1_7_0/adapters/chain_family.go | Adds GetFQAddressDynamic for CCV EVM chain-family adapter. |
| ccv/chains/evm/deployment/v1_7_0/adapters/chain_family_test.go | Adds assertion that the 2.0.0 EVM adapter implements DynamicFeeQuoter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dynamicConfig, err := onrampContract.GetDynamicConfig(&bind.CallOpts{}) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to call GetDynamicConfig on onramp contract for chain selector %d: %w", chainSelector, err) |
There was a problem hiding this comment.
This file imports github.com/ethereum/go-ethereum/accounts/abi/bind/v2, but the generated onramp contract wrapper (chains/evm/deployment/v1_6_0/operations/onramp) uses github.com/ethereum/go-ethereum/accounts/abi/bind and expects *bind.CallOpts. As written, &bind.CallOpts{} will be the wrong type and won’t compile; use the non-v2 bind package or simply pass nil call opts to GetDynamicConfig.
deployment/lanes/connect_chains.go
Outdated
| } | ||
|
|
||
| func populateAddresses(ds datastore.DataStore, chainDef *ChainDefinition, adapter LaneAdapter, version *semver.Version, isTestRouter bool) error { | ||
| func populateAddresses(chainDef *ChainDefinition, adapter LaneAdapter, version *semver.Version, isTestRouter bool, e cldf.Environment) error { |
There was a problem hiding this comment.
super nit but generally e cldf.Environment is the first param. And, while we're usually bad about enforcing this, passing it as a ptr is more efficient.
|
No description provided.