diff --git a/chains/solana/contracts/programs/mcm/src/event.rs b/chains/solana/contracts/programs/mcm/src/event.rs index 07871b080..6a47d9d85 100644 --- 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, + pub signers: Vec, } #[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 index 8f5cf1ab1..be7b7d7ff 100644 --- 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 = Vec::with_capacity(signer_addresses.len()); - let mut prev_signer = [0u8; EVM_ADDRESS_BYTES]; + { + let mut signers: Vec = 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 index e53283e1e..fa7adb61c 100644 --- 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 index 7fe7541b2..b4e7e5c0e 100644 --- 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 index bedb839d5..5989b39c0 100644 --- 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 } // 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 index eef6c6a42..c995684fa 100644 --- 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 index a50801301..b28e8b79a 100644 --- 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 index d62a8cf76..b1e1a9683 100644 --- 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