Skip to content

Commit

Permalink
pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@makramkd
makramkd committed Dec 12, 2024
1 parent 9492c9b commit eabcac1
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 38 deletions.
10 changes: 0 additions & 10 deletions deployment/ccip/changeset/cs_add_chain_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,9 @@ func TestAddChainInbound(t *testing.T) {

assertTimelockOwnership(t, e, initialDeploy, state)

nodes, err := deployment.NodeInfo(e.Env.NodeIDs, e.Env.Offchain)
require.NoError(t, err)

// TODO This currently is not working - Able to send the request here but request gets stuck in execution
// Send a new message and expect that this is delivered once the chain is completely set up as inbound
//TestSendRequest(t, e.Env, state, initialDeploy[0], newChain, true)
var nodeIDs []string
for _, node := range nodes {
nodeIDs = append(nodeIDs, node.NodeID)
}

_, err = commonchangeset.ApplyChangesets(t, e.Env, map[uint64]*commonchangeset.TimelockExecutionContracts{
e.HomeChainSel: {
Expand All @@ -212,7 +205,6 @@ func TestAddChainInbound(t *testing.T) {
FeedChainSelector: e.FeedChainSel,
DONChainSelector: newChain,
PluginType: types.PluginTypeCCIPCommit,
NodeIDs: nodeIDs,
CCIPOCRParams: DefaultOCRParams(
e.FeedChainSel,
tokenConfig.GetTokenInfo(logger.TestLogger(t), state.Chains[newChain].LinkToken, state.Chains[newChain].Weth9),
Expand All @@ -231,7 +223,6 @@ func TestAddChainInbound(t *testing.T) {
FeedChainSelector: e.FeedChainSel,
DONChainSelector: newChain,
PluginType: types.PluginTypeCCIPExec,
NodeIDs: nodeIDs,
CCIPOCRParams: DefaultOCRParams(
e.FeedChainSel,
tokenConfig.GetTokenInfo(logger.TestLogger(t), state.Chains[newChain].LinkToken, state.Chains[newChain].Weth9),
Expand All @@ -247,7 +238,6 @@ func TestAddChainInbound(t *testing.T) {
Config: PromoteAllCandidatesChangesetConfig{
HomeChainSelector: e.HomeChainSel,
DONChainSelector: newChain,
NodeIDs: nodeIDs,
MCMS: &MCMSConfig{
MinDelay: 0,
},
Expand Down
32 changes: 23 additions & 9 deletions deployment/ccip/changeset/cs_ccip_home.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ type PromoteAllCandidatesChangesetConfig struct {
// DONChainSelector is the chain selector of the DON that we want to promote the candidate config of.
// Note that each (chain, ccip capability version) pair has a unique DON ID.
DONChainSelector uint64
NodeIDs []string

// MCMS is optional MCMS configuration, if provided the changeset will generate an MCMS proposal.
// If nil, the changeset will execute the commands directly using the deployer key
Expand All @@ -47,7 +46,7 @@ func (p PromoteAllCandidatesChangesetConfig) Validate(e deployment.Environment,
if err := deployment.IsValidChainSelector(p.DONChainSelector); err != nil {
return nil, fmt.Errorf("don chain selector invalid: %w", err)
}
if len(p.NodeIDs) == 0 {
if len(e.NodeIDs) == 0 {
return nil, fmt.Errorf("NodeIDs must be set")
}
if state.Chains[p.HomeChainSelector].CCIPHome == nil {
Expand All @@ -56,8 +55,12 @@ func (p PromoteAllCandidatesChangesetConfig) Validate(e deployment.Environment,
if state.Chains[p.HomeChainSelector].CapabilityRegistry == nil {
return nil, fmt.Errorf("CapabilityRegistry contract does not exist")
}
if state.Chains[p.DONChainSelector].OffRamp == nil {
// should not be possible, but a defensive check.
return nil, fmt.Errorf("OffRamp contract does not exist")
}

nodes, err := deployment.NodeInfo(p.NodeIDs, e.Offchain)
nodes, err := deployment.NodeInfo(e.NodeIDs, e.Offchain)
if err != nil {
return nil, fmt.Errorf("fetch node info: %w", err)
}
Expand Down Expand Up @@ -103,7 +106,10 @@ func (p PromoteAllCandidatesChangesetConfig) Validate(e deployment.Environment,
}

// PromoteAllCandidatesChangeset generates a proposal to call promoteCandidate on the CCIPHome through CapReg.
// This needs to be called after SetCandidateProposal is executed.
// Note that a DON must exist prior to being able to use this changeset effectively,
// i.e AddDonAndSetCandidateChangeset must be called first.
// This can also be used to promote a 0x0 candidate config to be the active, effectively shutting down the DON.
// At that point you can call the RemoveDON changeset to remove the DON entirely from the capability registry.
func PromoteAllCandidatesChangeset(
e deployment.Environment,
cfg PromoteAllCandidatesChangesetConfig,
Expand Down Expand Up @@ -204,7 +210,6 @@ type SetCandidateChangesetConfig struct {
DONChainSelector uint64

PluginType types.PluginType
NodeIDs []string
CCIPOCRParams CCIPOCRParams

// MCMS is optional MCMS configuration, if provided the changeset will generate an MCMS proposal.
Expand All @@ -223,7 +228,7 @@ func (s SetCandidateChangesetConfig) Validate(e deployment.Environment, state CC
if err := deployment.IsValidChainSelector(s.DONChainSelector); err != nil {
return nil, fmt.Errorf("don chain selector invalid: %w", err)
}
if len(s.NodeIDs) == 0 {
if len(e.NodeIDs) == 0 {
return nil, fmt.Errorf("nodeIDs must be set")
}
if state.Chains[s.HomeChainSelector].CCIPHome == nil {
Expand All @@ -232,17 +237,22 @@ func (s SetCandidateChangesetConfig) Validate(e deployment.Environment, state CC
if state.Chains[s.HomeChainSelector].CapabilityRegistry == nil {
return nil, fmt.Errorf("CapabilityRegistry contract does not exist")
}
if state.Chains[s.DONChainSelector].OffRamp == nil {
// should not be possible, but a defensive check.
return nil, fmt.Errorf("OffRamp contract does not exist on don chain selector %d", s.DONChainSelector)
}
if s.PluginType != types.PluginTypeCCIPCommit &&
s.PluginType != types.PluginTypeCCIPExec {
return nil, fmt.Errorf("PluginType must be set to either CCIPCommit or CCIPExec")
}

nodes, err := deployment.NodeInfo(s.NodeIDs, e.Offchain)
nodes, err := deployment.NodeInfo(e.NodeIDs, e.Offchain)
if err != nil {
return nil, fmt.Errorf("get node info: %w", err)
}

// TODO: validate token config
// TODO: validate gas config

// check that chain config is set up for the new chain
chainConfig, err := state.Chains[s.HomeChainSelector].CCIPHome.GetChainConfig(nil, s.DONChainSelector)
Expand All @@ -269,6 +279,11 @@ func (s SetCandidateChangesetConfig) Validate(e deployment.Environment, state CC

// AddDonAndSetCandidateChangeset adds new DON for destination to home chain
// and sets the plugin config as candidateConfig for the don.
//
// This is the first step to creating a CCIP DON and must be executed before any
// other changesets (SetCandidateChangeset, PromoteAllCandidatesChangeset)
// can be executed.
//
// Note that these operations must be done together because the createDON call
// in the capability registry calls the capability config contract, so we must
// provide suitable calldata for CCIPHome.
Expand Down Expand Up @@ -409,8 +424,7 @@ func newDonWithCandidateOp(
}

// SetCandidateChangeset generates a proposal to call setCandidate on the CCIPHome through the capability registry.
// If the MCMS is enabled, it will generate an MCMS proposal.
// If the MCMS is disabled, it will execute the txes directly using the deployer key.
// A DON must exist in order to use this changeset effectively, i.e AddDonAndSetCandidateChangeset must be called first.
func SetCandidateChangeset(
e deployment.Environment,
cfg SetCandidateChangesetConfig,
Expand Down
19 changes: 0 additions & 19 deletions deployment/ccip/changeset/cs_ccip_home_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,6 @@ func Test_PromoteCandidate(t *testing.T) {
source := allChains[0]
dest := allChains[1]

nodes, err := deployment.NodeInfo(tenv.Env.NodeIDs, tenv.Env.Offchain)
require.NoError(t, err)

var nodeIDs []string
for _, node := range nodes {
nodeIDs = append(nodeIDs, node.NodeID)
}

if tc.mcmsEnabled {
// Transfer ownership to timelock so that we can promote the zero digest later down the line.
transferToTimelock(t, tenv, state, source, dest)
Expand Down Expand Up @@ -343,7 +335,6 @@ func Test_PromoteCandidate(t *testing.T) {
Config: PromoteAllCandidatesChangesetConfig{
HomeChainSelector: tenv.HomeChainSel,
DONChainSelector: dest,
NodeIDs: nodeIDs,
MCMS: mcmsConfig,
},
},
Expand Down Expand Up @@ -393,14 +384,6 @@ func Test_SetCandidate(t *testing.T) {
source := allChains[0]
dest := allChains[1]

nodes, err := deployment.NodeInfo(tenv.Env.NodeIDs, tenv.Env.Offchain)
require.NoError(t, err)

var nodeIDs []string
for _, node := range nodes {
nodeIDs = append(nodeIDs, node.NodeID)
}

if tc.mcmsEnabled {
// Transfer ownership to timelock so that we can promote the zero digest later down the line.
transferToTimelock(t, tenv, state, source, dest)
Expand Down Expand Up @@ -444,7 +427,6 @@ func Test_SetCandidate(t *testing.T) {
FeedChainSelector: tenv.FeedChainSel,
DONChainSelector: dest,
PluginType: types.PluginTypeCCIPCommit,
NodeIDs: nodeIDs,
CCIPOCRParams: DefaultOCRParams(
tenv.FeedChainSel,
tokenConfig.GetTokenInfo(logger.TestLogger(t), state.Chains[dest].LinkToken, state.Chains[dest].Weth9),
Expand All @@ -460,7 +442,6 @@ func Test_SetCandidate(t *testing.T) {
FeedChainSelector: tenv.FeedChainSel,
DONChainSelector: dest,
PluginType: types.PluginTypeCCIPExec,
NodeIDs: nodeIDs,
CCIPOCRParams: DefaultOCRParams(
tenv.FeedChainSel,
tokenConfig.GetTokenInfo(logger.TestLogger(t), state.Chains[dest].LinkToken, state.Chains[dest].Weth9),
Expand Down

0 comments on commit eabcac1

Please sign in to comment.