chore(rust/signed-doc): Cleanup Catalyst Signed Document Builder, make it type safe, add special test builder (2). (#380)

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* fix

* fix

* fix clippy

Author: Mr-Leshiy

rust/signed_doc/bins/mk_signed_doc.rs

@@ -63,13 +63,11 @@ impl Cli {
                 println!("{metadata}");
                 // Load Document from JSON file
                 let json_doc: serde_json::Value = load_json_from_file(&doc)?;
-                // Possibly encode if Metadata has an encoding set.
-                let payload = serde_json::to_vec(&json_doc)?;
                 // Start with no signatures.
                 let signed_doc = Builder::new()
                     .with_json_metadata(metadata)?
-                    .with_decoded_content(payload)?
-                    .build();
+                    .with_json_content(&json_doc)?
+                    .build()?;
                 println!(
                     "report {}",
                     serde_json::to_string(&signed_doc.problem_report())?
@@ -87,7 +85,7 @@ impl Cli {
                         |message| sk.sign::<()>(&message).to_bytes().to_vec(),
                         kid.clone(),
                     )?
-                    .build();
+                    .build()?;
                 save_signed_doc(new_signed_doc, &doc)?;
             },
             Self::Inspect { path } => {

rust/signed_doc/src/builder.rs

@@ -1,14 +1,29 @@
 //! Catalyst Signed Document Builder.
-use catalyst_types::{catalyst_id::CatalystId, problem_report::ProblemReport};
+use catalyst_types::catalyst_id::CatalystId;
 
 use crate::{
     signature::{tbs_data, Signature},
-    CatalystSignedDocument, Content, Metadata, Signatures,
+    CatalystSignedDocument, Content, ContentType, Metadata, Signatures,
 };
 
 /// Catalyst Signed Document Builder.
-#[derive(Debug)]
-pub struct Builder {
+/// Its a type sage state machine which iterates type safely during different stages of
+/// the Catalyst Signed Document build process:
+/// Setting Metadata -> Setting Content -> Setting Signatures
+pub type Builder = MetadataBuilder;
+
+/// Only `metadata` builder part
+pub struct MetadataBuilder(BuilderInner);
+
+/// Only `content` builder part
+pub struct ContentBuilder(BuilderInner);
+
+/// Only `Signatures` builder part
+pub struct SignaturesBuilder(BuilderInner);
+
+/// Inner state of the Catalyst Signed Documents `Builder`
+#[derive(Default)]
+pub struct BuilderInner {
     /// metadata
     metadata: Metadata,
     /// content
@@ -17,46 +32,57 @@ pub struct Builder {
     signatures: Signatures,
 }
 
-impl Default for Builder {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
-impl Builder {
+impl MetadataBuilder {
     /// Start building a signed document
     #[must_use]
+    #[allow(clippy::new_without_default)]
     pub fn new() -> Self {
-        Self {
-            metadata: Metadata::default(),
-            content: Content::default(),
-            signatures: Signatures::default(),
-        }
+        Self(BuilderInner::default())
     }
 
     /// Set document metadata in JSON format
     /// Collect problem report if some fields are missing.
     ///
     /// # Errors
     /// - Fails if it is invalid metadata fields JSON object.
-    pub fn with_json_metadata(mut self, json: serde_json::Value) -> anyhow::Result<Self> {
-        self.metadata = Metadata::from_json(json, &ProblemReport::new(""));
-        Ok(self)
+    pub fn with_json_metadata(mut self, json: serde_json::Value) -> anyhow::Result<ContentBuilder> {
+        self.0.metadata = Metadata::from_json(json)?;
+        Ok(ContentBuilder(self.0))
+    }
+}
+
+impl ContentBuilder {
+    /// Sets an empty content
+    pub fn empty_content(self) -> SignaturesBuilder {
+        SignaturesBuilder(self.0)
     }
 
-    /// Set decoded (original) document content bytes
+    /// Set the provided JSON content, applying already set `content-encoding`.
     ///
     /// # Errors
+    ///  - Verifies that `content-type` field is set to JSON
+    ///  - Cannot serialize provided JSON
     ///  - Compression failure
-    pub fn with_decoded_content(mut self, decoded: Vec<u8>) -> anyhow::Result<Self> {
-        if let Some(encoding) = self.metadata.content_encoding() {
-            self.content = encoding.encode(&decoded)?.into();
+    pub fn with_json_content(
+        mut self, json: &serde_json::Value,
+    ) -> anyhow::Result<SignaturesBuilder> {
+        anyhow::ensure!(
+            self.0.metadata.content_type()? == ContentType::Json,
+            "Already set metadata field `content-type` is not JSON value"
+        );
+
+        let content = serde_json::to_vec(&json)?;
+        if let Some(encoding) = self.0.metadata.content_encoding() {
+            self.0.content = encoding.encode(&content)?.into();
         } else {
-            self.content = decoded.into();
+            self.0.content = content.into();
         }
-        Ok(self)
+
+        Ok(SignaturesBuilder(self.0))
     }
+}
 
+impl SignaturesBuilder {
     /// Add a signature to the document
     ///
     /// # Errors
@@ -70,49 +96,104 @@ impl Builder {
         if kid.is_id() {
             anyhow::bail!("Provided kid should be in a uri format, kid: {kid}");
         }
-        let data_to_sign = tbs_data(&kid, &self.metadata, &self.content)?;
+        let data_to_sign = tbs_data(&kid, &self.0.metadata, &self.0.content)?;
         let sign_bytes = sign_fn(data_to_sign);
-        self.signatures.push(Signature::new(kid, sign_bytes));
+        self.0.signatures.push(Signature::new(kid, sign_bytes));
 
         Ok(self)
     }
 
-    /// Build a signed document with the collected error report.
-    /// Could provide an invalid document.
+    /// Builds a document from the set `metadata`, `content` and `signatures`.
     ///
-    /// # Panics
-    ///  Should not panic
-    #[must_use]
-    #[allow(
-        clippy::unwrap_used,
-        reason = "At this point all the data MUST be correctly encodable, and the final prepared bytes MUST be correctly decodable as a CatalystSignedDocument object."
-    )]
-    pub fn build(self) -> CatalystSignedDocument {
-        let mut e = minicbor::Encoder::new(Vec::new());
-        // COSE_Sign tag
-        // <!https://datatracker.ietf.org/doc/html/rfc8152#page-9>
-        e.tag(minicbor::data::Tag::new(98)).unwrap();
-        e.array(4).unwrap();
-        // protected headers (metadata fields)
-        e.bytes(minicbor::to_vec(&self.metadata).unwrap().as_slice())
-            .unwrap();
-        // empty unprotected headers
-        e.map(0).unwrap();
-        // content
-        e.encode(&self.content).unwrap();
-        // signatures
-        e.encode(self.signatures).unwrap();
-
-        CatalystSignedDocument::try_from(e.into_writer().as_slice()).unwrap()
+    /// # Errors:
+    ///  - CBOR encoding/decoding failures
+    pub fn build(self) -> anyhow::Result<CatalystSignedDocument> {
+        let doc = build_document(&self.0.metadata, &self.0.content, &self.0.signatures)?;
+        Ok(doc)
     }
 }
 
-impl From<&CatalystSignedDocument> for Builder {
+/// Build document from the provided `metadata`, `content` and `signatures`, performs all
+/// the decoding validation and collects a problem report.
+fn build_document(
+    metadata: &Metadata, content: &Content, signatures: &Signatures,
+) -> anyhow::Result<CatalystSignedDocument> {
+    let mut e = minicbor::Encoder::new(Vec::new());
+    // COSE_Sign tag
+    // <!https://datatracker.ietf.org/doc/html/rfc8152#page-9>
+    e.tag(minicbor::data::Tag::new(98))?;
+    e.array(4)?;
+    // protected headers (metadata fields)
+    e.bytes(minicbor::to_vec(metadata)?.as_slice())?;
+    // empty unprotected headers
+    e.map(0)?;
+    // content
+    e.encode(content)?;
+    // signatures
+    e.encode(signatures)?;
+    CatalystSignedDocument::try_from(e.into_writer().as_slice())
+}
+
+impl From<&CatalystSignedDocument> for SignaturesBuilder {
     fn from(value: &CatalystSignedDocument) -> Self {
-        Self {
+        Self(BuilderInner {
             metadata: value.inner.metadata.clone(),
             content: value.inner.content.clone(),
             signatures: value.inner.signatures.clone(),
+        })
+    }
+}
+
+#[cfg(test)]
+pub(crate) mod tests {
+    use crate::builder::SignaturesBuilder;
+
+    /// A test version of the builder, which allows to build a not fully valid catalyst
+    /// signed document
+    pub(crate) struct Builder(super::BuilderInner);
+
+    impl Default for Builder {
+        fn default() -> Self {
+            Self::new()
+        }
+    }
+
+    impl Builder {
+        /// Start building a signed document
+        #[must_use]
+        pub(crate) fn new() -> Self {
+            Self(super::BuilderInner::default())
+        }
+
+        /// Add provided `SupportedField` into the `Metadata`.
+        pub(crate) fn with_metadata_field(
+            mut self, field: crate::metadata::SupportedField,
+        ) -> Self {
+            self.0.metadata.add_field(field);
+            self
+        }
+
+        /// Set the content (COSE payload) to the document builder.
+        /// It will set the content as its provided, make sure by yourself that
+        /// `content-type` and `content-encoding` fields are aligned with the
+        /// provided content bytes.
+        pub(crate) fn with_content(mut self, content: Vec<u8>) -> Self {
+            self.0.content = content.into();
+            self
+        }
+
+        /// Add a signature to the document
+        pub(crate) fn add_signature(
+            mut self, sign_fn: impl FnOnce(Vec<u8>) -> Vec<u8>, kid: super::CatalystId,
+        ) -> anyhow::Result<Self> {
+            self.0 = SignaturesBuilder(self.0).add_signature(sign_fn, kid)?.0;
+            Ok(self)
+        }
+
+        /// Build a signed document with the collected error report.
+        /// Could provide an invalid document.
+        pub(crate) fn build(self) -> super::CatalystSignedDocument {
+            super::build_document(&self.0.metadata, &self.0.content, &self.0.signatures).unwrap()
         }
     }
 }

rust/signed_doc/src/lib.rs

@@ -29,6 +29,8 @@ pub use metadata::{
 use minicbor::{decode, encode, Decode, Decoder, Encode};
 pub use signature::{CatalystId, Signatures};
 
+use crate::builder::SignaturesBuilder;
+
 /// A problem report content string
 const PROBLEM_REPORT_CTX: &str = "Catalyst Signed Document";
 
@@ -203,7 +205,7 @@ impl CatalystSignedDocument {
     /// Returns a signed document `Builder` pre-loaded with the current signed document's
     /// data.
     #[must_use]
-    pub fn into_builder(&self) -> Builder {
+    pub fn into_builder(&self) -> SignaturesBuilder {
         self.into()
     }
 }

rust/signed_doc/src/metadata/mod.rs

@@ -23,13 +23,8 @@ pub use section::Section;
 use strum::IntoDiscriminant as _;
 use utils::{cose_protected_header_find, decode_document_field_from_protected_header, CborUuidV7};
 
-use crate::{
-    decode_context::DecodeContext,
-    metadata::{
-        supported_field::{SupportedField, SupportedLabel},
-        utils::decode_cose_protected_header_value,
-    },
-};
+pub(crate) use crate::metadata::supported_field::{SupportedField, SupportedLabel};
+use crate::{decode_context::DecodeContext, metadata::utils::decode_cose_protected_header_value};
 
 /// `content_encoding` field COSE key value
 const CONTENT_ENCODING_KEY: &str = "Content-Encoding";
@@ -171,6 +166,18 @@ impl Metadata {
             .and_then(SupportedField::try_as_parameters_ref)
     }
 
+    /// Add `SupportedField` into the `Metadata`.
+    ///
+    /// # Warning
+    ///
+    /// Building metadata by-field with this function doesn't ensure the presence of
+    /// required fields. Use [`Self::from_fields`] or [`Self::from_json`] if it's
+    /// important for metadata to be valid.
+    #[cfg(test)]
+    pub(crate) fn add_field(&mut self, field: SupportedField) {
+        self.0.insert(field.discriminant(), field);
+    }
+
     /// Build `Metadata` object from the metadata fields, doing all necessary validation.
     pub(crate) fn from_fields(fields: Vec<SupportedField>, report: &ProblemReport) -> Self {
         const REPORT_CONTEXT: &str = "Metadata building";
@@ -204,16 +211,12 @@ impl Metadata {
     }
 
     /// Build `Metadata` object from the metadata fields, doing all necessary validation.
-    pub(crate) fn from_json(fields: serde_json::Value, report: &ProblemReport) -> Self {
-        let fields = serde::Deserializer::deserialize_map(fields, MetadataDeserializeVisitor)
-            .inspect_err(|err| {
-                report.other(
-                    &format!("Unable to deserialize json: {err}"),
-                    "Metadata building from json",
-                );
-            })
-            .unwrap_or_default();
-        Self::from_fields(fields, report)
+    pub(crate) fn from_json(fields: serde_json::Value) -> anyhow::Result<Self> {
+        let fields = serde::Deserializer::deserialize_map(fields, MetadataDeserializeVisitor)?;
+        let report = ProblemReport::new("Deserializing metadata from json");
+        let metadata = Self::from_fields(fields, &report);
+        anyhow::ensure!(!report.is_problematic(), "{:?}", report);
+        Ok(metadata)
     }
 }
 

rust/signed_doc/src/metadata/supported_field.rs

@@ -95,7 +95,7 @@ impl Display for Label<'_> {
 )]
 #[non_exhaustive]
 #[repr(usize)]
-pub enum SupportedField {
+pub(crate) enum SupportedField {
     /// `content-type` field. In COSE it's represented as the signed integer `3` (see [RFC
     /// 8949 section 3.1]).
     ///