From 90cc29107fa5212d7b85ad54ca4f07f748d1aa36 Mon Sep 17 00:00:00 2001 From: JordyRo1 Date: Wed, 17 Jul 2024 19:42:47 +0200 Subject: [PATCH] audit: threshold set as parameter --- .../isms/aggregation/aggregation.cairo | 17 ++----- .../multisig/merkleroot_multisig_ism.cairo | 16 ++---- .../multisig/messageid_multisig_ism.cairo | 16 ++---- contracts/src/interfaces.cairo | 4 -- .../src/tests/isms/test_aggregation.cairo | 42 +++++++--------- .../tests/isms/test_merkleroot_multisig.cairo | 44 +++++++---------- .../tests/isms/test_messageid_multisig.cairo | 49 +++++++------------ .../test_default_fallback_routing_ism.cairo | 10 +--- .../routing/test_domain_routing_ism.cairo | 10 +--- contracts/src/tests/setup.cairo | 9 ++-- 10 files changed, 78 insertions(+), 139 deletions(-) diff --git a/contracts/src/contracts/isms/aggregation/aggregation.cairo b/contracts/src/contracts/isms/aggregation/aggregation.cairo index 578d008..078b1c8 100644 --- a/contracts/src/contracts/isms/aggregation/aggregation.cairo +++ b/contracts/src/contracts/isms/aggregation/aggregation.cairo @@ -47,13 +47,17 @@ pub mod aggregation { pub const MODULES_ALREADY_STORED: felt252 = 'Modules already stored'; pub const NO_MODULES_PROVIDED: felt252 = 'No modules provided'; pub const TOO_MANY_MODULES_PROVIDED: felt252 = 'Too many modules provided'; + pub const THRESHOLD_TOO_HIGH: felt252 = 'Threshold too high'; } #[constructor] - fn constructor(ref self: ContractState, _owner: ContractAddress, _modules: Span) { + fn constructor(ref self: ContractState, _owner: ContractAddress, _modules: Span, _threshold: u8) { self.ownable.initializer(_owner); assert(_modules.len() < 256, Errors::TOO_MANY_MODULES_PROVIDED); + assert(_threshold <= 255, Errors::THRESHOLD_TOO_HIGH); + self.threshold.write(_threshold); self.set_modules(_modules); + } #[abi(embed_v0)] @@ -132,17 +136,6 @@ pub mod aggregation { fn get_threshold(self: @ContractState) -> u8 { self.threshold.read() } - - /// Sets the threshold for validation - /// Dev: callable only by the owner - /// - /// # Arguments - /// - /// * - `_threshold` - The number of validator signatures needed - fn set_threshold(ref self: ContractState, _threshold: u8) { - self.ownable.assert_only_owner(); - self.threshold.write(_threshold); - } } #[generate_trait] impl InternalImpl of InternalTrait { diff --git a/contracts/src/contracts/isms/multisig/merkleroot_multisig_ism.cairo b/contracts/src/contracts/isms/multisig/merkleroot_multisig_ism.cairo index b15316a..e06a43a 100644 --- a/contracts/src/contracts/isms/multisig/merkleroot_multisig_ism.cairo +++ b/contracts/src/contracts/isms/multisig/merkleroot_multisig_ism.cairo @@ -36,6 +36,7 @@ pub mod merkleroot_multisig_ism { pub const EMPTY_METADATA: felt252 = 'Empty metadata'; pub const VALIDATOR_ADDRESS_CANNOT_BE_NULL: felt252 = 'Validator address cannot be 0'; pub const NO_VALIDATORS_PROVIDED: felt252 = 'No validators provided'; + pub const THRESHOLD_TOO_HIGH: felt252 = 'Threshold too high'; } #[event] @@ -46,9 +47,11 @@ pub mod merkleroot_multisig_ism { } #[constructor] - fn constructor(ref self: ContractState, _owner: ContractAddress, _validators: Span) { + fn constructor(ref self: ContractState, _owner: ContractAddress, _validators: Span, _threshold: u32) { self.ownable.initializer(_owner); self.set_validators(_validators); + assert(_threshold <= 0xffffffff, Errors::THRESHOLD_TOO_HIGH); + self.threshold.write(_threshold); } #[abi(embed_v0)] @@ -113,17 +116,6 @@ pub mod merkleroot_multisig_ism { self.threshold.read() } - /// Sets the threshold, the number of validator signatures needed to verify a message - /// Dev: callable only by the admin - /// - /// # Arguments - /// - /// * - `_threshold` - the number of validator signature needed - fn set_threshold(ref self: ContractState, _threshold: u32) { - self.ownable.assert_only_owner(); - self.threshold.write(_threshold); - } - /// Returns the set of validators responsible for verifying _message and the number of signatures required /// Dev: Can change based on the content of _message /// diff --git a/contracts/src/contracts/isms/multisig/messageid_multisig_ism.cairo b/contracts/src/contracts/isms/multisig/messageid_multisig_ism.cairo index c177338..cc60743 100644 --- a/contracts/src/contracts/isms/multisig/messageid_multisig_ism.cairo +++ b/contracts/src/contracts/isms/multisig/messageid_multisig_ism.cairo @@ -36,6 +36,7 @@ pub mod messageid_multisig_ism { pub const EMPTY_METADATA: felt252 = 'Empty metadata'; pub const VALIDATOR_ADDRESS_CANNOT_BE_NULL: felt252 = 'Validator address cannot be 0'; pub const NO_VALIDATORS_PROVIDED: felt252 = 'No validators provided'; + pub const THRESHOLD_TOO_HIGH: felt252 = 'Threshold too high'; } #[event] @@ -49,9 +50,11 @@ pub mod messageid_multisig_ism { #[constructor] - fn constructor(ref self: ContractState, _owner: ContractAddress, _validators: Span) { + fn constructor(ref self: ContractState, _owner: ContractAddress, _validators: Span, _threshold: u32) { self.ownable.initializer(_owner); self.set_validators(_validators); + assert(_threshold <= 0xffffffff, Errors::THRESHOLD_TOO_HIGH); + self.threshold.write(_threshold); } #[abi(embed_v0)] @@ -117,17 +120,6 @@ pub mod messageid_multisig_ism { } - /// Set the threshold for validation - /// Dev: callable only by the owner - /// - /// # Arguments - /// - /// * - `_threshold` - The number of validator signatures needed - fn set_threshold(ref self: ContractState, _threshold: u32) { - self.ownable.assert_only_owner(); - self.threshold.write(_threshold); - } - /// Returns the set of validators responsible for verifying _message and the number of signatures required /// Dev: Can change based on the content of _message /// diff --git a/contracts/src/interfaces.cairo b/contracts/src/interfaces.cairo index 08041b7..57ab189 100644 --- a/contracts/src/interfaces.cairo +++ b/contracts/src/interfaces.cairo @@ -105,8 +105,6 @@ pub trait IValidatorConfiguration { fn get_validators(self: @TContractState) -> Span; fn get_threshold(self: @TContractState) -> u32; - - fn set_threshold(ref self: TContractState, _threshold: u32); } #[starknet::interface] @@ -276,8 +274,6 @@ pub trait IAggregation { fn get_modules(self: @TContractState) -> Span; fn get_threshold(self: @TContractState) -> u8; - - fn set_threshold(ref self: TContractState, _threshold: u8); } diff --git a/contracts/src/tests/isms/test_aggregation.cairo b/contracts/src/tests/isms/test_aggregation.cairo index b27763c..ea46fff 100644 --- a/contracts/src/tests/isms/test_aggregation.cairo +++ b/contracts/src/tests/isms/test_aggregation.cairo @@ -19,25 +19,18 @@ use starknet::ContractAddress; #[test] fn test_aggregation_module_type() { - let aggregation = setup_aggregation(MODULES()); + let threshold = 4; + let aggregation = setup_aggregation(MODULES(), threshold); assert( aggregation.module_type() == ModuleType::AGGREGATION(aggregation.contract_address), 'Aggregation: Wrong module type' ); } -#[test] -fn test_aggregation_set_threshold() { - let threshold = 3; - let aggregation = setup_aggregation(MODULES()); - let ownable = IOwnableDispatcher { contract_address: aggregation.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - aggregation.set_threshold(threshold); -} - #[test] #[should_panic] fn test_aggregation_initialize_with_too_many_modules() { + let threshold = 4; let mut modules = array![]; let mut cur_idx = 0; loop { @@ -47,32 +40,35 @@ fn test_aggregation_initialize_with_too_many_modules() { modules.append('module_1'.into()); cur_idx += 1; }; - setup_aggregation(modules.span()); + setup_aggregation(modules.span(), threshold); } -#[test] -#[should_panic(expected: ('Threshold not set',))] -fn test_aggregation_verify_fails_if_treshold_not_set() { - let aggregation = setup_aggregation(MODULES()); - aggregation.verify(BytesTrait::new(42, array![]), MessageTrait::default()); -} - #[test] #[should_panic] fn test_setup_aggregation_with_null_module_address() { + let threshold = 2; let modules: Span = array![0, 'module_1'].span(); - setup_aggregation(modules); + setup_aggregation(modules, threshold); } #[test] fn test_get_modules() { - let aggregation = setup_aggregation(MODULES()); + let threshold = 2; + let aggregation = setup_aggregation(MODULES(), threshold); let ownable = IOwnableDispatcher { contract_address: aggregation.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); assert(aggregation.get_modules() == CONTRACT_MODULES(), 'set modules failed'); } +#[test] +fn test_get_treshold() { + let threshold = 2; + let aggregation = setup_aggregation(MODULES(), threshold); + let ownable = IOwnableDispatcher { contract_address: aggregation.contract_address }; + start_prank(CheatTarget::One(ownable.contract_address), OWNER()); + assert(aggregation.get_threshold() == threshold, 'threshold writing failed'); +} #[test] fn test_aggregation_verify() { @@ -97,7 +93,7 @@ fn test_aggregation_verify() { }; let (_, validators_address, _) = get_message_and_signature(); let (messageid, messageid_validator_configuration) = setup_messageid_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let origin_merkle_tree: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let root: u256 = 'root'.try_into().unwrap(); @@ -107,15 +103,13 @@ fn test_aggregation_verify() { contract_address: messageid_validator_configuration.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - messageid_validator_configuration.set_threshold(5); // Noop ism let noop_ism = setup_noop_ism(); let aggregation = setup_aggregation( - array![messageid.contract_address.into(), noop_ism.contract_address.into(),].span() + array![messageid.contract_address.into(), noop_ism.contract_address.into(),].span(), threshold.try_into().unwrap() ); let ownable = IOwnableDispatcher { contract_address: aggregation.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - aggregation.set_threshold(threshold); let mut concat_metadata = BytesTrait::new_empty(); concat_metadata.append_u128(0x00000010000001A0000001A0000001A9); concat_metadata.concat(@message_id_metadata); diff --git a/contracts/src/tests/isms/test_merkleroot_multisig.cairo b/contracts/src/tests/isms/test_merkleroot_multisig.cairo index 1296e2b..8415a50 100644 --- a/contracts/src/tests/isms/test_merkleroot_multisig.cairo +++ b/contracts/src/tests/isms/test_merkleroot_multisig.cairo @@ -21,10 +21,11 @@ use snforge_std::{start_prank, CheatTarget}; #[test] fn test_set_validators() { + let threshold = 4; let new_validators: Array = array![ VALIDATOR_ADDRESS_1().into(), VALIDATOR_ADDRESS_2().into() ]; - let (_, validators) = setup_merkleroot_multisig_ism(new_validators.span()); + let (_, validators) = setup_merkleroot_multisig_ism(new_validators.span(), threshold); let ownable = IOwnableDispatcher { contract_address: validators.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); let validators_span = validators.get_validators(); @@ -35,30 +36,20 @@ fn test_set_validators() { #[test] fn test_set_threshold() { - let new_threshold = 3; - let (_, validators) = setup_merkleroot_multisig_ism(array!['validator_1'].span()); + let threshold = 3; + let (_, validators) = setup_merkleroot_multisig_ism(array!['validator_1'].span(), threshold); let ownable = IOwnableDispatcher { contract_address: validators.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - validators.set_threshold(new_threshold); - assert(validators.get_threshold() == new_threshold, 'wrong validator threshold'); + assert(validators.get_threshold() == 2, 'wrong validator threshold'); } #[test] #[should_panic] fn test_set_validators_fails_if_null_validator() { + let threshold = 3; let new_validators = array![VALIDATOR_ADDRESS_1().into(), 0].span(); - setup_merkleroot_multisig_ism(new_validators); -} - -#[test] -#[should_panic(expected: ('Caller is not the owner',))] -fn test_set_threshold_fails_if_caller_not_owner() { - let new_threshold = 3; - let (_, validators) = setup_merkleroot_multisig_ism(array!['validator_1'].span()); - let ownable = IOwnableDispatcher { contract_address: validators.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), NEW_OWNER()); - validators.set_threshold(new_threshold); + setup_merkleroot_multisig_ism(new_validators, threshold); } @@ -108,7 +99,8 @@ fn test_merkleroot_ism_metadata() { #[test] fn test_merkle_root_multisig_module_type() { - let (merkleroot_ism, _) = setup_merkleroot_multisig_ism(array!['validator_1'].span()); + let threshold = 2; + let (merkleroot_ism, _) = setup_merkleroot_multisig_ism(array!['validator_1'].span(),threshold); assert( merkleroot_ism .module_type() == ModuleType::MERKLE_ROOT_MULTISIG(merkleroot_ism.contract_address), @@ -119,6 +111,7 @@ fn test_merkle_root_multisig_module_type() { #[test] fn test_merkle_root_multisig_verify_with_4_valid_signatures() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -136,7 +129,7 @@ fn test_merkle_root_multisig_verify_with_4_valid_signatures() { }; let (_, validators_address, _) = get_merkle_message_and_signature(); let (merkleroot_ism, merkleroot_validator_configuration) = setup_merkleroot_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let origin_merkle_tree_hook: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let message_index: u32 = 1; @@ -149,7 +142,6 @@ fn test_merkle_root_multisig_verify_with_4_valid_signatures() { contract_address: merkleroot_validator_configuration.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - merkleroot_validator_configuration.set_threshold(4); assert(merkleroot_ism.verify(metadata, message) == true, 'verification failed'); } @@ -157,6 +149,7 @@ fn test_merkle_root_multisig_verify_with_4_valid_signatures() { #[test] #[should_panic(expected: ('No match for given signature',))] fn test_merkle_root_multisig_verify_with_insufficient_valid_signatures() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -173,8 +166,8 @@ fn test_merkle_root_multisig_verify_with_insufficient_valid_signatures() { body: message_body.clone() }; let (_, validators_address, _) = get_merkle_message_and_signature(); - let (merkleroot_ism, merkleroot_validator_config) = setup_merkleroot_multisig_ism( - validators_address.span() + let (merkleroot_ism, _) = setup_merkleroot_multisig_ism( + validators_address.span(), threshold ); let origin_merkle_tree_hook: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let message_index: u32 = 1; @@ -186,7 +179,6 @@ fn test_merkle_root_multisig_verify_with_insufficient_valid_signatures() { metadata.update_at(1100, 0); let ownable = IOwnableDispatcher { contract_address: merkleroot_ism.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - merkleroot_validator_config.set_threshold(4); assert(merkleroot_ism.verify(metadata, message) == true, 'verification failed'); } @@ -194,6 +186,7 @@ fn test_merkle_root_multisig_verify_with_insufficient_valid_signatures() { #[test] #[should_panic(expected: ('Empty metadata',))] fn test_merkle_root_multisig_verify_with_empty_metadata() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -211,11 +204,10 @@ fn test_merkle_root_multisig_verify_with_empty_metadata() { }; let (_, validators_address, _) = get_merkle_message_and_signature(); let (merkle_root_ism, merkleroot_validator_config) = setup_merkleroot_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let ownable = IOwnableDispatcher { contract_address: merkle_root_ism.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - merkleroot_validator_config.set_threshold(4); let bytes_metadata = BytesTrait::new_empty(); assert(merkle_root_ism.verify(bytes_metadata, message) == true, 'verification failed'); } @@ -224,6 +216,7 @@ fn test_merkle_root_multisig_verify_with_empty_metadata() { #[test] #[should_panic(expected: ('No match for given signature',))] fn test_merkle_root_multisig_verify_with_4_valid_signatures_fails_if_duplicate_signatures() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -241,7 +234,7 @@ fn test_merkle_root_multisig_verify_with_4_valid_signatures_fails_if_duplicate_s }; let (_, validators_address, _) = get_merkle_message_and_signature(); let (merkleroot_ism, merkleroot_validator_configuration) = setup_merkleroot_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let origin_merkle_tree_hook: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let message_index: u32 = 1; @@ -254,6 +247,5 @@ fn test_merkle_root_multisig_verify_with_4_valid_signatures_fails_if_duplicate_s contract_address: merkleroot_validator_configuration.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - merkleroot_validator_configuration.set_threshold(4); assert(merkleroot_ism.verify(metadata, message) == true, 'verification failed'); } diff --git a/contracts/src/tests/isms/test_messageid_multisig.cairo b/contracts/src/tests/isms/test_messageid_multisig.cairo index 5851267..393ec8e 100644 --- a/contracts/src/tests/isms/test_messageid_multisig.cairo +++ b/contracts/src/tests/isms/test_messageid_multisig.cairo @@ -23,10 +23,11 @@ use snforge_std::{start_prank, CheatTarget}; #[test] fn test_set_validators() { + let threshold = 2; let new_validators: Array = array![ VALIDATOR_ADDRESS_1().into(), VALIDATOR_ADDRESS_2().into() ]; - let (_, validators) = setup_messageid_multisig_ism(new_validators.span()); + let (_, validators) = setup_messageid_multisig_ism(new_validators.span(), threshold); let ownable = IOwnableDispatcher { contract_address: validators.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); let validators_span = validators.get_validators(); @@ -36,33 +37,20 @@ fn test_set_validators() { #[test] fn test_set_threshold() { - let new_threshold = 3; - let (_, validators) = setup_messageid_multisig_ism(array!['validator_1'].span()); - let ownable = IOwnableDispatcher { contract_address: validators.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - validators.set_threshold(new_threshold); - assert(validators.get_threshold() == new_threshold, 'wrong validator threshold'); + let threshold = 3; + let (_, validators) = setup_messageid_multisig_ism(array!['validator_1'].span(), threshold); + assert(validators.get_threshold() == threshold, 'wrong validator threshold'); } #[test] #[should_panic] fn test_set_validators_fails_if_null_validator() { + let threshold = 2; let new_validators: Span = array![VALIDATOR_ADDRESS_1().try_into().unwrap(), 0].span(); - setup_messageid_multisig_ism(new_validators); + setup_messageid_multisig_ism(new_validators, threshold); } -#[test] -#[should_panic(expected: ('Caller is not the owner',))] -fn test_set_threshold_fails_if_caller_not_owner() { - let new_threshold = 3; - let (_, validators) = setup_messageid_multisig_ism(array!['validator_1'].span()); - let ownable = IOwnableDispatcher { contract_address: validators.contract_address }; - start_prank(CheatTarget::One(ownable.contract_address), NEW_OWNER()); - validators.set_threshold(new_threshold); -} - - #[test] fn test_message_id_ism_metadata() { let origin_merkle_tree: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); @@ -95,7 +83,8 @@ fn test_message_id_ism_metadata() { #[test] fn test_message_id_multisig_module_type() { - let (messageid, _) = setup_messageid_multisig_ism(array!['validator_1'].span()); + let threshold = 2; + let (messageid, _) = setup_messageid_multisig_ism(array!['validator_1'].span(),threshold); assert( messageid.module_type() == ModuleType::MESSAGE_ID_MULTISIG(messageid.contract_address), 'Wrong module type' @@ -105,6 +94,7 @@ fn test_message_id_multisig_module_type() { #[test] fn test_message_id_multisig_verify_with_4_valid_signatures() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -122,7 +112,7 @@ fn test_message_id_multisig_verify_with_4_valid_signatures() { }; let (_, validators_address, _) = get_message_and_signature(); let (messageid, messageid_validator_configuration) = setup_messageid_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let origin_merkle_tree: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let root: u256 = 'root'.try_into().unwrap(); @@ -132,7 +122,6 @@ fn test_message_id_multisig_verify_with_4_valid_signatures() { contract_address: messageid_validator_configuration.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - messageid_validator_configuration.set_threshold(4); assert(messageid.verify(metadata, message) == true, 'verification failed'); } @@ -140,6 +129,7 @@ fn test_message_id_multisig_verify_with_4_valid_signatures() { #[test] #[should_panic(expected: ('No match for given signature',))] fn test_message_id_multisig_verify_with_insufficient_valid_signatures() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -156,8 +146,8 @@ fn test_message_id_multisig_verify_with_insufficient_valid_signatures() { body: message_body.clone() }; let (_, validators_address, _) = get_message_and_signature(); - let (messageid, messageid_validator_config) = setup_messageid_multisig_ism( - validators_address.span() + let (messageid, _) = setup_messageid_multisig_ism( + validators_address.span(), threshold ); let origin_merkle_tree: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let root: u256 = 'root'.try_into().unwrap(); @@ -167,7 +157,6 @@ fn test_message_id_multisig_verify_with_insufficient_valid_signatures() { metadata.update_at(80, 0); let ownable = IOwnableDispatcher { contract_address: messageid.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - messageid_validator_config.set_threshold(4); assert(messageid.verify(metadata, message) == true, 'verification failed'); } @@ -175,6 +164,7 @@ fn test_message_id_multisig_verify_with_insufficient_valid_signatures() { #[test] #[should_panic(expected: ('Empty metadata',))] fn test_message_id_multisig_verify_with_empty_metadata() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -191,12 +181,11 @@ fn test_message_id_multisig_verify_with_empty_metadata() { body: message_body.clone() }; let (_, validators_address, _) = get_message_and_signature(); - let (messageid, messageid_validator_config) = setup_messageid_multisig_ism( - validators_address.span() + let (messageid, _) = setup_messageid_multisig_ism( + validators_address.span(), threshold ); let ownable = IOwnableDispatcher { contract_address: messageid.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - messageid_validator_config.set_threshold(4); let bytes_metadata = BytesTrait::new_empty(); assert(messageid.verify(bytes_metadata, message) == true, 'verification failed'); } @@ -205,6 +194,7 @@ fn test_message_id_multisig_verify_with_empty_metadata() { #[test] #[should_panic(expected: ('No match for given signature',))] fn test_message_id_multisig_verify_with_4_valid_signatures_fails_if_duplicate_signatures() { + let threshold = 4; let array = array![ 0x01020304050607080910111213141516, 0x01020304050607080910111213141516, @@ -222,7 +212,7 @@ fn test_message_id_multisig_verify_with_4_valid_signatures_fails_if_duplicate_si }; let (_, validators_address, _) = get_message_and_signature(); let (messageid, messageid_validator_configuration) = setup_messageid_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let origin_merkle_tree: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let root: u256 = 'root'.try_into().unwrap(); @@ -232,6 +222,5 @@ fn test_message_id_multisig_verify_with_4_valid_signatures_fails_if_duplicate_si contract_address: messageid_validator_configuration.contract_address }; start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - messageid_validator_configuration.set_threshold(4); assert(messageid.verify(metadata, message) == true, 'verification failed'); } diff --git a/contracts/src/tests/routing/test_default_fallback_routing_ism.cairo b/contracts/src/tests/routing/test_default_fallback_routing_ism.cairo index 41f7706..3e5b832 100644 --- a/contracts/src/tests/routing/test_default_fallback_routing_ism.cairo +++ b/contracts/src/tests/routing/test_default_fallback_routing_ism.cairo @@ -279,6 +279,7 @@ fn test_module_type() { // for this test, we will reuse existing tests #[test] fn test_verify() { + let threshold = 4; // ISM MESSAGE AND METADATA CONFIGURATION let array = array![ 0x01020304050607080910111213141516, @@ -297,20 +298,13 @@ fn test_verify() { }; let (_, validators_address, _) = get_message_and_signature(); let (messageid, messageid_validator_configuration) = setup_messageid_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let origin_merkle_tree: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let root: u256 = 'root'.try_into().unwrap(); let index = 1; let metadata = build_messageid_metadata(origin_merkle_tree, root, index); - // ISM CONFIGURATION - let ownable = IOwnableDispatcher { - contract_address: messageid_validator_configuration.contract_address - }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - messageid_validator_configuration.set_threshold(4); - // ROUTING TESTING let mut _domains = array![LOCAL_DOMAIN, 1123322, 312441]; let mut _modules: Array = array![ diff --git a/contracts/src/tests/routing/test_domain_routing_ism.cairo b/contracts/src/tests/routing/test_domain_routing_ism.cairo index f3ad8b2..769972e 100644 --- a/contracts/src/tests/routing/test_domain_routing_ism.cairo +++ b/contracts/src/tests/routing/test_domain_routing_ism.cairo @@ -281,6 +281,7 @@ fn test_module_type() { // for this test, we will reuse existing tests #[test] fn test_verify() { + let threshold = 4; // ISM MESSAGE AND METADATA CONFIGURATION let array = array![ 0x01020304050607080910111213141516, @@ -299,20 +300,13 @@ fn test_verify() { }; let (_, validators_address, _) = get_message_and_signature(); let (messageid, messageid_validator_configuration) = setup_messageid_multisig_ism( - validators_address.span() + validators_address.span(), threshold ); let origin_merkle_tree: u256 = 'origin_merkle_tree_hook'.try_into().unwrap(); let root: u256 = 'root'.try_into().unwrap(); let index = 1; let metadata = build_messageid_metadata(origin_merkle_tree, root, index); - // ISM CONFIGURATION - let ownable = IOwnableDispatcher { - contract_address: messageid_validator_configuration.contract_address - }; - start_prank(CheatTarget::One(ownable.contract_address), OWNER()); - messageid_validator_configuration.set_threshold(4); - // ROUTING TESTING let mut _domains = array![LOCAL_DOMAIN, 1123322, 312441]; let mut _modules: Array = array![ diff --git a/contracts/src/tests/setup.cairo b/contracts/src/tests/setup.cairo index 6071444..3faee89 100644 --- a/contracts/src/tests/setup.cairo +++ b/contracts/src/tests/setup.cairo @@ -207,12 +207,13 @@ pub fn setup_mock_ism() -> IInterchainSecurityModuleDispatcher { IInterchainSecurityModuleDispatcher { contract_address: mock_ism_addr } } pub fn setup_messageid_multisig_ism( - validators: Span + validators: Span, threshold: u32 ) -> (IInterchainSecurityModuleDispatcher, IValidatorConfigurationDispatcher) { let messageid_multisig_class = declare("messageid_multisig_ism").unwrap(); let mut parameters = Default::default(); Serde::serialize(@OWNER(), ref parameters); Serde::serialize(@validators, ref parameters); + Serde::serialize(@threshold, ref parameters); let (messageid_multisig_addr, _) = messageid_multisig_class.deploy(@parameters).unwrap(); ( IInterchainSecurityModuleDispatcher { contract_address: messageid_multisig_addr }, @@ -222,12 +223,13 @@ pub fn setup_messageid_multisig_ism( pub fn setup_merkleroot_multisig_ism( - validators: Span + validators: Span, threshold: u32 ) -> (IInterchainSecurityModuleDispatcher, IValidatorConfigurationDispatcher) { let merkleroot_multisig_class = declare("merkleroot_multisig_ism").unwrap(); let mut parameters = Default::default(); Serde::serialize(@OWNER(), ref parameters); Serde::serialize(@validators, ref parameters); + Serde::serialize(@threshold, ref parameters); let (merkleroot_multisig_addr, _) = merkleroot_multisig_class.deploy(@parameters).unwrap(); ( IInterchainSecurityModuleDispatcher { contract_address: merkleroot_multisig_addr }, @@ -295,11 +297,12 @@ pub fn setup_mock_validator_announce( IMockValidatorAnnounceDispatcher { contract_address: validator_announce_addr } } -pub fn setup_aggregation(modules: Span) -> IAggregationDispatcher { +pub fn setup_aggregation(modules: Span, threshold: u8) -> IAggregationDispatcher { let aggregation_class = declare("aggregation").unwrap(); let mut parameters = Default::default(); Serde::serialize(@OWNER(), ref parameters); Serde::serialize(@modules, ref parameters); + Serde::serialize(@threshold, ref parameters); let (aggregation_addr, _) = aggregation_class.deploy(@parameters).unwrap(); IAggregationDispatcher { contract_address: aggregation_addr } }