feat: identity reference validation #2993
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ABCI as State Transition ABCI
participant DocValidator as Document Validator (V2)
participant RefValidator as Reference Validator (V0)
participant Platform as Platform State
participant Identity as Identity Store
Client->>ABCI: Submit Document Create/Replace
ABCI->>DocValidator: validate_state_v2(...)
DocValidator->>DocValidator: validate_state_v1(...)
alt v1 fails
DocValidator-->>ABCI: return v1 error
else v1 passes
DocValidator->>RefValidator: validate_document_references(document_data, changed_fields, ...)
RefValidator->>RefValidator: collect properties with refersTo
loop each referenced property
alt refersTo.type == "identity" && mustExist == true
RefValidator->>Platform: fetch_identity_revision(identity_id)
Platform->>Identity: query by id
alt identity found
Identity-->>Platform: identity
Platform-->>RefValidator: success
else identity missing
Identity-->>Platform: not found
Platform-->>RefValidator: error -> ReferencedEntityNotFoundError
RefValidator-->>ABCI: return error
end
else mustExist == false
RefValidator-->>RefValidator: skip existence check
end
end
RefValidator-->>DocValidator: success
DocValidator-->>ABCI: success
end
sequenceDiagram
participant Contract as Contract Definition
participant Parser as Property Parser
participant SchemaCompat as Compatibility Validator
Contract->>Parser: parse document schema
Parser->>Parser: for each property
alt property is identifier
Parser->>Parser: parse refersTo if present -> DocumentPropertyReference
Parser-->>Contract: property with reference attached
else non-identifier + refersTo present
Parser-->>Contract: emit parse error
end
Contract->>SchemaCompat: schema update comparison
alt refersTo added/modified post-creation
SchemaCompat-->>Contract: IncompatibleDocumentTypeSchemaError
else unchanged
SchemaCompat-->>Contract: compatible
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lklimek
force-pushed
the
feat/reference-validation
branch
from
72fede5 to
2eab1ca
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/specs/reference-validation.md`:
- Around line 31-33: The spec currently names the error
ReferencedIdentityNotFoundError but the implementation defines
ReferencedEntityNotFoundError; update the spec text to use
ReferencedEntityNotFoundError everywhere (including any examples/fields like {
path, identityId } -> keep fields consistent with the implementation) so the
documented error name and payload match the actual error type used by the code
(ReferencedEntityNotFoundError).
In `@packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json`:
- Around line 126-144: The schema currently allows "refersTo" on any property,
which can let non-identifier fields pass validation; update the JSON Schema so
that whenever a property contains "refersTo" it is constrained to the identifier
media type and appropriate base type: add an if/then (or dependencies) clause
that checks for the presence of "refersTo" and then enforces "type": "string"
(or the correct identifier base type) and "mediaType":
"application/vnd.dash.identifier" (or the canonical identifier media type used
in the repo), and apply this guard to both occurrences of the "refersTo" block
(the one with properties including "type" and "mustExist" and the second similar
block later in the file) so reference validation no longer accepts
non-identifier fields.
In
`@packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs`:
- Around line 2251-2317: The test functions in this file use imperative names
instead of the required "should ..." convention; rename the four test functions
test_document_replace_fails_when_referenced_identity_missing,
test_document_replace_succeeds_when_must_exist_false,
test_document_replace_validates_only_changed_fields, and
test_document_replace_fails_when_reference_field_changed_to_missing_identity to
start with "should" (e.g.,
should_document_replace_fail_when_referenced_identity_missing,
should_document_replace_succeed_when_must_exist_false,
should_document_replace_validate_only_changed_fields,
should_document_replace_fail_when_reference_field_changed_to_missing_identity)
so they conform to the tests/** naming guideline and update any references to
these symbols accordingly (keep function bodies and assertions unchanged).
In `@packages/rs-json-schema-compatibility-validator/tests/rules.rs`:
- Around line 54-97: Rename the test function to follow the "should …" naming
convention: change the function named test_refers_to_addition_is_incompatible to
a "should" form (for example should_refers_to_addition_be_incompatible or
should_detect_incompatible_refers_to_addition) so the test name reflects the
guideline; update the function declaration (fn
test_refers_to_addition_is_incompatible) to the new name and leave the body,
assertions (result.is_compatible, result.incompatible_changes() check) and
referenced symbols (/properties/toUserId/refersTo,
validate_schemas_compatibility, Options) unchanged.
In `@packages/rs-platform-version/src/version/v12.rs`:
- Around line 33-34: Fix the typo in the module-level documentation comment in
v12.rs: change "Intruduced" to "Introduced" in the doc comment that begins "This
version introduces document reference validation..." so the sentence reads
"Introduced in Platform release 3.1.0." This is the doc comment near the top of
the v12.rs file that documents the version change.
🧹 Nitpick comments (2)
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs (1)
117-154: AlignrefersTovalidation across object/non-object paths.
insert_valuesonly callsparse_property_referenceon non-object properties, so arefersToplaced on an object property could be silently ignored, whileinsert_values_nestedrejects it. Consider validatingrefersTobefore the match to keep behavior consistent.♻️ Suggested adjustment
- let property_type = - DocumentPropertyType::try_from_value_map(&inner_properties, &config.into())?; + let property_type = + DocumentPropertyType::try_from_value_map(&inner_properties, &config.into())?; + let reference = parse_property_reference(&inner_properties, &property_type)?; match property_type { DocumentPropertyType::Object(_) => { if let Some(properties_as_value) = inner_properties.get(property_names::PROPERTIES) { ... } } property_type => { - let reference = parse_property_reference(&inner_properties, &property_type)?; document_properties.insert( prefixed_property_key, DocumentProperty { property_type, required: is_required, transient: is_transient, reference, }, ); } };packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)
35-40: Consider omittingreference: nullfrom serialized output.If
DocumentPropertyis serialized anywhere, the new field will emitreference: nullfor properties without references. If you want to preserve the previous JSON shape, skip serialization when it’sNone.♻️ Suggested adjustment
pub struct DocumentProperty { pub property_type: DocumentPropertyType, pub required: bool, pub transient: bool, + #[serde(skip_serializing_if = "Option::is_none")] pub reference: Option<DocumentPropertyReference>, }
.../execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…feat/reference-validation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.