refactor(rust/signed-doc): Move validate_signatures as a separate validation rule

.config/dictionaries/project.dic

@@ -192,6 +192,7 @@ MPMC
 msvc
 Multiaddr
 multiera
+mutlisig
 mypy
 nanos
 netkey

rust/signed_doc/src/providers.rs

@@ -8,12 +8,12 @@ use ed25519_dalek::VerifyingKey;
 use crate::{CatalystSignedDocument, DocumentRef};
 
 /// `VerifyingKey` Provider trait
-pub trait VerifyingKeyProvider {
+pub trait VerifyingKeyProvider: Send + Sync {
     /// Try to get `VerifyingKey`
     fn try_get_key(
         &self,
         kid: &CatalystId,
-    ) -> impl Future<Output = anyhow::Result<Option<VerifyingKey>>>;
+    ) -> impl Future<Output = anyhow::Result<Option<VerifyingKey>>> + Send;
 }
 
 /// `CatalystSignedDocument` Provider trait
@@ -53,11 +53,16 @@ pub mod tests {
     };
     use crate::{DocLocator, DocumentRef};
 
-    ///  Simple testing implementation of `CatalystSignedDocumentProvider`
+    /// Simple testing implementation of `CatalystSignedDocumentProvider`,
     #[derive(Default, Debug)]
-    pub struct TestCatalystSignedDocumentProvider(HashMap<DocumentRef, CatalystSignedDocument>);
+    pub struct TestCatalystProvider {
+        /// For `CatalystSignedDocumentProvider`.
+        signed_doc: HashMap<DocumentRef, CatalystSignedDocument>,
+        /// For `VerifyingKeyProvider`.
+        verifying_key: HashMap<CatalystId, VerifyingKey>,
+    }
 
