Ccip-4110 migration test #15854
Ccip-4110 migration test #15854
Conversation
AnieeG
requested review from
winder,
rstout,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara and
0xnogo
|
I see you updated files related to
|
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
| @@ -323,7 +323,7 @@ func (params CCIPJobSpecParams) BootstrapJob(contractID string) *OCR2TaskJobSpec | |||
| }, | |||
| } | |||
| return &OCR2TaskJobSpec{ | |||
| Name: fmt.Sprintf("%s-%s", Boostrap, params.DestChainName), | |||
| Name: fmt.Sprintf("%s-%s-%s", Boostrap, params.SourceChainName, params.DestChainName), | |||
There was a problem hiding this comment.
If we're here fixing stuff may as well fix the typo Boostrap
| }, | ||
| }) | ||
| require.NoError(t, err) | ||
| AddLaneWithDefaultPricesAndFeeQuoterConfig(t, &e, state, chain1, chain2, true) |
There was a problem hiding this comment.
Haha we removed this and now its back, I actually copy/pasted the above code for my PR but I guess having this helper is fine.
| if cfg.TestRouter { | ||
| if err := commoncs.ValidateOwnership(e.GetContext(), cfg.MCMS != nil, e.Chains[chainSel].DeployerKey.From, chainState.Timelock.Address(), chainState.TestRouter); err != nil { | ||
| return err | ||
| } | ||
| } else { | ||
| if err := commoncs.ValidateOwnership(e.GetContext(), cfg.MCMS != nil, e.Chains[chainSel].DeployerKey.From, chainState.Timelock.Address(), chainState.Router); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
q: if TestRouter is enabled why not validate ownership of it + the real router? Instead of either/or
There was a problem hiding this comment.
The real router can be owned by MCMS and test router can be owned by deployer key.
| RMNStaticConfig: NewTestRMNStaticConfig(), | ||
| NodeOperators: NewTestNodeOperator(e.Env.Chains[e.HomeChainSel].DeployerKey.From), | ||
| NodeP2PIDsPerNodeOpAdmin: map[string][][32]byte{ | ||
| "NodeOperator": envNodes.NonBootstraps().PeerIDs(), |
There was a problem hiding this comment.
Is "NodeOperator" the name of the nop or something? Would be nice as a constant or determined via some other way?
| @@ -233,15 +235,25 @@ func (d *DeployedEnv) SetupJobs(t *testing.T) { | |||
|
|
|||
| type MemoryEnvironment struct { | |||
| DeployedEnv | |||
| Chains map[uint64]deployment.Chain | |||
| TCConfig *TestConfigs | |||
There was a problem hiding this comment.
naming nit: just TestConfigs?
| var err error | ||
| envNodes, err := deployment.NodeInfo(e.Env.NodeIDs, e.Env.Offchain) |
There was a problem hiding this comment.
| var err error | |
| envNodes, err := deployment.NodeInfo(e.Env.NodeIDs, e.Env.Offchain) | |
| envNodes, err := deployment.NodeInfo(e.Env.NodeIDs, e.Env.Offchain) |
| // Adding to src1 and dest chains first, due to the open issue - | ||
| // if ccip contracts are added to all chains and only one lane is enabled between src1 and dest, the plugin will not work | ||
| // until lanes are added involving the rest of the chains |
There was a problem hiding this comment.
What open issue is this referring to?
…ccip-4110-migration-test
…t/chainlink into ccip-4110-migration-testi
…ccip-4110-migration-test
| require.NoError(t, testCfg.Validate(), "invalid test con"+ | ||
| "fig") |
There was a problem hiding this comment.
Why is the error string two lines?
| tEnv.UpdateDeployedEnvironment(e) | ||
| e = AddCCIPContractsToEnvironment(t, e.Env.AllChainSelectors(), tEnv) | ||
| // now we update RMNProxy to point to RMNRemote | ||
| e.Env, err = commonchangeset.ApplyChangesets(t, e.Env, nil, []commonchangeset.ChangesetApplication{ |
There was a problem hiding this comment.
Noticed that this isn't in AddCCIPContractsToEnvironment, is that intentional? This caught me when writing the add chain test
There was a problem hiding this comment.
Yeah. RMNProxy can be owned by timelock like in this test case. If owned by timelock, we will have to run this changeset with MCMS config but that's not the case for generic test set up. that's why I have kept it separate.
|
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
Requires
Supports