Skip to content

Commit 60f359b

Browse files
Mr-LeshiyMr-Leshiy
authored
feat(rust/signed-doc): DocLocator validation during fetching from the CatalystSignedDocumentProvider (#659)
* improve referenced_doc_id_and_ver_check with fully verify DocumentRef * refactor `ChainRule::check`, use `doc_refs_check` for properly verify chained document reference * fix clippy, move some code into separate `chain_check` method
1 parent ba6ad3e commit 60f359b

File tree

3 files changed

+141
-117
lines changed

3 files changed

+141
-117
lines changed

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

Lines changed: 125 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33
use anyhow::ensure;
44
use catalyst_signed_doc_spec::{
55
is_required::IsRequired,
6-
metadata::{chain::Chain, collaborators::Collaborators},
6+
metadata::{chain::Chain as ChainSpec, collaborators::Collaborators},
77
};
88

9-
use crate::{CatalystSignedDocument, providers::CatalystSignedDocumentProvider};
9+
use crate::{
10+
CatalystSignedDocument, Chain, providers::CatalystSignedDocumentProvider,
11+
validator::rules::doc_ref::doc_refs_check,
12+
};
1013

1114
#[cfg(test)]
1215
mod tests;
@@ -26,7 +29,7 @@ pub(crate) enum ChainRule {
2629
impl ChainRule {
2730
/// Generating `ChainRule` from specs
2831
pub(crate) fn new(
29-
spec: &Chain,
32+
spec: &ChainSpec,
3033
collaborators_spec: &Collaborators,
3134
) -> anyhow::Result<Self> {
3235
let optional = match spec.required {
@@ -56,9 +59,6 @@ impl ChainRule {
5659
{
5760
let chain = doc.doc_meta().chain();
5861

59-
// TODO: the current implementation is only for the direct chained doc,
60-
// make it recursively checks the entire chain with the same `id` docs.
61-
6262
if let Self::Specified { optional } = self {
6363
if chain.is_none() && !optional {
6464
doc.report()
@@ -68,85 +68,7 @@ impl ChainRule {
6868

6969
// perform integrity validation
7070
if let Some(doc_chain) = chain {
71-
if doc_chain.document_ref().is_none() && doc_chain.height() != 0 {
72-
doc.report().functional_validation(
73-
"The chain height must be zero when there is no chained doc",
74-
"Chained Documents validation",
75-
);
76-
return Ok(false);
77-
}
78-
if doc_chain.height() == 0 && doc_chain.document_ref().is_some() {
79-
doc.report().functional_validation(
80-
"The next Chained Document must not exist while the height is zero",
81-
"Chained Documents validation",
82-
);
83-
return Ok(false);
84-
}
85-
86-
if let Some(chained_ref) = doc_chain.document_ref() {
87-
let Some(chained_doc) = provider.try_get_doc(chained_ref).await? else {
88-
doc.report().other(
89-
&format!(
90-
"Cannot find the Chained Document ({chained_ref}) from the provider"
91-
),
92-
"Chained Documents validation",
93-
);
94-
return Ok(false);
95-
};
96-
97-
// have the same id as the document being chained to.
98-
if chained_doc.doc_id()? != doc.doc_id()? {
99-
doc.report().functional_validation(
100-
"Must have the same id as the document being chained to",
101-
"Chained Documents validation",
102-
);
103-
return Ok(false);
104-
}
105-
106-
// have a ver that is greater than the ver being chained to.
107-
if chained_doc.doc_ver()? > doc.doc_ver()? {
108-
doc.report().functional_validation(
109-
"Must have a ver that is greater than the ver being chained to",
110-
"Chained Documents validation",
111-
);
112-
return Ok(false);
113-
}
114-
115-
// have the same type as the chained document.
116-
if chained_doc.doc_type()? != doc.doc_type()? {
117-
doc.report().functional_validation(
118-
"Must have the same type as the chained document",
119-
"Chained Documents validation",
120-
);
121-
return Ok(false);
122-
}
123-
124-
if let Some(chained_height) =
125-
chained_doc.doc_meta().chain().map(crate::Chain::height)
126-
{
127-
// chain doc must not be negative
128-
if chained_height < 0 {
129-
doc.report().functional_validation(
130-
"The height of the document being chained to must be positive number",
131-
"Chained Documents validation",
132-
);
133-
return Ok(false);
134-
}
135-
136-
// have its absolute height exactly one more than the height of the
137-
// document being chained to.
138-
if !matches!(
139-
i32::abs(doc_chain.height()).checked_sub(i32::abs(chained_height)),
140-
Some(1)
141-
) {
142-
doc.report().functional_validation(
143-
"Must have its absolute height exactly one more than the height of the document being chained to",
144-
"Chained Documents validation",
145-
);
146-
return Ok(false);
147-
}
148-
}
149-
}
71+
return Self::chain_check(doc_chain, doc, provider).await;
15072
}
15173
}
15274
if let Self::NotSpecified = self
@@ -167,4 +89,122 @@ impl ChainRule {
16789

16890
Ok(true)
16991
}
92+
93+
/// `chain` metadata field checks
94+
async fn chain_check<Provider>(
95+
doc_chain: &Chain,
96+
doc: &CatalystSignedDocument,
97+
provider: &Provider,
98+
) -> anyhow::Result<bool>
99+
where
100+
Provider: CatalystSignedDocumentProvider,
101+
{
102+
const CONTEXT: &str = "Chained Documents validation";
103+
104+
if doc_chain.document_ref().is_none() && doc_chain.height() != 0 {
105+
doc.report().functional_validation(
106+
"The chain height must be zero when there is no chained doc",
107+
CONTEXT,
108+
);
109+
return Ok(false);
110+
}
111+
if doc_chain.height() == 0 && doc_chain.document_ref().is_some() {
112+
doc.report().functional_validation(
113+
"The next Chained Document must not exist while the height is zero",
114+
CONTEXT,
115+
);
116+
return Ok(false);
117+
}
118+
119+
if let Some(chained_ref) = doc_chain.document_ref() {
120+
let Ok(expected_doc_type) = doc.doc_type() else {
121+
doc.report().missing_field("type", CONTEXT);
122+
return Ok(false);
123+
};
124+
125+
let chain_validator = |chained_doc: &CatalystSignedDocument| {
126+
let Ok(doc_id) = doc.doc_id() else {
127+
doc.report()
128+
.missing_field("id", "Missing id field in the document");
129+
return false;
130+
};
131+
let Ok(chained_id) = chained_doc.doc_id() else {
132+
doc.report()
133+
.missing_field("id", "Missing id field in the chained document");
134+
return false;
135+
};
136+
// have the same id as the document being chained to.
137+
if chained_id != doc_id {
138+
doc.report().functional_validation(
139+
"Must have the same id as the document being chained to",
140+
CONTEXT,
141+
);
142+
return false;
143+
}
144+
145+
let Ok(doc_ver) = doc.doc_ver() else {
146+
doc.report()
147+
.missing_field("ver", "Missing ver field in the document");
148+
return false;
149+
};
150+
let Ok(chained_ver) = chained_doc.doc_ver() else {
151+
doc.report()
152+
.missing_field("ver", "Missing ver field in the chained document");
153+
return false;
154+
};
155+
// have a ver that is greater than the ver being chained to.
156+
if chained_ver > doc_ver {
157+
doc.report().functional_validation(
158+
"Must have a ver that is greater than the ver being chained to",
159+
CONTEXT,
160+
);
161+
return false;
162+
}
163+
164+
let Some(chained_height) = chained_doc.doc_meta().chain().map(crate::Chain::height)
165+
else {
166+
doc.report()
167+
.missing_field("chain", "Missing chain field in the chained document");
168+
return false;
169+
};
170+
171+
// chain doc must not be negative
172+
if chained_height < 0 {
173+
doc.report().functional_validation(
174+
"The height of the document being chained to must be positive number",
175+
CONTEXT,
176+
);
177+
return false;
178+
}
179+
180+
// have its absolute height exactly one more than the height of the
181+
// document being chained to.
182+
if !matches!(
183+
i32::abs(doc_chain.height()).checked_sub(i32::abs(chained_height)),
184+
Some(1)
185+
) {
186+
doc.report().functional_validation(
187+
"Must have its absolute height exactly one more than the height of the document being chained to",
188+
CONTEXT,
189+
);
190+
return false;
191+
}
192+
193+
true
194+
};
195+
196+
return doc_refs_check(
197+
&vec![chained_ref.clone()].into(),
198+
std::slice::from_ref(expected_doc_type),
199+
false,
200+
"chain",
201+
provider,
202+
doc.report(),
203+
chain_validator,
204+
)
205+
.await;
206+
}
207+
208+
Ok(true)
209+
}
170210
}

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

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ where
148148
for dr in doc_refs.iter() {
149149
if let Some(ref ref_doc) = provider.try_get_doc(dr).await? {
150150
let is_valid = referenced_doc_type_check(ref_doc, exp_ref_types, field_name, report)
151-
&& referenced_doc_id_and_ver_check(ref_doc, dr, field_name, report)
151+
&& referenced_doc_ref_check(ref_doc, dr, field_name, report)
152152
&& validator(ref_doc);
153153

154154
if !is_valid {
@@ -167,39 +167,27 @@ where
167167

168168
/// Validation check that the provided `ref_doc` is a correct referenced document found by
169169
/// `original_doc_ref`
170-
fn referenced_doc_id_and_ver_check(
170+
fn referenced_doc_ref_check(
171171
ref_doc: &CatalystSignedDocument,
172172
original_doc_ref: &DocumentRef,
173173
field_name: &str,
174174
report: &ProblemReport,
175175
) -> bool {
176-
let Ok(id) = ref_doc.doc_id() else {
177-
report.missing_field(
178-
"id",
179-
&format!("Referenced document validation for the `{field_name}` field"),
176+
let context = &format!("Referenced document validation for the `{field_name}` field");
177+
let Ok(doc_ref) = ref_doc.doc_ref() else {
178+
report.functional_validation(
179+
"Cannot calculate a document reference of the fetched document",
180+
context,
180181
);
181182
return false;
182183
};
183184

184-
let Ok(ver) = ref_doc.doc_ver() else {
185-
report.missing_field(
186-
"ver",
187-
&format!("Referenced document validation for the `{field_name}` field"),
188-
);
189-
return false;
190-
};
191-
192-
// id and version must match the values in ref doc
193-
if &id != original_doc_ref.id() && &ver != original_doc_ref.ver() {
185+
if &doc_ref != original_doc_ref {
194186
report.invalid_value(
195-
"id and version",
196-
&format!("id: {id}, ver: {ver}"),
197-
&format!(
198-
"id: {}, ver: {}",
199-
original_doc_ref.id(),
200-
original_doc_ref.ver()
201-
),
202-
&format!("Referenced document validation for the `{field_name}` field"),
187+
"document reference",
188+
&format!("{doc_ref}",),
189+
&format!("{original_doc_ref}",),
190+
context,
203191
);
204192
return false;
205193
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,23 +141,19 @@ use crate::{
141141
.with_metadata_field(SupportedField::Ver(UuidV7::new()))
142142
.with_metadata_field(SupportedField::Type(exp_types[0].clone()))
143143
.build();
144+
144145
let new_ref = create_dummy_doc_ref();
145-
let new_ref = DocumentRef::new(
146-
ref_doc.doc_id().unwrap(),
147-
ref_doc.doc_ver().unwrap(),
148-
new_ref.doc_locator().clone()
149-
);
150-
provider.add_document_with_ref(new_ref, &ref_doc);
146+
provider.add_document_with_ref(new_ref.clone(), &ref_doc);
151147
152148
Builder::new()
153149
.with_metadata_field(SupportedField::Ref(
154-
vec![ref_doc.doc_ref().unwrap()].into(),
150+
vec![new_ref].into(),
155151
))
156152
.build()
157153
}
158154
=> false
159155
;
160-
"invalid reference to the document, which has different id and ver fields as stated in the `ref` field"
156+
"invalid reference in the `ref` field to the document, which is different with the fetched document"
161157
)]
162158
#[test_case(
163159
|_, _| {

0 commit comments

Comments
 (0)