MCMS reader for Chainlink-CCIP produce interface #544
Conversation
huangzhen1997
changed the base branch from
main
to
NONEVM-3443/separate-tooling-api-adaptor
krebernisak
left a comment
krebernisak
left a comment
There was a problem hiding this comment.
Looks good! (with comments)
But also, this adapter feels very thin and makes me wonder can we generalize this across chains...
| "github.com/smartcontractkit/chainlink-ton/deployment/state" | ||
| ) | ||
|
|
||
| type MCMSReaderAdapter struct{} |
There was a problem hiding this comment.
nit: implement a constructor and return a CCIP shared type. This allows the compiler to show errors and the reader to understand which type is getting implemented here.
Or can use a pattern like:
var _ sdk.X = (*MCMSReaderAdapter)(nil)
| @@ -0,0 +1,70 @@ | |||
| package sequences | |||
There was a problem hiding this comment.
Not sure this fits the sequences pkg. Let's figure out a better structure eventually.
| e.DataStore.Addresses().Filter(), | ||
| chainSelector, | ||
| deployment.ContractType(state.MCMS), | ||
| &state.Version1_6_0, |
There was a problem hiding this comment.
Version 1.6 is not correct for MCMS, right?
Also, what's the point of matching a version here, why do we need an address for a specific version of MCMS per env?
There was a problem hiding this comment.
Fixed it, the version is a required param for the datastore GetAddressRef function
huangzhen1997
dismissed
krebernisak’s stale review
The base branch was changed.
There was a problem hiding this comment.
Pull request overview
This PR implements an MCMS (Multi-Chain Management System) reader adapter for Chainlink-CCIP on TON blockchain, enabling interaction with MCMS and Timelock contracts through a standardized interface.
Changes:
- Centralized MCMS and Timelock contract version definitions in the state package
- Created MCMSReaderAdapter implementing the MCMSReader interface for TON chains
- Refactored MCMS deployment to use centralized version constants
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| deployment/state/state.go | Added centralized version constants for MCMS (0.0.4) and Timelock (0.0.3) contracts |
| deployment/ccip/1_6_0/sequences/mcms_reader.go | New adapter implementing MCMSReader interface with methods to retrieve chain metadata and contract references |
| deployment/ccip/1_6_0/sequences/mcms.go | Refactored to use centralized version constants instead of local semver parsing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Duplicates of chainlink/deployment/ccip/ to avoid import loops | ||
| var ( | ||
| Version1_6_0 = *semver.MustParse("1.6.0") | ||
| // MCMS contract versions |
There was a problem hiding this comment.
The variable names TimelockVersion and MCMSVersion are inconsistent with the existing naming pattern. The existing version constant is named Version1_6_0 which includes the version number in the name. Consider renaming to TimelockVersion0_0_3 and MCMSVersion0_0_4 for consistency, or update the comment to be more descriptive of what these versions represent.
| // MCMS contract versions | |
| // MCMS-related contract versions (Timelock v0.0.3 and MCMS v0.0.4) |
|
|
||
| "github.com/smartcontractkit/chainlink-ton/deployment/state" | ||
| ) | ||
|
|
There was a problem hiding this comment.
The MCMSReaderAdapter struct lacks documentation. Add a comment explaining its purpose as an adapter for reading MCMS contract information from TON chains.
| // MCMSReaderAdapter is an adapter for reading MCMS contract information from TON chains. |
| ref := datastore_utils.GetAddressRef( | ||
| e.DataStore.Addresses().Filter(), | ||
| chainSelector, | ||
| deployment.ContractType(state.Timelock), |
There was a problem hiding this comment.
The code references state.Timelock which is not defined in the provided state.go diff. Verify that the Timelock constant is properly defined in the state package.
| ref := datastore_utils.GetAddressRef( | ||
| e.DataStore.Addresses().Filter(), | ||
| chainSelector, | ||
| deployment.ContractType(state.MCMS), |
There was a problem hiding this comment.
The code references state.MCMS which is not defined in the provided state.go diff. Verify that the MCMS constant is properly defined in the state package.
2 participants
Jira