-    impl TestCatalystSignedDocumentProvider {
+    impl TestCatalystProvider {
         /// Inserts document into the `TestCatalystSignedDocumentProvider` where
         /// if document reference is provided use that value.
         /// if not use the id and version of the provided doc.
@@ -71,29 +76,38 @@ pub mod tests {
             doc: &CatalystSignedDocument,
         ) -> anyhow::Result<()> {
             if let Some(dr) = doc_ref {
-                self.0.insert(dr, doc.clone());
+                self.signed_doc.insert(dr, doc.clone());
             } else {
                 let dr = DocumentRef::new(doc.doc_id()?, doc.doc_ver()?, DocLocator::default());
-                self.0.insert(dr, doc.clone());
+                self.signed_doc.insert(dr, doc.clone());
             }
             Ok(())
         }
+
+        /// Inserts public key into the `TestVerifyingKeyProvider`
+        pub fn add_pk(
+            &mut self,
+            kid: CatalystId,
+            pk: VerifyingKey,
+        ) {
+            self.verifying_key.insert(kid, pk);
+        }
     }
 
-    impl CatalystSignedDocumentProvider for TestCatalystSignedDocumentProvider {
+    impl CatalystSignedDocumentProvider for TestCatalystProvider {
         async fn try_get_doc(
             &self,
             doc_ref: &DocumentRef,
         ) -> anyhow::Result<Option<CatalystSignedDocument>> {
-            Ok(self.0.get(doc_ref).cloned())
+            Ok(self.signed_doc.get(doc_ref).cloned())
         }
 
         async fn try_get_last_doc(
             &self,
             id: catalyst_types::uuid::UuidV7,
         ) -> anyhow::Result<Option<CatalystSignedDocument>> {
             Ok(self
-                .0
+                .signed_doc
                 .iter()
                 .filter(|(doc_ref, _)| doc_ref.id() == &id)
                 .max_by_key(|(doc_ref, _)| doc_ref.ver().uuid())
@@ -109,27 +123,12 @@ pub mod tests {
         }
     }
 
-    /// Simple testing implementation of `VerifyingKeyProvider`
-    #[derive(Default)]
-    pub struct TestVerifyingKeyProvider(HashMap<CatalystId, VerifyingKey>);
-
-    impl TestVerifyingKeyProvider {
-        /// Inserts public key into the `TestVerifyingKeyProvider`
-        pub fn add_pk(
-            &mut self,
-            kid: CatalystId,
-            pk: VerifyingKey,
-        ) {
-            self.0.insert(kid, pk);
-        }
-    }
-
-    impl VerifyingKeyProvider for TestVerifyingKeyProvider {
+    impl VerifyingKeyProvider for TestCatalystProvider {
         async fn try_get_key(
             &self,
             kid: &CatalystId,
         ) -> anyhow::Result<Option<VerifyingKey>> {
-            Ok(self.0.get(kid).copied())
+            Ok(self.verifying_key.get(kid).copied())
         }
     }
 }

rust/signed_doc/src/validator/mod.rs

@@ -8,8 +8,7 @@ use std::{
     sync::{Arc, LazyLock},
 };
 
-use anyhow::Context;
-use catalyst_types::{catalyst_id::role_index::RoleId, problem_report::ProblemReport};
+use catalyst_types::catalyst_id::role_index::RoleId;
 use rules::{
     ContentEncodingRule, ContentRule, ContentSchema, ContentTypeRule, IdRule, ParametersRule,
     RefRule, ReplyRule, Rules, SectionRule, SignatureKidRule, VerRule,
@@ -22,7 +21,7 @@ use crate::{
     },
     metadata::DocType,
     providers::{CatalystSignedDocumentProvider, VerifyingKeyProvider},
-    signature::{tbs_data, Signature},
+    validator::rules::SignatureRule,
     CatalystSignedDocument, ContentEncoding, ContentType,
 };
 
@@ -62,6 +61,7 @@ fn proposal_rule() -> Rules {
         kid: SignatureKidRule {
             exp: &[RoleId::Proposer],
         },
+        signature: SignatureRule { mutlisig: false },
     }
 }
 
@@ -104,6 +104,7 @@ fn proposal_comment_rule() -> Rules {
         kid: SignatureKidRule {
             exp: &[RoleId::Role0],
         },
+        signature: SignatureRule { mutlisig: false },
     }
 }
 
@@ -152,6 +153,7 @@ fn proposal_submission_action_rule() -> Rules {
         kid: SignatureKidRule {
             exp: &[RoleId::Proposer],
         },
+        signature: SignatureRule { mutlisig: false },
     }
 }
 
@@ -185,7 +187,7 @@ pub async fn validate<Provider>(
     provider: &Provider,
 ) -> anyhow::Result<bool>
 where
-    Provider: CatalystSignedDocumentProvider,
+    Provider: CatalystSignedDocumentProvider + VerifyingKeyProvider,
 {
     let Ok(doc_type) = doc.doc_type() else {
         doc.report().missing_field(
@@ -207,82 +209,6 @@ where
     rules.check(doc, provider).await
 }
 
-/// Verify document signatures.
-/// Return true if all signatures are valid, otherwise return false.
-///
-/// # Errors
-/// If `provider` returns error, fails fast throwing that error.
-pub async fn validate_signatures(
-    doc: &CatalystSignedDocument,
-    provider: &impl VerifyingKeyProvider,
-) -> anyhow::Result<bool> {
-    if doc.signatures().is_empty() {
-        doc.report().other(
-            "Catalyst Signed Document is unsigned",
-            "During Catalyst Signed Document signature validation",
-        );
-        return Ok(false);
-    }
-
-    let sign_rules = doc
-        .signatures()
-        .iter()
-        .map(|sign| validate_signature(doc, sign, provider, doc.report()));
-
-    let res = futures::future::join_all(sign_rules)
-        .await
-        .into_iter()
-        .collect::<anyhow::Result<Vec<_>>>()?
-        .iter()
-        .all(|res| *res);
-
-    Ok(res)
-}
-
-/// A single signature validation function
-async fn validate_signature<Provider>(
-    doc: &CatalystSignedDocument,
-    sign: &Signature,
-    provider: &Provider,
-    report: &ProblemReport,
-) -> anyhow::Result<bool>
-where
-    Provider: VerifyingKeyProvider,
-{
-    let kid = sign.kid();
-
-    let Some(pk) = provider.try_get_key(kid).await? else {
-        report.other(
-            &format!("Missing public key for {kid}."),
-            "During public key extraction",
-        );
-        return Ok(false);
-    };
-
-    let tbs_data = tbs_data(kid, doc.doc_meta(), doc.content()).context("Probably a bug, cannot build CBOR COSE bytes for signature verification from the structurally valid COSE object.")?;
-
-    let Ok(signature_bytes) = sign.signature().try_into() else {
-        report.invalid_value(
-            "cose signature",
-            &format!("{}", sign.signature().len()),
-            &format!("must be {}", ed25519_dalek::Signature::BYTE_SIZE),
-            "During encoding cose signature to bytes",
-        );
-        return Ok(false);
-    };
-
-    let signature = ed25519_dalek::Signature::from_bytes(signature_bytes);
-    if pk.verify_strict(&tbs_data, &signature).is_err() {
-        report.functional_validation(
-            &format!("Verification failed for signature with Key ID {kid}"),
-            "During signature validation with verifying key",
-        );
-        return Ok(false);
-    }
-
-    Ok(true)
-}
-
 #[cfg(test)]
 mod tests {
     use crate::validator::document_rules_init;

rust/signed_doc/src/validator/rules/doc_ref.rs

@@ -188,8 +188,8 @@ mod tests {
 
     use super::*;
     use crate::{
-        builder::tests::Builder, metadata::SupportedField,
-        providers::tests::TestCatalystSignedDocumentProvider, DocLocator, DocumentRef,
+        builder::tests::Builder, metadata::SupportedField, providers::tests::TestCatalystProvider,
+        DocLocator, DocumentRef,
     };
 
     #[test_case(
@@ -397,12 +397,9 @@ mod tests {
     )]
     #[tokio::test]
     async fn ref_specified_test(
-        doc_gen: impl FnOnce(
-            &[DocType; 2],
-            &mut TestCatalystSignedDocumentProvider,
-        ) -> CatalystSignedDocument
+        doc_gen: impl FnOnce(&[DocType; 2], &mut TestCatalystProvider) -> CatalystSignedDocument
     ) -> bool {
-        let mut provider = TestCatalystSignedDocumentProvider::default();
+        let mut provider = TestCatalystProvider::default();
 
         let exp_types: [DocType; 2] = [UuidV4::new().into(), UuidV4::new().into()];
 
@@ -430,7 +427,7 @@ mod tests {
 
     #[tokio::test]
     async fn ref_specified_optional_test() {
-        let provider = TestCatalystSignedDocumentProvider::default();
+        let provider = TestCatalystProvider::default();
         let rule = RefRule::Specified {
             exp_ref_types: vec![UuidV4::new().into()],
             optional: true,
@@ -439,7 +436,7 @@ mod tests {
         let doc = Builder::new().build();
         assert!(rule.check(&doc, &provider).await.unwrap());
 
-        let provider = TestCatalystSignedDocumentProvider::default();
+        let provider = TestCatalystProvider::default();
         let rule = RefRule::Specified {
             exp_ref_types: vec![UuidV4::new().into()],
             optional: false,
@@ -452,7 +449,7 @@ mod tests {
     #[tokio::test]
     async fn ref_rule_not_specified_test() {
         let rule = RefRule::NotSpecified;
-        let provider = TestCatalystSignedDocumentProvider::default();
+        let provider = TestCatalystProvider::default();
 
         let doc = Builder::new().build();
         assert!(rule.check(&doc, &provider).await.unwrap());

rust/signed_doc/src/validator/rules/id.rs

@@ -101,8 +101,8 @@ mod tests {
 
     use super::*;
     use crate::{
-        builder::tests::Builder, metadata::SupportedField,
-        providers::tests::TestCatalystSignedDocumentProvider, UuidV7,
+        builder::tests::Builder, metadata::SupportedField, providers::tests::TestCatalystProvider,
+        UuidV7,
     };
 
     #[test_case(
@@ -170,9 +170,9 @@ mod tests {
     )]
     #[tokio::test]
     async fn id_test(
-        doc_gen: impl FnOnce(&TestCatalystSignedDocumentProvider) -> CatalystSignedDocument
+        doc_gen: impl FnOnce(&TestCatalystProvider) -> CatalystSignedDocument
     ) -> bool {
-        let provider = TestCatalystSignedDocumentProvider::default();
+        let provider = TestCatalystProvider::default();
         let doc = doc_gen(&provider);
 
         IdRule.check(&doc, &provider).await.unwrap()

rust/signed_doc/src/validator/rules/mod.rs

@@ -3,7 +3,10 @@
 
 use futures::FutureExt;
 
-use crate::{providers::CatalystSignedDocumentProvider, CatalystSignedDocument};
+use crate::{
+    providers::{CatalystSignedDocumentProvider, VerifyingKeyProvider},
+    CatalystSignedDocument,
+};
 
 mod content_encoding;
 mod content_type;
@@ -12,6 +15,7 @@ mod id;
 mod parameters;
 mod reply;
 mod section;
+mod signature;
 mod signature_kid;
 mod template;
 mod ver;
@@ -23,6 +27,7 @@ pub(crate) use id::IdRule;
 pub(crate) use parameters::ParametersRule;
 pub(crate) use reply::ReplyRule;
 pub(crate) use section::SectionRule;
+pub(crate) use signature::SignatureRule;
 pub(crate) use signature_kid::SignatureKidRule;
 pub(crate) use template::{ContentRule, ContentSchema};
 pub(crate) use ver::VerRule;
@@ -49,6 +54,8 @@ pub(crate) struct Rules {
     pub(crate) parameters: ParametersRule,
     /// `kid` field validation rule
     pub(crate) kid: SignatureKidRule,
+    /// document's signatures validation rule
+    pub(crate) signature: SignatureRule,
 }
 
 impl Rules {
@@ -59,7 +66,7 @@ impl Rules {
         provider: &Provider,
     ) -> anyhow::Result<bool>
     where
-        Provider: CatalystSignedDocumentProvider,
+        Provider: CatalystSignedDocumentProvider + VerifyingKeyProvider,
     {
         let rules = [
             self.id.check(doc, provider).boxed(),
@@ -72,6 +79,7 @@ impl Rules {
             self.section.check(doc).boxed(),
             self.parameters.check(doc, provider).boxed(),
             self.kid.check(doc).boxed(),
+            self.signature.check(doc, provider).boxed(),
         ];
 
         let res = futures::future::join_all(rules)
@@ -80,6 +88,7 @@ impl Rules {
             .collect::<anyhow::Result<Vec<_>>>()?
             .iter()
             .all(|res| *res);
+
         Ok(res)
     }
 }