corrections + tests

astraly-labs · May 15, 2024 · 2a7764f · 2a7764f
1 parent e43980f
commit 2a7764f
Show file tree

Hide file tree

Showing 9 changed files with 331 additions and 54 deletions.
diff --git a/src/contracts/isms/multisig/merkleroot_multisig_ism.cairo b/src/contracts/isms/multisig/merkleroot_multisig_ism.cairo
@@ -51,13 +51,13 @@ pub mod merkleroot_multisig_ism {
                 }
                 let (signature_r, signature_s) = get_signature_at(_metadata.clone(), i);
 
-                // we loop on the validators list public kew in order to find a match
+                // we loop on the validators list public key in order to find a match
                 let mut cur_idx = 0;
                 let is_signer_in_list = loop {
                     if (cur_idx == validators.len()) {
                         break false;
                     }
-                    let signer = *validators.at(cur_idx).address;
+                    let signer = *validators.at(cur_idx);
                     if check_ecdsa_signature(
                         digest, signer.try_into().unwrap(), signature_r, signature_s
                     ) {

diff --git a/src/contracts/isms/multisig/messageid_multisig_ism.cairo b/src/contracts/isms/multisig/messageid_multisig_ism.cairo
@@ -19,7 +19,7 @@ pub mod messageid_multisig_ism {
 
     mod Errors {
         pub const NO_MULTISIG_THRESHOLD_FOR_MESSAGE: felt252 = 'No MultisigISM treshold present';
-        pub const VERIFICATION_FAILED_THRESHOLD_NOT_REACHED: felt252 = 'Verify failed, < threshold';
+        pub const NO_MATCH_FOR_SIGNATURE: felt252 = 'No match for given signature';
         pub const EMPTY_METADATA: felt252 = 'Empty metadata';
     }
     #[abi(embed_v0)]
@@ -42,8 +42,6 @@ pub mod messageid_multisig_ism {
             let (validators, threshold) = validator_configuration
                 .validators_and_threshold(_message);
             assert(threshold > 0, Errors::NO_MULTISIG_THRESHOLD_FOR_MESSAGE);
-            let validator_count = validators.len();
-            let mut unmatched_signatures = 0;
             let mut matched_signatures = 0;
             let mut i = 0;
 
@@ -53,41 +51,28 @@ pub mod messageid_multisig_ism {
                     break ();
                 }
                 let signature = get_signature_at(_metadata.clone(), i);
-
-                // we loop on the validators list public kew in order to find a match
+                // we loop on the validators list public key in order to find a match
                 let mut cur_idx = 0;
                 let is_signer_in_list = loop {
                     if (cur_idx == validators.len()) {
                         break false;
                     }
-                    let signer = *validators.at(cur_idx).address;
-                    if bool_is_eth_signature_valid(
-                        digest.into(), signature, signer.try_into().unwrap()
-                    ) {
+                    let signer = *validators.at(cur_idx);
+                    if bool_is_eth_signature_valid(digest, signature, signer) {
                         // we found a match
                         break true;
                     }
                     cur_idx += 1;
                 };
-                if (!is_signer_in_list) {
-                    unmatched_signatures += 1;
-                } else {
-                    matched_signatures += 1;
-                }
-                assert(
-                    unmatched_signatures < validator_count - threshold,
-                    Errors::VERIFICATION_FAILED_THRESHOLD_NOT_REACHED
-                );
+                assert(is_signer_in_list, Errors::NO_MATCH_FOR_SIGNATURE);
                 i += 1;
             };
-            assert(
-                matched_signatures >= threshold, Errors::VERIFICATION_FAILED_THRESHOLD_NOT_REACHED
-            );
+            println!("matched_signatures: {}", matched_signatures);
             true
         }
     }
 
-    fn digest(_metadata: Bytes, _message: Message) -> felt252 {
+    fn digest(_metadata: Bytes, _message: Message) -> u256 {
         let origin_merkle_tree_hook = MessageIdIsmMetadata::origin_merkle_tree_hook(
             _metadata.clone()
         );
@@ -100,8 +85,6 @@ pub mod messageid_multisig_ism {
             index,
             MessageTrait::format_message(_message)
         )
-            .try_into()
-            .unwrap()
     }
 
     fn get_signature_at(_metadata: Bytes, _index: u32) -> Signature {

diff --git a/src/contracts/isms/multisig/multisig_ism.cairo b/src/contracts/isms/multisig/multisig_ism.cairo
@@ -15,15 +15,10 @@ pub mod multisig_ism {
     impl UpgradeableInternalImpl = UpgradeableComponent::InternalImpl<ContractState>;
     type Index = felt252;
 
-    #[derive(Serde, Copy, starknet::Store, Drop, PartialEq)]
-    pub struct ValidatorInformations {
-        pub address: EthAddress,
-        pub public_key: u256
-    }
 
     #[storage]
     struct Storage {
-        validators: LegacyMap<u32, ValidatorInformations>,
+        validators: LegacyMap<u32, EthAddress>,
         threshold: u32,
         #[substorage(v0)]
         ownable: OwnableComponent::Storage,
@@ -48,20 +43,19 @@ pub mod multisig_ism {
 
     mod Errors {
         pub const VALIDATOR_ADDRESS_CANNOT_BE_NULL: felt252 = 'Validator address cannot be 0';
-        pub const VALIDATOR_PUBLIC_KEY_CANNOT_BE_NULL: felt252 = 'Validator pk cannot be 0';
     }
 
     #[abi(embed_v0)]
     impl IMultisigIsmImpl of IMultisigIsm<ContractState> {
-        fn get_validators(self: @ContractState) -> Span<ValidatorInformations> {
+        fn get_validators(self: @ContractState) -> Span<EthAddress> {
             build_validators_span(self)
         }
 
         fn get_threshold(self: @ContractState) -> u32 {
             self.threshold.read()
         }
 
-        fn set_validators(ref self: ContractState, _validators: Span<ValidatorInformations>) {
+        fn set_validators(ref self: ContractState, _validators: Span<EthAddress>) {
             self.ownable.assert_only_owner();
             let mut cur_idx = 0;
 
@@ -71,10 +65,8 @@ pub mod multisig_ism {
                 }
                 let validator = *_validators.at(cur_idx);
                 assert(
-                    validator.address != 0.try_into().unwrap(),
-                    Errors::VALIDATOR_ADDRESS_CANNOT_BE_NULL
+                    validator != 0.try_into().unwrap(), Errors::VALIDATOR_ADDRESS_CANNOT_BE_NULL
                 );
-                // assert(validator.public_key != 0, Errors::VALIDATOR_PUBLIC_KEY_CANNOT_BE_NULL);
                 self.validators.write(cur_idx.into(), validator);
                 cur_idx += 1;
             }
@@ -87,20 +79,20 @@ pub mod multisig_ism {
 
         fn validators_and_threshold(
             self: @ContractState, _message: Message
-        ) -> (Span<ValidatorInformations>, u32) {
+        ) -> (Span<EthAddress>, u32) {
             // USER CONTRACT DEFINITION HERE
             // USER CAN SPECIFY VALIDATORS SELECTION CONDITIONS
             let threshold = self.threshold.read();
             (build_validators_span(self), threshold)
         }
     }
 
-    fn build_validators_span(self: @ContractState) -> Span<ValidatorInformations> {
+    fn build_validators_span(self: @ContractState) -> Span<EthAddress> {
         let mut validators = ArrayTrait::new();
         let mut cur_idx = 0;
         loop {
             let validator = self.validators.read(cur_idx);
-            if (validator.address == 0.try_into().unwrap()) {
+            if (validator == 0.try_into().unwrap()) {
                 break ();
             }
             validators.append(validator);

diff --git a/src/contracts/isms/multisig/validator_announce.cairo b/src/contracts/isms/multisig/validator_announce.cairo
@@ -130,10 +130,10 @@ pub mod validator_announce {
         let mailboxclient = IMailboxClientDispatcher {
             contract_address: self.mailboxclient.read()
         };
-        let mailboxcient_address: felt252 = self.mailboxclient.read().try_into().unwrap();
+        let mailboxclient_address: felt252 = self.mailboxclient.read().try_into().unwrap();
         let mut input: Array<u256> = array![
             mailboxclient.get_local_domain().into(),
-            mailboxcient_address.try_into().unwrap(),
+            mailboxclient_address.try_into().unwrap(),
             HYPERLANE_ANNOUNCEMENT.into()
         ];
         let hash = keccak_u256s_be_inputs(input.span());

diff --git a/src/contracts/libs/multisig/message_id_ism_metadata.cairo b/src/contracts/libs/multisig/message_id_ism_metadata.cairo
@@ -11,7 +11,7 @@ pub mod message_id_ism_metadata {
     pub const ORIGIN_MERKLE_TREE_HOOK_OFFSET: u32 = 0;
     pub const ROOT_OFFSET: u32 = 32;
     pub const INDEX_OFFSET: u32 = 64;
-    pub const SIGNATURE_OFFSET: u32 = 96;
+    pub const SIGNATURE_OFFSET: u32 = 80;
     impl MessagIdIsmMetadataImpl of MessageIdIsmMetadata {
         fn origin_merkle_tree_hook(_metadata: Bytes) -> u256 {
             let (_, felt) = _metadata.read_u256(ORIGIN_MERKLE_TREE_HOOK_OFFSET);
@@ -30,9 +30,9 @@ pub mod message_id_ism_metadata {
 
         fn signature_at(_metadata: Bytes, _index: u32) -> (u8, u256, u256) {
             // the first signer index is 0
-            let (_, r) = _metadata.read_u256(SIGNATURE_OFFSET + 96 * _index);
-            let (_, s) = _metadata.read_u256(SIGNATURE_OFFSET + 96 * _index);
-            let (_, v) = _metadata.read_u8(SIGNATURE_OFFSET + 96 * _index + 32);
+            let (index_r, r) = _metadata.read_u256(SIGNATURE_OFFSET + 80 * _index);
+            let (index_s, s) = _metadata.read_u256(index_r);
+            let (_, v) = _metadata.read_u8(index_s);
             (v, r, s)
         }
     }

diff --git a/src/interfaces.cairo b/src/interfaces.cairo
@@ -1,6 +1,5 @@
 use alexandria_bytes::Bytes;
 use core::array::ArrayTrait;
-use hyperlane_starknet::contracts::isms::multisig::multisig_ism::multisig_ism::ValidatorInformations;
 use hyperlane_starknet::contracts::libs::message::Message;
 use starknet::ContractAddress;
 use starknet::EthAddress;
@@ -222,13 +221,13 @@ pub trait IRouter<TContractState> {
 pub trait IMultisigIsm<TContractState> {
     fn validators_and_threshold(
         self: @TContractState, _message: Message
-    ) -> (Span<ValidatorInformations>, u32);
+    ) -> (Span<EthAddress>, u32);
 
-    fn get_validators(self: @TContractState) -> Span<ValidatorInformations>;
+    fn get_validators(self: @TContractState) -> Span<EthAddress>;
 
     fn get_threshold(self: @TContractState) -> u32;
 
-    fn set_validators(ref self: TContractState, _validators: Span<ValidatorInformations>);
+    fn set_validators(ref self: TContractState, _validators: Span<EthAddress>);
 
     fn set_threshold(ref self: TContractState, _threshold: u32);
 }

diff --git a/src/lib.cairo b/src/lib.cairo
@@ -37,5 +37,5 @@ mod utils {
 mod tests {
     pub mod setup;
     pub mod test_mailbox;
-// pub mod test_multisig;
+    pub mod test_multisig;
 }
diff --git a/src/tests/setup.cairo b/src/tests/setup.cairo
@@ -2,13 +2,17 @@ use core::result::ResultTrait;
 use hyperlane_starknet::contracts::mocks::message_recipient::message_recipient;
 use hyperlane_starknet::interfaces::{
     IMailboxDispatcher, IMailboxDispatcherTrait, IMessageRecipientDispatcher,
-    IMessageRecipientDispatcherTrait
+    IMessageRecipientDispatcherTrait, IInterchainSecurityModule,
+    IInterchainSecurityModuleDispatcher, IInterchainSecurityModuleDispatcherTrait,
+    IMultisigIsmDispatcher, IMultisigIsmDispatcherTrait, IValidatorAnnounceDispatcher,
+    IValidatorAnnounceDispatcherTrait, IMailboxClientDispatcher, IMailboxClientDispatcherTrait
 };
 use snforge_std::{
     declare, ContractClassTrait, CheatTarget, EventSpy, EventAssertions, spy_events, SpyOn
 };
+use starknet::secp256_trait::Signature;
 
-use starknet::{ContractAddress, contract_address_const};
+use starknet::{ContractAddress, contract_address_const, EthAddress};
 
 pub const LOCAL_DOMAIN: u32 = 534352;
 pub const DESTINATION_DOMAIN: u32 = 9841001;
@@ -49,6 +53,14 @@ pub fn RECIPIENT_ADDRESS() -> ContractAddress {
     contract_address_const::<'RECIPIENT_ADDRESS'>()
 }
 
+pub fn VALIDATOR_ADDRESS() -> EthAddress {
+    'VALIDATOR_ADDRESS'.try_into().unwrap()
+}
+
+pub fn VALIDATOR_PUBLIC_KEY() -> u256 {
+    'VALIDATOR_PUBLIC_KEY'
+}
+
 pub fn setup() -> (IMailboxDispatcher, EventSpy) {
     let mailbox_class = declare("mailbox").unwrap();
     let (mailbox_addr, _) = mailbox_class
@@ -64,3 +76,77 @@ pub fn mock_setup() -> IMessageRecipientDispatcher {
     let (message_recipient_addr, _) = message_recipient_class.deploy(@array![]).unwrap();
     IMessageRecipientDispatcher { contract_address: message_recipient_addr }
 }
+
+pub fn setup_messageid_multisig_ism() -> IInterchainSecurityModuleDispatcher {
+    let messageid_multisig_class = declare("messageid_multisig_ism").unwrap();
+
+    let (messageid_multisig_addr, _) = messageid_multisig_class.deploy(@array![]).unwrap();
+    IInterchainSecurityModuleDispatcher { contract_address: messageid_multisig_addr }
+}
+
+pub fn setup_multisig_ism() -> IMultisigIsmDispatcher {
+    let multisig_ism_class = declare("multisig_ism").unwrap();
+    let (multisig_ism_addr, _) = multisig_ism_class.deploy(@array![OWNER().into()]).unwrap();
+    IMultisigIsmDispatcher { contract_address: multisig_ism_addr }
+}
+
+pub fn setup_mailbox_client() -> IMailboxClientDispatcher {
+    let (mailbox, _) = setup();
+    let mailboxclient_class = declare("mailboxclient").unwrap();
+    let (mailboxclient_addr, _) = mailboxclient_class
+        .deploy(@array![mailbox.contract_address.into(), OWNER().into()])
+        .unwrap();
+    IMailboxClientDispatcher { contract_address: mailboxclient_addr }
+}
+
+
+pub fn setup_validator_announce() -> IValidatorAnnounceDispatcher {
+    let validator_announce_class = declare("validator_announce").unwrap();
+    let mailboxclient = setup_mailbox_client();
+    let (validator_announce_addr, _) = validator_announce_class
+        .deploy(@array![mailboxclient.contract_address.into()])
+        .unwrap();
+    IValidatorAnnounceDispatcher { contract_address: validator_announce_addr }
+}
+
+
+// Configuration from the main cairo repo: https://github.com/starkware-libs/cairo/blob/main/corelib/src/test/secp256k1_test.cairo
+pub fn get_message_and_signature(y_parity: bool) -> (u256, Array<EthAddress>, Array<Signature>) {
+    let msg_hash = 0xfbff8940be2153ce000c0e1933bf32e179c60f53c45f56b4ac84b2e90f1f6214;
+    let validators_array: Array<EthAddress> = array![
+        0x2cb1a91F2F23D6eC7FD22d2f7996f55B71EB32dc.try_into().unwrap(),
+        0x0fb1A81BcefDEc06154279219F227938D00B1c12.try_into().unwrap(),
+        0xF650b555CFDEfF61d225058e26326266E69660c2.try_into().unwrap(),
+        0x03aC66d13dc1B5b10fc363fC32f324ca947CDac1.try_into().unwrap(),
+        0x5711B186cdCAFD9E7aa1f78c0A0c30d3C7A2Af77.try_into().unwrap()
+    ];
+    let signatures = array![
+        Signature {
+            r: 0xb994fec0137776002d05dcf847bbba338285f1210c9ca7829109578ac876519f,
+            s: 0x0a42bb91f22ef042ca82fdcf8c8a5846e0debbce509dc2a0ce28a988dcbe4a16,
+            y_parity
+        },
+        Signature {
+            r: 0xf81a5dd3f871ad2d27a3b538e73663d723f8263fb3d289514346d43d000175f5,
+            s: 0x083df770623e9ae52a7bb154473961e24664bb003bdfdba6100fb5e540875ce1,
+            y_parity
+        },
+        Signature {
+            r: 0x76b194f951f94492ca582dab63dc413b9ac1ca9992c22bc2186439e9ab8fdd3c,
+            s: 0x62a6a6f402edaa53e9bdc715070a61edb0d98d4e14e182f60bdd4ae932b40b29,
+            y_parity
+        },
+        Signature {
+            r: 0x35932eefd85897d868aaacd4ba7aee81a2384e42ba062133f6d37fdfebf94ad4,
+            s: 0x78cce49db96ee27c3f461800388ac95101476605baa64a194b7dd4d56d2d4a4d,
+            y_parity
+        },
+        Signature {
+            r: 0x6b38d4353d69396e91c57542254348d16459d448ab887574e9476a6ff76d49a1,
+            s: 0x3527627295bde423d7d799afef22affac4f00c70a5b651ad14c8879aeb9b6e03,
+            y_parity
+        }
+    ];
+
+    (msg_hash, validators_array, signatures)
+}