-
Notifications
You must be signed in to change notification settings - Fork 53
fix(dpp): remove erroneous keywords field from document-meta schema and fix contract keywords docs #3471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3.1-dev
Are you sure you want to change the base?
fix(dpp): remove erroneous keywords field from document-meta schema and fix contract keywords docs #3471
Changes from all commits
e47d531
44d7038
6d234e9
8c4244c
85081a8
3df26c1
f1ef834
e89e0fc
1fad422
73b972a
aa60910
4249c3a
6aa00c9
1239e2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4357,6 +4357,93 @@ mod tests { | |
| valid_keywords_for_verification.retain(|&x| x != keyword); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_document_type_keywords_rejected_by_v1_meta_schema() { | ||
| use dpp::ProtocolError; | ||
|
|
||
| // `keywords` is a contract-level field only. The v1 document-type | ||
| // meta schema (active as of protocol v12) must reject it on any | ||
| // document type via its root-level `additionalProperties: false`. | ||
| // Pinned to v12 because this is the specific version that introduced | ||
| // v1 meta schema enforcement. | ||
| // | ||
| // No platform/identity setup: this test exercises meta-schema | ||
| // validation inside `DataContract::from_value`, which is a pure DPP | ||
| // call and never reaches Drive or the state-transition pipeline. | ||
| let platform_version = PlatformVersion::get(12).expect("expected v12"); | ||
|
|
||
| let data_contract = json_document_to_contract_with_ids( | ||
| "tests/supporting_files/contract/keyword_test/keyword_base_contract.json", | ||
| None, | ||
| None, | ||
| false, | ||
| platform_version, | ||
| ) | ||
| .expect("expected to load contract"); | ||
|
|
||
| let mut contract_value = data_contract | ||
| .to_value(platform_version) | ||
| .expect("to_value failed"); | ||
|
|
||
| // Inject `keywords` onto the `preorder` document type schema — the | ||
| // wrong place for it. This should be rejected by the v1 meta | ||
| // schema during `DataContract::from_value` full validation. | ||
| contract_value["documentSchemas"]["preorder"]["keywords"] = | ||
| Value::Array(vec![Value::Text("invalid".to_string())]); | ||
|
|
||
| let err = DataContract::from_value(contract_value, true, platform_version) | ||
| .expect_err("meta schema validation must reject document-type keywords"); | ||
|
|
||
| // Assert the failure is specifically a JSON schema validation error | ||
| // (i.e. the meta schema rejected the unknown `keywords` property), | ||
| // not an unrelated error such as a serialization or structural issue. | ||
| match err { | ||
| ProtocolError::ConsensusError(consensus_err) => match *consensus_err { | ||
| ConsensusError::BasicError(BasicError::JsonSchemaError(js_err)) => { | ||
| // The rejection must be driven by `additionalProperties` | ||
| // / `unevaluatedProperties`, and the offending property | ||
| // name must be `keywords` — not just any schema error | ||
| // whose summary happens to mention the string. | ||
| let keyword = js_err.keyword(); | ||
| assert!( | ||
| matches!( | ||
| keyword, | ||
| "additionalProperties" | "unevaluatedProperties" | ||
| ), | ||
| "expected additionalProperties/unevaluatedProperties rejection, got keyword={keyword:?}, summary={}", | ||
| js_err.error_summary() | ||
| ); | ||
|
|
||
| let param_key = if keyword == "additionalProperties" { | ||
| "additionalProperties" | ||
| } else { | ||
| "unexpected" | ||
| }; | ||
| let unexpected = js_err | ||
| .params() | ||
| .get(param_key) | ||
| .ok() | ||
| .flatten() | ||
| .and_then(|v| v.as_array()) | ||
| .unwrap_or_else(|| { | ||
| panic!( | ||
| "expected params[{param_key:?}] array, got params={:?}", | ||
| js_err.params() | ||
| ) | ||
| }); | ||
| assert!( | ||
| unexpected.iter().any(|v| v.as_str() == Some("keywords")), | ||
| "expected `keywords` in rejected properties, got {unexpected:?}" | ||
| ); | ||
|
Comment on lines
+4404
to
+4438
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 Nitpick: Test couples to validator's internal The test branches on source: ['claude'] |
||
| } | ||
| other => panic!( | ||
| "expected BasicError::JsonSchemaError, got ConsensusError: {other:?}" | ||
| ), | ||
| }, | ||
| other => panic!("expected ProtocolError::ConsensusError, got: {other:?}"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| mod descriptions { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 Nitpick: The assertion is weaker than the error type allows and can pass on unrelated failures mentioning
keywordsThe test only checks
js_err.error_summary().contains("keywords"). In this codebaseJsonSchemaErrorexposes structured fields such askeyword(),instance_path(), andproperty_name(), so the test can assert that the failure came from the document-type schema rejecting an unexpected property rather than from any future validation error whose summary also happens to mentionkeywords. As written, the test proves that some schema error referenced the string, not that the intendedadditionalPropertiesorunevaluatedPropertiesrejection occurred at the injected document-type path.source: ['claude']