Skip to content

CCIP - make required config types public and update legacy snapshot function args #1415

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ogtownsend
Copy link
Contributor

@ogtownsend ogtownsend commented Jul 18, 2025

  • A few of the config poller related structs were private when they were defined in cl-ccip. However, since they now live in cl-common and we need to access them in cl-ccip and other repos, they need to be public.
  • Updates GetAllConfigLegacySnapshot(...) function args since it needs to have knowledge of which chain is the dest chain when constructing the batch read query

@ogtownsend ogtownsend changed the title CCIP - make required config types public and update legacy snapshot f… CCIP - make required config types public and update legacy snapshot function args Jul 18, 2025
@ogtownsend ogtownsend marked this pull request as ready for review July 18, 2025 05:11
@ogtownsend ogtownsend requested a review from a team as a code owner July 18, 2025 05:11
Comment on lines +67 to +68
destChainSelector ChainSelector,
sourceChainSelectors []ChainSelector,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't destChainSelector implicit? Thats the only chain the accessor is able to read from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@winder it's for this check: https://github.com/smartcontractkit/chainlink-ccip/blob/main/pkg/reader/ccip.go#L1224

the accessor needs to know which configs it should fetch (unless want to remove that check and attempt to fetch them all)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we could just check if destChain is contained within sourceChainSelectors, but that might not be exhaustive if sourceChainSelectors only has a subset of chains

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that check staying outside of the accessor? Maybe we need a way for the accessor user to get the associated selector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that check would be within the accessor since that function is very specific to the chain reader, here is a WIP: https://github.com/smartcontractkit/chainlink-ccip/compare/cal-implement-GetAllConfigLegacySnapshot?expand=1

@ogtownsend ogtownsend force-pushed the ogt/cal-make-required-ccip-config-structs-public branch from f87ac6b to 7847fa7 Compare July 28, 2025 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants