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.

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

Accept home chain selector id in commit plugin factory #140

Merged
merged 23 commits into from
Sep 30, 2024

Conversation

0xnogo
Copy link
Contributor

@0xnogo 0xnogo commented Sep 19, 2024

Accept the hom chain selector id in the under the rmnEnabled flag (otherwise integration test will fail as the RMNHome contract is not ready + RMNHomeAddress is not yet defined in OCR3Config.

The Commit factory will instantiate a RMNHome reader per plugin instance as the rmnHomeAddress is part of the OCR3Config, hence not uniquely defined (1 per don id)

The build of chainlink is failing: here is the PR to mirror the interface changes made in this PR: smartcontractkit/chainlink#14500.

@0xnogo 0xnogo changed the title Accept home chain selector id in comming plugin factory Accept home chain selector id in commit plugin factory Sep 19, 2024
@0xnogo 0xnogo changed the base branch from ng/rmn-readers to main September 19, 2024 18:49
@0xnogo 0xnogo marked this pull request as ready for review September 26, 2024 10:44
@0xnogo 0xnogo requested a review from a team as a code owner September 26, 2024 10:44
commit/factory.go Show resolved Hide resolved
@@ -358,6 +358,7 @@ type OCR3Config struct {
F uint8 `json:"F"`
OffchainConfigVersion uint64 `json:"offchainConfigVersion"`
OfframpAddress []byte `json:"offrampAddress"`
RmnHomeAddress []byte `json:"rmnHomeAddress"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will cause all tests to fail because this is not on-chain yet. I think we should remove this for now and maybe add later when its finally in there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be just empty? We are not using it (under the rmn feature flag).

It seems like ccip integ test are passing here: smartcontractkit/chainlink#14500

Copy link

Test Coverage

Branch Coverage
ng/chain-selector-id 73.5%
main 73.8%

Copy link

Metric ng/chain-selector-id main
Coverage 71.9% 72.2%

@0xnogo 0xnogo merged commit 4a660be into main Sep 30, 2024
3 of 4 checks passed
@0xnogo 0xnogo deleted the ng/chain-selector-id branch September 30, 2024 05:26
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