chore: adjusted max signers

smartcontractkit · Dec 19, 2024 · 27d4f90 · 27d4f90
1 parent 72eea70
commit 27d4f90
Show file tree

Hide file tree

Showing 8 changed files with 108 additions and 76 deletions.
diff --git a/chains/solana/contracts/programs/mcm/src/event.rs b/chains/solana/contracts/programs/mcm/src/event.rs
@@ -1,6 +1,6 @@
 use anchor_lang::prelude::*;
 
-use crate::constant::*;
+use crate::{constant::*, state::config::*};
 
 #[event]
 /// @dev Emitted when a new root is set.
@@ -22,8 +22,7 @@ pub struct ConfigSet {
     pub group_parents: [u8; NUM_GROUPS],
     pub group_quorums: [u8; NUM_GROUPS],
     pub is_root_cleared: bool,
-    // todo: emitting all signers causes a memory overflow, need to find a way to emit them
-    // pub signers: Vec<McmSigner>,
+    pub signers: Vec<McmSigner>,
 }
 
 #[event]

diff --git a/chains/solana/contracts/programs/mcm/src/instructions/set_config.rs b/chains/solana/contracts/programs/mcm/src/instructions/set_config.rs
@@ -16,6 +16,7 @@ pub fn set_config(
     group_parents: [u8; NUM_GROUPS],
     clear_root: bool,
 ) -> Result<()> {
+    let config = &mut ctx.accounts.multisig_config;
     // signer addresses are preloaded in the ConfigSigners account through InitSigners, AppendSigners and FinalizeSigners instructions
     let signer_addresses = &ctx.accounts.config_signers.signer_addresses;
 
@@ -28,86 +29,88 @@ pub fn set_config(
         signer_addresses.len() == signer_groups.len(),
         McmError::MismatchedInputSignerVectorsLength
     );
-
-    // count the number of children for each group while validating group structure
-    let mut group_children_counts = signer_groups.iter().try_fold(
-        [0u8; NUM_GROUPS],
-        |mut acc, &group| -> Result<[u8; NUM_GROUPS]> {
-            // make sure the specified signer group is in bound
-            require!(
-                (group as usize) < NUM_GROUPS,
-                McmError::MismatchedInputGroupArraysLength
-            );
-            acc[group as usize] = acc[group as usize]
-                .checked_add(1)
-                .ok_or(McmError::Overflow)?;
-
-            Ok(acc)
-        },
-    )?;
-
-    const ROOT_GROUP: usize = 0;
-    // check if the group structure is a tree
-    for i in (0..NUM_GROUPS).rev() {
-        // validate group structure in backwards(root is 0)
-
-        match i {
-            // root should have itself as parent
-            ROOT_GROUP => require!(
-                group_parents[ROOT_GROUP] == ROOT_GROUP as u8,
-                McmError::GroupTreeNotWellFormed
-            ),
-            // make sure the parent group is at a higher level(lower index) than the current group
-            _ => require!(group_parents[i] < i as u8, McmError::GroupTreeNotWellFormed),
-        }
-
-        let disabled: bool = group_quorums[i] == 0;
-
-        match disabled {
-            true => {
-                // validate disabled group has no children
+    {
+        // count the number of children for each group while validating group structure
+        let mut group_children_counts = signer_groups.iter().try_fold(
+            [0u8; NUM_GROUPS],
+            |mut acc, &group| -> Result<[u8; NUM_GROUPS]> {
+                // make sure the specified signer group is in bound
                 require!(
-                    group_children_counts[i] == 0,
-                    McmError::SignerInDisabledGroup
+                    (group as usize) < NUM_GROUPS,
+                    McmError::MismatchedInputGroupArraysLength
                 );
-            }
-            false => {
-                // ensure the group quorum can be met(i.e. have more signers than the quorum)
-                require!(
-                    group_children_counts[i] >= group_quorums[i],
-                    McmError::OutOfBoundsGroupQuorum
-                );
-
-                // increase the parent group's children count
-                let parent_index = group_parents[i] as usize;
-                group_children_counts[parent_index] = group_children_counts[parent_index]
+                acc[group as usize] = acc[group as usize]
                     .checked_add(1)
                     .ok_or(McmError::Overflow)?;
+
+                Ok(acc)
+            },
+        )?;
+
+        const ROOT_GROUP: usize = 0;
+        // check if the group structure is a tree
+        for i in (0..NUM_GROUPS).rev() {
+            // validate group structure in backwards(root is 0)
+
+            match i {
+                // root should have itself as parent
+                ROOT_GROUP => require!(
+                    group_parents[ROOT_GROUP] == ROOT_GROUP as u8,
+                    McmError::GroupTreeNotWellFormed
+                ),
+                // make sure the parent group is at a higher level(lower index) than the current group
+                _ => require!(group_parents[i] < i as u8, McmError::GroupTreeNotWellFormed),
+            }
+
+            let disabled: bool = group_quorums[i] == 0;
+
+            match disabled {
+                true => {
+                    // validate disabled group has no children
+                    require!(
+                        group_children_counts[i] == 0,
+                        McmError::SignerInDisabledGroup
+                    );
+                }
+                false => {
+                    // ensure the group quorum can be met(i.e. have more signers than the quorum)
+                    require!(
+                        group_children_counts[i] >= group_quorums[i],
+                        McmError::OutOfBoundsGroupQuorum
+                    );
+
+                    // increase the parent group's children count
+                    let parent_index = group_parents[i] as usize;
+                    group_children_counts[parent_index] = group_children_counts[parent_index]
+                        .checked_add(1)
+                        .ok_or(McmError::Overflow)?;
+                }
             }
         }
     }
 
-    let config = &mut ctx.accounts.multisig_config;
-    let mut signers: Vec<McmSigner> = Vec::with_capacity(signer_addresses.len());
-    let mut prev_signer = [0u8; EVM_ADDRESS_BYTES];
+    {
+        let mut signers: Vec<McmSigner> = Vec::with_capacity(signer_addresses.len());
+        let mut prev_signer = [0u8; EVM_ADDRESS_BYTES];
 
-    for (index, &evm_addr) in signer_addresses.iter().enumerate() {
-        require!(
-            evm_addr > prev_signer,
-            McmError::SignersAddressesMustBeStrictlyIncreasing
-        );
+        for (index, &evm_addr) in signer_addresses.iter().enumerate() {
+            require!(
+                evm_addr > prev_signer,
+                McmError::SignersAddressesMustBeStrictlyIncreasing
+            );
 
-        // update prev signer
-        prev_signer = evm_addr;
+            // update prev signer
+            prev_signer = evm_addr;
 
-        signers.push(McmSigner {
-            evm_address: evm_addr,
-            index: u8::try_from(index).unwrap(), // This is safe due to previous check on signer_addresses length
-            group: signer_groups[index],
-        })
+            signers.push(McmSigner {
+                evm_address: evm_addr,
+                index: u8::try_from(index).unwrap(), // This is safe due to previous check on signer_addresses length
+                group: signer_groups[index],
+            })
+        }
+        config.signers = signers;
     }
 
-    config.signers = signers;
     config.group_quorums = group_quorums;
     config.group_parents = group_parents;
 
@@ -125,8 +128,8 @@ pub fn set_config(
         expiring_root.op_count = current_op_count;
 
         // set root metadata to a cleared state
-        root_metadata.chain_id = ctx.accounts.multisig_config.chain_id;
-        root_metadata.multisig = ctx.accounts.multisig_config.key();
+        root_metadata.chain_id = config.chain_id;
+        root_metadata.multisig = config.key();
         root_metadata.pre_op_count = current_op_count;
         root_metadata.post_op_count = current_op_count;
         root_metadata.override_previous_root = true;
@@ -136,8 +139,7 @@ pub fn set_config(
         group_parents,
         group_quorums,
         is_root_cleared: clear_root,
-        // todo: memory inefficient, finding workaround
-        // signers: config.signers.clone(),
+        signers: config.signers.clone(),
     });
 
     Ok(())

diff --git a/chains/solana/contracts/target/idl/mcm.json b/chains/solana/contracts/target/idl/mcm.json
@@ -1102,6 +1102,15 @@
           "name": "isRootCleared",
           "type": "bool",
           "index": false
+        },
+        {
+          "name": "signers",
+          "type": {
+            "vec": {
+              "defined": "McmSigner"
+            }
+          },
+          "index": false
         }
       ]
     },

diff --git a/chains/solana/contracts/tests/config/mcm_config.go b/chains/solana/contracts/tests/config/mcm_config.go
@@ -23,7 +23,7 @@ var (
 	TestMsigNamePaddedBuffer = [32]byte{
 		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x74, 0x65, 0x73, 0x74, 0x2d, 0x6d, 0x63, 0x6d,
 	}
-	MaxNumSigners               = 200
+	MaxNumSigners               = 180
 	MaxAppendSignerBatchSize    = 45
 	MaxAppendSignatureBatchSize = 13
 

diff --git a/chains/solana/contracts/tests/mcms/mcm_events.go b/chains/solana/contracts/tests/mcms/mcm_events.go
@@ -2,6 +2,7 @@ package contracts
 
 import (
 	"github.com/gagliardetto/solana-go"
+	"github.com/smartcontractkit/chainlink-ccip/chains/solana/gobindings/mcm"
 )
 
 // Events - temporary event struct to decode
@@ -25,10 +26,10 @@ const numGroups = 32
 
 // ConfigSet represents an event emitted when a new config is set
 type ConfigSet struct {
-	// Note: Rust comment indicates signers are omitted due to memory overflow
 	GroupParents  [numGroups]byte // group_parents
 	GroupQuorums  [numGroups]byte // group_quorums
 	IsRootCleared bool            // is_root_cleared
+	Signers       []mcm.McmSigner // data: Vec<u8>
 }
 
 // OpExecuted represents an event emitted when an op is successfully executed

diff --git a/chains/solana/contracts/tests/mcms/mcm_set_config_test.go b/chains/solana/contracts/tests/mcms/mcm_set_config_test.go
@@ -279,6 +279,12 @@ func TestMcmSetConfig(t *testing.T) {
 				require.Equal(t, mcmConfig.GroupQuorums, event.GroupQuorums)
 				require.Equal(t, mcmConfig.ClearRoot, event.IsRootCleared)
 
+				for i, signer := range event.Signers {
+					require.Equal(t, mcmConfig.SignerAddresses[i], signer.EvmAddress)
+					require.Equal(t, uint8(i), signer.Index)
+					require.Equal(t, mcmConfig.SignerGroups[i], signer.Group)
+				}
+
 				// get config and validate
 				var configAccount mcm.MultisigConfig
 				err = utils.GetAccountDataBorshInto(ctx, solanaGoClient, multisigConfigPDA, config.DefaultCommitment, &configAccount)
@@ -416,6 +422,11 @@ func TestMcmSetConfig(t *testing.T) {
 			require.Equal(t, mcmConfig.GroupParents, event.GroupParents)
 			require.Equal(t, mcmConfig.GroupQuorums, event.GroupQuorums)
 			require.Equal(t, mcmConfig.ClearRoot, event.IsRootCleared)
+			for i, signer := range event.Signers {
+				require.Equal(t, mcmConfig.SignerAddresses[i], signer.EvmAddress)
+				require.Equal(t, uint8(i), signer.Index)
+				require.Equal(t, mcmConfig.SignerGroups[i], signer.Group)
+			}
 
 			// get config and validate
 			var configAccount mcm.MultisigConfig

diff --git a/chains/solana/contracts/tests/mcms/mcm_set_root_execute_test.go b/chains/solana/contracts/tests/mcms/mcm_set_root_execute_test.go
@@ -536,6 +536,11 @@ func TestMcmSetRootAndExecute(t *testing.T) {
 				require.Equal(t, newMcmConfig.GroupParents, event.GroupParents)
 				require.Equal(t, newMcmConfig.GroupQuorums, event.GroupQuorums)
 				require.Equal(t, true, event.IsRootCleared)
+				for i, signer := range event.Signers {
+					require.Equal(t, newMcmConfig.SignerAddresses[i], signer.EvmAddress)
+					require.Equal(t, uint8(i), signer.Index)
+					require.Equal(t, newMcmConfig.SignerGroups[i], signer.Group)
+				}
 
 				// get config and validate
 				var configAccount mcm.MultisigConfig

diff --git a/chains/solana/contracts/tests/mcms/mcm_timelock_test.go b/chains/solana/contracts/tests/mcms/mcm_timelock_test.go
@@ -195,6 +195,11 @@ func TestMcmWithTimelock(t *testing.T) {
 						require.Equal(t, msig.RawConfig.GroupParents, event.GroupParents)
 						require.Equal(t, msig.RawConfig.GroupQuorums, event.GroupQuorums)
 						require.Equal(t, msig.RawConfig.ClearRoot, event.IsRootCleared)
+						for i, signer := range event.Signers {
+							require.Equal(t, msig.RawConfig.SignerAddresses[i], signer.EvmAddress)
+							require.Equal(t, uint8(i), signer.Index)
+							require.Equal(t, msig.RawConfig.SignerGroups[i], signer.Group)
+						}
 
 						// get config and validate
 						var configAccount mcm.MultisigConfig