From 9ca4940f87020ea3caccceae61f453870eb92050 Mon Sep 17 00:00:00 2001 From: AnieeG Date: Thu, 21 Nov 2024 11:39:50 -0800 Subject: [PATCH] more review comments --- deployment/ccip/changeset/add_chain_test.go | 2 +- deployment/ccip/changeset/deploy.go | 8 ++++---- deployment/ccip/changeset/deploy_chain.go | 4 ++-- deployment/common/changeset/test_helpers.go | 19 +++++++++++++------ deployment/environment.go | 11 ----------- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/deployment/ccip/changeset/add_chain_test.go b/deployment/ccip/changeset/add_chain_test.go index 5f8380436bd..d369a481c7d 100644 --- a/deployment/ccip/changeset/add_chain_test.go +++ b/deployment/ccip/changeset/add_chain_test.go @@ -59,7 +59,7 @@ func TestAddChainInbound(t *testing.T) { require.NoError(t, e.Env.ExistingAddresses.Merge(out.AddressBook)) newAddresses = deployment.NewMemoryAddressBook() tokenConfig := NewTestTokenConfig(state.Chains[e.FeedChainSel].USDFeeds) - err = DeployCCIPContracts(e.Env, newAddresses, NewChainsConfig{ + err = deployCCIPContracts(e.Env, newAddresses, NewChainsConfig{ HomeChainSel: e.HomeChainSel, FeedChainSel: e.FeedChainSel, ChainsToDeploy: initialDeploy, diff --git a/deployment/ccip/changeset/deploy.go b/deployment/ccip/changeset/deploy.go index f3cd5ebf210..8d2b6a63e12 100644 --- a/deployment/ccip/changeset/deploy.go +++ b/deployment/ccip/changeset/deploy.go @@ -405,7 +405,7 @@ func configureChain( return nil } -// DeployCCIPContracts assumes the following contracts are deployed: +// deployCCIPContracts assumes the following contracts are deployed: // - Capability registry // - CCIP home // - RMN home @@ -414,11 +414,11 @@ func configureChain( // It then deploys the rest of the CCIP chain contracts to the selected chains // registers the nodes with the capability registry and creates a DON for // each new chain. -func DeployCCIPContracts( +func deployCCIPContracts( e deployment.Environment, ab deployment.AddressBook, c NewChainsConfig) error { - err := DeployChainContractsForChains(e, ab, c.HomeChainSel, c.ChainsToDeploy) + err := deployChainContractsForChains(e, ab, c.HomeChainSel, c.ChainsToDeploy) if err != nil { e.Logger.Errorw("Failed to deploy chain contracts", "err", err) return err @@ -437,7 +437,7 @@ func DeployCCIPContracts( return nil } -func DeployChainContractsForChains( +func deployChainContractsForChains( e deployment.Environment, ab deployment.AddressBook, homeChainSel uint64, diff --git a/deployment/ccip/changeset/deploy_chain.go b/deployment/ccip/changeset/deploy_chain.go index c1ab11a994e..4c37603c64d 100644 --- a/deployment/ccip/changeset/deploy_chain.go +++ b/deployment/ccip/changeset/deploy_chain.go @@ -12,12 +12,12 @@ var _ deployment.ChangeSet[DeployChainContractsConfig] = DeployChainContracts // DeployChainContracts deploys all new CCIP v1.6 or later contracts for the given chains. // It returns the new addresses for the contracts. -// If there is an error, it will return the successfully deployed addresses and the error so that the caller can call the +// DeployChainContracts is idempotent. If there is an error, it will return the successfully deployed addresses and the error so that the caller can call the // changeset again with the same input to retry the failed deployment. // Caller should update the environment's address book with the returned addresses. func DeployChainContracts(env deployment.Environment, c DeployChainContractsConfig) (deployment.ChangesetOutput, error) { newAddresses := deployment.NewMemoryAddressBook() - err := DeployChainContractsForChains(env, newAddresses, c.HomeChainSelector, c.ChainSelectors) + err := deployChainContractsForChains(env, newAddresses, c.HomeChainSelector, c.ChainSelectors) if err != nil { env.Logger.Errorw("Failed to deploy CCIP contracts", "err", err, "newAddresses", newAddresses) return deployment.ChangesetOutput{AddressBook: newAddresses}, deployment.MaybeDataErr(err) diff --git a/deployment/common/changeset/test_helpers.go b/deployment/common/changeset/test_helpers.go index cf6dc809d0a..ae20ba61abf 100644 --- a/deployment/common/changeset/test_helpers.go +++ b/deployment/common/changeset/test_helpers.go @@ -8,6 +8,7 @@ import ( "github.com/smartcontractkit/ccip-owner-contracts/pkg/gethwrappers" jobv1 "github.com/smartcontractkit/chainlink-protos/job-distributor/v1/job" "github.com/smartcontractkit/chainlink-testing-framework/lib/utils/testcontext" + "github.com/stretchr/testify/require" "github.com/smartcontractkit/chainlink/deployment" ) @@ -34,12 +35,18 @@ func WrapChangeSet[C any](fn deployment.ChangeSet[C]) func(e deployment.Environm // ApplyChangesets applies the changeset applications to the environment and returns the updated environment. func ApplyChangesets(t *testing.T, e deployment.Environment, timelocksPerChain map[uint64]*gethwrappers.RBACTimelock, changesetApplications []ChangesetApplication) (deployment.Environment, error) { - currentEnv, err := e.Copy() - if err != nil { - return e, fmt.Errorf("failed to copy environment: %w", err) - } + addrBook := deployment.NewMemoryAddressBook() + require.NoError(t, addrBook.Merge(e.ExistingAddresses)) + currentEnv := deployment.NewEnvironment( + e.Name, + e.Logger, + addrBook, + e.Chains, + e.NodeIDs, + e.Offchain, + ) for i, csa := range changesetApplications { - out, err := csa.Changeset(currentEnv, csa.Config) + out, err := csa.Changeset(*currentEnv, csa.Config) if err != nil { return e, fmt.Errorf("failed to apply changeset at index %d: %w", i, err) } @@ -83,5 +90,5 @@ func ApplyChangesets(t *testing.T, e deployment.Environment, timelocksPerChain m } } } - return currentEnv, nil + return *currentEnv, nil } diff --git a/deployment/environment.go b/deployment/environment.go index 3918d1ba5e1..b9f3700bdc4 100644 --- a/deployment/environment.go +++ b/deployment/environment.go @@ -76,17 +76,6 @@ type Environment struct { Offchain OffchainClient } -func (e Environment) Copy() (Environment, error) { - newEnv := e - addr := NewMemoryAddressBook() - err := addr.Merge(e.ExistingAddresses) - if err != nil { - return Environment{}, err - } - newEnv.ExistingAddresses = addr - return newEnv, nil -} - func NewEnvironment( name string, logger logger.Logger,