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.

Sign up for GitHub

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

follow up the chain selector refactor for CCIP plugin, fix potential bug in CR/CW config #15482

Merged
merged 3 commits into from
Dec 5, 2024
Merged

follow up the chain selector refactor for CCIP plugin, fix potential bug in CR/CW config #15482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cbe3922
fix chain family logic
huangzhen1997 Dec 2, 2024
7d9d00b
add changeset
huangzhen1997 Dec 3, 2024
1dad686
Merge branch 'develop' of github.com:smartcontractkit/chainlink into …
huangzhen1997 Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/breezy-kings-clean.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

Potential bug introduced from chain selector refactor, not causing issue now since only EVM is used, but need to fix #bugfix
10 changes: 5 additions & 5 deletions core/capabilities/ccip/oraclecreator/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ func (i *pluginOracleCreator) createReadersAndWriters(
pluginType cctypes.PluginType,
config cctypes.OCR3ConfigWithMeta,
publicCfg ocr3confighelper.PublicConfig,
chainFamily string,
destChainFamily string,
) (
map[cciptypes.ChainSelector]types.ContractReader,
map[cciptypes.ChainSelector]types.ChainWriter,
Expand Down Expand Up @@ -328,8 +328,8 @@ func (i *pluginOracleCreator) createReadersAndWriters(
chainWriters := make(map[cciptypes.ChainSelector]types.ChainWriter)
for relayID, relayer := range i.relayers {
chainID := relayID.ChainID

chainSelector, err1 := i.getChainSelector(chainID, chainFamily)
relayChainFamily := relayID.Network
chainSelector, err1 := i.getChainSelector(chainID, relayChainFamily)
if err1 != nil {
return nil, nil, fmt.Errorf("failed to get chain selector from chain ID %s: %w", chainID, err1)
}
Expand All @@ -344,7 +344,7 @@ func (i *pluginOracleCreator) createReadersAndWriters(
return nil, nil, err1
}

if chainID == destChainID {
if chainID == destChainID && destChainFamily == relayChainFamily {
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the difference between a chainID and a chain selector?

Is it true that this extra check is to ensure we don't mis-match in case the same chainID is used in different chain families?

Copy link
Contributor Author

@huangzhen1997 huangzhen1997 Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, chain selector is a randomly generated uint64 and used by CCIP, and it is unique across all chains/families.

The chain id is specific to a chain family, so it doesn't provide this guarantee. For example chainid: 1 can either beethereum-mainnet or aptos-mainnet

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I was mixing up chainID and chainSelector. They're both differen't ways to ID a chain so it's hard to reason about, some kind of (chainID, chainFamily) tuple type and/or converting to one or the other rather than passing them around is needed. Having an ID with type string doesn't help.

I believe getChainReaderConfig has this same bug.

offrampAddressHex := common.BytesToAddress(config.Config.OfframpAddress).Hex()
err2 := cr.Bind(ctx, []types.BoundContract{
{
Expand All @@ -367,7 +367,7 @@ func (i *pluginOracleCreator) createReadersAndWriters(
relayer,
i.transmitters,
execBatchGasLimit,
chainFamily)
relayChainFamily)
if err1 != nil {
return nil, nil, err1
}
Expand Down
Loading