diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo index 1baf28180..0d5f79684 100644 --- a/contracts/src/mcms.cairo +++ b/contracts/src/mcms.cairo @@ -38,14 +38,14 @@ trait IManyChainMultiSig { fn get_root_metadata(self: @TContractState) -> RootMetadata; } -#[derive(Copy, Drop, Serde, starknet::Store)] +#[derive(Copy, Drop, Serde, starknet::Store, PartialEq, Debug)] struct Signer { address: EthAddress, index: u8, group: u8 } -#[derive(Copy, Drop, Serde, starknet::Store)] +#[derive(Copy, Drop, Serde, starknet::Store, PartialEq)] struct RootMetadata { chain_id: u256, multisig: ContractAddress, @@ -67,14 +67,14 @@ struct Op { } // does not implement Storage trait because structs cannot support arrays or maps -#[derive(Copy, Drop, Serde)] +#[derive(Copy, Drop, Serde, PartialEq)] struct Config { signers: Span, group_quorums: Span, group_parents: Span } -#[derive(Copy, Drop, Serde, starknet::Store)] +#[derive(Copy, Drop, Serde, starknet::Store, PartialEq)] struct ExpiringRootAndOpCount { root: u256, valid_until: u32, @@ -249,11 +249,11 @@ mod ManyChainMultiSig { }; assert( - to_u256(prev_address) < to_u256(signer_address), + to_u256(prev_address) < to_u256(signer_address.clone()), 'signer address must increase' ); - let signer = self.s_signers.read(signer_address); + let signer = self.get_signer_by_address(signer_address); assert(signer.address == signer_address, 'invalid signer'); let mut group = signer.group; @@ -478,6 +478,7 @@ mod ManyChainMultiSig { self.s_signers.write(old_signer.address, empty_signer); // reset _s_config_signers self._s_config_signers.write(i.into(), empty_signer); + i += 1; }; // reset _s_config_signers_len self._s_config_signers_len.write(0); @@ -515,6 +516,9 @@ mod ManyChainMultiSig { i += 1; }; + // length will always be less than MAX_NUM_SIGNERS so try_into will never panic + self._s_config_signers_len.write(signer_addresses.len().try_into().unwrap()); + if clear_root { let op_count = self.s_expiring_root_and_op_count.read().op_count; self @@ -566,6 +570,7 @@ mod ManyChainMultiSig { while i < NUM_GROUPS { group_quorums.append(self._s_config_group_quorums.read(i)); group_parents.append(self._s_config_group_parents.read(i)); + i += 1; }; Config { @@ -596,6 +601,10 @@ mod ManyChainMultiSig { ) { let _response = call_contract_syscall(target, selector, data).unwrap_syscall(); } + + fn get_signer_by_address(ref self: ContractState, signer_address: EthAddress) -> Signer { + self.s_signers.read(signer_address) + } } } diff --git a/contracts/src/tests/test_mcms.cairo b/contracts/src/tests/test_mcms.cairo index d9c0a3952..a59d2f976 100644 --- a/contracts/src/tests/test_mcms.cairo +++ b/contracts/src/tests/test_mcms.cairo @@ -1,12 +1,23 @@ use core::array::{SpanTrait, ArrayTrait}; -use starknet::{ContractAddress, EthAddress, EthAddressZeroable}; +use starknet::{ContractAddress, EthAddress, EthAddressZeroable, contract_address_const}; use chainlink::mcms::{ - ManyChainMultiSig, IManyChainMultiSigDispatcher, IManyChainMultiSigSafeDispatcher, - IManyChainMultiSigSafeDispatcherTrait, ManyChainMultiSig::{MAX_NUM_SIGNERS} + ExpiringRootAndOpCount, RootMetadata, Config, Signer, ManyChainMultiSig, + ManyChainMultiSig::{ + InternalFunctionsTrait, contract_state_for_testing, s_signersContractMemberStateTrait, + s_expiring_root_and_op_countContractMemberStateTrait, + s_root_metadataContractMemberStateTrait + }, + IManyChainMultiSigDispatcher, IManyChainMultiSigDispatcherTrait, + IManyChainMultiSigSafeDispatcher, IManyChainMultiSigSafeDispatcherTrait, IManyChainMultiSig, + ManyChainMultiSig::{MAX_NUM_SIGNERS}, }; use snforge_std::{ - declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global + declare, ContractClassTrait, start_cheat_caller_address_global, start_cheat_caller_address, + stop_cheat_caller_address, stop_cheat_caller_address_global, spy_events, + EventSpyAssertionsTrait, // Add for assertions on the EventSpy + test_address, // the contract being tested, + start_cheat_chain_id }; // set_config tests @@ -661,3 +672,352 @@ fn test_set_config_signer_addresses_not_sorted() { } } +// test success, root not cleared, event emitted +// 12. successful => test without clearing root. test the state of storage variables and that event was emitted +// +// ┌──────┐ +// ┌─►│2-of-2│ +// │ └──────┘ +// │ ▲ +// │ │ +// ┌──┴───┐ ┌──┴───┐ +// signer 1 signer 2 +// └──────┘ └──────┘ +#[test] +fn test_set_config_success_dont_clear_root() { + let (mcms_address, mcms, _) = setup(); + + let signer_address_1: EthAddress = u256 { high: 0, low: 1 }.into(); + let signer_address_2: EthAddress = u256 { high: 0, low: 2 }.into(); + let signer_addresses: Array = array![signer_address_1, signer_address_2]; + let signer_groups = array![0, 0]; + let group_quorums = array![ + 2, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0 + ]; + let group_parents = array![ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0 + ]; + let clear_root = false; + + let mut spy = spy_events(); + + mcms + .set_config( + signer_addresses.span(), + signer_groups.span(), + group_quorums.span(), + group_parents.span(), + clear_root + ); + + let expected_signer_1 = Signer { address: signer_address_1, index: 0, group: 0 }; + let expected_signer_2 = Signer { address: signer_address_2, index: 1, group: 0 }; + + let expected_config = Config { + signers: array![expected_signer_1, expected_signer_2].span(), + group_quorums: group_quorums.span(), + group_parents: group_parents.span(), + }; + + spy + .assert_emitted( + @array![ + ( + mcms_address, + ManyChainMultiSig::Event::ConfigSet( + ManyChainMultiSig::ConfigSet { + config: expected_config, is_root_cleared: false + } + ) + ) + ] + ); + let config = mcms.get_config(); + // assert(config.signers == expected_config.signers, 'signers not equal'); + // assert(config.group_quorums == expected_config.group_quorums, 'group quorums not equal'); + // assert(config.group_parents == expected_config.group_parents, 'group parents not equal'); + // test the members + assert(config == expected_config, 'config should be same'); + + // mock the contract state + let test_address = test_address(); + start_cheat_caller_address(test_address, contract_address_const::<777>()); + + // test internal function state + let mut state = contract_state_for_testing(); + ManyChainMultiSig::constructor(ref state); + state + .set_config( + signer_addresses.span(), + signer_groups.span(), + group_quorums.span(), + group_parents.span(), + clear_root + ); + + let signer_1 = state.get_signer_by_address(signer_address_1); + let signer_2 = state.get_signer_by_address(signer_address_2); + + println!("expected signer 1 {:?}", expected_signer_1); + println!("signer 1 {:?}", signer_1); + + println!("expected signer 2 {:?}", expected_signer_2); + println!("signer 2 {:?}", signer_2); + + assert(signer_1 == expected_signer_1, 'signer 1 not equal'); + assert(signer_2 == expected_signer_2, 'signer 2 not equal'); + + // test second set_config + let new_signer_address_1: EthAddress = u256 { high: 0, low: 3 }.into(); + let new_signer_address_2: EthAddress = u256 { high: 0, low: 4 }.into(); + let new_signer_addresses = array![new_signer_address_1, new_signer_address_2]; + + mcms + .set_config( + new_signer_addresses.span(), + signer_groups.span(), + group_quorums.span(), + group_parents.span(), + clear_root + ); + + let new_config = mcms.get_config(); + + let new_expected_signer_1 = Signer { address: new_signer_address_1, index: 0, group: 0 }; + let new_expected_signer_2 = Signer { address: new_signer_address_2, index: 1, group: 0 }; + + let new_expected_config = Config { + signers: array![new_expected_signer_1, new_expected_signer_2].span(), + group_quorums: group_quorums.span(), + group_parents: group_parents.span(), + }; + + assert(new_config == new_expected_config, 'new config should be same'); + + state + .set_config( + new_signer_addresses.span(), + signer_groups.span(), + group_quorums.span(), + group_parents.span(), + clear_root + ); + + let new_signer_1 = state.get_signer_by_address(new_signer_address_1); + let new_signer_2 = state.get_signer_by_address(new_signer_address_2); + + println!("new expected signer 1 {:?}", new_expected_signer_1); + println!("new signer 1 {:?}", new_signer_1); + + println!("new expected signer 2 {:?}", new_expected_signer_2); + println!("new signer 2 {:?}", new_signer_2); + + assert(new_signer_1 == new_expected_signer_1, 'new signer 1 not equal'); + assert(new_signer_2 == new_expected_signer_2, 'new signer 2 not equal'); + + // test old signers were reset + let old_signer_1 = state.get_signer_by_address(signer_address_1); + let old_signer_2 = state.get_signer_by_address(signer_address_2); + assert(old_signer_1.address == EthAddressZeroable::zero(), 'old signer 1 should be reset'); + assert(old_signer_2.address == EthAddressZeroable::zero(), 'old signer 1 should be reset'); +} + + +// test that the config was reset +#[test] +fn test_set_config_success_and_clear_root() { + // mock the contract state + let test_address = test_address(); + let mock_chain_id = 990; + start_cheat_caller_address(test_address, contract_address_const::<777>()); + start_cheat_chain_id(test_address, mock_chain_id); + + let mut state = contract_state_for_testing(); + ManyChainMultiSig::constructor(ref state); + + // initialize s_expiring_root_and_op_count & s_root_metadata + state + .s_expiring_root_and_op_count + .write( + ExpiringRootAndOpCount { + root: u256 { high: 777, low: 777 }, valid_until: 102934894, op_count: 134 + } + ); + + state + .s_root_metadata + .write( + RootMetadata { + chain_id: 123123, + multisig: contract_address_const::<111>(), + pre_op_count: 20, + post_op_count: 155, + override_previous_root: false + } + ); + + let signer_address_1: EthAddress = u256 { high: 0, low: 1 }.into(); + let signer_address_2: EthAddress = u256 { high: 0, low: 2 }.into(); + let signer_addresses: Array = array![signer_address_1, signer_address_2]; + let signer_groups = array![0, 0]; + let group_quorums = array![ + 2, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0 + ]; + let group_parents = array![ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0 + ]; + let clear_root = true; + + state + .set_config( + signer_addresses.span(), + signer_groups.span(), + group_quorums.span(), + group_parents.span(), + clear_root + ); + + let expected_s_expiring_root_and_op_count = ExpiringRootAndOpCount { + root: u256 { high: 0, low: 0 }, valid_until: 0, op_count: 134 + }; + let s_expiring_root_and_op_count = state.s_expiring_root_and_op_count.read(); + assert!( + s_expiring_root_and_op_count == expected_s_expiring_root_and_op_count, + "s_expiring_root_and_op_count not equal" + ); + + let expected_s_root_metadata = RootMetadata { + chain_id: mock_chain_id.into(), + multisig: test_address, + pre_op_count: 134, + post_op_count: 134, + override_previous_root: true + }; + let s_root_metadata = state.s_root_metadata.read(); + assert(expected_s_root_metadata == s_root_metadata, 's_root_metadata not equal'); +} +