From 6c192d0eeea3d88dfea063a3035862fb8653b47f Mon Sep 17 00:00:00 2001 From: bkioshn Date: Fri, 13 Jun 2025 18:59:40 +0700 Subject: [PATCH 01/12] fix(signed-doc): wip apply new doc ref Signed-off-by: bkioshn --- rust/signed_doc/src/doc_types/mod.rs | 106 ++++++++++++++---- .../src/metadata/document_refs/mod.rs | 4 +- rust/signed_doc/src/metadata/mod.rs | 82 +++++--------- rust/signed_doc/src/metadata/utils.rs | 13 +++ rust/signed_doc/src/validator/mod.rs | 71 +++++++----- .../src/validator/rules/parameters.rs | 26 +++-- 6 files changed, 185 insertions(+), 117 deletions(-) diff --git a/rust/signed_doc/src/doc_types/mod.rs b/rust/signed_doc/src/doc_types/mod.rs index 2a2c22735e4..f6f5fe545d9 100644 --- a/rust/signed_doc/src/doc_types/mod.rs +++ b/rust/signed_doc/src/doc_types/mod.rs @@ -4,16 +4,38 @@ use std::sync::LazyLock; use catalyst_types::uuid::Uuid; -use deprecated::{ - COMMENT_DOCUMENT_UUID_TYPE, PROPOSAL_ACTION_DOCUMENT_UUID_TYPE, PROPOSAL_DOCUMENT_UUID_TYPE, -}; use crate::DocType; +/// -------------- Document Types -------------- +/// Brand document type. +pub static BRAND_PARAMETERS: LazyLock = LazyLock::new(|| { + let ids = &[BRAND_BASE_TYPE]; + ids.to_vec() + .try_into() + .expect("Failed to convert brand base types Uuid to DocType") +}); + +/// Campaign Parameters document type. +pub static CAMPAIGN_PARAMETERS: LazyLock = LazyLock::new(|| { + let ids = &[CAMPAIGN_BASE_TYPE]; + ids.to_vec() + .try_into() + .expect("Failed to convert campaign base types Uuid to DocType") +}); + +/// Category Parameters document type. +pub static CATEGORY_PARAMETERS: LazyLock = LazyLock::new(|| { + let ids = &[CATEGORY_BASE_TYPE]; + ids.to_vec() + .try_into() + .expect("Failed to convert category base types Uuid to DocType") +}); + /// Proposal document type. #[allow(clippy::expect_used)] -pub static PROPOSAL_DOC_TYPE: LazyLock = LazyLock::new(|| { - let ids = &[PROPOSAL_UUID_TYPE]; +pub static PROPOSAL: LazyLock = LazyLock::new(|| { + let ids = &[PROPOSAL_BASE_TYPE]; ids.to_vec() .try_into() .expect("Failed to convert proposal document Uuid to DocType") @@ -21,8 +43,8 @@ pub static PROPOSAL_DOC_TYPE: LazyLock = LazyLock::new(|| { /// Proposal comment document type. #[allow(clippy::expect_used)] -pub static PROPOSAL_COMMENT_DOC: LazyLock = LazyLock::new(|| { - let ids = &[COMMENT_UUID_TYPE, PROPOSAL_UUID_TYPE]; +pub static PROPOSAL_COMMENT: LazyLock = LazyLock::new(|| { + let ids = &[COMMENT_BASE_TYPE, PROPOSAL_BASE_TYPE]; ids.to_vec() .try_into() .expect("Failed to convert proposal comment document Uuid to DocType") @@ -30,26 +52,72 @@ pub static PROPOSAL_COMMENT_DOC: LazyLock = LazyLock::new(|| { /// Proposal action document type. #[allow(clippy::expect_used)] -pub static PROPOSAL_ACTION_DOC: LazyLock = LazyLock::new(|| { +pub static PROPOSAL_SUBMISSION_ACTION: LazyLock = LazyLock::new(|| { let ids = &[ - ACTION_UUID_TYPE, - PROPOSAL_UUID_TYPE, - SUBMISSION_ACTION_UUID_TYPE, + ACTION_BASE_TYPE, + PROPOSAL_BASE_TYPE, + SUBMISSION_ACTION_BASE_TYPE, ]; ids.to_vec() .try_into() .expect("Failed to convert proposal action document Uuid to DocType") }); -/// Submission Action UUID type. -pub const SUBMISSION_ACTION_UUID_TYPE: Uuid = +/// Proposal Comment Meta Template document type. +#[allow(clippy::expect_used)] +pub static PROPOSAL_COMMENT_META_TEMPLATE: LazyLock = LazyLock::new(|| { + let ids = &[ + TEMPLATE_BASE_TYPE, + TEMPLATE_BASE_TYPE, + COMMENT_BASE_TYPE, + PROPOSAL_BASE_TYPE, + ]; + ids.to_vec() + .try_into() + .expect("Failed to convert proposal comment meta template document Uuid to DocType") +}); + +/// Proposal Comment Template document type. +#[allow(clippy::expect_used)] +pub static PROPOSAL_COMMENT_TEMPLATE: LazyLock = LazyLock::new(|| { + let ids = &[TEMPLATE_BASE_TYPE, COMMENT_BASE_TYPE, PROPOSAL_BASE_TYPE]; + ids.to_vec() + .try_into() + .expect("Failed to convert proposal comment template document Uuid to DocType") +}); + +/// Proposal Template document type. +#[allow(clippy::expect_used)] +pub static PROPOSAL_TEMPLATE: LazyLock = LazyLock::new(|| { + let ids = &[TEMPLATE_BASE_TYPE, PROPOSAL_BASE_TYPE]; + ids.to_vec() + .try_into() + .expect("Failed to convert proposal template document Uuid to DocType") +}); + +/// -------------- Base Types -------------- +/// Action UUID base type. +pub const ACTION_BASE_TYPE: Uuid = Uuid::from_u128(0x5E60_E623_AD02_4A1B_A1AC_406D_B978_EE48); +/// Brand UUID base type. +pub const BRAND_BASE_TYPE: Uuid = Uuid::from_u128(0xEBCA_BEEB_5BC5_4F95_91E8_CAB8_CA72_4172); +/// Campaign UUID base type. +pub const CAMPAIGN_BASE_TYPE: Uuid = Uuid::from_u128(0x5EF3_2D5D_F240_462C_A7A4_BA4A_F221_FA23); +/// Category UUID base type. +pub const CATEGORY_BASE_TYPE: Uuid = Uuid::from_u128(0x8189_38C3_3139_4DAA_AFE6_974C_7848_8E95); +/// Comment UUID base type. +pub const COMMENT_BASE_TYPE: Uuid = Uuid::from_u128(0xB679_DED3_0E7C_41BA_89F8_DA62_A178_98EA); +/// Decision UUID base type. +pub const DECISION_BASE_TYPE: Uuid = Uuid::from_u128(0x788F_F4C6_D65A_451F_BB33_575F_E056_B411); +/// Moderation Action UUID base type. +pub const MODERATION_ACTION_BASE_TYPE: Uuid = + Uuid::from_u128(0xA5D2_32B8_5E03_4117_9AFD_BE32_B878_FCDD); +/// Proposal UUID base type. +pub const PROPOSAL_BASE_TYPE: Uuid = Uuid::from_u128(0x7808_D2BA_D511_40AF_84E8_C0D1_625F_DFDC); +/// Submission Action UUID base type. +pub const SUBMISSION_ACTION_BASE_TYPE: Uuid = Uuid::from_u128(0x7892_7329_CFD9_4EA1_9C71_0E01_9B12_6A65); -/// Proposal UUID type. -pub const PROPOSAL_UUID_TYPE: Uuid = PROPOSAL_DOCUMENT_UUID_TYPE; -/// Comment UUID type. -pub const COMMENT_UUID_TYPE: Uuid = COMMENT_DOCUMENT_UUID_TYPE; -/// Action UUID type. -pub const ACTION_UUID_TYPE: Uuid = PROPOSAL_ACTION_DOCUMENT_UUID_TYPE; +/// Template UUID base type. +pub const TEMPLATE_BASE_TYPE: Uuid = Uuid::from_u128(0x0CE8_AB38_9258_4FBC_A62E_7FAA_6E58_318F); /// Document type which will be deprecated. pub mod deprecated { diff --git a/rust/signed_doc/src/metadata/document_refs/mod.rs b/rust/signed_doc/src/metadata/document_refs/mod.rs index e5ef0cad093..7d13304e406 100644 --- a/rust/signed_doc/src/metadata/document_refs/mod.rs +++ b/rust/signed_doc/src/metadata/document_refs/mod.rs @@ -203,9 +203,7 @@ impl TryFrom<&DocumentRefs> for Value { impl<'de> Deserialize<'de> for DocumentRefs { fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { + where D: Deserializer<'de> { /// Old structure deserialize as map {id, ver} #[derive(Deserialize)] struct OldRef { diff --git a/rust/signed_doc/src/metadata/mod.rs b/rust/signed_doc/src/metadata/mod.rs index 2dba04931a8..4194eb1c85c 100644 --- a/rust/signed_doc/src/metadata/mod.rs +++ b/rust/signed_doc/src/metadata/mod.rs @@ -16,17 +16,20 @@ pub(crate) mod utils; use catalyst_types::{problem_report::ProblemReport, uuid::UuidV7}; pub use content_encoding::ContentEncoding; pub use content_type::ContentType; -use coset::{cbor::Value, iana::CoapContentFormat, CborSerializable}; +use coset::{cbor::Value, iana::CoapContentFormat}; pub use doc_type::DocType; -pub use document_refs::{DocLocator, DocumentRef, DocumentRefs}; -use minicbor::{Decode, Decoder}; +pub use document_refs::{DocLocator, DocumentRefs}; +use minicbor::Decoder; 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}, + metadata::{ + supported_field::{SupportedField, SupportedLabel}, + utils::decode_cose_protected_header_value, + }, }; /// `content_encoding` field COSE key value @@ -83,13 +86,13 @@ pub(crate) struct InnerMetadata { content_encoding: Option, /// Reference to the latest document. #[serde(rename = "ref", skip_serializing_if = "Option::is_none")] - doc_ref: Option, + doc_ref: Option, /// Reference to the document template. #[serde(skip_serializing_if = "Option::is_none")] - template: Option, + template: Option, /// Reference to the document reply. #[serde(skip_serializing_if = "Option::is_none")] - reply: Option, + reply: Option, /// Reference to the document section. #[serde(skip_serializing_if = "Option::is_none")] section: Option
, @@ -98,7 +101,7 @@ pub(crate) struct InnerMetadata { collabs: Vec, /// Reference to the parameters document. #[serde(skip_serializing_if = "Option::is_none")] - parameters: Option, + parameters: Option, } impl InnerMetadata { @@ -181,29 +184,26 @@ impl Metadata { /// Return `ref` field. #[must_use] - pub fn doc_ref(&self) -> Option { + pub fn doc_ref(&self) -> Option<&DocumentRefs> { self.0 .get(&SupportedLabel::Ref) .and_then(SupportedField::try_as_ref_ref) - .copied() } /// Return `template` field. #[must_use] - pub fn template(&self) -> Option { + pub fn template(&self) -> Option<&DocumentRefs> { self.0 .get(&SupportedLabel::Template) .and_then(SupportedField::try_as_template_ref) - .copied() } /// Return `reply` field. #[must_use] - pub fn reply(&self) -> Option { + pub fn reply(&self) -> Option<&DocumentRefs> { self.0 .get(&SupportedLabel::Reply) .and_then(SupportedField::try_as_reply_ref) - .copied() } /// Return `section` field. @@ -225,11 +225,10 @@ impl Metadata { /// Return `parameters` field. #[must_use] - pub fn parameters(&self) -> Option { + pub fn parameters(&self) -> Option<&DocumentRefs> { self.0 .get(&SupportedLabel::Parameters) .and_then(SupportedField::try_as_parameters_ref) - .copied() } /// Build `Metadata` object from the metadata fields, doing all necessary validation. @@ -315,18 +314,6 @@ impl InnerMetadata { } } - metadata.doc_type = cose_protected_header_find( - protected, - |key| matches!(key, coset::Label::Text(label) if label.eq_ignore_ascii_case(TYPE_KEY)), - ) - .and_then(|value| { - DocType::decode( - &mut Decoder::new(&value.clone().to_vec().unwrap_or_default()), - context, - ) - .ok() - }); - metadata.id = decode_document_field_from_protected_header::( protected, ID_KEY, @@ -343,24 +330,12 @@ impl InnerMetadata { ) .map(|v| v.0); - metadata.doc_ref = decode_document_field_from_protected_header( - protected, - REF_KEY, - COSE_DECODING_CONTEXT, - context.report, - ); - metadata.template = decode_document_field_from_protected_header( - protected, - TEMPLATE_KEY, - COSE_DECODING_CONTEXT, - context.report, - ); - metadata.reply = decode_document_field_from_protected_header( - protected, - REPLY_KEY, - COSE_DECODING_CONTEXT, - context.report, - ); + // DocType and DocRef now using cbor decoding. + metadata.doc_type = decode_cose_protected_header_value(&protected, context, TYPE_KEY); + metadata.doc_ref = decode_cose_protected_header_value(&protected, context, REF_KEY); + metadata.template = decode_cose_protected_header_value(&protected, context, TEMPLATE_KEY); + metadata.reply = decode_cose_protected_header_value(&protected, context, REPLY_KEY); + metadata.section = decode_document_field_from_protected_header( protected, SECTION_KEY, @@ -376,20 +351,15 @@ impl InnerMetadata { CATEGORY_ID_KEY, ] .iter() - .filter_map(|field_name| -> Option { - decode_document_field_from_protected_header( - protected, - field_name, - COSE_DECODING_CONTEXT, - context.report, - ) + .filter_map(|field_name| -> Option { + return decode_cose_protected_header_value(&protected, context, field_name); }) .fold((None, false), |(res, _), v| (Some(v), res.is_some())); if has_multiple_fields { context.report.duplicate_field( - "brand_id, campaign_id, category_id", - "Only value at the same time is allowed parameters, brand_id, campaign_id, category_id", - "Validation of parameters field aliases" + "Parameters field", + "Only one parameter can be used at a time: either brand_id, campaign_id, category_id", + COSE_DECODING_CONTEXT ); } metadata.parameters = parameters; diff --git a/rust/signed_doc/src/metadata/utils.rs b/rust/signed_doc/src/metadata/utils.rs index 0e54f10c439..698f0a5f1ff 100644 --- a/rust/signed_doc/src/metadata/utils.rs +++ b/rust/signed_doc/src/metadata/utils.rs @@ -5,6 +5,19 @@ use catalyst_types::{ uuid::{CborContext, UuidV4, UuidV7}, }; use coset::{CborSerializable, Label, ProtectedHeader}; +use minicbor::{Decode, Decoder}; + +/// Decode cose protected header value using minicbor decoder. +pub(crate) fn decode_cose_protected_header_value( + protected: &ProtectedHeader, context: &mut C, label: &str, +) -> Option +where T: for<'a> Decode<'a, C> { + cose_protected_header_find(protected, |key| matches!(key, Label::Text(l) if l == label)) + .and_then(|value| { + let bytes = value.clone().to_vec().unwrap_or_default(); + Decoder::new(&bytes).decode_with(context).ok() + }) +} /// Find a value for a predicate in the protected header. pub(crate) fn cose_protected_header_find( diff --git a/rust/signed_doc/src/validator/mod.rs b/rust/signed_doc/src/validator/mod.rs index da1b4604d2a..24b36dda5c4 100644 --- a/rust/signed_doc/src/validator/mod.rs +++ b/rust/signed_doc/src/validator/mod.rs @@ -22,11 +22,8 @@ use rules::{ use crate::{ doc_types::{ - deprecated::{ - CATEGORY_DOCUMENT_UUID_TYPE, COMMENT_TEMPLATE_UUID_TYPE, PROPOSAL_TEMPLATE_UUID_TYPE, - }, - COMMENT_UUID_TYPE, PROPOSAL_ACTION_DOC, PROPOSAL_COMMENT_DOC, PROPOSAL_DOC_TYPE, - PROPOSAL_UUID_TYPE, + BRAND_PARAMETERS, CAMPAIGN_PARAMETERS, CATEGORY_PARAMETERS, PROPOSAL, PROPOSAL_COMMENT, + PROPOSAL_COMMENT_TEMPLATE, PROPOSAL_SUBMISSION_ACTION, PROPOSAL_TEMPLATE, }, metadata::DocType, providers::{CatalystSignedDocumentProvider, VerifyingKeyProvider}, @@ -51,9 +48,18 @@ where /// `DOCUMENT_RULES` initialization function #[allow(clippy::expect_used)] fn document_rules_init() -> HashMap { + // Parameter can be either brand, campaign or category + let parameters = vec![ + BRAND_PARAMETERS.clone(), + CAMPAIGN_PARAMETERS.clone(), + CATEGORY_PARAMETERS.clone(), + ]; + let mut document_rules_map = HashMap::new(); - let proposal_document_rules = Rules { + // Proposal + // + let proposal_rules = Rules { content_type: ContentTypeRule { exp: ContentType::Json, }, @@ -62,11 +68,11 @@ fn document_rules_init() -> HashMap { optional: false, }, content: ContentRule::Templated { - exp_template_type: expect_doc_type(PROPOSAL_TEMPLATE_UUID_TYPE), + exp_template_type: PROPOSAL_TEMPLATE.clone(), }, parameters: ParametersRule::Specified { - exp_parameters_type: expect_doc_type(CATEGORY_DOCUMENT_UUID_TYPE), - optional: true, + exp_parameters_type: parameters.clone(), + optional: false, }, doc_ref: RefRule::NotSpecified, reply: ReplyRule::NotSpecified, @@ -76,9 +82,11 @@ fn document_rules_init() -> HashMap { }, }; - document_rules_map.insert(PROPOSAL_DOC_TYPE.clone(), proposal_document_rules); + document_rules_map.insert(PROPOSAL.clone(), proposal_rules); - let comment_document_rules = Rules { + // Proposal Comment + // + let proposal_comment_rules = Rules { content_type: ContentTypeRule { exp: ContentType::Json, }, @@ -87,33 +95,40 @@ fn document_rules_init() -> HashMap { optional: false, }, content: ContentRule::Templated { - exp_template_type: expect_doc_type(COMMENT_TEMPLATE_UUID_TYPE), + exp_template_type: PROPOSAL_COMMENT_TEMPLATE.clone(), }, doc_ref: RefRule::Specified { - exp_ref_type: expect_doc_type(PROPOSAL_UUID_TYPE), + exp_ref_type: PROPOSAL.clone(), optional: false, }, reply: ReplyRule::Specified { - exp_reply_type: expect_doc_type(COMMENT_UUID_TYPE), + exp_reply_type: PROPOSAL_COMMENT.clone(), optional: true, }, - section: SectionRule::Specified { optional: true }, - parameters: ParametersRule::NotSpecified, + // FIXME: section ref? optional + section: SectionRule::NotSpecified, + parameters: ParametersRule::Specified { + exp_parameters_type: parameters.clone(), + optional: false, + }, kid: SignatureKidRule { exp: &[RoleId::Role0], }, }; - document_rules_map.insert(PROPOSAL_COMMENT_DOC.clone(), comment_document_rules); + document_rules_map.insert(PROPOSAL_COMMENT.clone(), proposal_comment_rules); let proposal_action_json_schema = jsonschema::options() - .with_draft(jsonschema::Draft::Draft7) - .build( - &serde_json::from_str(include_str!( - "./../../../../specs/signed_docs/docs/payload_schemas/proposal_submission_action.schema.json" - )) - .expect("Must be a valid json file"), - ) - .expect("Must be a valid json scheme file"); + .with_draft(jsonschema::Draft::Draft7) + .build( + &serde_json::from_str(include_str!( + "./../../../../specs/signed_docs/docs/payload_schemas/proposal_submission_action.schema.json" + )) + .expect("Must be a valid json file"), + ) + .expect("Must be a valid json scheme file"); + + // Proposal Submission Action + // let proposal_submission_action_rules = Rules { content_type: ContentTypeRule { exp: ContentType::Json, @@ -124,11 +139,11 @@ fn document_rules_init() -> HashMap { }, content: ContentRule::Static(ContentSchema::Json(proposal_action_json_schema)), parameters: ParametersRule::Specified { - exp_parameters_type: expect_doc_type(CATEGORY_DOCUMENT_UUID_TYPE), + exp_parameters_type: parameters, optional: true, }, doc_ref: RefRule::Specified { - exp_ref_type: expect_doc_type(PROPOSAL_UUID_TYPE), + exp_ref_type: PROPOSAL.clone(), optional: false, }, reply: ReplyRule::NotSpecified, @@ -139,7 +154,7 @@ fn document_rules_init() -> HashMap { }; document_rules_map.insert( - PROPOSAL_ACTION_DOC.clone(), + PROPOSAL_SUBMISSION_ACTION.clone(), proposal_submission_action_rules, ); diff --git a/rust/signed_doc/src/validator/rules/parameters.rs b/rust/signed_doc/src/validator/rules/parameters.rs index 41ad404df07..b18294ff68e 100644 --- a/rust/signed_doc/src/validator/rules/parameters.rs +++ b/rust/signed_doc/src/validator/rules/parameters.rs @@ -12,7 +12,7 @@ pub(crate) enum ParametersRule { /// Is `parameters` specified Specified { /// expected `type` field of the parameter doc - exp_parameters_type: DocType, + exp_parameters_type: Vec, /// optional flag for the `parameters` field optional: bool, }, @@ -33,12 +33,10 @@ impl ParametersRule { { if let Some(parameters) = doc.doc_meta().parameters() { let parameters_validator = |replied_doc: CatalystSignedDocument| { - referenced_doc_check( - &replied_doc, - exp_parameters_type, - "parameters", - doc.report(), - ) + // For "parameters", it should be one of these types + exp_parameters_type.iter().any(|exp_type| { + referenced_doc_check(&replied_doc, exp_type, "parameters", doc.report()) + }) }; return validate_provided_doc( ¶meters, @@ -79,7 +77,8 @@ mod tests { async fn ref_rule_specified_test() { let mut provider = TestCatalystSignedDocumentProvider::default(); - let exp_parameters_type = UuidV4::new(); + // Parameter types can contains multiple, but should match one of it. + let exp_parameters_type: Vec = vec![vec![UuidV4::new()].try_into().unwrap()]; let valid_category_doc_id = UuidV7::new(); let valid_category_doc_ver = UuidV7::new(); @@ -90,11 +89,16 @@ mod tests { // prepare replied documents { + let exp_parameters_type_str: Vec = exp_parameters_type + .iter() + .map(|dt| dt.to_string()) + .collect(); + let ref_doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": valid_category_doc_id.to_string(), "ver": valid_category_doc_ver.to_string(), - "type": exp_parameters_type.to_string() + "type": exp_parameters_type_str, })) .unwrap() .build(); @@ -124,7 +128,7 @@ mod tests { // all correct let rule = ParametersRule::Specified { - exp_parameters_type: exp_parameters_type.into(), + exp_parameters_type: exp_parameters_type.clone().into(), optional: false, }; let doc = Builder::new() @@ -137,7 +141,7 @@ mod tests { // all correct, `parameters` field is missing, but its optional let rule = ParametersRule::Specified { - exp_parameters_type: exp_parameters_type.into(), + exp_parameters_type: exp_parameters_type.clone().into(), optional: true, }; let doc = Builder::new().build(); From eda0d5ac8aee7060bdfdb0b608bbe95c836f31da Mon Sep 17 00:00:00 2001 From: bkioshn Date: Thu, 19 Jun 2025 15:57:32 +0700 Subject: [PATCH 02/12] fix(signed-doc): apply new doc ref and implement validation rule for doc ref Signed-off-by: bkioshn --- rust/signed_doc/src/lib.rs | 2 +- rust/signed_doc/src/metadata/doc_type.rs | 99 +++++--- .../src/metadata/document_refs/doc_ref.rs | 2 +- rust/signed_doc/src/metadata/mod.rs | 20 +- .../src/metadata/supported_field.rs | 35 ++- rust/signed_doc/src/providers.rs | 34 ++- rust/signed_doc/src/validator/mod.rs | 50 +++- .../signed_doc/src/validator/rules/doc_ref.rs | 119 ++++++--- rust/signed_doc/src/validator/rules/mod.rs | 7 +- .../src/validator/rules/param_link_ref.rs | 168 +++++++++++++ .../src/validator/rules/parameters.rs | 139 ++++++---- rust/signed_doc/src/validator/rules/reply.rs | 105 ++++---- .../signed_doc/src/validator/rules/section.rs | 1 + .../src/validator/rules/template.rs | 67 ++--- rust/signed_doc/src/validator/utils.rs | 25 +- rust/signed_doc/tests/comment.rs | 238 +++++++++++------- rust/signed_doc/tests/common/mod.rs | 2 +- rust/signed_doc/tests/decoding.rs | 32 +-- rust/signed_doc/tests/proposal.rs | 98 ++++++-- rust/signed_doc/tests/signature.rs | 2 +- rust/signed_doc/tests/submission.rs | 126 ++++++++-- 21 files changed, 967 insertions(+), 404 deletions(-) create mode 100644 rust/signed_doc/src/validator/rules/param_link_ref.rs diff --git a/rust/signed_doc/src/lib.rs b/rust/signed_doc/src/lib.rs index 8284032e841..78efd04161f 100644 --- a/rust/signed_doc/src/lib.rs +++ b/rust/signed_doc/src/lib.rs @@ -25,7 +25,7 @@ pub use content::Content; use coset::{CborSerializable, Header, TaggedCborSerializable}; use decode_context::{CompatibilityPolicy, DecodeContext}; pub use metadata::{ - ContentEncoding, ContentType, DocLocator, DocType, DocumentRef, Metadata, Section, + ContentEncoding, ContentType, DocLocator, DocType, DocumentRef, DocumentRefs, Metadata, Section, }; use minicbor::{decode, encode, Decode, Decoder, Encode}; pub use signature::{CatalystId, Signatures}; diff --git a/rust/signed_doc/src/metadata/doc_type.rs b/rust/signed_doc/src/metadata/doc_type.rs index a7c2ca205bb..20a537dc237 100644 --- a/rust/signed_doc/src/metadata/doc_type.rs +++ b/rust/signed_doc/src/metadata/doc_type.rs @@ -16,10 +16,7 @@ use tracing::warn; use crate::{ decode_context::{CompatibilityPolicy, DecodeContext}, - doc_types::{ - ACTION_UUID_TYPE, COMMENT_UUID_TYPE, PROPOSAL_ACTION_DOC, PROPOSAL_COMMENT_DOC, - PROPOSAL_DOC_TYPE, PROPOSAL_UUID_TYPE, - }, + doc_types::{deprecated, PROPOSAL, PROPOSAL_COMMENT, PROPOSAL_SUBMISSION_ACTION}, }; /// List of `UUIDv4` document type. @@ -46,21 +43,6 @@ impl DocType { pub fn doc_types(&self) -> &Vec { &self.0 } - - /// Convert `DocType` to coset `Value`. - pub(crate) fn to_value(&self) -> Value { - Value::Array( - self.0 - .iter() - .map(|uuidv4| { - Value::Tag( - UUID_CBOR_TAG, - Box::new(Value::Bytes(uuidv4.uuid().as_bytes().to_vec())), - ) - }) - .collect(), - ) - } } impl Hash for DocType { @@ -106,6 +88,18 @@ impl TryFrom> for DocType { } } +impl From for Vec { + fn from(value: DocType) -> Vec { + value.0.into_iter().map(Uuid::from).collect() + } +} + +impl From for Vec { + fn from(val: DocType) -> Self { + val.0.into_iter().map(|uuid| uuid.to_string()).collect() + } +} + impl TryFrom> for DocType { type Error = DocTypeError; @@ -255,9 +249,11 @@ impl Decode<'_, DecodeContext<'_>> for DocType { /// fn map_doc_type(uuid: Uuid) -> anyhow::Result { match uuid { - id if id == PROPOSAL_UUID_TYPE => Ok(PROPOSAL_DOC_TYPE.clone()), - id if id == COMMENT_UUID_TYPE => Ok(PROPOSAL_COMMENT_DOC.clone()), - id if id == ACTION_UUID_TYPE => Ok(PROPOSAL_ACTION_DOC.clone()), + id if id == deprecated::PROPOSAL_DOCUMENT_UUID_TYPE => Ok(PROPOSAL.clone()), + id if id == deprecated::COMMENT_DOCUMENT_UUID_TYPE => Ok(PROPOSAL_COMMENT.clone()), + id if id == deprecated::PROPOSAL_ACTION_DOCUMENT_UUID_TYPE => { + Ok(PROPOSAL_SUBMISSION_ACTION.clone()) + }, _ => anyhow::bail!("Unknown document type: {uuid}"), } } @@ -275,13 +271,13 @@ impl Encode for DocType { } e.array(self.0.len().try_into().map_err(|_| { - report.other("Unable to encode array length", CONTEXT); - minicbor::encode::Error::message(format!("{CONTEXT}, unable to encode array length")) + report.invalid_encoding("Array", "Invalid array", "Valid array", CONTEXT); + minicbor::encode::Error::message(format!("{CONTEXT}, array length encoding failed")) })?)?; for id in &self.0 { id.encode(e, &mut CborContext::Tagged).map_err(|_| { - report.other("Failed to encode UUIDv4", CONTEXT); + report.invalid_encoding("UUIDv4", &id.to_string(), "Valid UUIDv4", CONTEXT); minicbor::encode::Error::message(format!("{CONTEXT}: UUIDv4 encoding failed")) })?; } @@ -317,15 +313,35 @@ impl<'de> Deserialize<'de> for DocType { } } +impl From for Value { + fn from(value: DocType) -> Self { + Value::Array( + value + .0 + .iter() + .map(|uuidv4| { + Value::Tag( + UUID_CBOR_TAG, + Box::new(Value::Bytes(uuidv4.uuid().as_bytes().to_vec())), + ) + }) + .collect(), + ) + } +} + // This is needed to preserve backward compatibility with the old solution. impl PartialEq for DocType { fn eq(&self, other: &Self) -> bool { // List of special-case (single UUID) -> new DocType // The old one should equal to the new one let special_cases = [ - (PROPOSAL_UUID_TYPE, &*PROPOSAL_DOC_TYPE), - (COMMENT_UUID_TYPE, &*PROPOSAL_COMMENT_DOC), - (ACTION_UUID_TYPE, &*PROPOSAL_ACTION_DOC), + (deprecated::PROPOSAL_DOCUMENT_UUID_TYPE, &*PROPOSAL), + (deprecated::COMMENT_DOCUMENT_UUID_TYPE, &*PROPOSAL_COMMENT), + ( + deprecated::PROPOSAL_ACTION_DOCUMENT_UUID_TYPE, + &*PROPOSAL_SUBMISSION_ACTION, + ), ]; for (uuid, expected) in special_cases { match DocType::try_from(uuid) { @@ -447,14 +463,19 @@ mod tests { let input = vec!["not-a-uuid".to_string()]; let result = DocType::try_from(input); assert!(matches!(result, Err(DocTypeError::StringConversion(s)) if s == "not-a-uuid")); + + let e: Vec = vec![vec![UuidV4::new()].try_into().unwrap()]; + let exp_parameters_type_str: Vec = + e.iter().map(std::string::ToString::to_string).collect(); + assert!(!exp_parameters_type_str.is_empty()); } #[test] fn test_doc_type_to_value() { let uuid = uuid::Uuid::new_v4(); - let doc_type = DocType(vec![UuidV4::try_from(uuid).unwrap()]); + let doc_type: Value = DocType(vec![UuidV4::try_from(uuid).unwrap()]).into(); - for d in &doc_type.to_value().into_array().unwrap() { + for d in &doc_type.into_array().unwrap() { let t = d.clone().into_tag().unwrap(); assert_eq!(t.0, UUID_CBOR_TAG); assert_eq!(t.1.as_bytes().unwrap().len(), 16); @@ -464,18 +485,18 @@ mod tests { #[test] fn test_doctype_equal_special_cases() { // Direct equal - let uuid = PROPOSAL_UUID_TYPE; + let uuid = deprecated::PROPOSAL_DOCUMENT_UUID_TYPE; let dt1 = DocType::try_from(vec![uuid]).unwrap(); let dt2 = DocType::try_from(vec![uuid]).unwrap(); assert_eq!(dt1, dt2); // single -> special mapped type - let single = DocType::try_from(PROPOSAL_UUID_TYPE).unwrap(); - assert_eq!(single, *PROPOSAL_DOC_TYPE); - let single = DocType::try_from(COMMENT_UUID_TYPE).unwrap(); - assert_eq!(single, *PROPOSAL_COMMENT_DOC); - let single = DocType::try_from(ACTION_UUID_TYPE).unwrap(); - assert_eq!(single, *PROPOSAL_ACTION_DOC); + let single = DocType::try_from(deprecated::PROPOSAL_DOCUMENT_UUID_TYPE).unwrap(); + assert_eq!(single, *PROPOSAL); + let single = DocType::try_from(deprecated::COMMENT_DOCUMENT_UUID_TYPE).unwrap(); + assert_eq!(single, *PROPOSAL_COMMENT); + let single = DocType::try_from(deprecated::PROPOSAL_ACTION_DOCUMENT_UUID_TYPE).unwrap(); + assert_eq!(single, *PROPOSAL_SUBMISSION_ACTION); } #[test] @@ -504,10 +525,10 @@ mod tests { #[test] fn test_deserialize_special_case() { - let uuid = PROPOSAL_UUID_TYPE.to_string(); + let uuid = deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.to_string(); let json = json!(uuid); let dt: DocType = serde_json::from_value(json).unwrap(); - assert_eq!(dt, *PROPOSAL_DOC_TYPE); + assert_eq!(dt, *PROPOSAL); } } diff --git a/rust/signed_doc/src/metadata/document_refs/doc_ref.rs b/rust/signed_doc/src/metadata/document_refs/doc_ref.rs index e8aae51d0ce..33361351ed9 100644 --- a/rust/signed_doc/src/metadata/document_refs/doc_ref.rs +++ b/rust/signed_doc/src/metadata/document_refs/doc_ref.rs @@ -167,7 +167,7 @@ mod test { use catalyst_types::uuid::{UuidV7, UUID_CBOR_TAG}; use coset::cbor::Value; - use crate::{metadata::document_refs::doc_ref::DOC_REF_ARR_ITEM, DocumentRef}; + use crate::metadata::document_refs::{doc_ref::DOC_REF_ARR_ITEM, DocumentRef}; #[test] #[allow(clippy::indexing_slicing)] diff --git a/rust/signed_doc/src/metadata/mod.rs b/rust/signed_doc/src/metadata/mod.rs index 4194eb1c85c..124519decd8 100644 --- a/rust/signed_doc/src/metadata/mod.rs +++ b/rust/signed_doc/src/metadata/mod.rs @@ -18,7 +18,7 @@ pub use content_encoding::ContentEncoding; pub use content_type::ContentType; use coset::{cbor::Value, iana::CoapContentFormat}; pub use doc_type::DocType; -pub use document_refs::{DocLocator, DocumentRefs}; +pub use document_refs::{DocLocator, DocumentRef, DocumentRefs}; use minicbor::Decoder; pub use section::Section; use strum::IntoDiscriminant as _; @@ -330,11 +330,11 @@ impl InnerMetadata { ) .map(|v| v.0); - // DocType and DocRef now using cbor decoding. - metadata.doc_type = decode_cose_protected_header_value(&protected, context, TYPE_KEY); - metadata.doc_ref = decode_cose_protected_header_value(&protected, context, REF_KEY); - metadata.template = decode_cose_protected_header_value(&protected, context, TEMPLATE_KEY); - metadata.reply = decode_cose_protected_header_value(&protected, context, REPLY_KEY); + // DocType and DocRef now using minicbor decoding. + metadata.doc_type = decode_cose_protected_header_value(protected, context, TYPE_KEY); + metadata.doc_ref = decode_cose_protected_header_value(protected, context, REF_KEY); + metadata.template = decode_cose_protected_header_value(protected, context, TEMPLATE_KEY); + metadata.reply = decode_cose_protected_header_value(protected, context, REPLY_KEY); metadata.section = decode_document_field_from_protected_header( protected, @@ -352,7 +352,7 @@ impl InnerMetadata { ] .iter() .filter_map(|field_name| -> Option { - return decode_cose_protected_header_value(&protected, context, field_name); + decode_cose_protected_header_value(protected, context, field_name) }) .fold((None, false), |(res, _), v| (Some(v), res.is_some())); if has_multiple_fields { @@ -436,7 +436,7 @@ impl TryFrom<&Metadata> for coset::Header { } builder = builder - .text_value(TYPE_KEY.to_string(), meta.doc_type()?.to_value()) + .text_value(TYPE_KEY.to_string(), meta.doc_type()?.clone().into()) .text_value( ID_KEY.to_string(), Value::try_from(CborUuidV7(meta.doc_id()?))?, @@ -519,8 +519,8 @@ pub(crate) struct MetadataDecodeContext { } impl MetadataDecodeContext { - /// [`DocType`] decoding context. - fn doc_type_context(&mut self) -> crate::decode_context::DecodeContext { + /// [`DocType`, `DocumentRef`] decoding context. + fn doc_type_ref_context(&mut self) -> crate::decode_context::DecodeContext { crate::decode_context::DecodeContext { compatibility_policy: self.compatibility_policy, report: &mut self.report, diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index 9ce4ecb122a..db67c0dcad9 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -9,7 +9,7 @@ use strum::{EnumDiscriminants, EnumTryAs, IntoDiscriminant as _}; use crate::{ metadata::{custom_transient_decode_error, MetadataDecodeContext, MetadataEncodeContext}, - ContentEncoding, ContentType, DocType, DocumentRef, Section, + ContentEncoding, ContentType, DocType, DocumentRefs, Section, }; /// COSE label. May be either a signed integer or a string. @@ -102,21 +102,21 @@ pub enum SupportedField { /// `id` field. Id(UuidV7) = 1, /// `ref` field. - Ref(DocumentRef) = 2, + Ref(DocumentRefs) = 2, /// `ver` field. Ver(UuidV7) = 3, /// `type` field. Type(DocType) = 4, /// `reply` field. - Reply(DocumentRef) = 5, + Reply(DocumentRefs) = 5, /// `collabs` field. Collabs(Vec) = 7, /// `section` field. Section(Section) = 8, /// `template` field. - Template(DocumentRef) = 9, + Template(DocumentRefs) = 9, /// `parameters` field. - Parameters(DocumentRef) = 10, + Parameters(DocumentRefs) = 10, /// `Content-Encoding` field. ContentEncoding(ContentEncoding) = 11, } @@ -196,14 +196,29 @@ impl minicbor::Decode<'_, MetadataDecodeContext> for SupportedField { let field = match key { SupportedLabel::ContentType => todo!(), SupportedLabel::Id => d.decode_with(&mut ctx.uuid_context).map(Self::Id), - SupportedLabel::Ref => todo!(), + SupportedLabel::Ref => { + d.decode_with(&mut ctx.doc_type_ref_context()) + .map(Self::Ref) + }, SupportedLabel::Ver => d.decode_with(&mut ctx.uuid_context).map(Self::Ver), - SupportedLabel::Type => d.decode_with(&mut ctx.doc_type_context()).map(Self::Type), - SupportedLabel::Reply => todo!(), + SupportedLabel::Type => { + d.decode_with(&mut ctx.doc_type_ref_context()) + .map(Self::Type) + }, + SupportedLabel::Reply => { + d.decode_with(&mut ctx.doc_type_ref_context()) + .map(Self::Reply) + }, SupportedLabel::Collabs => todo!(), SupportedLabel::Section => todo!(), - SupportedLabel::Template => todo!(), - SupportedLabel::Parameters => todo!(), + SupportedLabel::Template => { + d.decode_with(&mut ctx.doc_type_ref_context()) + .map(Self::Template) + }, + SupportedLabel::Parameters => { + d.decode_with(&mut ctx.doc_type_ref_context()) + .map(Self::Parameters) + }, SupportedLabel::ContentEncoding => todo!(), }?; diff --git a/rust/signed_doc/src/providers.rs b/rust/signed_doc/src/providers.rs index 9fd41d1c63c..b839c8166e7 100644 --- a/rust/signed_doc/src/providers.rs +++ b/rust/signed_doc/src/providers.rs @@ -17,7 +17,7 @@ pub trait VerifyingKeyProvider { /// `CatalystSignedDocument` Provider trait pub trait CatalystSignedDocumentProvider: Send + Sync { - /// Try to get `CatalystSignedDocument` + /// Try to get `CatalystSignedDocument`from document reference fn try_get_doc( &self, doc_ref: &DocumentRef, ) -> impl Future>> + Send; @@ -38,24 +38,34 @@ pub mod tests { use std::{collections::HashMap, time::Duration}; - use catalyst_types::uuid::Uuid; - use super::{ - CatalystId, CatalystSignedDocument, CatalystSignedDocumentProvider, DocumentRef, - VerifyingKey, VerifyingKeyProvider, + CatalystId, CatalystSignedDocument, CatalystSignedDocumentProvider, VerifyingKey, + VerifyingKeyProvider, }; + use crate::{DocLocator, DocumentRef}; /// Simple testing implementation of `CatalystSignedDocumentProvider` - #[derive(Default)] - pub struct TestCatalystSignedDocumentProvider(HashMap); + #[derive(Default, Debug)] + + pub struct TestCatalystSignedDocumentProvider(HashMap); impl TestCatalystSignedDocumentProvider { - /// Inserts document into the `TestCatalystSignedDocumentProvider` + /// 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. /// /// # Errors - /// - Missing document id - pub fn add_document(&mut self, doc: CatalystSignedDocument) -> anyhow::Result<()> { - self.0.insert(doc.doc_id()?.uuid(), doc); + /// Returns error if document reference is not provided and its fail to create one + /// from the given doc. + pub fn add_document( + &mut self, doc_ref: Option, doc: &CatalystSignedDocument, + ) -> anyhow::Result<()> { + if let Some(dr) = doc_ref { + self.0.insert(dr, doc.clone()); + } else { + let dr = DocumentRef::new(doc.doc_id()?, doc.doc_ver()?, DocLocator::default()); + self.0.insert(dr, doc.clone()); + } Ok(()) } } @@ -64,7 +74,7 @@ pub mod tests { async fn try_get_doc( &self, doc_ref: &DocumentRef, ) -> anyhow::Result> { - Ok(self.0.get(&doc_ref.id.uuid()).cloned()) + Ok(self.0.get(doc_ref).cloned()) } fn future_threshold(&self) -> Option { diff --git a/rust/signed_doc/src/validator/mod.rs b/rust/signed_doc/src/validator/mod.rs index 24b36dda5c4..f5e7db1bbe0 100644 --- a/rust/signed_doc/src/validator/mod.rs +++ b/rust/signed_doc/src/validator/mod.rs @@ -5,7 +5,7 @@ pub(crate) mod utils; use std::{ collections::HashMap, - sync::LazyLock, + sync::{Arc, LazyLock}, time::{Duration, SystemTime}, }; @@ -16,12 +16,13 @@ use catalyst_types::{ }; use coset::{CoseSign, CoseSignature}; use rules::{ - ContentEncodingRule, ContentRule, ContentSchema, ContentTypeRule, ParametersRule, RefRule, - ReplyRule, Rules, SectionRule, SignatureKidRule, + ContentEncodingRule, ContentRule, ContentSchema, ContentTypeRule, LinkField, + ParameterLinkRefRule, ParametersRule, RefRule, ReplyRule, Rules, SectionRule, SignatureKidRule, }; use crate::{ doc_types::{ + deprecated::{self}, BRAND_PARAMETERS, CAMPAIGN_PARAMETERS, CATEGORY_PARAMETERS, PROPOSAL, PROPOSAL_COMMENT, PROPOSAL_COMMENT_TEMPLATE, PROPOSAL_SUBMISSION_ACTION, PROPOSAL_TEMPLATE, }, @@ -31,7 +32,7 @@ use crate::{ }; /// A table representing a full set or validation rules per document id. -static DOCUMENT_RULES: LazyLock> = LazyLock::new(document_rules_init); +static DOCUMENT_RULES: LazyLock>> = LazyLock::new(document_rules_init); /// Returns an `DocType` from the provided argument. /// Reduce redundant conversion. @@ -46,8 +47,8 @@ where } /// `DOCUMENT_RULES` initialization function -#[allow(clippy::expect_used)] -fn document_rules_init() -> HashMap { +#[allow(clippy::expect_used, clippy::too_many_lines)] +fn document_rules_init() -> HashMap> { // Parameter can be either brand, campaign or category let parameters = vec![ BRAND_PARAMETERS.clone(), @@ -58,6 +59,7 @@ fn document_rules_init() -> HashMap { let mut document_rules_map = HashMap::new(); // Proposal + // Require field: type, id, ver, template, parameters // let proposal_rules = Rules { content_type: ContentTypeRule { @@ -80,11 +82,13 @@ fn document_rules_init() -> HashMap { kid: SignatureKidRule { exp: &[RoleId::Proposer], }, + param_link_ref: ParameterLinkRefRule::Specified { + field: LinkField::Template, + }, }; - document_rules_map.insert(PROPOSAL.clone(), proposal_rules); - // Proposal Comment + // Require field: type, id, ver, ref, template, parameters // let proposal_comment_rules = Rules { content_type: ContentTypeRule { @@ -105,7 +109,6 @@ fn document_rules_init() -> HashMap { exp_reply_type: PROPOSAL_COMMENT.clone(), optional: true, }, - // FIXME: section ref? optional section: SectionRule::NotSpecified, parameters: ParametersRule::Specified { exp_parameters_type: parameters.clone(), @@ -114,8 +117,11 @@ fn document_rules_init() -> HashMap { kid: SignatureKidRule { exp: &[RoleId::Role0], }, + // Link field can be either template or ref + param_link_ref: ParameterLinkRefRule::Specified { + field: LinkField::Template, + }, }; - document_rules_map.insert(PROPOSAL_COMMENT.clone(), proposal_comment_rules); let proposal_action_json_schema = jsonschema::options() .with_draft(jsonschema::Draft::Draft7) @@ -128,6 +134,7 @@ fn document_rules_init() -> HashMap { .expect("Must be a valid json scheme file"); // Proposal Submission Action + // Require fields: type, id, ver, ref, parameters // let proposal_submission_action_rules = Rules { content_type: ContentTypeRule { @@ -140,7 +147,7 @@ fn document_rules_init() -> HashMap { content: ContentRule::Static(ContentSchema::Json(proposal_action_json_schema)), parameters: ParametersRule::Specified { exp_parameters_type: parameters, - optional: true, + optional: false, }, doc_ref: RefRule::Specified { exp_ref_type: PROPOSAL.clone(), @@ -151,11 +158,30 @@ fn document_rules_init() -> HashMap { kid: SignatureKidRule { exp: &[RoleId::Proposer], }, + param_link_ref: ParameterLinkRefRule::Specified { + field: LinkField::Ref, + }, }; + let proposal_rules = Arc::new(proposal_rules); + let comment_rules = Arc::new(proposal_comment_rules); + let action_rules = Arc::new(proposal_submission_action_rules); + + document_rules_map.insert(PROPOSAL.clone(), Arc::clone(&proposal_rules)); + document_rules_map.insert(PROPOSAL_COMMENT.clone(), Arc::clone(&comment_rules)); document_rules_map.insert( PROPOSAL_SUBMISSION_ACTION.clone(), - proposal_submission_action_rules, + Arc::clone(&action_rules), + ); + + // Insert old rules (for backward compatibility) + document_rules_map.insert( + expect_doc_type(deprecated::COMMENT_DOCUMENT_UUID_TYPE), + Arc::clone(&comment_rules), + ); + document_rules_map.insert( + expect_doc_type(deprecated::PROPOSAL_ACTION_DOCUMENT_UUID_TYPE), + Arc::clone(&action_rules), ); document_rules_map diff --git a/rust/signed_doc/src/validator/rules/doc_ref.rs b/rust/signed_doc/src/validator/rules/doc_ref.rs index 977893b0614..7dcf6938fd2 100644 --- a/rust/signed_doc/src/validator/rules/doc_ref.rs +++ b/rust/signed_doc/src/validator/rules/doc_ref.rs @@ -3,8 +3,8 @@ use catalyst_types::problem_report::ProblemReport; use crate::{ - metadata::DocType, providers::CatalystSignedDocumentProvider, - validator::utils::validate_provided_doc, CatalystSignedDocument, + providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + CatalystSignedDocument, DocType, }; /// `ref` field validation rule @@ -26,6 +26,7 @@ impl RefRule { &self, doc: &CatalystSignedDocument, provider: &Provider, ) -> anyhow::Result where Provider: CatalystSignedDocumentProvider { + let context: &str = "Ref rule check"; if let Self::Specified { exp_ref_type, optional, @@ -35,11 +36,17 @@ impl RefRule { let ref_validator = |ref_doc: CatalystSignedDocument| { referenced_doc_check(&ref_doc, exp_ref_type, "ref", doc.report()) }; - return validate_provided_doc(&doc_ref, provider, doc.report(), ref_validator) - .await; + for dr in doc_ref.doc_refs() { + let result = + validate_provided_doc(dr, provider, doc.report(), ref_validator).await?; + // Reference ALL of them + if !result { + return Ok(false); + } + } + return Ok(true); } else if !optional { - doc.report() - .missing_field("ref", "Document must have a ref field"); + doc.report().missing_field("ref", context); return Ok(false); } } @@ -48,7 +55,7 @@ impl RefRule { doc.report().unknown_field( "ref", &doc_ref.to_string(), - "Document does not expect to have a ref field", + &format!("{context}, document does not expect to have a ref field"), ); return Ok(false); } @@ -67,11 +74,12 @@ pub(crate) fn referenced_doc_check( report.missing_field("type", "Referenced document must have type field"); return false; }; + if ref_doc_type != exp_ref_type { report.invalid_value( field_name, - ref_doc_type.to_string().as_str(), - exp_ref_type.to_string().as_str(), + &ref_doc_type.to_string(), + &exp_ref_type.to_string(), "Invalid referenced document type", ); return false; @@ -80,11 +88,14 @@ pub(crate) fn referenced_doc_check( } #[cfg(test)] +#[allow(clippy::similar_names, clippy::too_many_lines)] mod tests { use catalyst_types::uuid::{UuidV4, UuidV7}; use super::*; - use crate::{providers::tests::TestCatalystSignedDocumentProvider, Builder}; + use crate::{ + providers::tests::TestCatalystSignedDocumentProvider, Builder, DocLocator, DocumentRef, + }; #[tokio::test] async fn ref_rule_specified_test() { @@ -94,14 +105,17 @@ mod tests { let valid_referenced_doc_id = UuidV7::new(); let valid_referenced_doc_ver = UuidV7::new(); + let different_id_and_ver_referenced_doc_id = UuidV7::new(); + let different_id_and_ver_referenced_doc_ver = UuidV7::new(); let another_type_referenced_doc_id = UuidV7::new(); let another_type_referenced_doc_ver = UuidV7::new(); let missing_type_referenced_doc_id = UuidV7::new(); let missing_type_referenced_doc_ver = UuidV7::new(); - // prepare replied documents + // Prepare provider documents { - let ref_doc = Builder::new() + // Valid one + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": valid_referenced_doc_id.to_string(), "ver": valid_referenced_doc_ver.to_string(), @@ -109,10 +123,22 @@ mod tests { })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); + + // Having different id and ver in registered reference + let doc_ref = DocumentRef::new(UuidV7::new(), UuidV7::new(), DocLocator::default()); + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "id": different_id_and_ver_referenced_doc_id.to_string(), + "ver": different_id_and_ver_referenced_doc_ver.to_string(), + "type": exp_ref_type.to_string(), + })) + .unwrap() + .build(); + provider.add_document(Some(doc_ref), &doc).unwrap(); - // reply doc with other `type` field - let ref_doc = Builder::new() + // Having another `type` field + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": another_type_referenced_doc_id.to_string(), "ver": another_type_referenced_doc_ver.to_string(), @@ -120,33 +146,59 @@ mod tests { })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); - // missing `type` field in the referenced document - let ref_doc = Builder::new() + // Missing `type` field in the referenced document + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": missing_type_referenced_doc_id.to_string(), "ver": missing_type_referenced_doc_ver.to_string(), })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); } - // all correct + // Create a document where `ref` field is required and referencing a valid document in + // provider. Using doc ref of new implementation. let rule = RefRule::Specified { exp_ref_type: exp_ref_type.into(), optional: false, }; let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "ref": {"id": valid_referenced_doc_id.to_string(), "ver": valid_referenced_doc_ver.to_string() } - })) + "ref": [{"id": valid_referenced_doc_id.to_string(), "ver":valid_referenced_doc_ver.to_string(), "cid": "0x" }]})) + .unwrap() + .build(); + assert!(rule.check(&doc, &provider).await.unwrap()); + + // Checking backward compatible + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "ref": {"id": valid_referenced_doc_id.to_string(), "ver":valid_referenced_doc_ver.to_string()}})) .unwrap() .build(); assert!(rule.check(&doc, &provider).await.unwrap()); - // all correct, `ref` field is missing, but its optional + // Having multiple refs, where one ref doc is not found. + // Checking match all of + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "ref": [{"id": valid_referenced_doc_id.to_string(), "ver":valid_referenced_doc_ver.to_string(), "cid": "0x" }, + {"id": different_id_and_ver_referenced_doc_id.to_string(), "ver":different_id_and_ver_referenced_doc_ver.to_string(), "cid": "0x" }]})) + .unwrap() + .build(); + assert!(!rule.check(&doc, &provider).await.unwrap()); + + // Invalid the ref doc id and ver doesn't match the id and ver in ref doc ref + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "ref": [{"id": different_id_and_ver_referenced_doc_id.to_string(), "ver":different_id_and_ver_referenced_doc_ver.to_string(), "cid": "0x" }]})) + .unwrap() + .build(); + assert!(!rule.check(&doc, &provider).await.unwrap()); + + // All correct, `ref` field is missing, but its optional let rule = RefRule::Specified { exp_ref_type: exp_ref_type.into(), optional: true, @@ -154,7 +206,7 @@ mod tests { let doc = Builder::new().build(); assert!(rule.check(&doc, &provider).await.unwrap()); - // missing `ref` field, but its required + // Missing `ref` field, but its required let rule = RefRule::Specified { exp_ref_type: exp_ref_type.into(), optional: false, @@ -162,20 +214,20 @@ mod tests { let doc = Builder::new().build(); assert!(!rule.check(&doc, &provider).await.unwrap()); - // reference to the document with another `type` field + // Reference to the document with another `type` field let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "ref": {"id": another_type_referenced_doc_id.to_string(), "ver": another_type_referenced_doc_ver.to_string() } - })) + "ref": {"id": another_type_referenced_doc_id.to_string(), "ver": +another_type_referenced_doc_ver.to_string() } })) .unwrap() .build(); assert!(!rule.check(&doc, &provider).await.unwrap()); - // missing `type` field in the referenced document + // Missing `type` field in the referenced document let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "ref": {"id": missing_type_referenced_doc_id.to_string(), "ver": missing_type_referenced_doc_ver.to_string() } - })) + "ref": {"id": missing_type_referenced_doc_id.to_string(), "ver": +missing_type_referenced_doc_ver.to_string() } })) .unwrap() .build(); assert!(!rule.check(&doc, &provider).await.unwrap()); @@ -183,8 +235,8 @@ mod tests { // cannot find a referenced document let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "ref": {"id": UuidV7::new().to_string(), "ver": UuidV7::new().to_string() } - })) + "ref": {"id": UuidV7::new().to_string(), "ver": +UuidV7::new().to_string() } })) .unwrap() .build(); assert!(!rule.check(&doc, &provider).await.unwrap()); @@ -201,7 +253,8 @@ mod tests { let ref_id = UuidV7::new(); let ref_ver = UuidV7::new(); let doc = Builder::new() - .with_json_metadata(serde_json::json!({"ref": {"id": ref_id.to_string(), "ver": ref_ver.to_string() } })) + .with_json_metadata(serde_json::json!({"ref": {"id": ref_id.to_string(), +"ver": ref_ver.to_string() } })) .unwrap() .build(); assert!(!rule.check(&doc, &provider).await.unwrap()); diff --git a/rust/signed_doc/src/validator/rules/mod.rs b/rust/signed_doc/src/validator/rules/mod.rs index 165dcb043ac..7efc6044b6c 100644 --- a/rust/signed_doc/src/validator/rules/mod.rs +++ b/rust/signed_doc/src/validator/rules/mod.rs @@ -8,6 +8,7 @@ use crate::{providers::CatalystSignedDocumentProvider, CatalystSignedDocument}; mod content_encoding; mod content_type; mod doc_ref; +mod param_link_ref; mod parameters; mod reply; mod section; @@ -17,6 +18,7 @@ mod template; pub(crate) use content_encoding::ContentEncodingRule; pub(crate) use content_type::ContentTypeRule; pub(crate) use doc_ref::RefRule; +pub(crate) use param_link_ref::{LinkField, ParameterLinkRefRule}; pub(crate) use parameters::ParametersRule; pub(crate) use reply::ReplyRule; pub(crate) use section::SectionRule; @@ -31,6 +33,7 @@ pub(crate) struct Rules { pub(crate) content_encoding: ContentEncodingRule, /// 'ref' field validation rule pub(crate) doc_ref: RefRule, + #[allow(dead_code)] /// document's content validation rule pub(crate) content: ContentRule, /// 'reply' field validation rule @@ -41,6 +44,8 @@ pub(crate) struct Rules { pub(crate) parameters: ParametersRule, /// `kid` field validation rule pub(crate) kid: SignatureKidRule, + /// Link reference rule + pub(crate) param_link_ref: ParameterLinkRefRule, } impl Rules { @@ -53,11 +58,11 @@ impl Rules { self.content_type.check(doc).boxed(), self.content_encoding.check(doc).boxed(), self.doc_ref.check(doc, provider).boxed(), - self.content.check(doc, provider).boxed(), self.reply.check(doc, provider).boxed(), self.section.check(doc).boxed(), self.parameters.check(doc, provider).boxed(), self.kid.check(doc).boxed(), + self.param_link_ref.check(doc, provider).boxed(), ]; let res = futures::future::join_all(rules) diff --git a/rust/signed_doc/src/validator/rules/param_link_ref.rs b/rust/signed_doc/src/validator/rules/param_link_ref.rs new file mode 100644 index 00000000000..7690d4a4669 --- /dev/null +++ b/rust/signed_doc/src/validator/rules/param_link_ref.rs @@ -0,0 +1,168 @@ +//! Parameter linked reference rule impl. + +use crate::{ + providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + CatalystSignedDocument, +}; + +/// Filed that is being used for linked ref +pub(crate) enum LinkField { + /// Ref field + Ref, + /// Template field + Template, +} + +/// Parameter Link reference validation rule +pub(crate) enum ParameterLinkRefRule { + /// Link ref specified + Specified { + /// Filed that is being used for linked ref + field: LinkField, + }, + /// Link ref is not specified + #[allow(dead_code)] + NotSpecified, +} + +impl ParameterLinkRefRule { + /// Validation rule + pub(crate) async fn check( + &self, doc: &CatalystSignedDocument, provider: &Provider, + ) -> anyhow::Result + where Provider: CatalystSignedDocumentProvider { + if let Self::Specified { field } = self { + let param_link_ref_validator = |ref_doc: CatalystSignedDocument| { + // The parameters MUST be the same + doc.doc_meta().parameters() == ref_doc.doc_meta().parameters() + }; + + // Which field is use for linked reference + let param_link_ref = match field { + LinkField::Ref => doc.doc_meta().doc_ref(), + LinkField::Template => doc.doc_meta().template(), + }; + + let Some(param_link_ref) = param_link_ref else { + doc.report() + .missing_field("Link ref", "Invalid link reference"); + return Ok(false); + }; + + for dr in param_link_ref.doc_refs() { + let result = + validate_provided_doc(dr, provider, doc.report(), param_link_ref_validator) + .await?; + if !result { + return Ok(false); + } + } + } + Ok(true) + } +} + +#[cfg(test)] +mod tests { + use catalyst_types::uuid::{UuidV4, UuidV7}; + + use crate::{ + providers::tests::TestCatalystSignedDocumentProvider, + validator::rules::param_link_ref::{LinkField, ParameterLinkRefRule}, + Builder, + }; + #[tokio::test] + async fn param_link_ref_specified_test() { + let mut provider = TestCatalystSignedDocumentProvider::default(); + + let doc1_id = UuidV7::new(); + let doc1_ver = UuidV7::new(); + let doc2_id = UuidV7::new(); + let doc2_ver = UuidV7::new(); + + let doc_type = UuidV4::new(); + + let category_id = UuidV7::new(); + let category_ver = UuidV7::new(); + let category_type = UuidV4::new(); + + let campaign_id = UuidV7::new(); + let campaign_ver = UuidV7::new(); + let campaign_type = UuidV4::new(); + + // Prepare provider documents + { + // Doc being referenced - parameter MUST match + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "id": doc1_id.to_string(), + "ver": doc1_ver.to_string(), + "type": doc_type.to_string(), + "parameters": [{"id": category_id.to_string(), "ver": category_ver.to_string(), "cid": "0x" }, {"id": campaign_id.to_string(), "ver": campaign_ver.to_string(), "cid": "0x" }] + })) + .unwrap() + .build(); + provider.add_document(None, &doc).unwrap(); + + // Doc being referenced - parameter does not match + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "id": doc2_id.to_string(), + "ver": doc2_ver.to_string(), + "type": doc_type.to_string(), + "parameters": [{"id": campaign_id.to_string(), "ver": campaign_ver.to_string(), "cid": "0x" }] + })) + .unwrap() + .build(); + provider.add_document(None, &doc).unwrap(); + + // Category doc + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "id": category_id.to_string(), + "ver": category_ver.to_string(), + "type": category_type.to_string(), + })) + .unwrap() + .build(); + provider.add_document(None, &doc).unwrap(); + + // Campaign doc + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "id": campaign_id.to_string(), + "ver": campaign_ver.to_string(), + "type": campaign_type.to_string(), + })) + .unwrap() + .build(); + provider.add_document(None, &doc).unwrap(); + } + + // Use Ref as a linked reference + let rule = ParameterLinkRefRule::Specified { + field: LinkField::Ref, + }; + // Parameter must match + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "ref": [{"id": doc1_id.to_string(), "ver": doc1_ver.to_string(), "cid": "0x" }], + "parameters": + [{"id": category_id.to_string(), "ver": category_ver.to_string(), "cid": "0x" }, {"id": campaign_id.to_string(), "ver": campaign_ver.to_string(), "cid": "0x" }] + })) + .unwrap() + .build(); + assert!(rule.check(&doc, &provider).await.unwrap()); + + // Parameter does not match + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "ref": {"id": doc2_id.to_string(), "ver": doc2_ver.to_string()}, + "parameters": + [{"id": category_id.to_string(), "ver": category_ver.to_string(), "cid": "0x" }, {"id": campaign_id.to_string(), "ver": campaign_ver.to_string(), "cid": "0x" }] + })) + .unwrap() + .build(); + assert!(!rule.check(&doc, &provider).await.unwrap()); + } +} diff --git a/rust/signed_doc/src/validator/rules/parameters.rs b/rust/signed_doc/src/validator/rules/parameters.rs index b18294ff68e..1e7784bf8f5 100644 --- a/rust/signed_doc/src/validator/rules/parameters.rs +++ b/rust/signed_doc/src/validator/rules/parameters.rs @@ -2,8 +2,8 @@ use super::doc_ref::referenced_doc_check; use crate::{ - metadata::DocType, providers::CatalystSignedDocumentProvider, - validator::utils::validate_provided_doc, CatalystSignedDocument, + providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + CatalystSignedDocument, DocType, }; /// `parameters` field validation rule @@ -17,6 +17,7 @@ pub(crate) enum ParametersRule { optional: bool, }, /// `parameters` is not specified + #[allow(unused)] NotSpecified, } @@ -26,28 +27,31 @@ impl ParametersRule { &self, doc: &CatalystSignedDocument, provider: &Provider, ) -> anyhow::Result where Provider: CatalystSignedDocumentProvider { + let context: &str = "Parameter rule check"; if let Self::Specified { exp_parameters_type, optional, } = self { if let Some(parameters) = doc.doc_meta().parameters() { - let parameters_validator = |replied_doc: CatalystSignedDocument| { - // For "parameters", it should be one of these types + let parameters_validator = |ref_doc: CatalystSignedDocument| { + // Check that the type matches one of the expected ones exp_parameters_type.iter().any(|exp_type| { - referenced_doc_check(&replied_doc, exp_type, "parameters", doc.report()) + referenced_doc_check(&ref_doc, exp_type, "parameters", doc.report()) }) }; - return validate_provided_doc( - ¶meters, - provider, - doc.report(), - parameters_validator, - ) - .await; + for dr in parameters.doc_refs() { + let result = + validate_provided_doc(dr, provider, doc.report(), parameters_validator) + .await?; + // Reference ALL of them + if !result { + return Ok(false); + } + } + return Ok(true); } else if !optional { - doc.report() - .missing_field("parameters", "Document must have a parameters field"); + doc.report().missing_field("parameters", context); return Ok(false); } } @@ -56,7 +60,7 @@ impl ParametersRule { doc.report().unknown_field( "parameters", ¶meters.to_string(), - "Document does not expect to have a parameters field", + &format!("{context}, document does not expect to have a parameters field"), ); return Ok(false); } @@ -67,6 +71,7 @@ impl ParametersRule { } #[cfg(test)] +#[allow(clippy::similar_names, clippy::too_many_lines)] mod tests { use catalyst_types::uuid::{UuidV4, UuidV7}; @@ -77,35 +82,51 @@ mod tests { async fn ref_rule_specified_test() { let mut provider = TestCatalystSignedDocumentProvider::default(); - // Parameter types can contains multiple, but should match one of it. - let exp_parameters_type: Vec = vec![vec![UuidV4::new()].try_into().unwrap()]; + let exp_parameters_cat_type = UuidV4::new(); + let exp_parameters_cam_type = UuidV4::new(); + let exp_parameters_brand_type = UuidV4::new(); + + let exp_param_type: Vec = vec![ + exp_parameters_cat_type.into(), + exp_parameters_cam_type.into(), + exp_parameters_brand_type.into(), + ]; let valid_category_doc_id = UuidV7::new(); let valid_category_doc_ver = UuidV7::new(); + let valid_brand_doc_id = UuidV7::new(); + let valid_brand_doc_ver = UuidV7::new(); let another_type_category_doc_id = UuidV7::new(); let another_type_category_doc_ver = UuidV7::new(); let missing_type_category_doc_id = UuidV7::new(); let missing_type_category_doc_ver = UuidV7::new(); - // prepare replied documents + // Prepare provider documents { - let exp_parameters_type_str: Vec = exp_parameters_type - .iter() - .map(|dt| dt.to_string()) - .collect(); - - let ref_doc = Builder::new() + // Category doc + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": valid_category_doc_id.to_string(), "ver": valid_category_doc_ver.to_string(), - "type": exp_parameters_type_str, + "type": exp_parameters_cat_type.to_string(), + })) + .unwrap() + .build(); + provider.add_document(None, &doc).unwrap(); + + // Brand doc + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "id": valid_brand_doc_id.to_string(), + "ver": valid_brand_doc_ver.to_string(), + "type": exp_parameters_cat_type.to_string(), })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); - // reply doc with other `type` field - let ref_doc = Builder::new() + // Other type + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": another_type_category_doc_id.to_string(), "ver": another_type_category_doc_ver.to_string(), @@ -113,49 +134,83 @@ mod tests { })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); - // missing `type` field in the referenced document - let ref_doc = Builder::new() + // Missing `type` field in the referenced document + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": missing_type_category_doc_id.to_string(), "ver": missing_type_category_doc_ver.to_string(), })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); } - // all correct + // Create a document where `parameters` field is required and referencing a valid document + // in provider. Using doc ref of new implementation. let rule = ParametersRule::Specified { - exp_parameters_type: exp_parameters_type.clone().into(), + exp_parameters_type: exp_param_type.clone(), optional: false, }; let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "parameters": {"id": valid_category_doc_id.to_string(), "ver": valid_category_doc_ver } + "parameters": + [{"id": valid_category_doc_id.to_string(), "ver": valid_category_doc_ver.to_string(), "cid": "0x" }] + })) + .unwrap() + .build(); + assert!(rule.check(&doc, &provider).await.unwrap()); + + // Parameters contain multiple ref + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "parameters": + [{"id": valid_category_doc_id.to_string(), "ver": valid_category_doc_ver.to_string(), "cid": "0x" }, + {"id": valid_brand_doc_id.to_string(), "ver": valid_brand_doc_ver.to_string(), "cid": "0x" }] + })) + .unwrap() + .build(); + assert!(rule.check(&doc, &provider).await.unwrap()); + + // Parameters contain multiple ref, but one of them is invalid (not registered). + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "parameters": + [{"id": valid_category_doc_id.to_string(), "ver": valid_category_doc_ver.to_string(), "cid": "0x" }, + {"id": UuidV7::new().to_string() , "ver": UuidV7::new().to_string(), "cid": "0x" }] + })) + .unwrap() + .build(); + assert!(!rule.check(&doc, &provider).await.unwrap()); + + // Checking backward compatible + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "parameters": + {"id": valid_category_doc_id.to_string(), "ver": valid_category_doc_ver.to_string()} })) .unwrap() .build(); assert!(rule.check(&doc, &provider).await.unwrap()); - // all correct, `parameters` field is missing, but its optional + // All correct, `parameters` field is missing, but its optional let rule = ParametersRule::Specified { - exp_parameters_type: exp_parameters_type.clone().into(), + exp_parameters_type: exp_param_type.clone(), optional: true, }; let doc = Builder::new().build(); assert!(rule.check(&doc, &provider).await.unwrap()); - // missing `parameters` field, but its required + // Missing `parameters` field, but its required let rule = ParametersRule::Specified { - exp_parameters_type: exp_parameters_type.into(), + exp_parameters_type: exp_param_type, optional: false, }; let doc = Builder::new().build(); assert!(!rule.check(&doc, &provider).await.unwrap()); - // reference to the document with another `type` field + // Reference to the document with another `type` field let doc = Builder::new() .with_json_metadata(serde_json::json!({ "parameters": {"id": another_type_category_doc_id.to_string(), "ver": another_type_category_doc_ver.to_string() } @@ -164,7 +219,7 @@ mod tests { .build(); assert!(!rule.check(&doc, &provider).await.unwrap()); - // missing `type` field in the referenced document + // Missing `type` field in the referenced document let doc = Builder::new() .with_json_metadata(serde_json::json!({ "parameters": {"id": missing_type_category_doc_id.to_string(), "ver": missing_type_category_doc_ver.to_string() } @@ -173,7 +228,7 @@ mod tests { .build(); assert!(!rule.check(&doc, &provider).await.unwrap()); - // cannot find a referenced document + // Cannot find a referenced document let doc = Builder::new() .with_json_metadata(serde_json::json!({ "parameters": {"id": UuidV7::new().to_string(), "ver": UuidV7::new().to_string() } diff --git a/rust/signed_doc/src/validator/rules/reply.rs b/rust/signed_doc/src/validator/rules/reply.rs index 09b48ea28d9..be846af9343 100644 --- a/rust/signed_doc/src/validator/rules/reply.rs +++ b/rust/signed_doc/src/validator/rules/reply.rs @@ -2,8 +2,8 @@ use super::doc_ref::referenced_doc_check; use crate::{ - metadata::DocType, providers::CatalystSignedDocumentProvider, - validator::utils::validate_provided_doc, CatalystSignedDocument, + providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + CatalystSignedDocument, DocType, }; /// `reply` field validation rule @@ -26,45 +26,45 @@ impl ReplyRule { &self, doc: &CatalystSignedDocument, provider: &Provider, ) -> anyhow::Result where Provider: CatalystSignedDocumentProvider { + let context: &str = "Reply rule check"; if let Self::Specified { exp_reply_type, optional, } = self { if let Some(reply) = doc.doc_meta().reply() { - let reply_validator = |replied_doc: CatalystSignedDocument| { - if !referenced_doc_check(&replied_doc, exp_reply_type, "reply", doc.report()) { + let reply_validator = |ref_doc: CatalystSignedDocument| { + // Validate type + if !referenced_doc_check(&ref_doc, exp_reply_type, "reply", doc.report()) { return false; } - let Some(doc_ref) = doc.doc_meta().doc_ref() else { - doc.report() - .missing_field("ref", "Document must have a ref field"); - return false; - }; - let Some(replied_doc_ref) = replied_doc.doc_meta().doc_ref() else { - doc.report() - .missing_field("ref", "Referenced document must have ref field"); + // Get `ref` from both the doc and the ref doc + let Some(ref_doc_dr) = ref_doc.doc_meta().doc_ref() else { + doc.report().missing_field("Ref doc `ref` field", context); return false; }; - if replied_doc_ref.id != doc_ref.id { - doc.report().invalid_value( - "reply", - doc_ref.id .to_string().as_str(), - replied_doc_ref.id.to_string().as_str(), - "Invalid referenced document. Document ID should aligned with the replied document.", - ); + let Some(doc_dr) = doc.doc_meta().doc_ref() else { + doc.report().missing_field("Doc `ref` field", context); return false; - } + }; - true + // Checking the ref field of ref doc, it should match the ref field of the doc + ref_doc_dr == doc_dr }; - return validate_provided_doc(&reply, provider, doc.report(), reply_validator) - .await; + + for dr in reply.doc_refs() { + let result = + validate_provided_doc(dr, provider, doc.report(), reply_validator).await?; + // Reference ALL of them + if !result { + return Ok(false); + }; + } + return Ok(true); } else if !optional { - doc.report() - .missing_field("reply", "Document must have a reply field"); + doc.report().missing_field("reply", context); return Ok(false); } } @@ -73,7 +73,7 @@ impl ReplyRule { doc.report().unknown_field( "reply", &reply.to_string(), - "Document does not expect to have a reply field", + &format!("{context}, document does not expect to have a reply field"), ); return Ok(false); } @@ -99,53 +99,45 @@ mod tests { let common_ref_id = UuidV7::new(); let common_ref_ver = UuidV7::new(); + let doc1_id = UuidV7::new(); + let doc1_ver = UuidV7::new(); + let doc2_id = UuidV7::new(); + let doc2_ver = UuidV7::new(); + let valid_replied_doc_id = UuidV7::new(); let valid_replied_doc_ver = UuidV7::new(); let another_type_replied_doc_ver = UuidV7::new(); let another_type_replied_doc_id = UuidV7::new(); - let missing_ref_replied_doc_ver = UuidV7::new(); let missing_ref_replied_doc_id = UuidV7::new(); let missing_type_replied_doc_ver = UuidV7::new(); let missing_type_replied_doc_id = UuidV7::new(); - // prepare replied documents + // Prepare provider documents { - let ref_doc = Builder::new() + let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "ref": { "id": common_ref_id.to_string(), "ver": common_ref_ver.to_string() }, - "id": valid_replied_doc_id.to_string(), - "ver": valid_replied_doc_ver.to_string(), - "type": exp_reply_type.to_string() + "id": doc1_id.to_string(), + "ver": doc1_ver.to_string(), + "type": exp_reply_type.to_string(), + "ref": { "id": doc2_id.to_string(), "ver": doc2_ver.to_string(), "cid": "0x" } })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); - // reply doc with other `type` field - let ref_doc = Builder::new() + // Reply doc with other `type` field + let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "ref": { "id": common_ref_id.to_string(), "ver": common_ref_ver.to_string() }, "id": another_type_replied_doc_id.to_string(), "ver": another_type_replied_doc_ver.to_string(), "type": UuidV4::new().to_string() })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); - // missing `ref` field in the referenced document - let ref_doc = Builder::new() - .with_json_metadata(serde_json::json!({ - "id": missing_ref_replied_doc_id.to_string(), - "ver": missing_ref_replied_doc_ver.to_string(), - "type": exp_reply_type.to_string() - })) - .unwrap() - .build(); - provider.add_document(ref_doc).unwrap(); - - // missing `type` field in the referenced document - let ref_doc = Builder::new() + // Missing `type` field in the referenced document + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "ref": { "id": common_ref_id.to_string(), "ver": common_ref_ver.to_string() }, "id": missing_type_replied_doc_id.to_string(), @@ -153,18 +145,21 @@ mod tests { })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); } - // all correct + // Create a document where `reply` field is required and referencing a valid document in + // provider. let rule = ReplyRule::Specified { exp_reply_type: exp_reply_type.into(), optional: false, }; + + // Doc1 ref reply to doc2. Doc1 ref filed should match doc2 ref field let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "ref": { "id": common_ref_id.to_string(), "ver": common_ref_ver.to_string() }, - "reply": { "id": valid_replied_doc_id.to_string(), "ver": valid_replied_doc_ver.to_string() } + "ref": { "id": doc2_id.to_string(), "ver": doc2_ver.to_string() }, + "reply": { "id": doc1_id.to_string(), "ver": doc1_ver.to_string() } })) .unwrap() .build(); diff --git a/rust/signed_doc/src/validator/rules/section.rs b/rust/signed_doc/src/validator/rules/section.rs index 87203534254..b31b7fe0fb5 100644 --- a/rust/signed_doc/src/validator/rules/section.rs +++ b/rust/signed_doc/src/validator/rules/section.rs @@ -5,6 +5,7 @@ use crate::CatalystSignedDocument; /// `section` field validation rule pub(crate) enum SectionRule { /// Is 'section' specified + #[allow(dead_code)] Specified { /// optional flag for the `section` field optional: bool, diff --git a/rust/signed_doc/src/validator/rules/template.rs b/rust/signed_doc/src/validator/rules/template.rs index 13fea3b5449..56368fb0f5e 100644 --- a/rust/signed_doc/src/validator/rules/template.rs +++ b/rust/signed_doc/src/validator/rules/template.rs @@ -4,10 +4,8 @@ use std::fmt::Write; use super::doc_ref::referenced_doc_check; use crate::{ - metadata::{ContentType, DocType}, - providers::CatalystSignedDocumentProvider, - validator::utils::validate_provided_doc, - CatalystSignedDocument, + metadata::ContentType, providers::CatalystSignedDocumentProvider, + validator::utils::validate_provided_doc, CatalystSignedDocument, DocType, }; /// Enum represents different content schemas, against which documents content would be @@ -21,6 +19,7 @@ pub(crate) enum ContentSchema { /// Document's content validation rule pub(crate) enum ContentRule { /// Based on the 'template' field and loaded corresponding template document + #[allow(dead_code)] Templated { /// expected `type` field of the template exp_template_type: DocType, @@ -36,27 +35,27 @@ pub(crate) enum ContentRule { impl ContentRule { /// Field validation rule + #[allow(dead_code)] pub(crate) async fn check( &self, doc: &CatalystSignedDocument, provider: &Provider, ) -> anyhow::Result where Provider: CatalystSignedDocumentProvider { + let context = "Content/Template rule check"; if let Self::Templated { exp_template_type } = self { let Some(template_ref) = doc.doc_meta().template() else { doc.report() - .missing_field("template", "Document must have a template field"); + .missing_field("template", &format!("{context}, doc")); return Ok(false); }; - let template_validator = |template_doc: CatalystSignedDocument| { if !referenced_doc_check(&template_doc, exp_template_type, "template", doc.report()) { return false; } - let Ok(template_content_type) = template_doc.doc_content_type() else { doc.report().missing_field( "content-type", - "Referenced template document must have a content-type field", + &format!("{context}, referenced document must have a content-type field"), ); return false; }; @@ -68,20 +67,21 @@ impl ContentRule { }, } }; - return validate_provided_doc( - &template_ref, - provider, - doc.report(), - template_validator, - ) - .await; + for dr in template_ref.doc_refs() { + let result = + validate_provided_doc(dr, provider, doc.report(), template_validator).await?; + if !result { + return Ok(false); + } + } + return Ok(true); } if let Self::Static(content_schema) = self { if let Some(template) = doc.doc_meta().template() { doc.report().unknown_field( "template", &template.to_string(), - "Document does not expect to have a template field", + &format!("{context} Static, Document does not expect to have a template field",) ); return Ok(false); } @@ -93,7 +93,7 @@ impl ContentRule { doc.report().unknown_field( "template", &template.to_string(), - "Document does not expect to have a template field", + &format!("{context} Not Specified, Document does not expect to have a template field",) ); return Ok(false); } @@ -135,7 +135,7 @@ fn templated_json_schema_check( content_schema_check(doc, &ContentSchema::Json(schema_validator)) } - +#[allow(dead_code)] /// Validating the document's content against the provided schema fn content_schema_check(doc: &CatalystSignedDocument, schema: &ContentSchema) -> bool { let Ok(doc_content) = doc.doc_content().decoded_bytes() else { @@ -199,9 +199,9 @@ mod tests { let missing_content_template_doc_id = UuidV7::new(); let invalid_content_template_doc_id = UuidV7::new(); - // prepare replied documents + // Prepare provider documents { - let ref_doc = Builder::new() + let doc = Builder::new() .with_json_metadata(serde_json::json!({ "id": valid_template_doc_id.to_string(), "ver": valid_template_doc_id.to_string(), @@ -211,7 +211,7 @@ mod tests { .unwrap() .with_decoded_content(json_schema.clone()) .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &doc).unwrap(); // reply doc with other `type` field let ref_doc = Builder::new() @@ -224,7 +224,7 @@ mod tests { .unwrap() .with_decoded_content(json_schema.clone()) .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &ref_doc).unwrap(); // missing `type` field in the referenced document let ref_doc = Builder::new() @@ -236,7 +236,7 @@ mod tests { .unwrap() .with_decoded_content(json_schema.clone()) .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &ref_doc).unwrap(); // missing `content-type` field in the referenced document let ref_doc = Builder::new() @@ -248,7 +248,7 @@ mod tests { .unwrap() .with_decoded_content(json_schema.clone()) .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &ref_doc).unwrap(); // missing content let ref_doc = Builder::new() @@ -260,7 +260,7 @@ mod tests { })) .unwrap() .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &ref_doc).unwrap(); // invalid content, must be json encoded let ref_doc = Builder::new() @@ -273,16 +273,27 @@ mod tests { .unwrap() .with_decoded_content(vec![]) .build(); - provider.add_document(ref_doc).unwrap(); + provider.add_document(None, &ref_doc).unwrap(); } - // all correct + // Create a document where `templates` field is required and referencing a valid document + // in provider. Using doc ref of new implementation. let rule = ContentRule::Templated { exp_template_type: exp_template_type.into(), }; let doc = Builder::new() .with_json_metadata(serde_json::json!({ - "template": {"id": valid_template_doc_id.to_string(), "ver": valid_template_doc_id.to_string() } + "template": [{"id": valid_template_doc_id.to_string(), "ver": valid_template_doc_id.to_string(), "cid": "0x" }] + })) + .unwrap() + .with_decoded_content(json_content.clone()) + .build(); + assert!(rule.check(&doc, &provider).await.unwrap()); + + // Checking backward compatible + let doc = Builder::new() + .with_json_metadata(serde_json::json!({ + "template": {"id": valid_template_doc_id.to_string(), "ver": valid_template_doc_id.to_string()} })) .unwrap() .with_decoded_content(json_content.clone()) diff --git a/rust/signed_doc/src/validator/utils.rs b/rust/signed_doc/src/validator/utils.rs index 4b25a9bbfaf..79ef5d2e9ae 100644 --- a/rust/signed_doc/src/validator/utils.rs +++ b/rust/signed_doc/src/validator/utils.rs @@ -13,12 +13,35 @@ where Provider: CatalystSignedDocumentProvider, Validator: Fn(CatalystSignedDocument) -> bool, { + const CONTEXT: &str = "Validation data provider"; + + // General check for document ref + + // Getting the Signed Document instance from a doc ref. + // The reference document must exist if let Some(doc) = provider.try_get_doc(doc_ref).await? { + let id = doc + .doc_id() + .inspect_err(|_| report.missing_field("id", CONTEXT))?; + + let ver = doc + .doc_ver() + .inspect_err(|_| report.missing_field("ver", CONTEXT))?; + // id and version must match the values in ref doc + if &id != doc_ref.id() && &ver != doc_ref.ver() { + report.invalid_value( + "id and version", + &format!("id: {id}, ver: {ver}"), + &format!("id: {}, ver: {}", doc_ref.id(), doc_ref.ver()), + CONTEXT, + ); + return Ok(false); + } Ok(validator(doc)) } else { report.functional_validation( format!("Cannot retrieve a document {doc_ref}").as_str(), - "Validation data provider could not return a corresponding document.", + CONTEXT, ); Ok(false) } diff --git a/rust/signed_doc/tests/comment.rs b/rust/signed_doc/tests/comment.rs index 5725e3080c7..1f80ef17f27 100644 --- a/rust/signed_doc/tests/comment.rs +++ b/rust/signed_doc/tests/comment.rs @@ -1,169 +1,229 @@ -//! Integration test for comment document validation part. +//! Test for Proposal Comment document. +//! Require fields: type, id, ver, ref, template, parameters +//! -use catalyst_signed_doc::{providers::tests::TestCatalystSignedDocumentProvider, *}; +use catalyst_signed_doc::{ + doc_types::deprecated, providers::tests::TestCatalystSignedDocumentProvider, *, +}; use catalyst_types::catalyst_id::role_index::RoleId; mod common; +// Given a proposal comment document `doc`: +// +// - Parameters: +// The `parameters` field in `doc` points to a brand document. +// The parameter rule defines the link reference as `template`, This mean the document +// that `ref` field in `doc` points to (in this case = template_doc), must have the same +// `parameters` value as `doc`. +// +// - Reply: +// The `reply` field in `doc` points to another comment (`ref_doc`). +// The rule requires that the `ref` field in `ref_doc` must match the `ref` field in `doc` #[tokio::test] async fn test_valid_comment_doc() { let (proposal_doc, proposal_doc_id, proposal_doc_ver) = - common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); - let (template_doc, template_doc_id, template_doc_ver) = - common::create_dummy_doc(doc_types::deprecated::COMMENT_TEMPLATE_UUID_TYPE).unwrap(); + common::create_dummy_doc(&doc_types::PROPOSAL.clone()).unwrap(); + let (brand_doc, brand_doc_id, brand_doc_ver) = + common::create_dummy_doc(&doc_types::BRAND_PARAMETERS.clone()).unwrap(); - let uuid_v7 = UuidV7::new(); - let (doc, ..) = common::create_dummy_signed_doc( - serde_json::json!({ + let ref_doc_id = UuidV7::new(); + let ref_doc_ver = UuidV7::new(); + + let template_doc_id = UuidV7::new(); + let template_doc_ver = UuidV7::new(); + + // Create a ref document, which is a proposal comment type + let (ref_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_COMMENT_DOC.clone(), - "id": uuid_v7.to_string(), - "ver": uuid_v7.to_string(), + "type": doc_types::PROPOSAL_COMMENT.clone(), + "id": ref_doc_id.to_string(), + "ver": ref_doc_ver.to_string(), + "ref": { + "id": proposal_doc_id, + "ver": proposal_doc_ver + }, "template": { "id": template_doc_id, "ver": template_doc_ver }, + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver }})), + serde_json::to_vec(&serde_json::Value::Null).unwrap(), + RoleId::Role0, + ) + .unwrap(); + + // Create a template document + let (template_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ + "content-type": ContentType::Json.to_string(), + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": doc_types::PROPOSAL_TEMPLATE.clone(), + "id": template_doc_id.to_string(), + "ver": template_doc_ver.to_string(), + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + })), + serde_json::to_vec(&serde_json::Value::Null).unwrap(), + RoleId::Role0, + ) + .unwrap(); + + let comment_doc_id = UuidV7::new(); + let comment_doc_ver = UuidV7::new(); + // Create a main comment doc, contain all fields mention in the document (except + // revocations and section) + let (doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ + "content-type": ContentType::Json.to_string(), + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": doc_types::PROPOSAL_COMMENT.clone(), + "id": comment_doc_id.to_string(), + "ver": comment_doc_ver.to_string(), "ref": { "id": proposal_doc_id, "ver": proposal_doc_ver - } - }), + }, + "template": { + "id": template_doc_id, + "ver": template_doc_ver + }, + "reply": { + "id": ref_doc_id, + "ver": ref_doc_ver + }, + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + })), serde_json::to_vec(&serde_json::Value::Null).unwrap(), RoleId::Role0, ) .unwrap(); - let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(template_doc).unwrap(); - provider.add_document(proposal_doc).unwrap(); - let is_valid = validator::validate(&doc, &provider).await.unwrap(); + provider.add_document(None, &brand_doc).unwrap(); + provider.add_document(None, &proposal_doc).unwrap(); + provider.add_document(None, &ref_doc).unwrap(); + provider.add_document(None, &template_doc).unwrap(); - assert!(is_valid); + let is_valid = validator::validate(&doc, &provider).await.unwrap(); + assert!(is_valid, "{:?}", doc.problem_report()); } +// The same as above but test with the old type #[tokio::test] async fn test_valid_comment_doc_old_type() { let (proposal_doc, proposal_doc_id, proposal_doc_ver) = - common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); - let (template_doc, template_doc_id, template_doc_ver) = - common::create_dummy_doc(doc_types::deprecated::COMMENT_TEMPLATE_UUID_TYPE).unwrap(); + common::create_dummy_doc(&deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.try_into().unwrap()) + .unwrap(); + let (brand_doc, brand_doc_id, brand_doc_ver) = + common::create_dummy_doc(&doc_types::BRAND_PARAMETERS.clone()).unwrap(); - let uuid_v7 = UuidV7::new(); - let (doc, ..) = common::create_dummy_signed_doc( - serde_json::json!({ + let ref_doc_id = UuidV7::new(); + let ref_doc_ver = UuidV7::new(); + + let template_doc_id = UuidV7::new(); + let template_doc_ver = UuidV7::new(); + + // Create a ref document, which is a proposal comment type + let (ref_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - // Using old (single uuid) - "type": doc_types::deprecated::COMMENT_DOCUMENT_UUID_TYPE, - "id": uuid_v7.to_string(), - "ver": uuid_v7.to_string(), + "type": deprecated::COMMENT_DOCUMENT_UUID_TYPE, + "id": ref_doc_id.to_string(), + "ver": ref_doc_ver.to_string(), + "ref": { + "id": proposal_doc_id, + "ver": proposal_doc_ver + }, "template": { "id": template_doc_id, "ver": template_doc_ver }, - "ref": { - "id": proposal_doc_id, - "ver": proposal_doc_ver - } - }), + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver }})), serde_json::to_vec(&serde_json::Value::Null).unwrap(), RoleId::Role0, ) .unwrap(); - let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(template_doc).unwrap(); - provider.add_document(proposal_doc).unwrap(); - - let is_valid = validator::validate(&doc, &provider).await.unwrap(); - - assert!(is_valid); -} - -#[tokio::test] -async fn test_valid_comment_doc_with_reply() { - let empty_json = serde_json::to_vec(&serde_json::json!({})).unwrap(); - - let (proposal_doc, proposal_doc_id, proposal_doc_ver) = - common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); - let (template_doc, template_doc_id, template_doc_ver) = - common::create_dummy_doc(doc_types::deprecated::COMMENT_TEMPLATE_UUID_TYPE).unwrap(); + // Create a template document + let (template_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ + "content-type": ContentType::Json.to_string(), + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": doc_types::PROPOSAL_TEMPLATE.clone(), + "id": template_doc_id.to_string(), + "ver": template_doc_ver.to_string(), + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + })), + serde_json::to_vec(&serde_json::Value::Null).unwrap(), + RoleId::Role0, + ) + .unwrap(); let comment_doc_id = UuidV7::new(); let comment_doc_ver = UuidV7::new(); - let comment_doc = Builder::new() - .with_json_metadata(serde_json::json!({ - "id": comment_doc_id, - "ver": comment_doc_ver, - "type": doc_types::PROPOSAL_COMMENT_DOC.clone(), + // Create a main comment doc, Contain all fields mention in the document (except + // revocations and section) + let (doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ "content-type": ContentType::Json.to_string(), - "template": { "id": template_doc_id.to_string(), "ver": template_doc_ver.to_string() }, + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": deprecated::COMMENT_DOCUMENT_UUID_TYPE.clone(), + "id": comment_doc_id.to_string(), + "ver": comment_doc_ver.to_string(), "ref": { "id": proposal_doc_id, "ver": proposal_doc_ver }, - })) - .unwrap() - .with_decoded_content(empty_json.clone()) - .build(); - - let uuid_v7 = UuidV7::new(); - let (doc, ..) = common::create_dummy_signed_doc( - serde_json::json!({ - "content-type": ContentType::Json.to_string(), - "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_COMMENT_DOC.clone(), - "id": uuid_v7.to_string(), - "ver": uuid_v7.to_string(), "template": { "id": template_doc_id, "ver": template_doc_ver }, - "ref": { - "id": proposal_doc_id, - "ver": proposal_doc_ver - }, "reply": { - "id": comment_doc_id, - "ver": comment_doc_ver - } - }), + "id": ref_doc_id, + "ver": ref_doc_ver + }, + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + })), serde_json::to_vec(&serde_json::Value::Null).unwrap(), RoleId::Role0, ) .unwrap(); - let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(template_doc).unwrap(); - provider.add_document(proposal_doc).unwrap(); - provider.add_document(comment_doc).unwrap(); - let is_valid = validator::validate(&doc, &provider).await.unwrap(); + provider.add_document(None, &brand_doc).unwrap(); + provider.add_document(None, &proposal_doc).unwrap(); + provider.add_document(None, &ref_doc).unwrap(); + provider.add_document(None, &template_doc).unwrap(); - assert!(is_valid); + let is_valid = validator::validate(&doc, &provider).await.unwrap(); + assert!(is_valid, "{:?}", doc.problem_report()); } #[tokio::test] async fn test_invalid_comment_doc() { - let (proposal_doc, ..) = common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); + let (proposal_doc, ..) = + common::create_dummy_doc(&deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.try_into().unwrap()) + .unwrap(); let (template_doc, template_doc_id, template_doc_ver) = - common::create_dummy_doc(doc_types::deprecated::COMMENT_TEMPLATE_UUID_TYPE).unwrap(); + common::create_dummy_doc(&deprecated::COMMENT_TEMPLATE_UUID_TYPE.try_into().unwrap()) + .unwrap(); let uuid_v7 = UuidV7::new(); + // Missing parameters field let (doc, ..) = common::create_dummy_signed_doc( serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_COMMENT_DOC.clone(), + "type": doc_types::PROPOSAL_COMMENT.clone(), "id": uuid_v7.to_string(), "ver": uuid_v7.to_string(), "template": { "id": template_doc_id, "ver": template_doc_ver }, - // without ref "ref": serde_json::Value::Null }), serde_json::to_vec(&serde_json::Value::Null).unwrap(), @@ -172,8 +232,8 @@ async fn test_invalid_comment_doc() { .unwrap(); let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(template_doc).unwrap(); - provider.add_document(proposal_doc).unwrap(); + provider.add_document(None, &template_doc).unwrap(); + provider.add_document(None, &proposal_doc).unwrap(); let is_valid = validator::validate(&doc, &provider).await.unwrap(); diff --git a/rust/signed_doc/tests/common/mod.rs b/rust/signed_doc/tests/common/mod.rs index ae3d348f8a6..8f8acb75dc6 100644 --- a/rust/signed_doc/tests/common/mod.rs +++ b/rust/signed_doc/tests/common/mod.rs @@ -68,7 +68,7 @@ pub fn create_dummy_key_pair( } pub fn create_dummy_doc( - doc_type_id: Uuid, + doc_type_id: &DocType, ) -> anyhow::Result<(CatalystSignedDocument, UuidV7, UuidV7)> { let empty_json = serde_json::to_vec(&serde_json::json!({}))?; diff --git a/rust/signed_doc/tests/decoding.rs b/rust/signed_doc/tests/decoding.rs index 795183e6990..99986c51823 100644 --- a/rust/signed_doc/tests/decoding.rs +++ b/rust/signed_doc/tests/decoding.rs @@ -1,6 +1,6 @@ //! Integration test for COSE decoding part. -use catalyst_signed_doc::{providers::tests::TestVerifyingKeyProvider, *}; +use catalyst_signed_doc::{doc_types::deprecated, providers::tests::TestVerifyingKeyProvider, *}; use catalyst_types::catalyst_id::role_index::RoleId; use common::create_dummy_key_pair; use coset::TaggedCborSerializable; @@ -12,7 +12,7 @@ mod common; fn catalyst_signed_doc_cbor_roundtrip_kid_as_id_test() { catalyst_signed_doc_cbor_roundtrip_kid_as_id(common::test_metadata()); catalyst_signed_doc_cbor_roundtrip_kid_as_id(common::test_metadata_specific_type( - Some(doc_types::PROPOSAL_UUID_TYPE.try_into().unwrap()), + Some(deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.try_into().unwrap()), None, )); } @@ -41,7 +41,7 @@ fn catalyst_signed_doc_cbor_roundtrip_kid_as_id(data: (UuidV7, UuidV4, serde_jso async fn catalyst_signed_doc_parameters_aliases_test() { catalyst_signed_doc_parameters_aliases(common::test_metadata()).await; catalyst_signed_doc_parameters_aliases(common::test_metadata_specific_type( - Some(doc_types::PROPOSAL_UUID_TYPE.try_into().unwrap()), + Some(deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.try_into().unwrap()), None, )) .await; @@ -201,6 +201,8 @@ fn signed_doc_with_all_fields_case() -> TestCase { let uuid_v7 = UuidV7::new(); let uuid_v4 = UuidV4::new(); let (sk, _, kid) = create_dummy_key_pair(RoleId::Role0).unwrap(); + let doc_refs: DocumentRefs = + vec![DocumentRef::new(uuid_v7, uuid_v7, DocLocator::default())].into(); TestCase { name: "Catalyst Signed Doc with ALL defined metadata fields and signatures", @@ -239,26 +241,10 @@ fn signed_doc_with_all_fields_case() -> TestCase { && (doc.doc_ver().unwrap() == uuid_v7) && (doc.doc_content_type().unwrap() == ContentType::Json) && (doc.doc_content_encoding().unwrap() == ContentEncoding::Brotli) - && (doc.doc_meta().doc_ref().unwrap() - == DocumentRef { - id: uuid_v7, - ver: uuid_v7, - }) - && (doc.doc_meta().reply().unwrap() - == DocumentRef { - id: uuid_v7, - ver: uuid_v7, - }) - && (doc.doc_meta().template().unwrap() - == DocumentRef { - id: uuid_v7, - ver: uuid_v7, - }) - && (doc.doc_meta().parameters().unwrap() - == DocumentRef { - id: uuid_v7, - ver: uuid_v7, - }) + && (doc.doc_meta().doc_ref().unwrap() == &doc_refs) + && (doc.doc_meta().reply().unwrap() == &doc_refs) + && (doc.doc_meta().template().unwrap() == &doc_refs) + && (doc.doc_meta().parameters().unwrap() == &doc_refs) && (doc.doc_meta().section().unwrap() == &"$".parse::
().unwrap()) && (doc.doc_meta().collabs() == ["Alex1".to_string(), "Alex2".to_string()]) && (doc.doc_content().decoded_bytes().unwrap() diff --git a/rust/signed_doc/tests/proposal.rs b/rust/signed_doc/tests/proposal.rs index 7e6f4f21d7c..a2f3e9fef17 100644 --- a/rust/signed_doc/tests/proposal.rs +++ b/rust/signed_doc/tests/proposal.rs @@ -1,27 +1,62 @@ //! Integration test for proposal document validation part. +//! Require fields: type, id, ver, template, parameters +//! -use catalyst_signed_doc::{providers::tests::TestCatalystSignedDocumentProvider, *}; +use catalyst_signed_doc::{ + doc_types::deprecated, providers::tests::TestCatalystSignedDocumentProvider, *, +}; use catalyst_types::catalyst_id::role_index::RoleId; mod common; +// Given a proposal document `doc`: +// +// - Parameters: +// The `parameters` field in `doc` points to a brand document. +// The parameter rule defines the link reference as `template`, This mean the document +// that `ref` field in `doc` points to (in this case = `template_doc`), must have the same +// `parameters` value as `doc`. #[tokio::test] async fn test_valid_proposal_doc() { - let (template_doc, template_doc_id, template_doc_ver) = - common::create_dummy_doc(doc_types::deprecated::PROPOSAL_TEMPLATE_UUID_TYPE).unwrap(); + let (brand_doc, brand_doc_id, brand_doc_ver) = + common::create_dummy_doc(&doc_types::BRAND_PARAMETERS.clone()).unwrap(); - let uuid_v7 = UuidV7::new(); + let template_doc_id = UuidV7::new(); + let template_doc_ver = UuidV7::new(); + // Create a template document + let (template_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ + "content-type": ContentType::Json.to_string(), + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": doc_types::PROPOSAL_TEMPLATE.clone(), + "id": template_doc_id.to_string(), + "ver": template_doc_ver.to_string(), + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + })), + serde_json::to_vec(&serde_json::Value::Null).unwrap(), + RoleId::Role0, + ) + .unwrap(); + + let proposal_doc_id = UuidV7::new(); + let proposal_doc_ver = UuidV7::new(); + // Create a main proposal doc, contain all fields mention in the document (except + // collaborations and revocations) let (doc, ..) = common::create_dummy_signed_doc( serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_DOC_TYPE.clone(), - "id": uuid_v7.to_string(), - "ver": uuid_v7.to_string(), + "type": doc_types::PROPOSAL.clone(), + "id": proposal_doc_id.to_string(), + "ver": proposal_doc_ver.to_string(), "template": { "id": template_doc_id, "ver": template_doc_ver }, + "parameters": { + "id": brand_doc_id, + "ver": brand_doc_ver + } }), serde_json::to_vec(&serde_json::Value::Null).unwrap(), RoleId::Proposer, @@ -29,7 +64,9 @@ async fn test_valid_proposal_doc() { .unwrap(); let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(template_doc).unwrap(); + + provider.add_document(None, &template_doc).unwrap(); + provider.add_document(None, &brand_doc).unwrap(); let is_valid = validator::validate(&doc, &provider).await.unwrap(); @@ -38,22 +75,45 @@ async fn test_valid_proposal_doc() { #[tokio::test] async fn test_valid_proposal_doc_old_type() { - let (template_doc, template_doc_id, template_doc_ver) = - common::create_dummy_doc(doc_types::deprecated::PROPOSAL_TEMPLATE_UUID_TYPE).unwrap(); + let (brand_doc, brand_doc_id, brand_doc_ver) = + common::create_dummy_doc(&doc_types::BRAND_PARAMETERS.clone()).unwrap(); - let uuid_v7 = UuidV7::new(); + let template_doc_id = UuidV7::new(); + let template_doc_ver = UuidV7::new(); + // Create a template document + let (template_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!(serde_json::json!({ + "content-type": ContentType::Json.to_string(), + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": doc_types::PROPOSAL_TEMPLATE.clone(), + "id": template_doc_id.to_string(), + "ver": template_doc_ver.to_string(), + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + })), + serde_json::to_vec(&serde_json::Value::Null).unwrap(), + RoleId::Role0, + ) + .unwrap(); + + let proposal_doc_id = UuidV7::new(); + let proposal_doc_ver = UuidV7::new(); + // Create a main proposal doc, contain all fields mention in the document (except + // collaborations and revocations) let (doc, ..) = common::create_dummy_signed_doc( serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - // Using old (single uuid) - "type": doc_types::deprecated::PROPOSAL_DOCUMENT_UUID_TYPE, - "id": uuid_v7.to_string(), - "ver": uuid_v7.to_string(), + "type": deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.clone(), + "id": proposal_doc_id.to_string(), + "ver": proposal_doc_ver.to_string(), "template": { "id": template_doc_id, "ver": template_doc_ver }, + "parameters": { + "id": brand_doc_id, + "ver": brand_doc_ver + } }), serde_json::to_vec(&serde_json::Value::Null).unwrap(), RoleId::Proposer, @@ -61,7 +121,9 @@ async fn test_valid_proposal_doc_old_type() { .unwrap(); let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(template_doc).unwrap(); + + provider.add_document(None, &template_doc).unwrap(); + provider.add_document(None, &brand_doc).unwrap(); let is_valid = validator::validate(&doc, &provider).await.unwrap(); @@ -79,7 +141,7 @@ async fn test_valid_proposal_doc_with_empty_provider() { serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_DOC_TYPE.clone(), + "type": doc_types::PROPOSAL.clone(), "id": uuid_v7.to_string(), "ver": uuid_v7.to_string(), "template": { @@ -106,7 +168,7 @@ async fn test_invalid_proposal_doc() { serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_DOC_TYPE.clone(), + "type": doc_types::PROPOSAL.clone(), "id": uuid_v7.to_string(), "ver": uuid_v7.to_string(), // without specifying template id diff --git a/rust/signed_doc/tests/signature.rs b/rust/signed_doc/tests/signature.rs index 5c93ec25bb5..7155e7939a6 100644 --- a/rust/signed_doc/tests/signature.rs +++ b/rust/signed_doc/tests/signature.rs @@ -33,7 +33,7 @@ async fn single_signature_validation_test() { ); // case: missing signatures - let (unsigned_doc, ..) = common::create_dummy_doc(UuidV4::new().into()).unwrap(); + let (unsigned_doc, ..) = common::create_dummy_doc(&UuidV4::new().into()).unwrap(); assert!(!validator::validate_signatures(&unsigned_doc, &provider) .await .unwrap()); diff --git a/rust/signed_doc/tests/submission.rs b/rust/signed_doc/tests/submission.rs index dc2ea5d56a4..68620254fd4 100644 --- a/rust/signed_doc/tests/submission.rs +++ b/rust/signed_doc/tests/submission.rs @@ -1,27 +1,64 @@ -//! Test for proposal submission action. +//! Test for Proposal Submission Action. +//! Require fields: type, id, ver, ref, parameters +//! -use catalyst_signed_doc::{providers::tests::TestCatalystSignedDocumentProvider, *}; +use catalyst_signed_doc::{ + doc_types::deprecated, providers::tests::TestCatalystSignedDocumentProvider, *, +}; use catalyst_types::catalyst_id::role_index::RoleId; mod common; +// Given a proposal comment document `doc`: +// +// - Parameters: +// The `parameters` field in `doc` points to a brand document. +// The parameter rule defines the link reference as `ref`, This mean the document that +// `ref` field in `doc` points to (in this case = `proposal_doc`), must have the same +// `parameters` value as `doc`. #[tokio::test] async fn test_valid_submission_action() { - let (proposal_doc, proposal_doc_id, proposal_doc_ver) = - common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); + let (brand_doc, brand_doc_id, brand_doc_ver) = + common::create_dummy_doc(&doc_types::BRAND_PARAMETERS.clone()).unwrap(); - let uuid_v7 = UuidV7::new(); + let proposal_doc_id = UuidV7::new(); + let proposal_doc_ver = UuidV7::new(); + let (proposal_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!({ + "content-type": ContentType::Json.to_string(), + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": doc_types::PROPOSAL.clone(), + "id": proposal_doc_id.to_string(), + "ver": proposal_doc_ver.to_string(), + "ref": { + "id": UuidV7::new(), + "ver": UuidV7::new() + }, + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + }), + serde_json::to_vec(&serde_json::json!({ + "action": "final" + })) + .unwrap(), + RoleId::Proposer, + ) + .unwrap(); + + let doc_id = UuidV7::new(); + let doc_ver = UuidV7::new(); + // Create a main proposal submission doc, contain all fields mention in the document let (doc, ..) = common::create_dummy_signed_doc( serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_ACTION_DOC.clone(), - "id": uuid_v7.to_string(), - "ver": uuid_v7.to_string(), + "type": doc_types::PROPOSAL_SUBMISSION_ACTION.clone(), + "id": doc_id.to_string(), + "ver": doc_ver.to_string(), "ref": { "id": proposal_doc_id, "ver": proposal_doc_ver }, + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } }), serde_json::to_vec(&serde_json::json!({ "action": "final" @@ -32,29 +69,57 @@ async fn test_valid_submission_action() { .unwrap(); let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(proposal_doc).unwrap(); + + provider.add_document(None, &proposal_doc).unwrap(); + provider.add_document(None, &brand_doc).unwrap(); + let is_valid = validator::validate(&doc, &provider).await.unwrap(); assert!(is_valid, "{:?}", doc.problem_report()); } #[tokio::test] async fn test_valid_submission_action_old_type() { - let (proposal_doc, proposal_doc_id, proposal_doc_ver) = - common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); + let (brand_doc, brand_doc_id, brand_doc_ver) = + common::create_dummy_doc(&doc_types::BRAND_PARAMETERS.clone()).unwrap(); - let uuid_v7 = UuidV7::new(); + let proposal_doc_id = UuidV7::new(); + let proposal_doc_ver = UuidV7::new(); + let (proposal_doc, ..) = common::create_dummy_signed_doc( + serde_json::json!({ + "content-type": ContentType::Json.to_string(), + "content-encoding": ContentEncoding::Brotli.to_string(), + "type": deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.clone(), + "id": proposal_doc_id.to_string(), + "ver": proposal_doc_ver.to_string(), + "ref": { + "id": UuidV7::new(), + "ver": UuidV7::new() + }, + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } + }), + serde_json::to_vec(&serde_json::json!({ + "action": "final" + })) + .unwrap(), + RoleId::Proposer, + ) + .unwrap(); + + let doc_id = UuidV7::new(); + let doc_ver = UuidV7::new(); + // Create a main proposal submission doc, contain all fields mention in the document let (doc, ..) = common::create_dummy_signed_doc( serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - // Using old (single uuid) - "type": doc_types::deprecated::PROPOSAL_ACTION_DOCUMENT_UUID_TYPE, - "id": uuid_v7.to_string(), - "ver": uuid_v7.to_string(), + "type": deprecated::PROPOSAL_ACTION_DOCUMENT_UUID_TYPE.clone(), + "id": doc_id.to_string(), + "ver": doc_ver.to_string(), "ref": { "id": proposal_doc_id, "ver": proposal_doc_ver }, + "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } }), serde_json::to_vec(&serde_json::json!({ "action": "final" @@ -65,7 +130,10 @@ async fn test_valid_submission_action_old_type() { .unwrap(); let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(proposal_doc).unwrap(); + + provider.add_document(None, &proposal_doc).unwrap(); + provider.add_document(None, &brand_doc).unwrap(); + let is_valid = validator::validate(&doc, &provider).await.unwrap(); assert!(is_valid, "{:?}", doc.problem_report()); } @@ -80,7 +148,7 @@ async fn test_valid_submission_action_with_empty_provider() { serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_ACTION_DOC.clone(), + "type": doc_types::PROPOSAL_SUBMISSION_ACTION.clone(), "id": uuid_v7.to_string(), "ver": uuid_v7.to_string(), "ref": { @@ -99,7 +167,6 @@ async fn test_valid_submission_action_with_empty_provider() { let provider = TestCatalystSignedDocumentProvider::default(); let is_valid = validator::validate(&doc, &provider).await.unwrap(); - assert!(!is_valid); } @@ -111,10 +178,9 @@ async fn test_invalid_submission_action() { serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_ACTION_DOC.clone(), + "type": doc_types::PROPOSAL_SUBMISSION_ACTION.clone(), "id": uuid_v7.to_string(), "ver": uuid_v7.to_string(), - // without specifying ref "ref": serde_json::Value::Null, }), serde_json::to_vec(&serde_json::json!({ @@ -131,13 +197,14 @@ async fn test_invalid_submission_action() { // corrupted JSON let (proposal_doc, proposal_doc_id, proposal_doc_ver) = - common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); + common::create_dummy_doc(&deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.try_into().unwrap()) + .unwrap(); let uuid_v7 = UuidV7::new(); let (doc, ..) = common::create_dummy_signed_doc( serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::ACTION_UUID_TYPE, + "type": deprecated::PROPOSAL_ACTION_DOCUMENT_UUID_TYPE, "id": uuid_v7.to_string(), "ver": uuid_v7.to_string(), "ref": { @@ -151,19 +218,22 @@ async fn test_invalid_submission_action() { .unwrap(); let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(proposal_doc).unwrap(); + + provider.add_document(None, &proposal_doc).unwrap(); + let is_valid = validator::validate(&doc, &provider).await.unwrap(); assert!(!is_valid); // empty content let (proposal_doc, proposal_doc_id, proposal_doc_ver) = - common::create_dummy_doc(doc_types::PROPOSAL_UUID_TYPE).unwrap(); + common::create_dummy_doc(&deprecated::PROPOSAL_DOCUMENT_UUID_TYPE.try_into().unwrap()) + .unwrap(); let uuid_v7 = UuidV7::new(); let (doc, ..) = common::create_dummy_signed_doc( serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_ACTION_DOC.clone(), + "type": doc_types::PROPOSAL_SUBMISSION_ACTION.clone(), "id": uuid_v7.to_string(), "ver": uuid_v7.to_string(), "ref": { @@ -177,7 +247,9 @@ async fn test_invalid_submission_action() { .unwrap(); let mut provider = TestCatalystSignedDocumentProvider::default(); - provider.add_document(proposal_doc).unwrap(); + + provider.add_document(None, &proposal_doc).unwrap(); + let is_valid = validator::validate(&doc, &provider).await.unwrap(); assert!(!is_valid); } From b8953f469cd34ec7980a4f711d1c625110cea71b Mon Sep 17 00:00:00 2001 From: bkioshn Date: Fri, 20 Jun 2025 19:22:57 +0700 Subject: [PATCH 03/12] fix(signed-doc): revert doc type change Signed-off-by: bkioshn --- rust/signed_doc/src/metadata/doc_type.rs | 37 +++++++----------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/rust/signed_doc/src/metadata/doc_type.rs b/rust/signed_doc/src/metadata/doc_type.rs index 0a2e3254d33..c55a45be5e9 100644 --- a/rust/signed_doc/src/metadata/doc_type.rs +++ b/rust/signed_doc/src/metadata/doc_type.rs @@ -5,10 +5,7 @@ use std::{ hash::{Hash, Hasher}, }; -use catalyst_types::{ - problem_report::ProblemReport, - uuid::{CborContext, Uuid, UuidV4, UUID_CBOR_TAG}, -}; +use catalyst_types::uuid::{CborContext, Uuid, UuidV4, UUID_CBOR_TAG}; use coset::cbor::Value; use minicbor::{Decode, Decoder, Encode}; use serde::{Deserialize, Deserializer}; @@ -253,28 +250,19 @@ fn map_doc_type(uuid: UuidV4) -> DocType { } } -impl Encode for DocType { +impl Encode for DocType { fn encode( - &self, e: &mut minicbor::Encoder, report: &mut ProblemReport, + &self, e: &mut minicbor::Encoder, _ctx: &mut C, ) -> Result<(), minicbor::encode::Error> { - const CONTEXT: &str = "DocType encoding"; - if self.0.is_empty() { - report.invalid_value("DocType", "empty", "DocType cannot be empty", CONTEXT); - return Err(minicbor::encode::Error::message(format!( - "{CONTEXT}: DocType cannot be empty" - ))); - } - - e.array(self.0.len().try_into().map_err(|_| { - report.invalid_encoding("Array", "Invalid array", "Valid array", CONTEXT); - minicbor::encode::Error::message(format!("{CONTEXT}, array length encoding failed")) - })?)?; + e.array( + self.0 + .len() + .try_into() + .map_err(minicbor::encode::Error::message)?, + )?; for id in &self.0 { - id.encode(e, &mut CborContext::Tagged).map_err(|_| { - report.invalid_encoding("UUIDv4", &id.to_string(), "Valid UUIDv4", CONTEXT); - minicbor::encode::Error::message(format!("{CONTEXT}: UUIDv4 encoding failed")) - })?; + id.encode(e, &mut CborContext::Tagged)?; } Ok(()) } @@ -458,11 +446,6 @@ mod tests { let input = vec!["not-a-uuid".to_string()]; let result = DocType::try_from(input); assert!(matches!(result, Err(DocTypeError::StringConversion(s)) if s == "not-a-uuid")); - - let e: Vec = vec![vec![UuidV4::new()].try_into().unwrap()]; - let exp_parameters_type_str: Vec = - e.iter().map(std::string::ToString::to_string).collect(); - assert!(!exp_parameters_type_str.is_empty()); } #[test] From 906e587c72354737b15c2cb050fbc5b85c8683b7 Mon Sep 17 00:00:00 2001 From: bkioshn Date: Fri, 20 Jun 2025 19:32:23 +0700 Subject: [PATCH 04/12] fix(signed-doc): remove problem report in encoder Signed-off-by: bkioshn --- .../src/metadata/document_refs/mod.rs | 10 ++--- .../src/metadata/supported_field.rs | 42 ++++++++++--------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/rust/signed_doc/src/metadata/document_refs/mod.rs b/rust/signed_doc/src/metadata/document_refs/mod.rs index babc8d8df41..3c3cf6704f5 100644 --- a/rust/signed_doc/src/metadata/document_refs/mod.rs +++ b/rust/signed_doc/src/metadata/document_refs/mod.rs @@ -4,10 +4,7 @@ mod doc_locator; mod doc_ref; use std::{fmt::Display, str::FromStr}; -use catalyst_types::{ - problem_report::ProblemReport, - uuid::{CborContext, UuidV7}, -}; +use catalyst_types::uuid::{CborContext, UuidV7}; use coset::cbor::Value; pub use doc_locator::DocLocator; pub use doc_ref::DocumentRef; @@ -267,6 +264,7 @@ impl<'de> Deserialize<'de> for DocumentRefs { #[cfg(test)] mod tests { + use catalyst_types::problem_report::ProblemReport; use minicbor::Encoder; use serde_json::json; @@ -352,11 +350,11 @@ mod tests { let doc_refs = DocumentRefs(vec![doc_ref.clone(), doc_ref]); let mut buffer = Vec::new(); let mut encoder = Encoder::new(&mut buffer); - doc_refs.encode(&mut encoder, &mut report).unwrap(); + doc_refs.encode(&mut encoder, &mut ()).unwrap(); let mut decoder = Decoder::new(&buffer); let mut decoded_context = DecodeContext { compatibility_policy: CompatibilityPolicy::Accept, - report: &mut report.clone(), + report: &mut report, }; let decoded_doc_refs = DocumentRefs::decode(&mut decoder, &mut decoded_context).unwrap(); assert_eq!(decoded_doc_refs, doc_refs); diff --git a/rust/signed_doc/src/metadata/supported_field.rs b/rust/signed_doc/src/metadata/supported_field.rs index 69e6e3384ec..88849e04eaf 100644 --- a/rust/signed_doc/src/metadata/supported_field.rs +++ b/rust/signed_doc/src/metadata/supported_field.rs @@ -4,7 +4,7 @@ use std::fmt::{self, Display}; #[cfg(test)] use std::{cmp, convert::Infallible}; -use catalyst_types::{problem_report::ProblemReport, uuid::UuidV7}; +use catalyst_types::uuid::UuidV7; use strum::{EnumDiscriminants, EnumTryAs, IntoDiscriminant as _}; use crate::{ @@ -44,10 +44,12 @@ impl<'a, C> minicbor::Decode<'a, C> for Label<'a> { match d.datatype()? { minicbor::data::Type::U8 => d.u8().map(Self::U8), minicbor::data::Type::String => d.str().map(Self::Str), - _ => Err(minicbor::decode::Error::message( - "Datatype is neither 8bit signed integer nor text", - ) - .at(d.position())), + _ => { + Err(minicbor::decode::Error::message( + "Datatype is neither 8bit signed integer nor text", + ) + .at(d.position())) + }, } } } @@ -193,13 +195,15 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte let field = match key { SupportedLabel::ContentType => todo!(), - SupportedLabel::Id => d - .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Id), + SupportedLabel::Id => { + d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Id) + }, SupportedLabel::Ref => d.decode_with(ctx).map(Self::Ref), - SupportedLabel::Ver => d - .decode_with(&mut catalyst_types::uuid::CborContext::Tagged) - .map(Self::Ver), + SupportedLabel::Ver => { + d.decode_with(&mut catalyst_types::uuid::CborContext::Tagged) + .map(Self::Ver) + }, SupportedLabel::Type => d.decode_with(ctx).map(Self::Type), SupportedLabel::Reply => d.decode_with(ctx).map(Self::Reply), SupportedLabel::Collabs => todo!(), @@ -213,23 +217,23 @@ impl minicbor::Decode<'_, crate::decode_context::DecodeContext<'_>> for Supporte } } -impl minicbor::Encode for SupportedField { +impl minicbor::Encode<()> for SupportedField { fn encode( - &self, e: &mut minicbor::Encoder, report: &mut ProblemReport, + &self, e: &mut minicbor::Encoder, ctx: &mut (), ) -> Result<(), minicbor::encode::Error> { let key = self.discriminant().to_cose(); e.encode(key)?; match self { - SupportedField::ContentType(content_type) => content_type.encode(e, &mut ()), + SupportedField::ContentType(content_type) => content_type.encode(e, ctx), SupportedField::Id(uuid_v7) | SupportedField::Ver(uuid_v7) => { uuid_v7.encode(e, &mut catalyst_types::uuid::CborContext::Tagged) }, SupportedField::Ref(document_ref) | SupportedField::Reply(document_ref) | SupportedField::Template(document_ref) - | SupportedField::Parameters(document_ref) => document_ref.encode(e, report), - SupportedField::Type(doc_type) => doc_type.encode(e, report), + | SupportedField::Parameters(document_ref) => document_ref.encode(e, ctx), + SupportedField::Type(doc_type) => doc_type.encode(e, ctx), SupportedField::Collabs(collabs) => { if !collabs.is_empty() { e.array( @@ -244,10 +248,8 @@ impl minicbor::Encode for SupportedField { } Ok(()) }, - SupportedField::Section(section) => section.encode(e, &mut ()), - SupportedField::ContentEncoding(content_encoding) => { - content_encoding.encode(e, &mut ()) - }, + SupportedField::Section(section) => section.encode(e, ctx), + SupportedField::ContentEncoding(content_encoding) => content_encoding.encode(e, ctx), } } } From 396f319322a9b1b0426e4748ff7dec1403d6b095 Mon Sep 17 00:00:00 2001 From: bkioshn Date: Fri, 20 Jun 2025 20:42:06 +0700 Subject: [PATCH 05/12] fix(signed-doc): metadata decode Signed-off-by: bkioshn --- rust/signed_doc/src/metadata/mod.rs | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/rust/signed_doc/src/metadata/mod.rs b/rust/signed_doc/src/metadata/mod.rs index 0ab2bdfec88..06f393fdeae 100644 --- a/rust/signed_doc/src/metadata/mod.rs +++ b/rust/signed_doc/src/metadata/mod.rs @@ -287,12 +287,30 @@ impl Metadata { } // DocType and DocRef now using minicbor decoding. - metadata.doc_type = decode_cose_protected_header_value(protected, context, TYPE_KEY); - metadata.doc_ref = decode_cose_protected_header_value(protected, context, REF_KEY); - metadata.template = decode_cose_protected_header_value(protected, context, TEMPLATE_KEY); - metadata.reply = decode_cose_protected_header_value(protected, context, REPLY_KEY); + if let Some(value) = decode_cose_protected_header_value::( + protected, context, TYPE_KEY, + ) { + metadata_fields.push(SupportedField::Type(value)); + }; + if let Some(value) = decode_cose_protected_header_value::( + protected, context, REF_KEY, + ) { + metadata_fields.push(SupportedField::Ref(value)); + }; + if let Some(value) = decode_cose_protected_header_value::( + protected, + context, + TEMPLATE_KEY, + ) { + metadata_fields.push(SupportedField::Template(value)); + } + if let Some(value) = decode_cose_protected_header_value::( + protected, context, REPLY_KEY, + ) { + metadata_fields.push(SupportedField::Reply(value)); + } - metadata.section = decode_document_field_from_protected_header( + if let Some(value) = decode_document_field_from_protected_header( protected, SECTION_KEY, COSE_DECODING_CONTEXT, From f4dccb56a8193393e64779d33470f97b8e51dcb7 Mon Sep 17 00:00:00 2001 From: bkioshn Date: Fri, 20 Jun 2025 20:43:38 +0700 Subject: [PATCH 06/12] chore(signed-doc): revert change Signed-off-by: bkioshn --- rust/signed_doc/tests/decoding.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/rust/signed_doc/tests/decoding.rs b/rust/signed_doc/tests/decoding.rs index af39f27ca56..1ec2661a627 100644 --- a/rust/signed_doc/tests/decoding.rs +++ b/rust/signed_doc/tests/decoding.rs @@ -175,8 +175,6 @@ fn decoding_empty_bytes_case() -> TestCase { fn signed_doc_with_all_fields_case() -> TestCase { let uuid_v7 = UuidV7::new(); let uuid_v4 = UuidV4::new(); - let doc_refs: DocumentRefs = - vec![DocumentRef::new(uuid_v7, uuid_v7, DocLocator::default())].into(); TestCase { name: "Catalyst Signed Doc with minimally defined metadata fields, signed (one signature), CBOR tagged.", @@ -222,10 +220,6 @@ fn signed_doc_with_all_fields_case() -> TestCase { && (doc.doc_id().unwrap() == uuid_v7) && (doc.doc_ver().unwrap() == uuid_v7) && (doc.doc_content_type().unwrap() == ContentType::Json) - && (doc.doc_meta().doc_ref().unwrap() == &doc_refs) - && (doc.doc_meta().reply().unwrap() == &doc_refs) - && (doc.doc_meta().template().unwrap() == &doc_refs) - && (doc.doc_meta().parameters().unwrap() == &doc_refs) && (doc.encoded_content() == serde_json::to_vec(&serde_json::Value::Null).unwrap()) && doc.kids().len() == 1 } From 3f21a9e540a2e286325141f6107820a8aedb91d1 Mon Sep 17 00:00:00 2001 From: bkioshn Date: Mon, 23 Jun 2025 11:03:28 +0700 Subject: [PATCH 07/12] test(signed-doc): add docref to decoding test Signed-off-by: bkioshn --- rust/signed_doc/tests/decoding.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/rust/signed_doc/tests/decoding.rs b/rust/signed_doc/tests/decoding.rs index 1ec2661a627..2ff3e2053b9 100644 --- a/rust/signed_doc/tests/decoding.rs +++ b/rust/signed_doc/tests/decoding.rs @@ -175,7 +175,8 @@ fn decoding_empty_bytes_case() -> TestCase { fn signed_doc_with_all_fields_case() -> TestCase { let uuid_v7 = UuidV7::new(); let uuid_v4 = UuidV4::new(); - + let dr: DocumentRefs = vec![DocumentRef::new(UuidV7::new(), UuidV7::new(), DocLocator::default())].into(); + let check_dr = dr.clone(); TestCase { name: "Catalyst Signed Doc with minimally defined metadata fields, signed (one signature), CBOR tagged.", bytes_gen: Box::new({ @@ -188,12 +189,15 @@ fn signed_doc_with_all_fields_case() -> TestCase { // protected headers (metadata fields) let mut p_headers = Encoder::new(Vec::new()); - p_headers.map(4)?; + p_headers.map(8)?; p_headers.u8(3)?.encode(ContentType::Json)?; p_headers.str("type")?.encode_with(uuid_v4, &mut catalyst_types::uuid::CborContext::Tagged)?; p_headers.str("id")?.encode_with(uuid_v7, &mut catalyst_types::uuid::CborContext::Tagged)?; p_headers.str("ver")?.encode_with(uuid_v7, &mut catalyst_types::uuid::CborContext::Tagged)?; - + p_headers.str("ref")?.encode_with(dr.clone(), &mut ())?; + p_headers.str("reply")?.encode_with(dr.clone(), &mut ())?; + p_headers.str("template")?.encode_with(dr.clone(), &mut ())?; + p_headers.str("parameters")?.encode_with(dr.clone(), &mut ())?; e.bytes(p_headers.into_writer().as_slice())?; // empty unprotected headers e.map(0)?; @@ -220,6 +224,10 @@ fn signed_doc_with_all_fields_case() -> TestCase { && (doc.doc_id().unwrap() == uuid_v7) && (doc.doc_ver().unwrap() == uuid_v7) && (doc.doc_content_type().unwrap() == ContentType::Json) + && doc.doc_meta().doc_ref().unwrap() == &check_dr + && doc.doc_meta().template().unwrap() == &check_dr + && doc.doc_meta().reply().unwrap() == &check_dr + && doc.doc_meta().parameters().unwrap() == &check_dr && (doc.encoded_content() == serde_json::to_vec(&serde_json::Value::Null).unwrap()) && doc.kids().len() == 1 } From 6ee751182ed21d922016e06476d89c74b1f7197d Mon Sep 17 00:00:00 2001 From: bkioshn Date: Mon, 23 Jun 2025 11:06:20 +0700 Subject: [PATCH 08/12] fix(signed-doc): format Signed-off-by: bkioshn --- rust/signed_doc/tests/decoding.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rust/signed_doc/tests/decoding.rs b/rust/signed_doc/tests/decoding.rs index 2ff3e2053b9..d0a5a1fffe6 100644 --- a/rust/signed_doc/tests/decoding.rs +++ b/rust/signed_doc/tests/decoding.rs @@ -175,7 +175,12 @@ fn decoding_empty_bytes_case() -> TestCase { fn signed_doc_with_all_fields_case() -> TestCase { let uuid_v7 = UuidV7::new(); let uuid_v4 = UuidV4::new(); - let dr: DocumentRefs = vec![DocumentRef::new(UuidV7::new(), UuidV7::new(), DocLocator::default())].into(); + let dr: DocumentRefs = vec![DocumentRef::new( + UuidV7::new(), + UuidV7::new(), + DocLocator::default(), + )] + .into(); let check_dr = dr.clone(); TestCase { name: "Catalyst Signed Doc with minimally defined metadata fields, signed (one signature), CBOR tagged.", From 16979ff5b03e9dfa0d0496ea137d5b163ef2d4ab Mon Sep 17 00:00:00 2001 From: bkioshn Date: Mon, 23 Jun 2025 14:19:07 +0700 Subject: [PATCH 09/12] fix(signed-doc): refactor and improvement Signed-off-by: bkioshn --- rust/signed_doc/src/validator/mod.rs | 66 ++++++++++++------- .../signed_doc/src/validator/rules/doc_ref.rs | 15 ++--- rust/signed_doc/src/validator/rules/mod.rs | 2 +- .../src/validator/rules/parameters.rs | 26 ++++---- rust/signed_doc/src/validator/rules/reply.rs | 20 ++---- .../src/validator/rules/template.rs | 12 +--- rust/signed_doc/src/validator/utils.rs | 24 ++++++- 7 files changed, 93 insertions(+), 72 deletions(-) diff --git a/rust/signed_doc/src/validator/mod.rs b/rust/signed_doc/src/validator/mod.rs index f0ccaf0a224..e14429a8673 100644 --- a/rust/signed_doc/src/validator/mod.rs +++ b/rust/signed_doc/src/validator/mod.rs @@ -43,22 +43,17 @@ where t.try_into().expect("Failed to convert to DocType") } -/// `DOCUMENT_RULES` initialization function -#[allow(clippy::expect_used, clippy::too_many_lines)] -fn document_rules_init() -> HashMap> { +/// Proposal +/// Require field: type, id, ver, template, parameters +/// +fn proposal_rule() -> Rules { // Parameter can be either brand, campaign or category let parameters = vec![ BRAND_PARAMETERS.clone(), CAMPAIGN_PARAMETERS.clone(), CATEGORY_PARAMETERS.clone(), ]; - - let mut document_rules_map = HashMap::new(); - - // Proposal - // Require field: type, id, ver, template, parameters - // - let proposal_rules = Rules { + Rules { content_type: ContentTypeRule { exp: ContentType::Json, }, @@ -82,12 +77,20 @@ fn document_rules_init() -> HashMap> { param_link_ref: ParameterLinkRefRule::Specified { field: LinkField::Template, }, - }; + } +} - // Proposal Comment - // Require field: type, id, ver, ref, template, parameters - // - let proposal_comment_rules = Rules { +/// Proposal Comment +/// Require field: type, id, ver, ref, template, parameters +/// +fn proposal_comment_rule() -> Rules { + // Parameter can be either brand, campaign or category + let parameters = vec![ + BRAND_PARAMETERS.clone(), + CAMPAIGN_PARAMETERS.clone(), + CATEGORY_PARAMETERS.clone(), + ]; + Rules { content_type: ContentTypeRule { exp: ContentType::Json, }, @@ -118,7 +121,20 @@ fn document_rules_init() -> HashMap> { param_link_ref: ParameterLinkRefRule::Specified { field: LinkField::Template, }, - }; + } +} + +/// Proposal Submission Action +/// Require fields: type, id, ver, ref, parameters +/// +#[allow(clippy::expect_used)] +fn proposal_submission_action_rule() -> Rules { + // Parameter can be either brand, campaign or category + let parameters = vec![ + BRAND_PARAMETERS.clone(), + CAMPAIGN_PARAMETERS.clone(), + CATEGORY_PARAMETERS.clone(), + ]; let proposal_action_json_schema = jsonschema::options() .with_draft(jsonschema::Draft::Draft7) @@ -130,10 +146,7 @@ fn document_rules_init() -> HashMap> { ) .expect("Must be a valid json scheme file"); - // Proposal Submission Action - // Require fields: type, id, ver, ref, parameters - // - let proposal_submission_action_rules = Rules { + Rules { content_type: ContentTypeRule { exp: ContentType::Json, }, @@ -158,11 +171,16 @@ fn document_rules_init() -> HashMap> { param_link_ref: ParameterLinkRefRule::Specified { field: LinkField::Ref, }, - }; + } +} + +/// `DOCUMENT_RULES` initialization function +fn document_rules_init() -> HashMap> { + let mut document_rules_map = HashMap::new(); - let proposal_rules = Arc::new(proposal_rules); - let comment_rules = Arc::new(proposal_comment_rules); - let action_rules = Arc::new(proposal_submission_action_rules); + let proposal_rules = Arc::new(proposal_rule()); + let comment_rules = Arc::new(proposal_comment_rule()); + let action_rules = Arc::new(proposal_submission_action_rule()); document_rules_map.insert(PROPOSAL.clone(), Arc::clone(&proposal_rules)); document_rules_map.insert(PROPOSAL_COMMENT.clone(), Arc::clone(&comment_rules)); diff --git a/rust/signed_doc/src/validator/rules/doc_ref.rs b/rust/signed_doc/src/validator/rules/doc_ref.rs index 7dcf6938fd2..94f39c3f7a3 100644 --- a/rust/signed_doc/src/validator/rules/doc_ref.rs +++ b/rust/signed_doc/src/validator/rules/doc_ref.rs @@ -3,7 +3,7 @@ use catalyst_types::problem_report::ProblemReport; use crate::{ - providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + providers::CatalystSignedDocumentProvider, validator::utils::validate_doc_refs, CatalystSignedDocument, DocType, }; @@ -36,17 +36,10 @@ impl RefRule { let ref_validator = |ref_doc: CatalystSignedDocument| { referenced_doc_check(&ref_doc, exp_ref_type, "ref", doc.report()) }; - for dr in doc_ref.doc_refs() { - let result = - validate_provided_doc(dr, provider, doc.report(), ref_validator).await?; - // Reference ALL of them - if !result { - return Ok(false); - } - } - return Ok(true); + return validate_doc_refs(doc_ref, provider, doc.report(), ref_validator).await; } else if !optional { - doc.report().missing_field("ref", context); + doc.report() + .missing_field("ref", &format!("{context}, document must have ref field")); return Ok(false); } } diff --git a/rust/signed_doc/src/validator/rules/mod.rs b/rust/signed_doc/src/validator/rules/mod.rs index 7efc6044b6c..47fafa5cb79 100644 --- a/rust/signed_doc/src/validator/rules/mod.rs +++ b/rust/signed_doc/src/validator/rules/mod.rs @@ -33,7 +33,6 @@ pub(crate) struct Rules { pub(crate) content_encoding: ContentEncodingRule, /// 'ref' field validation rule pub(crate) doc_ref: RefRule, - #[allow(dead_code)] /// document's content validation rule pub(crate) content: ContentRule, /// 'reply' field validation rule @@ -57,6 +56,7 @@ impl Rules { let rules = [ self.content_type.check(doc).boxed(), self.content_encoding.check(doc).boxed(), + self.content.check(doc, provider).boxed(), self.doc_ref.check(doc, provider).boxed(), self.reply.check(doc, provider).boxed(), self.section.check(doc).boxed(), diff --git a/rust/signed_doc/src/validator/rules/parameters.rs b/rust/signed_doc/src/validator/rules/parameters.rs index 1e7784bf8f5..c7d40b15e5b 100644 --- a/rust/signed_doc/src/validator/rules/parameters.rs +++ b/rust/signed_doc/src/validator/rules/parameters.rs @@ -2,7 +2,7 @@ use super::doc_ref::referenced_doc_check; use crate::{ - providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + providers::CatalystSignedDocumentProvider, validator::utils::validate_doc_refs, CatalystSignedDocument, DocType, }; @@ -33,25 +33,25 @@ impl ParametersRule { optional, } = self { - if let Some(parameters) = doc.doc_meta().parameters() { + if let Some(parameters_ref) = doc.doc_meta().parameters() { let parameters_validator = |ref_doc: CatalystSignedDocument| { // Check that the type matches one of the expected ones exp_parameters_type.iter().any(|exp_type| { referenced_doc_check(&ref_doc, exp_type, "parameters", doc.report()) }) }; - for dr in parameters.doc_refs() { - let result = - validate_provided_doc(dr, provider, doc.report(), parameters_validator) - .await?; - // Reference ALL of them - if !result { - return Ok(false); - } - } - return Ok(true); + return validate_doc_refs( + parameters_ref, + provider, + doc.report(), + parameters_validator, + ) + .await; } else if !optional { - doc.report().missing_field("parameters", context); + doc.report().missing_field( + "parameters", + &format!("{context}, document must have parameters field"), + ); return Ok(false); } } diff --git a/rust/signed_doc/src/validator/rules/reply.rs b/rust/signed_doc/src/validator/rules/reply.rs index be846af9343..6dccd07efdd 100644 --- a/rust/signed_doc/src/validator/rules/reply.rs +++ b/rust/signed_doc/src/validator/rules/reply.rs @@ -2,7 +2,7 @@ use super::doc_ref::referenced_doc_check; use crate::{ - providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + providers::CatalystSignedDocumentProvider, validator::utils::validate_doc_refs, CatalystSignedDocument, DocType, }; @@ -32,7 +32,7 @@ impl ReplyRule { optional, } = self { - if let Some(reply) = doc.doc_meta().reply() { + if let Some(reply_ref) = doc.doc_meta().reply() { let reply_validator = |ref_doc: CatalystSignedDocument| { // Validate type if !referenced_doc_check(&ref_doc, exp_reply_type, "reply", doc.report()) { @@ -53,18 +53,12 @@ impl ReplyRule { // Checking the ref field of ref doc, it should match the ref field of the doc ref_doc_dr == doc_dr }; - - for dr in reply.doc_refs() { - let result = - validate_provided_doc(dr, provider, doc.report(), reply_validator).await?; - // Reference ALL of them - if !result { - return Ok(false); - }; - } - return Ok(true); + return validate_doc_refs(reply_ref, provider, doc.report(), reply_validator).await; } else if !optional { - doc.report().missing_field("reply", context); + doc.report().missing_field( + "reply", + &format!("{context}, document must have reply field"), + ); return Ok(false); } } diff --git a/rust/signed_doc/src/validator/rules/template.rs b/rust/signed_doc/src/validator/rules/template.rs index 4c50b834d9a..849487bd6de 100644 --- a/rust/signed_doc/src/validator/rules/template.rs +++ b/rust/signed_doc/src/validator/rules/template.rs @@ -5,7 +5,7 @@ use std::fmt::Write; use super::doc_ref::referenced_doc_check; use crate::{ metadata::ContentType, providers::CatalystSignedDocumentProvider, - validator::utils::validate_provided_doc, CatalystSignedDocument, DocType, + validator::utils::validate_doc_refs, CatalystSignedDocument, DocType, }; /// Enum represents different content schemas, against which documents content would be @@ -67,14 +67,8 @@ impl ContentRule { }, } }; - for dr in template_ref.doc_refs() { - let result = - validate_provided_doc(dr, provider, doc.report(), template_validator).await?; - if !result { - return Ok(false); - } - } - return Ok(true); + return validate_doc_refs(template_ref, provider, doc.report(), template_validator) + .await; } if let Self::Static(content_schema) = self { if let Some(template) = doc.doc_meta().template() { diff --git a/rust/signed_doc/src/validator/utils.rs b/rust/signed_doc/src/validator/utils.rs index 79ef5d2e9ae..1dc2a06cb34 100644 --- a/rust/signed_doc/src/validator/utils.rs +++ b/rust/signed_doc/src/validator/utils.rs @@ -2,7 +2,9 @@ use catalyst_types::problem_report::ProblemReport; -use crate::{providers::CatalystSignedDocumentProvider, CatalystSignedDocument, DocumentRef}; +use crate::{ + providers::CatalystSignedDocumentProvider, CatalystSignedDocument, DocumentRef, DocumentRefs, +}; /// A helper validation document function, which validates a document from the /// `ValidationDataProvider`. @@ -46,3 +48,23 @@ where Ok(false) } } + +/// Validate the document references +/// Document all possible error in doc report (no fail fast) +pub(crate) async fn validate_doc_refs( + doc_refs: &DocumentRefs, provider: &Provider, report: &ProblemReport, validator: Validator, +) -> anyhow::Result +where + Provider: CatalystSignedDocumentProvider, + Validator: Fn(CatalystSignedDocument) -> bool, +{ + let mut all_valid = true; + + for dr in doc_refs.doc_refs() { + let is_valid = validate_provided_doc(dr, provider, report, &validator).await?; + if !is_valid { + all_valid = false; + } + } + Ok(all_valid) +} From f90192af8d572db17fcb26335e2d586f60e1ba8f Mon Sep 17 00:00:00 2001 From: bkioshn Date: Mon, 23 Jun 2025 14:23:42 +0700 Subject: [PATCH 10/12] fix(signed-doc): refactor Signed-off-by: bkioshn --- .../src/validator/rules/param_link_ref.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/rust/signed_doc/src/validator/rules/param_link_ref.rs b/rust/signed_doc/src/validator/rules/param_link_ref.rs index 7690d4a4669..dbbede9df81 100644 --- a/rust/signed_doc/src/validator/rules/param_link_ref.rs +++ b/rust/signed_doc/src/validator/rules/param_link_ref.rs @@ -1,7 +1,7 @@ //! Parameter linked reference rule impl. use crate::{ - providers::CatalystSignedDocumentProvider, validator::utils::validate_provided_doc, + providers::CatalystSignedDocumentProvider, validator::utils::validate_doc_refs, CatalystSignedDocument, }; @@ -49,14 +49,13 @@ impl ParameterLinkRefRule { return Ok(false); }; - for dr in param_link_ref.doc_refs() { - let result = - validate_provided_doc(dr, provider, doc.report(), param_link_ref_validator) - .await?; - if !result { - return Ok(false); - } - } + return validate_doc_refs( + param_link_ref, + provider, + doc.report(), + param_link_ref_validator, + ) + .await; } Ok(true) } From a3910d56e39226da34cce40145b41daffd7dca86 Mon Sep 17 00:00:00 2001 From: bkioshn Date: Mon, 23 Jun 2025 14:52:08 +0700 Subject: [PATCH 11/12] test(signed-doc): fix content test Signed-off-by: bkioshn --- rust/signed_doc/tests/comment.rs | 27 +++++++++++++++++++++------ rust/signed_doc/tests/proposal.rs | 22 ++++++++++++++++++---- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/rust/signed_doc/tests/comment.rs b/rust/signed_doc/tests/comment.rs index 1f80ef17f27..63225acab93 100644 --- a/rust/signed_doc/tests/comment.rs +++ b/rust/signed_doc/tests/comment.rs @@ -60,12 +60,19 @@ async fn test_valid_comment_doc() { serde_json::json!(serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_TEMPLATE.clone(), + "type": doc_types::PROPOSAL_COMMENT_TEMPLATE.clone(), "id": template_doc_id.to_string(), "ver": template_doc_ver.to_string(), "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } })), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + serde_json::to_vec(&serde_json::json!({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {}, + "required": [], + "additionalProperties": false + })) + .unwrap(), RoleId::Role0, ) .unwrap(); @@ -95,7 +102,8 @@ async fn test_valid_comment_doc() { }, "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } })), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + // Validate against the ref template + serde_json::to_vec(&serde_json::json!({})).unwrap(), RoleId::Role0, ) .unwrap(); @@ -152,12 +160,19 @@ async fn test_valid_comment_doc_old_type() { serde_json::json!(serde_json::json!({ "content-type": ContentType::Json.to_string(), "content-encoding": ContentEncoding::Brotli.to_string(), - "type": doc_types::PROPOSAL_TEMPLATE.clone(), + "type": doc_types::PROPOSAL_COMMENT_TEMPLATE.clone(), "id": template_doc_id.to_string(), "ver": template_doc_ver.to_string(), "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } })), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + serde_json::to_vec(&serde_json::json!({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {}, + "required": [], + "additionalProperties": false + })) + .unwrap(), RoleId::Role0, ) .unwrap(); @@ -187,7 +202,7 @@ async fn test_valid_comment_doc_old_type() { }, "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } })), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + serde_json::to_vec(&serde_json::json!({})).unwrap(), RoleId::Role0, ) .unwrap(); diff --git a/rust/signed_doc/tests/proposal.rs b/rust/signed_doc/tests/proposal.rs index a2f3e9fef17..1b0847656dd 100644 --- a/rust/signed_doc/tests/proposal.rs +++ b/rust/signed_doc/tests/proposal.rs @@ -33,7 +33,14 @@ async fn test_valid_proposal_doc() { "ver": template_doc_ver.to_string(), "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } })), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + serde_json::to_vec(&serde_json::json!({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {}, + "required": [], + "additionalProperties": false + })) + .unwrap(), RoleId::Role0, ) .unwrap(); @@ -58,7 +65,7 @@ async fn test_valid_proposal_doc() { "ver": brand_doc_ver } }), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + serde_json::to_vec(&serde_json::json!({})).unwrap(), RoleId::Proposer, ) .unwrap(); @@ -90,7 +97,14 @@ async fn test_valid_proposal_doc_old_type() { "ver": template_doc_ver.to_string(), "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } })), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + serde_json::to_vec(&serde_json::json!({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": {}, + "required": [], + "additionalProperties": false + })) + .unwrap(), RoleId::Role0, ) .unwrap(); @@ -115,7 +129,7 @@ async fn test_valid_proposal_doc_old_type() { "ver": brand_doc_ver } }), - serde_json::to_vec(&serde_json::Value::Null).unwrap(), + serde_json::to_vec(&serde_json::json!({})).unwrap(), RoleId::Proposer, ) .unwrap(); From a11569d76e4a0d3aee0957101e00522c3eb7a167 Mon Sep 17 00:00:00 2001 From: bkioshn Date: Mon, 23 Jun 2025 14:53:50 +0700 Subject: [PATCH 12/12] test(signed-doc): fix content test Signed-off-by: bkioshn --- rust/signed_doc/tests/comment.rs | 1 + rust/signed_doc/tests/proposal.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/rust/signed_doc/tests/comment.rs b/rust/signed_doc/tests/comment.rs index 63225acab93..6c5ca8c5c81 100644 --- a/rust/signed_doc/tests/comment.rs +++ b/rust/signed_doc/tests/comment.rs @@ -202,6 +202,7 @@ async fn test_valid_comment_doc_old_type() { }, "parameters": { "id": brand_doc_id, "ver": brand_doc_ver } })), + // Validate against the ref template serde_json::to_vec(&serde_json::json!({})).unwrap(), RoleId::Role0, ) diff --git a/rust/signed_doc/tests/proposal.rs b/rust/signed_doc/tests/proposal.rs index 1b0847656dd..4b4adf5ad88 100644 --- a/rust/signed_doc/tests/proposal.rs +++ b/rust/signed_doc/tests/proposal.rs @@ -65,6 +65,7 @@ async fn test_valid_proposal_doc() { "ver": brand_doc_ver } }), + // Validate against the ref template serde_json::to_vec(&serde_json::json!({})).unwrap(), RoleId::Proposer, )