-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Granular MCMS optional CCIP chain contract changesets #15651
Granular MCMS optional CCIP chain contract changesets #15651
Conversation
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
}, | ||
} | ||
} | ||
|
||
type PromoteAllCandidatesChangesetConfig struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made promote/add/setcandidate all support multiple DONs at once. Relevant for signer changes as an example
EncodableChainConifg chainconfig.ChainConfig | ||
} | ||
|
||
type UpdateChainConfigConfig struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new changeset here for managing chain config. Parameterizing the readers in particular will allow us to test heterogeneous chain support
) | ||
|
||
var ( | ||
_ deployment.ChangeSet[UpdateOnRampDestsConfig] = UpdateOnRampsDests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collectively these changesets allow for:
- Adding/removing lanes and chains
- Changing fee config
- Promoting lanes to real router
_ deployment.ChangeSet[UpdateOffRampSourcesConfig] = UpdateOffRampSources | ||
_ deployment.ChangeSet[UpdateRouterRampsConfig] = UpdateRouterRamps | ||
_ deployment.ChangeSet[UpdateFeeQuoterDestsConfig] = UpdateFeeQuoterDests | ||
_ deployment.ChangeSet[SetOCR3OffRampConfig] = SetOCR3OffRamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note SetOCR3OffRamp technically has test coverage because its used in the env setup. Can add an explicit unit test in a follow up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!! 👏
Replacing the add chain/add lane with more granular/flexible changesets which are MCMS optional. Flexibility is needed for when we want to test new chains/lanes via test router. Also moving towards a
Validate(e deployment.Environment) error
pattern to standardize (loading the state twice not a particularly big deal) and I added an ValidateOwnership function for general use with MCMS optional changesets.Follow up work: replace AddLanes with a composition of chain contract changesets and add a AddNewChain test.