From 73363fc81aaee39ff746253a49738815ff060182 Mon Sep 17 00:00:00 2001 From: connorwstein Date: Wed, 18 Dec 2024 11:28:47 -0500 Subject: [PATCH] More ownership validation --- deployment/ccip/changeset/cs_ccip_home.go | 64 ++++++++++++++---- .../ccip/changeset/cs_chain_contracts.go | 66 ++++++++----------- 2 files changed, 79 insertions(+), 51 deletions(-) diff --git a/deployment/ccip/changeset/cs_ccip_home.go b/deployment/ccip/changeset/cs_ccip_home.go index 9f3f6822edd..04d9b5ee73f 100644 --- a/deployment/ccip/changeset/cs_ccip_home.go +++ b/deployment/ccip/changeset/cs_ccip_home.go @@ -118,19 +118,36 @@ type PromoteAllCandidatesChangesetConfig struct { MCMS *MCMSConfig } -func (p PromoteAllCandidatesChangesetConfig) Validate(e deployment.Environment, state CCIPOnChainState) ([]uint32, error) { +func (p PromoteAllCandidatesChangesetConfig) Validate(e deployment.Environment) ([]uint32, error) { + state, err := LoadOnchainState(e) + if err != nil { + return nil, err + } if err := deployment.IsValidChainSelector(p.HomeChainSelector); err != nil { return nil, fmt.Errorf("home chain selector invalid: %w", err) } + homeChainState, exists := state.Chains[p.HomeChainSelector] + if !exists { + return nil, fmt.Errorf("home chain %d does not exist", p.HomeChainSelector) + } + if err := ValidateOwnership(e.GetContext(), p.MCMS != nil, e.Chains[p.HomeChainSelector].DeployerKey.From, homeChainState.Timelock.Address(), homeChainState.CapabilityRegistry); err != nil { + return nil, err + } + var donIDs []uint32 for _, chainSelector := range p.DONChainSelectors { if err := deployment.IsValidChainSelector(chainSelector); err != nil { return nil, fmt.Errorf("don chain selector invalid: %w", err) } - if state.Chains[chainSelector].OffRamp == nil { + chainState, exists := state.Chains[chainSelector] + if !exists { + return nil, fmt.Errorf("chain %d does not exist", chainSelector) + } + if chainState.OffRamp == nil { // should not be possible, but a defensive check. return nil, fmt.Errorf("OffRamp contract does not exist") } + donID, err := internal.DonIDForChain( state.Chains[p.HomeChainSelector].CapabilityRegistry, state.Chains[p.HomeChainSelector].CCIPHome, @@ -190,14 +207,13 @@ func PromoteAllCandidatesChangeset( e deployment.Environment, cfg PromoteAllCandidatesChangesetConfig, ) (deployment.ChangesetOutput, error) { - state, err := LoadOnchainState(e) + donIDs, err := cfg.Validate(e) if err != nil { - return deployment.ChangesetOutput{}, err + return deployment.ChangesetOutput{}, fmt.Errorf("%w: %w", deployment.ErrInvalidConfig, err) } - - donIDs, err := cfg.Validate(e, state) + state, err := LoadOnchainState(e) if err != nil { - return deployment.ChangesetOutput{}, fmt.Errorf("%w: %w", deployment.ErrInvalidConfig, err) + return deployment.ChangesetOutput{}, err } nodes, err := deployment.NodeInfo(e.NodeIDs, e.Offchain) @@ -285,6 +301,14 @@ func (s SetCandidateConfigBase) Validate(e deployment.Environment, state CCIPOnC if err := deployment.IsValidChainSelector(s.FeedChainSelector); err != nil { return fmt.Errorf("feed chain selector invalid: %w", err) } + homeChainState, exists := state.Chains[s.HomeChainSelector] + if !exists { + return fmt.Errorf("home chain %d does not exist", s.HomeChainSelector) + } + if err := ValidateOwnership(e.GetContext(), s.MCMS != nil, e.Chains[s.HomeChainSelector].DeployerKey.From, homeChainState.Timelock.Address(), homeChainState.CapabilityRegistry); err != nil { + return err + } + for chainSelector, params := range s.DONChainSelector { if err := deployment.IsValidChainSelector(chainSelector); err != nil { return fmt.Errorf("don chain selector invalid: %w", err) @@ -835,6 +859,13 @@ func (r RevokeCandidateChangesetConfig) Validate(e deployment.Environment, state if state.Chains[r.HomeChainSelector].CapabilityRegistry == nil { return 0, fmt.Errorf("CapabilityRegistry contract does not exist") } + homeChainState, exists := state.Chains[r.HomeChainSelector] + if !exists { + return 0, fmt.Errorf("home chain %d does not exist", r.HomeChainSelector) + } + if err := ValidateOwnership(e.GetContext(), r.MCMS != nil, e.Chains[r.HomeChainSelector].DeployerKey.From, homeChainState.Timelock.Address(), homeChainState.CapabilityRegistry); err != nil { + return 0, err + } // check that the don exists for this chain donID, err = internal.DonIDForChain( @@ -997,13 +1028,24 @@ type UpdateChainConfigConfig struct { MCMS *MCMSConfig } -func (c UpdateChainConfigConfig) Validate(state CCIPOnChainState) error { +func (c UpdateChainConfigConfig) Validate(e deployment.Environment) error { + state, err := LoadOnchainState(e) + if err != nil { + return err + } if err := deployment.IsValidChainSelector(c.HomeChainSelector); err != nil { return fmt.Errorf("home chain selector invalid: %w", err) } if len(c.ChainRemoves) == 0 && len(c.ChainAdds) == 0 { return fmt.Errorf("no chain adds or removes") } + homeChainState, exists := state.Chains[c.HomeChainSelector] + if !exists { + return fmt.Errorf("home chain %d does not exist", c.HomeChainSelector) + } + if err := ValidateOwnership(e.GetContext(), c.MCMS != nil, e.Chains[c.HomeChainSelector].DeployerKey.From, homeChainState.Timelock.Address(), homeChainState.CCIPHome); err != nil { + return err + } for _, remove := range c.ChainRemoves { if err := deployment.IsValidChainSelector(remove); err != nil { return fmt.Errorf("chain remove selector invalid: %w", err) @@ -1030,13 +1072,13 @@ func (c UpdateChainConfigConfig) Validate(state CCIPOnChainState) error { } func UpdateChainConfig(e deployment.Environment, cfg UpdateChainConfigConfig) (deployment.ChangesetOutput, error) { + if err := cfg.Validate(e); err != nil { + return deployment.ChangesetOutput{}, fmt.Errorf("%w: %w", deployment.ErrInvalidConfig, err) + } state, err := LoadOnchainState(e) if err != nil { return deployment.ChangesetOutput{}, err } - if err := cfg.Validate(state); err != nil { - return deployment.ChangesetOutput{}, fmt.Errorf("%w: %w", deployment.ErrInvalidConfig, err) - } txOpts := e.Chains[cfg.HomeChainSelector].DeployerKey txOpts.Context = e.GetContext() if cfg.MCMS != nil { diff --git a/deployment/ccip/changeset/cs_chain_contracts.go b/deployment/ccip/changeset/cs_chain_contracts.go index fcbfb84d336..7c00996899a 100644 --- a/deployment/ccip/changeset/cs_chain_contracts.go +++ b/deployment/ccip/changeset/cs_chain_contracts.go @@ -16,6 +16,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink/deployment" "github.com/smartcontractkit/chainlink/deployment/ccip/changeset/internal" + "github.com/smartcontractkit/chainlink/deployment/common/changeset" "github.com/smartcontractkit/chainlink/deployment/common/proposalutils" cctypes "github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/types" "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/ccip/generated/fee_quoter" @@ -65,14 +66,8 @@ func (cfg UpdateOnRampDestsConfig) Validate(e deployment.Environment) error { if chainState.OnRamp == nil { return fmt.Errorf("missing onramp onramp for chain %d", chainSel) } - owner, err := chainState.OnRamp.Owner(&bind.CallOpts{Context: e.GetContext()}) - if err != nil { - return fmt.Errorf("failed to get onramp owner %s: %w", chainState.OnRamp.Address(), err) - } - if cfg.MCMS != nil && owner != chainState.Timelock.Address() { - return fmt.Errorf("onramp %s is not owned by the timelock", chainState.OnRamp.Address()) - } else if cfg.MCMS == nil && owner != e.Chains[chainSel].DeployerKey.From { - return fmt.Errorf("onramp %s is not owned by the deployer key", chainState.OnRamp.Address()) + if err := ValidateOwnership(e.GetContext(), cfg.MCMS != nil, e.Chains[chainSel].DeployerKey.From, chainState.Timelock.Address(), chainState.OnRamp); err != nil { + return err } for destination := range updates { @@ -199,15 +194,10 @@ func (cfg UpdateFeeQuoterDestsConfig) Validate(e deployment.Environment) error { if chainState.OnRamp == nil { return fmt.Errorf("missing onramp onramp for chain %d", chainSel) } - owner, err := chainState.FeeQuoter.Owner(&bind.CallOpts{Context: e.GetContext()}) - if err != nil { - return fmt.Errorf("failed to get fee quoter owner %s: %w", chainState.FeeQuoter.Address(), err) - } - if cfg.MCMS != nil && owner != chainState.Timelock.Address() { - return fmt.Errorf("fq %s is not owned by the timelock", chainState.FeeQuoter.Address()) - } else if cfg.MCMS == nil && owner != e.Chains[chainSel].DeployerKey.From { - return fmt.Errorf("fq %s is not owned by the deployer key", chainState.FeeQuoter.Address()) + if err := ValidateOwnership(e.GetContext(), cfg.MCMS != nil, e.Chains[chainSel].DeployerKey.From, chainState.Timelock.Address(), chainState.FeeQuoter); err != nil { + return err } + for destination := range updates { // Destination cannot be an unknown destination. if _, ok := supportedChains[destination]; !ok { @@ -322,15 +312,10 @@ func (cfg UpdateOffRampSourcesConfig) Validate(e deployment.Environment) error { if chainState.OffRamp == nil { return fmt.Errorf("missing onramp onramp for chain %d", chainSel) } - owner, err := chainState.OffRamp.Owner(&bind.CallOpts{Context: e.GetContext()}) - if err != nil { - return fmt.Errorf("failed to get offramp owner %s: %w", chainState.OffRamp.Address(), err) - } - if cfg.MCMS != nil && owner != chainState.Timelock.Address() { - return fmt.Errorf("offramp %s is not owned by the timelock", chainState.OffRamp.Address()) - } else if cfg.MCMS == nil && owner != e.Chains[chainSel].DeployerKey.From { - return fmt.Errorf("offramp %s is not owned by the deployer key", chainState.OffRamp.Address()) + if err := ValidateOwnership(e.GetContext(), cfg.MCMS != nil, e.Chains[chainSel].DeployerKey.From, chainState.Timelock.Address(), chainState.OffRamp); err != nil { + return err } + for source := range updates { // Source cannot be an unknown if _, ok := supportedChains[source]; !ok { @@ -463,14 +448,8 @@ func (cfg UpdateRouterRampsConfig) Validate(e deployment.Environment) error { if chainState.OffRamp == nil { return fmt.Errorf("missing onramp onramp for chain %d", chainSel) } - owner, err := chainState.Router.Owner(&bind.CallOpts{Context: e.GetContext()}) - if err != nil { - return fmt.Errorf("failed to get router owner %s: %w", chainState.Router.Address(), err) - } - if cfg.MCMS != nil && owner != chainState.Timelock.Address() { - return fmt.Errorf("router %s is not owned by the timelock", chainState.Router.Address()) - } else if cfg.MCMS == nil && owner != e.Chains[chainSel].DeployerKey.From { - return fmt.Errorf("router %s is not owned by the deployer key", chainState.Router.Address()) + if err := ValidateOwnership(e.GetContext(), cfg.MCMS != nil, e.Chains[chainSel].DeployerKey.From, chainState.Timelock.Address(), chainState.Router); err != nil { + return err } for source := range update.OffRampUpdates { @@ -629,14 +608,8 @@ func (c SetOCR3OffRampConfig) Validate(e deployment.Environment) error { if !ok { return fmt.Errorf("remote chain %d not found in onchain state", remote) } - owner, err := state.Chains[remote].OffRamp.Owner(&bind.CallOpts{Context: e.GetContext()}) - if err != nil { - return fmt.Errorf("failed to get router owner %s: %w", chainState.OffRamp.Address(), err) - } - if c.MCMS != nil && owner != chainState.Timelock.Address() { - return fmt.Errorf("offramp %s is not owned by the timelock", chainState.OffRamp.Address()) - } else if c.MCMS == nil && owner != e.Chains[remote].DeployerKey.From { - return fmt.Errorf("offramp %s is not owned by the deployer key", chainState.OffRamp.Address()) + if err := ValidateOwnership(e.GetContext(), c.MCMS != nil, e.Chains[remote].DeployerKey.From, chainState.Timelock.Address(), chainState.OffRamp); err != nil { + return err } } return nil @@ -782,3 +755,16 @@ func isOCR3ConfigSetOnOffRamp( } return true, nil } + +func ValidateOwnership(ctx context.Context, mcms bool, deployerKey, timelock common.Address, contract changeset.Ownable) error { + owner, err := contract.Owner(&bind.CallOpts{Context: ctx}) + if err != nil { + return fmt.Errorf("failed to get owner: %w", err) + } + if mcms && owner != timelock { + return fmt.Errorf("%s not owned by deployer key", contract.Address()) + } else if !mcms && owner != deployerKey { + return fmt.Errorf("%s not owned by deployer key", contract.Address()) + } + return nil +}