From 6b99dad8301efccd1305e89c409f3cefa60b3efe Mon Sep 17 00:00:00 2001 From: gustavogama-cll <165679773+gustavogama-cll@users.noreply.github.com> Date: Thu, 13 Feb 2025 14:56:18 -0300 Subject: [PATCH 1/4] fix: execute all operations from proposals when using new mcms lib (#16368) * fix: execute all operations from proposals when using new mcms lib As opposed to executing "per chain". The existing code that executed per chain was broken as it (1) did not build the executors map properly, (2) did not skip execution of operations from different chains in the inner for loop, and (3) uses the `IsReady` call, which checks the readiness of *all* operations, regardless of chain. The new implementations simply executes all the proposal operations, in order, regardless of chain. It seems to suit the requirements of the existing changeset tests. * review: drop unnecessary "chains" map * review: drop unnecessary chainSelectors map --- deployment/common/changeset/test_helpers.go | 27 +---- .../common/proposalutils/mcms_test_helpers.go | 114 +++++++++++------- 2 files changed, 72 insertions(+), 69 deletions(-) diff --git a/deployment/common/changeset/test_helpers.go b/deployment/common/changeset/test_helpers.go index f9876f50e4c..7b29af4cac0 100644 --- a/deployment/common/changeset/test_helpers.go +++ b/deployment/common/changeset/test_helpers.go @@ -89,34 +89,15 @@ func ApplyChangesets(t *testing.T, e deployment.Environment, timelockContractsPe } if out.MCMSTimelockProposals != nil { for _, prop := range out.MCMSTimelockProposals { - chains := mapset.NewSet[uint64]() - for _, op := range prop.Operations { - chains.Add(uint64(op.ChainSelector)) - } - - p := proposalutils.SignMCMSTimelockProposal(t, e, &prop) - for _, sel := range chains.ToSlice() { - timelockContracts, ok := timelockContractsPerChain[sel] - if !ok || timelockContracts == nil { - return deployment.Environment{}, fmt.Errorf("timelock contracts not found for chain %d", sel) - } - - proposalutils.ExecuteMCMSProposalV2(t, e, p, sel) - proposalutils.ExecuteMCMSTimelockProposalV2(t, e, &prop, sel) - } + mcmProp := proposalutils.SignMCMSTimelockProposal(t, e, &prop) + proposalutils.ExecuteMCMSProposalV2(t, e, mcmProp) + proposalutils.ExecuteMCMSTimelockProposalV2(t, e, &prop) } } if out.MCMSProposals != nil { for _, prop := range out.MCMSProposals { - chains := mapset.NewSet[uint64]() - for _, op := range prop.Operations { - chains.Add(uint64(op.ChainSelector)) - } - p := proposalutils.SignMCMSProposal(t, e, &prop) - for _, sel := range chains.ToSlice() { - proposalutils.ExecuteMCMSProposalV2(t, e, p, sel) - } + proposalutils.ExecuteMCMSProposalV2(t, e, p) } } currentEnv = deployment.Environment{ diff --git a/deployment/common/proposalutils/mcms_test_helpers.go b/deployment/common/proposalutils/mcms_test_helpers.go index e0d39127ee2..f9f4be153ec 100644 --- a/deployment/common/proposalutils/mcms_test_helpers.go +++ b/deployment/common/proposalutils/mcms_test_helpers.go @@ -172,51 +172,66 @@ func SignMCMSProposal(t *testing.T, env deployment.Environment, proposal *mcmsli } // ExecuteMCMSProposalV2 - Executes an MCMS proposal on a chain. For timelock proposal, use ExecuteMCMSTimelockProposalV2 instead. -func ExecuteMCMSProposalV2(t *testing.T, env deployment.Environment, proposal *mcmslib.Proposal, sel uint64) { - t.Log("Executing proposal on chain", sel) +func ExecuteMCMSProposalV2(t *testing.T, env deployment.Environment, proposal *mcmslib.Proposal) { + t.Log("Executing proposal") - executorsMap := map[types.ChainSelector]sdk.Executor{} encoders, err := proposal.GetEncoders() require.NoError(t, err) - family, err := chainsel.GetSelectorFamily(sel) - require.NoError(t, err) + // build a map with chainSelector => executor + executorsMap := map[types.ChainSelector]sdk.Executor{} + for _, op := range proposal.Operations { + family, err := chainsel.GetSelectorFamily(uint64(op.ChainSelector)) + require.NoError(t, err) - chainSel := types.ChainSelector(sel) - - switch family { - case chainsel.FamilyEVM: - encoder := encoders[chainSel].(*evm.Encoder) - chain := env.Chains[sel] - executorsMap[chainSel] = evm.NewExecutor(encoder, chain.Client, chain.DeployerKey) - case chainsel.FamilySolana: - encoder := encoders[chainSel].(*solana.Encoder) - chain := env.SolChains[sel] - executorsMap[chainSel] = solana.NewExecutor(encoder, chain.Client, *chain.DeployerKey) - default: - require.FailNow(t, "unsupported chain family") + switch family { + case chainsel.FamilyEVM: + encoder := encoders[op.ChainSelector].(*evm.Encoder) + executorsMap[op.ChainSelector] = evm.NewExecutor( + encoder, + env.Chains[uint64(op.ChainSelector)].Client, + env.Chains[uint64(op.ChainSelector)].DeployerKey) + case chainsel.FamilySolana: + encoder := encoders[op.ChainSelector].(*solana.Encoder) + executorsMap[op.ChainSelector] = solana.NewExecutor( + encoder, + env.SolChains[uint64(op.ChainSelector)].Client, + *env.SolChains[uint64(op.ChainSelector)].DeployerKey) + default: + require.FailNow(t, "unsupported chain family") + } } executable, err := mcmslib.NewExecutable(proposal, executorsMap) require.NoError(t, err) - root, err := executable.SetRoot(env.GetContext(), chainSel) - require.NoError(t, deployment.MaybeDataErr(err)) + // call SetRoot for each chain + for chainSelector := range executorsMap { + root, err := executable.SetRoot(env.GetContext(), chainSelector) + require.NoError(t, deployment.MaybeDataErr(err)) - // no need to confirm transaction on solana as the MCMS sdk confirms it internally - if family == chainsel.FamilyEVM { - chain := env.Chains[sel] - evmTransaction := root.RawTransaction.(*gethtypes.Transaction) - _, err = chain.Confirm(evmTransaction) + family, err := chainsel.GetSelectorFamily(uint64(chainSelector)) require.NoError(t, err) + + // no need to confirm transaction on solana as the MCMS sdk confirms it internally + if family == chainsel.FamilyEVM { + chain := env.Chains[uint64(chainSelector)] + evmTransaction := root.RawTransaction.(*gethtypes.Transaction) + _, err = chain.Confirm(evmTransaction) + require.NoError(t, err) + } } - for i := range proposal.Operations { + // execute each operation sequentially + for i, op := range proposal.Operations { result, err := executable.Execute(env.GetContext(), i) require.NoError(t, err) + family, err := chainsel.GetSelectorFamily(uint64(op.ChainSelector)) + require.NoError(t, err) + if family == chainsel.FamilyEVM { - chain := env.Chains[sel] + chain := env.Chains[uint64(op.ChainSelector)] evmTransaction := result.RawTransaction.(*gethtypes.Transaction) _, err = chain.Confirm(evmTransaction) require.NoError(t, err) @@ -226,40 +241,47 @@ func ExecuteMCMSProposalV2(t *testing.T, env deployment.Environment, proposal *m // ExecuteMCMSTimelockProposalV2 - Includes an option to set callProxy to execute the calls through a proxy. // If the callProxy is not set, the calls will be executed directly to the timelock. -func ExecuteMCMSTimelockProposalV2(t *testing.T, env deployment.Environment, timelockProposal *mcmslib.TimelockProposal, sel uint64, opts ...mcmslib.Option) { - t.Log("Executing timelock proposal on chain", sel) +func ExecuteMCMSTimelockProposalV2(t *testing.T, env deployment.Environment, timelockProposal *mcmslib.TimelockProposal, opts ...mcmslib.Option) { + t.Log("Executing timelock proposal") - tExecutors := map[types.ChainSelector]sdk.TimelockExecutor{} - family, err := chainsel.GetSelectorFamily(sel) - require.NoError(t, err) + // build a "chainSelector => executor" map + executorsMap := map[types.ChainSelector]sdk.TimelockExecutor{} + for _, op := range timelockProposal.Operations { + family, err := chainsel.GetSelectorFamily(uint64(op.ChainSelector)) + require.NoError(t, err) - switch family { - case chainsel.FamilyEVM: - tExecutors[types.ChainSelector(sel)] = evm.NewTimelockExecutor( - env.Chains[sel].Client, - env.Chains[sel].DeployerKey) - case chainsel.FamilySolana: - tExecutors[types.ChainSelector(sel)] = solana.NewTimelockExecutor( - env.SolChains[sel].Client, - *env.SolChains[sel].DeployerKey) - default: - require.FailNow(t, "unsupported chain family") + switch family { + case chainsel.FamilyEVM: + executorsMap[op.ChainSelector] = evm.NewTimelockExecutor( + env.Chains[uint64(op.ChainSelector)].Client, + env.Chains[uint64(op.ChainSelector)].DeployerKey) + case chainsel.FamilySolana: + executorsMap[op.ChainSelector] = solana.NewTimelockExecutor( + env.SolChains[uint64(op.ChainSelector)].Client, + *env.SolChains[uint64(op.ChainSelector)].DeployerKey) + default: + require.FailNow(t, "unsupported chain family") + } } - timelockExecutable, err := mcmslib.NewTimelockExecutable(timelockProposal, tExecutors) + timelockExecutable, err := mcmslib.NewTimelockExecutable(timelockProposal, executorsMap) require.NoError(t, err) err = timelockExecutable.IsReady(env.GetContext()) require.NoError(t, err) + // execute each operation sequentially var tx = types.TransactionResult{} - for i := range timelockProposal.Operations { + for i, op := range timelockProposal.Operations { tx, err = timelockExecutable.Execute(env.GetContext(), i, opts...) require.NoError(t, err) + family, err := chainsel.GetSelectorFamily(uint64(op.ChainSelector)) + require.NoError(t, err) + // no need to confirm transaction on solana as the MCMS sdk confirms it internally if family == chainsel.FamilyEVM { - chain := env.Chains[sel] + chain := env.Chains[uint64(op.ChainSelector)] evmTransaction := tx.RawTransaction.(*gethtypes.Transaction) _, err = chain.Confirm(evmTransaction) require.NoError(t, err) From 1de1a493c35911ec5a5d68b53cd087263a82322d Mon Sep 17 00:00:00 2001 From: Erik Burton Date: Thu, 13 Feb 2025 10:52:54 -0800 Subject: [PATCH 2/4] fix: disable go module index during ci-core tests (#16393) --- .github/workflows/ci-core.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-core.yml b/.github/workflows/ci-core.yml index 8c164840535..574fa7de456 100644 --- a/.github/workflows/ci-core.yml +++ b/.github/workflows/ci-core.yml @@ -277,7 +277,10 @@ jobs: env: OUTPUT_FILE: ./output.txt CL_DATABASE_URL: ${{ env.DB_URL }} - run: ./tools/bin/${{ matrix.type.cmd }} ./... + run: | + # See: https://github.com/golang/go/issues/69179 + GODEBUG=goindex=0 + ./tools/bin/${{ matrix.type.cmd }} ./... - name: Print Races id: print-races From 10cbdc2d7089f66aa255172fa33bf6bc2c335be9 Mon Sep 17 00:00:00 2001 From: Austin <107539019+0xAustinWang@users.noreply.github.com> Date: Fri, 14 Feb 2025 03:41:50 +0800 Subject: [PATCH 3/4] parallelize deployer group per chain (#16387) * parallelize operations in the deployer group * goimport --- deployment/ccip/changeset/deployer_group.go | 30 ++++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/deployment/ccip/changeset/deployer_group.go b/deployment/ccip/changeset/deployer_group.go index 0768c95e9ed..6f3732a760b 100644 --- a/deployment/ccip/changeset/deployer_group.go +++ b/deployment/ccip/changeset/deployer_group.go @@ -7,6 +7,8 @@ import ( "slices" "time" + "golang.org/x/sync/errgroup" + "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" @@ -272,18 +274,26 @@ func getBatchCountForChain(chain mcms.ChainIdentifier, m *timelock.MCMSWithTimel func (d *DeployerGroup) enactDeployer() (deployment.ChangesetOutput, error) { contexts := d.getContextChainInOrder() for _, c := range contexts { + g := errgroup.Group{} for selector, txs := range c.transactions { - for _, tx := range txs { - err := d.e.Chains[selector].Client.SendTransaction(context.Background(), tx) - if err != nil { - return deployment.ChangesetOutput{}, fmt.Errorf("failed to send transaction: %w", err) - } - // TODO how to pass abi here to decode error reason - _, err = deployment.ConfirmIfNoError(d.e.Chains[selector], tx, err) - if err != nil { - return deployment.ChangesetOutput{}, fmt.Errorf("waiting for tx to be mined failed: %w", err) + selector, txs := selector, txs + g.Go(func() error { + for _, tx := range txs { + err := d.e.Chains[selector].Client.SendTransaction(context.Background(), tx) + if err != nil { + return fmt.Errorf("failed to send transaction: %w", err) + } + // TODO how to pass abi here to decode error reason + _, err = deployment.ConfirmIfNoError(d.e.Chains[selector], tx, err) + if err != nil { + return fmt.Errorf("waiting for tx to be mined failed: %w", err) + } } - } + return nil + }) + } + if err := g.Wait(); err != nil { + return deployment.ChangesetOutput{}, err } } return deployment.ChangesetOutput{}, nil From 8aab3d22aeb7f8a419385f5bf851c58f81dd37a6 Mon Sep 17 00:00:00 2001 From: Anindita Ghosh <88458927+AnieeG@users.noreply.github.com> Date: Thu, 13 Feb 2025 12:06:33 -0800 Subject: [PATCH 4/4] Ccip-2672 nonce manager checks (#16370) * nonce manager checks * fixes --- .../ccip/changeset/cs_chain_contracts.go | 37 +++++++------------ .../common/changeset/deploy_link_token.go | 33 +++++++++++------ .../ccip/ccip_migration_to_v_1_6_test.go | 4 -- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/deployment/ccip/changeset/cs_chain_contracts.go b/deployment/ccip/changeset/cs_chain_contracts.go index 26c78f1a2fd..49f777e32b2 100644 --- a/deployment/ccip/changeset/cs_chain_contracts.go +++ b/deployment/ccip/changeset/cs_chain_contracts.go @@ -75,8 +75,6 @@ type NonceManagerUpdate struct { type PreviousRampCfg struct { RemoteChainSelector uint64 OverrideExisting bool - EnableOnRamp bool - EnableOffRamp bool } func (cfg UpdateNonceManagerConfig) Validate(e deployment.Environment) error { @@ -106,22 +104,18 @@ func (cfg UpdateNonceManagerConfig) Validate(e deployment.Environment) error { if _, ok := state.Chains[prevRamp.RemoteChainSelector]; !ok { return fmt.Errorf("dest chain %d not found in onchain state for chain %d", prevRamp.RemoteChainSelector, sourceSel) } - if !prevRamp.EnableOnRamp && !prevRamp.EnableOffRamp { - return errors.New("must specify either onramp or offramp") + // If one of the onRamp or OffRamp is set with non-zero address and other is set with zero address, + // it will not be possible to update the previous ramps later. + // see https://github.com/smartcontractkit/chainlink/blob/develop/contracts/src/v0.8/ccip/NonceManager.sol#L139-L142 + if prevOnRamp := state.Chains[sourceSel].EVM2EVMOnRamp; prevOnRamp == nil { + return fmt.Errorf("no previous onramp for source chain %d", sourceSel) + } else if prevOnRamp[prevRamp.RemoteChainSelector] == nil || prevOnRamp[prevRamp.RemoteChainSelector].Address() == (common.Address{}) { + return fmt.Errorf("no previous onramp for source chain %d and dest chain %d", sourceSel, prevRamp.RemoteChainSelector) } - if prevRamp.EnableOnRamp { - if prevOnRamp := state.Chains[sourceSel].EVM2EVMOnRamp; prevOnRamp == nil { - return fmt.Errorf("no previous onramp for source chain %d", sourceSel) - } else if prevOnRamp[prevRamp.RemoteChainSelector] == nil { - return fmt.Errorf("no previous onramp for source chain %d and dest chain %d", sourceSel, prevRamp.RemoteChainSelector) - } - } - if prevRamp.EnableOffRamp { - if prevOffRamp := state.Chains[sourceSel].EVM2EVMOffRamp; prevOffRamp == nil { - return fmt.Errorf("missing previous offramps for chain %d", sourceSel) - } else if prevOffRamp[prevRamp.RemoteChainSelector] == nil { - return fmt.Errorf("no previous offramp for source chain %d and dest chain %d", prevRamp.RemoteChainSelector, sourceSel) - } + if prevOffRamp := state.Chains[sourceSel].EVM2EVMOffRamp; prevOffRamp == nil { + return fmt.Errorf("missing previous offramps for chain %d", sourceSel) + } else if prevOffRamp[prevRamp.RemoteChainSelector] == nil || prevOffRamp[prevRamp.RemoteChainSelector].Address() == (common.Address{}) { + return fmt.Errorf("no previous offramp for source chain %d and dest chain %d", prevRamp.RemoteChainSelector, sourceSel) } } } @@ -165,13 +159,8 @@ func UpdateNonceManagersChangeset(e deployment.Environment, cfg UpdateNonceManag if len(updates.PreviousRampsArgs) > 0 { previousRampsArgs := make([]nonce_manager.NonceManagerPreviousRampsArgs, 0) for _, prevRamp := range updates.PreviousRampsArgs { - var onRamp, offRamp common.Address - if prevRamp.EnableOnRamp { - onRamp = s.Chains[chainSel].EVM2EVMOnRamp[prevRamp.RemoteChainSelector].Address() - } - if prevRamp.EnableOffRamp { - offRamp = s.Chains[chainSel].EVM2EVMOffRamp[prevRamp.RemoteChainSelector].Address() - } + onRamp := s.Chains[chainSel].EVM2EVMOnRamp[prevRamp.RemoteChainSelector].Address() + offRamp := s.Chains[chainSel].EVM2EVMOffRamp[prevRamp.RemoteChainSelector].Address() previousRampsArgs = append(previousRampsArgs, nonce_manager.NonceManagerPreviousRampsArgs{ RemoteChainSelector: prevRamp.RemoteChainSelector, OverrideExistingRamps: prevRamp.OverrideExisting, diff --git a/deployment/common/changeset/deploy_link_token.go b/deployment/common/changeset/deploy_link_token.go index 86783ea4118..81540d40547 100644 --- a/deployment/common/changeset/deploy_link_token.go +++ b/deployment/common/changeset/deploy_link_token.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/smartcontractkit/chainlink-common/pkg/logger" + "golang.org/x/sync/errgroup" "github.com/gagliardetto/solana-go" chainsel "github.com/smartcontractkit/chain-selectors" @@ -31,31 +32,41 @@ func DeployLinkToken(e deployment.Environment, chains []uint64) (deployment.Chan return deployment.ChangesetOutput{}, err } newAddresses := deployment.NewMemoryAddressBook() + deployGrp := errgroup.Group{} for _, chain := range chains { family, err := chainsel.GetSelectorFamily(chain) if err != nil { return deployment.ChangesetOutput{AddressBook: newAddresses}, err } + var deployFn func() error switch family { case chainsel.FamilyEVM: // Deploy EVM LINK token - _, err := deployLinkTokenContractEVM( - e.Logger, e.Chains[chain], newAddresses, - ) - if err != nil { - return deployment.ChangesetOutput{AddressBook: newAddresses}, err + deployFn = func() error { + _, err := deployLinkTokenContractEVM( + e.Logger, e.Chains[chain], newAddresses, + ) + return err } case chainsel.FamilySolana: // Deploy Solana LINK token - err := deployLinkTokenContractSolana( - e.Logger, e.SolChains[chain], newAddresses, - ) - if err != nil { - return deployment.ChangesetOutput{AddressBook: newAddresses}, err + deployFn = func() error { + err := deployLinkTokenContractSolana( + e.Logger, e.SolChains[chain], newAddresses, + ) + return err } } + deployGrp.Go(func() error { + err := deployFn() + if err != nil { + e.Logger.Errorw("Failed to deploy link token", "chain", chain, "err", err) + return fmt.Errorf("failed to deploy link token for chain %d: %w", chain, err) + } + return nil + }) } - return deployment.ChangesetOutput{AddressBook: newAddresses}, nil + return deployment.ChangesetOutput{AddressBook: newAddresses}, deployGrp.Wait() } // DeployStaticLinkToken deploys a static link token contract to the chain identified by the ChainSelector. diff --git a/integration-tests/smoke/ccip/ccip_migration_to_v_1_6_test.go b/integration-tests/smoke/ccip/ccip_migration_to_v_1_6_test.go index 1f5f5b850a2..3273b113717 100644 --- a/integration-tests/smoke/ccip/ccip_migration_to_v_1_6_test.go +++ b/integration-tests/smoke/ccip/ccip_migration_to_v_1_6_test.go @@ -206,7 +206,6 @@ func TestMigrateFromV1_5ToV1_6(t *testing.T) { PreviousRampsArgs: []changeset.PreviousRampCfg{ { RemoteChainSelector: dest, - EnableOnRamp: true, }, }, }, @@ -214,7 +213,6 @@ func TestMigrateFromV1_5ToV1_6(t *testing.T) { PreviousRampsArgs: []changeset.PreviousRampCfg{ { RemoteChainSelector: dest, - EnableOnRamp: true, }, }, }, @@ -222,11 +220,9 @@ func TestMigrateFromV1_5ToV1_6(t *testing.T) { PreviousRampsArgs: []changeset.PreviousRampCfg{ { RemoteChainSelector: src1, - EnableOffRamp: true, }, { RemoteChainSelector: src2, - EnableOffRamp: true, }, }, },