CCIP-4158 chain contract changeset #15294
Conversation
|
|
||
| var _ deployment.ChangeSet[DeployChainContractsConfig] = DeployChainContracts | ||
|
|
||
| func DeployChainContracts(env deployment.Environment, c DeployChainContractsConfig) (deployment.ChangesetOutput, error) { |
There was a problem hiding this comment.
Comments on these would be nice
| return deployment.ChangesetOutput{AddressBook: newAddresses}, deployment.MaybeDataErr(err) | ||
| } | ||
| return deployment.ChangesetOutput{ | ||
| Proposals: []timelock.MCMSWithTimelockProposal{}, |
There was a problem hiding this comment.
Are all of these changesets going to be converted to use MCMS? Or is MCMS for post-deployment processes only (transferring ownership, setting config, etc.)
There was a problem hiding this comment.
MCMS for post deployment
| "NodeOperator": p2pIds, | ||
| }, | ||
| } | ||
| output, err := DeployHomeChain(e, homeChainCfg) |
There was a problem hiding this comment.
Figuring out the correct order of incantations to get a functioning environment seems to be getting harder with all these new changesets, are we planning a more "hard to mess up" API in order to do things like this or nah?
There was a problem hiding this comment.
Or maybe for tests, a simple function that composes everything needed in order to get a functioning env
There was a problem hiding this comment.
a simple function that composes everything needed in order to get a functioning env
I added this in #15288 FYI
| ccipdeployment "github.com/smartcontractkit/chainlink/deployment/ccip" | ||
| ) | ||
|
|
||
| func Jobspec(env deployment.Environment, _ any) (deployment.ChangesetOutput, error) { |
There was a problem hiding this comment.
Comment + how this is expected to be used in conjunction with all the other stuff would be useful
There was a problem hiding this comment.
Also naming is kinda not great on this one
| return err | ||
| } | ||
| if cr != CCIPCapabilityID { | ||
| return fmt.Errorf("capability registry does not support CCIP %s %s", hexutil.Encode(cr[:]), hexutil.Encode(CCIPCapabilityID[:])) |
There was a problem hiding this comment.
| return fmt.Errorf("capability registry does not support CCIP %s %s", hexutil.Encode(cr[:]), hexutil.Encode(CCIPCapabilityID[:])) | |
| return fmt.Errorf("unexpected mismatch between calculated ccip capability id (%s) and expected ccip capability id constant (%s)", hexutil.Encode(cr[:]), hexutil.Encode(CCIPCapabilityID[:])) |
There was a problem hiding this comment.
This would ultimately be a bug in the constant
There was a problem hiding this comment.
LGTM, I don't have any comments but please fix @makramkd remarks first
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* initial deploy chain * more updates * rename * fix DeployChainContracts
Requires
Supports