From 11ed8f9f7ceffa74df849d89bb5c232219f84795 Mon Sep 17 00:00:00 2001 From: "Simon B.Robert" Date: Thu, 12 Dec 2024 09:46:54 -0500 Subject: [PATCH 1/2] Use RMN changeset in e2e tests and refactor RMN remote changeset RMN remote changeset was changed to allow more flexibility when configuring remote configs. Allowing a subset of chains to be updated in a single changeset --- .../ccip/changeset/cs_update_rmn_config.go | 38 ++++++--- .../changeset/cs_update_rmn_config_test.go | 13 ++- integration-tests/smoke/ccip/ccip_rmn_test.go | 79 +++++++------------ 3 files changed, 63 insertions(+), 67 deletions(-) diff --git a/deployment/ccip/changeset/cs_update_rmn_config.go b/deployment/ccip/changeset/cs_update_rmn_config.go index 25ae8308eb5..4ae97affe2d 100644 --- a/deployment/ccip/changeset/cs_update_rmn_config.go +++ b/deployment/ccip/changeset/cs_update_rmn_config.go @@ -340,10 +340,14 @@ func buildRMNRemotePerChain(e deployment.Environment, state CCIPOnChainState) ma return timelocksPerChain } +type RMNRemoteConfig struct { + Signers []rmn_remote.RMNRemoteSigner + F uint64 +} + type SetRMNRemoteConfig struct { HomeChainSelector uint64 - Signers []rmn_remote.RMNRemoteSigner - F uint64 + RMNRemoteConfigs map[uint64]RMNRemoteConfig MCMSConfig *MCMSConfig } @@ -353,14 +357,21 @@ func (c SetRMNRemoteConfig) Validate() error { return err } - for i := 0; i < len(c.Signers)-1; i++ { - if c.Signers[i].NodeIndex >= c.Signers[i+1].NodeIndex { - return fmt.Errorf("signers must be in ascending order of nodeIndex") + for chain, config := range c.RMNRemoteConfigs { + err := deployment.IsValidChainSelector(chain) + if err != nil { + return err } - } - if len(c.Signers) < 2*int(c.F)+1 { - return fmt.Errorf("signers count must greater than or equal to %d", 2*c.F+1) + for i := 0; i < len(config.Signers)-1; i++ { + if config.Signers[i].NodeIndex >= config.Signers[i+1].NodeIndex { + return fmt.Errorf("signers must be in ascending order of nodeIndex") + } + } + + if len(config.Signers) < 2*int(config.F)+1 { + return fmt.Errorf("signers count must greater than or equal to %d", 2*config.F+1) + } } return nil @@ -397,9 +408,10 @@ func NewSetRMNRemoteConfigChangeset(e deployment.Environment, config SetRMNRemot rmnRemotePerChain := buildRMNRemotePerChain(e, state) batches := make([]timelock.BatchChainOperation, 0) - for chain, remote := range rmnRemotePerChain { - if remote == nil { - continue + for chain, remoteConfig := range config.RMNRemoteConfigs { + remote, ok := rmnRemotePerChain[chain] + if !ok { + return deployment.ChangesetOutput{}, fmt.Errorf("RMNRemote not found for chain %d", chain) } currentVersionConfig, err := remote.GetVersionedConfig(nil) @@ -409,8 +421,8 @@ func NewSetRMNRemoteConfigChangeset(e deployment.Environment, config SetRMNRemot newConfig := rmn_remote.RMNRemoteConfig{ RmnHomeContractConfigDigest: activeConfig, - Signers: config.Signers, - F: config.F, + Signers: remoteConfig.Signers, + F: remoteConfig.F, } if reflect.DeepEqual(currentVersionConfig.Config, newConfig) { diff --git a/deployment/ccip/changeset/cs_update_rmn_config_test.go b/deployment/ccip/changeset/cs_update_rmn_config_test.go index 3ec309182aa..46422877e90 100644 --- a/deployment/ccip/changeset/cs_update_rmn_config_test.go +++ b/deployment/ccip/changeset/cs_update_rmn_config_test.go @@ -166,10 +166,19 @@ func updateRMNConfig(t *testing.T, tc updateRMNConfigTestCase) { signers = append(signers, nop.ToRMNRemoteSigner()) } + remoteConfigs := make(map[uint64]RMNRemoteConfig) + for _, chain := range e.Env.Chains { + remoteConfig := RMNRemoteConfig{ + Signers: signers, + F: 0, + } + + remoteConfigs[chain.Selector] = remoteConfig + } + setRemoteConfig := SetRMNRemoteConfig{ HomeChainSelector: e.HomeChainSel, - Signers: signers, - F: 0, + RMNRemoteConfigs: remoteConfigs, MCMSConfig: mcmsConfig, } diff --git a/integration-tests/smoke/ccip/ccip_rmn_test.go b/integration-tests/smoke/ccip/ccip_rmn_test.go index adf07be290f..9f485f1f8cb 100644 --- a/integration-tests/smoke/ccip/ccip_rmn_test.go +++ b/integration-tests/smoke/ccip/ccip_rmn_test.go @@ -254,9 +254,6 @@ func runRmnTestCase(t *testing.T, tc rmnTestCase) { require.NoError(t, err) t.Logf("onChainState: %#v", onChainState) - homeChain, ok := envWithRMN.Env.Chains[envWithRMN.HomeChainSel] - require.True(t, ok) - homeChainState, ok := onChainState.Chains[envWithRMN.HomeChainSel] require.True(t, ok) @@ -270,23 +267,28 @@ func runRmnTestCase(t *testing.T, tc rmnTestCase) { dynamicConfig := rmn_home.RMNHomeDynamicConfig{SourceChains: tc.pf.rmnHomeSourceChains, OffchainConfig: []byte{}} t.Logf("Setting RMNHome candidate with staticConfig: %+v, dynamicConfig: %+v, current candidateDigest: %x", staticConfig, dynamicConfig, allDigests.CandidateConfigDigest[:]) - tx, err := homeChainState.RMNHome.SetCandidate(homeChain.DeployerKey, staticConfig, dynamicConfig, allDigests.CandidateConfigDigest) + + candidateDigest, err := homeChainState.RMNHome.GetCandidateDigest(&bind.CallOpts{Context: ctx}) require.NoError(t, err) - _, err = deployment.ConfirmIfNoError(homeChain, tx, err) + _, err = changeset.NewSetRMNHomeCandidateConfigChangeset(envWithRMN.Env, changeset.SetRMNHomeCandidateConfig{ + HomeChainSelector: envWithRMN.HomeChainSel, + RMNStaticConfig: staticConfig, + RMNDynamicConfig: dynamicConfig, + DigestToOverride: candidateDigest, + }) require.NoError(t, err) - candidateDigest, err := homeChainState.RMNHome.GetCandidateDigest(&bind.CallOpts{Context: ctx}) + candidateDigest, err = homeChainState.RMNHome.GetCandidateDigest(&bind.CallOpts{Context: ctx}) require.NoError(t, err) t.Logf("RMNHome candidateDigest after setting new candidate: %x", candidateDigest[:]) t.Logf("Promoting RMNHome candidate with candidateDigest: %x", candidateDigest[:]) - tx, err = homeChainState.RMNHome.PromoteCandidateAndRevokeActive( - homeChain.DeployerKey, candidateDigest, allDigests.ActiveConfigDigest) - require.NoError(t, err) - - _, err = deployment.ConfirmIfNoError(homeChain, tx, err) + _, err = changeset.NewPromoteCandidateConfigChangeset(envWithRMN.Env, changeset.PromoteRMNHomeCandidateConfig{ + HomeChainSelector: envWithRMN.HomeChainSel, + DigestToPromote: candidateDigest, + }) require.NoError(t, err) // check the active digest is the same as the candidate digest @@ -296,7 +298,20 @@ func runRmnTestCase(t *testing.T, tc rmnTestCase) { "active digest should be the same as the previously candidate digest after promotion, previous candidate: %x, active: %x", candidateDigest[:], activeDigest[:]) - tc.setRmnRemoteConfig(ctx, t, onChainState, activeDigest, envWithRMN) + rmnRemoteConfig := make(map[uint64]changeset.RMNRemoteConfig) + for _, remoteCfg := range tc.remoteChainsConfig { + selector := tc.pf.chainSelectors[remoteCfg.chainIdx] + rmnRemoteConfig[selector] = changeset.RMNRemoteConfig{ + F: uint64(remoteCfg.f), + Signers: tc.pf.rmnRemoteSigners, + } + } + + _, err = changeset.NewSetRMNRemoteConfigChangeset(envWithRMN.Env, changeset.SetRMNRemoteConfig{ + HomeChainSelector: envWithRMN.HomeChainSel, + RMNRemoteConfigs: rmnRemoteConfig, + }) + require.NoError(t, err) tc.killMarkedRmnNodes(t, rmnCluster) @@ -500,46 +515,6 @@ func (tc rmnTestCase) validate() error { return nil } -func (tc rmnTestCase) setRmnRemoteConfig( - ctx context.Context, - t *testing.T, - onChainState changeset.CCIPOnChainState, - activeDigest [32]byte, - envWithRMN changeset.DeployedEnv) { - for _, remoteCfg := range tc.remoteChainsConfig { - remoteSel := tc.pf.chainSelectors[remoteCfg.chainIdx] - chState, ok := onChainState.Chains[remoteSel] - require.True(t, ok) - if remoteCfg.f < 0 { - t.Fatalf("negative F: %d", remoteCfg.f) - } - rmnRemoteConfig := rmn_remote.RMNRemoteConfig{ - RmnHomeContractConfigDigest: activeDigest, - Signers: tc.pf.rmnRemoteSigners, - F: uint64(remoteCfg.f), - } - - chain := envWithRMN.Env.Chains[tc.pf.chainSelectors[remoteCfg.chainIdx]] - - t.Logf("Setting RMNRemote config with RMNHome active digest: %x, cfg: %+v", activeDigest[:], rmnRemoteConfig) - tx2, err2 := chState.RMNRemote.SetConfig(chain.DeployerKey, rmnRemoteConfig) - require.NoError(t, err2) - _, err2 = deployment.ConfirmIfNoError(chain, tx2, err2) - require.NoError(t, err2) - - // confirm the config is set correctly - config, err2 := chState.RMNRemote.GetVersionedConfig(&bind.CallOpts{Context: ctx}) - require.NoError(t, err2) - require.Equalf(t, - activeDigest, - config.Config.RmnHomeContractConfigDigest, - "RMNRemote config digest should be the same as the active digest of RMNHome after setting, RMNHome active: %x, RMNRemote config: %x", - activeDigest[:], config.Config.RmnHomeContractConfigDigest[:]) - - t.Logf("RMNRemote config digest after setting: %x", config.Config.RmnHomeContractConfigDigest[:]) - } -} - func (tc rmnTestCase) killMarkedRmnNodes(t *testing.T, rmnCluster devenv.RMNCluster) { for _, n := range tc.rmnNodes { if n.forceExit { From 8c1374544c798c9730a43ef1ec98d3f851b79517 Mon Sep 17 00:00:00 2001 From: "Simon B.Robert" Date: Thu, 12 Dec 2024 10:07:16 -0500 Subject: [PATCH 2/2] Address PR comments --- deployment/ccip/changeset/cs_update_rmn_config.go | 6 +++--- deployment/ccip/changeset/cs_update_rmn_config_test.go | 2 +- integration-tests/smoke/ccip/ccip_rmn_test.go | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/deployment/ccip/changeset/cs_update_rmn_config.go b/deployment/ccip/changeset/cs_update_rmn_config.go index 4ae97affe2d..62d90d9648d 100644 --- a/deployment/ccip/changeset/cs_update_rmn_config.go +++ b/deployment/ccip/changeset/cs_update_rmn_config.go @@ -365,12 +365,12 @@ func (c SetRMNRemoteConfig) Validate() error { for i := 0; i < len(config.Signers)-1; i++ { if config.Signers[i].NodeIndex >= config.Signers[i+1].NodeIndex { - return fmt.Errorf("signers must be in ascending order of nodeIndex") + return fmt.Errorf("signers must be in ascending order of nodeIndex, but found %d >= %d", config.Signers[i].NodeIndex, config.Signers[i+1].NodeIndex) } } if len(config.Signers) < 2*int(config.F)+1 { - return fmt.Errorf("signers count must greater than or equal to %d", 2*config.F+1) + return fmt.Errorf("signers count (%d) must be greater than or equal to %d", len(config.Signers), 2*config.F+1) } } @@ -411,7 +411,7 @@ func NewSetRMNRemoteConfigChangeset(e deployment.Environment, config SetRMNRemot for chain, remoteConfig := range config.RMNRemoteConfigs { remote, ok := rmnRemotePerChain[chain] if !ok { - return deployment.ChangesetOutput{}, fmt.Errorf("RMNRemote not found for chain %d", chain) + return deployment.ChangesetOutput{}, fmt.Errorf("RMNRemote contract not found for chain %d", chain) } currentVersionConfig, err := remote.GetVersionedConfig(nil) diff --git a/deployment/ccip/changeset/cs_update_rmn_config_test.go b/deployment/ccip/changeset/cs_update_rmn_config_test.go index 46422877e90..46b5c673540 100644 --- a/deployment/ccip/changeset/cs_update_rmn_config_test.go +++ b/deployment/ccip/changeset/cs_update_rmn_config_test.go @@ -166,7 +166,7 @@ func updateRMNConfig(t *testing.T, tc updateRMNConfigTestCase) { signers = append(signers, nop.ToRMNRemoteSigner()) } - remoteConfigs := make(map[uint64]RMNRemoteConfig) + remoteConfigs := make(map[uint64]RMNRemoteConfig, len(e.Env.Chains)) for _, chain := range e.Env.Chains { remoteConfig := RMNRemoteConfig{ Signers: signers, diff --git a/integration-tests/smoke/ccip/ccip_rmn_test.go b/integration-tests/smoke/ccip/ccip_rmn_test.go index 9f485f1f8cb..db8c6187a93 100644 --- a/integration-tests/smoke/ccip/ccip_rmn_test.go +++ b/integration-tests/smoke/ccip/ccip_rmn_test.go @@ -301,6 +301,9 @@ func runRmnTestCase(t *testing.T, tc rmnTestCase) { rmnRemoteConfig := make(map[uint64]changeset.RMNRemoteConfig) for _, remoteCfg := range tc.remoteChainsConfig { selector := tc.pf.chainSelectors[remoteCfg.chainIdx] + if remoteCfg.f < 0 { + t.Fatalf("remoteCfg.f is negative: %d", remoteCfg.f) + } rmnRemoteConfig[selector] = changeset.RMNRemoteConfig{ F: uint64(remoteCfg.f), Signers: tc.pf.rmnRemoteSigners,