CCIP- 4158 deploy home changeset #15143
Conversation
Flaky Test Detector for
|
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
AnieeG
changed the title
| if c.RMNStaticConfig.OffchainConfig == nil { | ||
| return fmt.Errorf("offchain config for RMNHomeStaticConfig must be set") | ||
| } | ||
| if c.NodeOperators == nil || len(c.NodeOperators) == 0 { |
There was a problem hiding this comment.
nit, but len(slice) == 0 already covers nil slices, so c.NodeOperators == nil is redundant, right?
| if nop.Name == "" { | ||
| return fmt.Errorf("node operator name must be set") | ||
| } | ||
| if c.NodeP2PIDsPerNodeOpAdmin[nop.Name] == nil || len(c.NodeP2PIDsPerNodeOpAdmin[nop.Name]) == 0 { |
deployment/ccip/deploy_home_chain.go
Outdated
| ab deployment.AddressBook, | ||
| chain deployment.Chain, | ||
| ) (*ContractDeploy[*capabilities_registry.CapabilitiesRegistry], error) { | ||
| capRegAddress, err := deployment.SearchAddressBook(ab, chain.Selector, CapabilitiesRegistry) |
There was a problem hiding this comment.
Hmm, not sure about that. So if error is nil we assume we found it, but if error is present we assume it's not there and needs deployment? What if the error is not related to a lack of capabilities registry but some other transient issue that would trigger unnecessary deployment?
I would consider a more explicit interface here, for instance, something like this
// bool added to returned types
func SearchAddressBook(ab AddressBook, chain uint64, typ ContractType) (string, bool, error) {}
capRegAddress, ok, err := deployment.SearchAddressBook(ab, chain.Selector, CapabilitiesRegistry)
if err != nil {
return nil, err
}
if !ok {
deployContract
} else {
returnAlreadyDeployed
}or returning a pointer to distinguish between the error and contract not being deployed
There was a problem hiding this comment.
Also, do we have any tests to cover these cases?
There was a problem hiding this comment.
There can be two scenarios -
- If Cap reg is present and not deployed again through DeployHomeChain- is covered by in-memory test. Here we deploy CapReg first and then call DeployHomeChain.
- If Cap reg is not present and deployed through DeployHomeChain - will be covered by docker tests( I will create a ticket to cover this in next pr) - In this case we will be restarting docker nodes once home chain is deployed with newly deployed cap reg address.
Flaky Test Detector for
|
| if err != nil { | ||
| return deployment.ChangesetOutput{}, errors.Wrapf(deployment.ErrInvalidConfig, "%v", err) | ||
| } | ||
| ab := deployment.NewMemoryAddressBook() |
There was a problem hiding this comment.
the is an addressbook in the env now. should be able to use it
There was a problem hiding this comment.
This is intentional. wanted to - keep env's Existing address unchanged, and then once the changeset is executed merge the address book with newly generated addresses.
In this way we can know from each changeset which addresses got deployed.
Based on discussion with @connorwstein the format of every changeset should be -
func DoThingChangeset(e deployment.Environment, config interface {}) deployment.ChangesetOutput {
// create new address book each time so that changeset output can denote
// which addresses are newly deployed
newAddresses := deployment.NewAddressBook()
// deployX should mutate newAddresses adding them.
// it can use e.ExistingAddresses to load existing state
// and do checks
deployX(newAddresses, e)
return Output{addresses: newAddresses}
}
| RMNStaticConfig rmn_home.RMNHomeStaticConfig | ||
| RMNDynamicConfig rmn_home.RMNHomeDynamicConfig | ||
| NodeOperators []capabilities_registry.CapabilitiesRegistryNodeOperator | ||
| NodeP2PIDsPerNodeOpAdmin map[string][][32]byte |
There was a problem hiding this comment.
nit/ i've been using p2pkey.PeerID when i want peer ids and converting to [32]byte for the contract when needed specifically for the contract.
have utils we can share/promote if you want to do the same
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
DeployHomeChain Changeset