-
Notifications
You must be signed in to change notification settings - Fork 54
fix(sdk): validate batch base structure before signing #3133
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?
Changes from all commits
2446db0
472bcad
956c192
3235ac7
5647f51
fde8031
555940b
89610b9
c773a81
b81e065
52a04ff
8125b54
10e173a
868e1a6
534d40a
8742a0b
9ab76e3
0ac3718
b7d7471
02f8159
f54eb96
f11960b
ea12f77
1f4c357
db0e002
be00490
11fb8c4
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 |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| use crate::state_transition::batch_transition::document_create_transition::validate_structure::v0::DocumentCreateTransitionStructureValidationV0; | ||
| use crate::state_transition::batch_transition::DocumentCreateTransition; | ||
| use crate::validation::SimpleConsensusValidationResult; | ||
| use crate::ProtocolError; | ||
| use platform_value::Identifier; | ||
| use platform_version::version::PlatformVersion; | ||
|
|
||
| mod v0; | ||
|
|
||
| pub(crate) trait DocumentCreateTransitionStructureValidation { | ||
| fn validate_structure( | ||
| &self, | ||
| owner_id: Identifier, | ||
| platform_version: &PlatformVersion, | ||
| ) -> Result<SimpleConsensusValidationResult, ProtocolError>; | ||
| } | ||
|
Comment on lines
+8
to
+16
Collaborator
Author
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. 🟡 Suggestion: Constructor-only document
source: ['claude-rust-quality', 'codex-rust-quality'] 🤖 Fix this with AI agents |
||
|
|
||
| impl DocumentCreateTransitionStructureValidation for DocumentCreateTransition { | ||
| fn validate_structure( | ||
| &self, | ||
| owner_id: Identifier, | ||
| platform_version: &PlatformVersion, | ||
| ) -> Result<SimpleConsensusValidationResult, ProtocolError> { | ||
| // Dispatch via the DPP-owned version field. The drive-abci action | ||
| // validator has a separate field under | ||
| // `drive_abci.validation_and_processing.state_transitions.batch_state_transition` | ||
| // and is intentionally decoupled from this DPP/SDK pre-sign helper. | ||
| match platform_version | ||
| .dpp | ||
| .state_transitions | ||
| .documents | ||
| .documents_batch_transition | ||
| .validation | ||
| .document_create_transition_structure_validation | ||
| { | ||
| 0 => self.validate_structure_v0(owner_id), | ||
| version => Err(ProtocolError::UnknownVersionMismatch { | ||
| method: "DocumentCreateTransition::validate_structure".to_string(), | ||
| known_versions: vec![0], | ||
| received: version, | ||
| }), | ||
| } | ||
| } | ||
|
Comment on lines
+28
to
+43
Collaborator
Author
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. 🟡 Suggestion: SDK pre-sign The new source: ['claude-general'] 🤖 Fix this with AI agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| use crate::consensus::basic::document::InvalidDocumentTransitionIdError; | ||
| use crate::consensus::basic::BasicError; | ||
| use crate::consensus::ConsensusError; | ||
| use crate::document::Document; | ||
| use crate::state_transition::batch_transition::document_base_transition::document_base_transition_trait::DocumentBaseTransitionAccessors; | ||
| use crate::state_transition::batch_transition::document_base_transition::v0::v0_methods::DocumentBaseTransitionV0Methods; | ||
| use crate::state_transition::batch_transition::document_create_transition::v0::v0_methods::DocumentCreateTransitionV0Methods; | ||
| use crate::state_transition::batch_transition::DocumentCreateTransition; | ||
| use crate::validation::SimpleConsensusValidationResult; | ||
| use crate::ProtocolError; | ||
| use platform_value::Identifier; | ||
|
|
||
| pub(super) trait DocumentCreateTransitionStructureValidationV0 { | ||
| fn validate_structure_v0( | ||
| &self, | ||
| owner_id: Identifier, | ||
| ) -> Result<SimpleConsensusValidationResult, ProtocolError>; | ||
| } | ||
|
|
||
| impl DocumentCreateTransitionStructureValidationV0 for DocumentCreateTransition { | ||
| fn validate_structure_v0( | ||
| &self, | ||
| owner_id: Identifier, | ||
| ) -> Result<SimpleConsensusValidationResult, ProtocolError> { | ||
| let (expected_id, invalid_id) = match self { | ||
| DocumentCreateTransition::V0(transition) => ( | ||
| Document::generate_document_id_v0( | ||
| &transition.base().data_contract_id(), | ||
| &owner_id, | ||
| transition.base().document_type_name(), | ||
| &transition.entropy(), | ||
| ), | ||
| transition.base().id(), | ||
| ), | ||
| }; | ||
|
|
||
| if invalid_id != expected_id { | ||
| return Ok(SimpleConsensusValidationResult::new_with_error( | ||
| ConsensusError::BasicError(BasicError::InvalidDocumentTransitionIdError( | ||
| InvalidDocumentTransitionIdError::new(expected_id, invalid_id), | ||
| )), | ||
| )); | ||
| } | ||
|
|
||
| Ok(SimpleConsensusValidationResult::default()) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ use crate::prelude::IdentityNonce; | |
| use crate::ProtocolError; | ||
| use platform_version::version::{FeatureVersion, PlatformVersion}; | ||
|
|
||
| use crate::state_transition::batch_transition::batched_transition::document_delete_transition::validate_structure::DocumentDeleteTransitionStructureValidation; | ||
| use crate::state_transition::batch_transition::batched_transition::document_delete_transition::DocumentDeleteTransitionV0; | ||
| use crate::state_transition::batch_transition::batched_transition::DocumentDeleteTransition; | ||
| use crate::tokens::token_payment_info::TokenPaymentInfo; | ||
|
|
@@ -27,15 +28,25 @@ impl DocumentDeleteTransition { | |
| .bounds | ||
| .default_current_version, | ||
| ) { | ||
| 0 => Ok(DocumentDeleteTransitionV0::from_document( | ||
| document, | ||
| document_type, | ||
| token_payment_info, | ||
| identity_contract_nonce, | ||
| platform_version, | ||
| base_feature_version, | ||
| )? | ||
| .into()), | ||
| 0 => { | ||
| let transition: DocumentDeleteTransition = | ||
| DocumentDeleteTransitionV0::from_document( | ||
| document, | ||
| document_type, | ||
| token_payment_info, | ||
| identity_contract_nonce, | ||
| platform_version, | ||
| base_feature_version, | ||
| )? | ||
| .into(); | ||
| if let Some(error) = transition | ||
| .validate_structure(document_type, platform_version)? | ||
| .errors_to_consensus_protocol_error() | ||
| { | ||
| return Err(error); | ||
| } | ||
|
Comment on lines
+42
to
+47
Collaborator
Author
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: 5 from_document constructors silently truncate validate_structure errors to the first one Each of source: ['claude-rust-quality']
Comment on lines
+42
to
+47
Collaborator
Author
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. 🟡 Suggestion: Five Each of source: ['claude-general', 'claude-rust-quality', 'codex-rust-quality'] 🤖 Fix this with AI agents |
||
| Ok(transition) | ||
| } | ||
| version => Err(ProtocolError::UnknownVersionMismatch { | ||
| method: "DocumentDeleteTransition::from_document".to_string(), | ||
| known_versions: vec![0], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| use crate::data_contract::document_type::DocumentTypeRef; | ||
| use crate::state_transition::batch_transition::batched_transition::document_delete_transition::validate_structure::v0::DocumentDeleteTransitionStructureValidationV0; | ||
| use crate::state_transition::batch_transition::batched_transition::DocumentDeleteTransition; | ||
| use crate::validation::SimpleConsensusValidationResult; | ||
| use crate::ProtocolError; | ||
| use platform_version::version::PlatformVersion; | ||
|
|
||
| mod v0; | ||
|
|
||
| pub(crate) trait DocumentDeleteTransitionStructureValidation { | ||
| fn validate_structure( | ||
| &self, | ||
| document_type: DocumentTypeRef, | ||
| platform_version: &PlatformVersion, | ||
| ) -> Result<SimpleConsensusValidationResult, ProtocolError>; | ||
| } | ||
|
|
||
| impl DocumentDeleteTransitionStructureValidation for DocumentDeleteTransition { | ||
| fn validate_structure( | ||
| &self, | ||
| document_type: DocumentTypeRef, | ||
| platform_version: &PlatformVersion, | ||
| ) -> Result<SimpleConsensusValidationResult, ProtocolError> { | ||
| match platform_version | ||
| .dpp | ||
| .state_transitions | ||
| .documents | ||
| .documents_batch_transition | ||
| .validation | ||
| .document_delete_transition_structure_validation | ||
| { | ||
| 0 => self.validate_structure_v0(document_type), | ||
| version => Err(ProtocolError::UnknownVersionMismatch { | ||
| method: "DocumentDeleteTransition::validate_structure".to_string(), | ||
| known_versions: vec![0], | ||
| received: version, | ||
| }), | ||
| } | ||
| } | ||
| } |
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.
🟡 Suggestion:
#[non_exhaustive]on publicProtocolErroris a downstream-visible break not noted in PR descriptionProtocolErroris re-exported fromdppand pattern-matched in many crates outside this workspace. Adding#[non_exhaustive]forces all exhaustive matches in other crates (no_ =>arm) to fail with E0004. The CHANGELOG entry calls this out; the PR description currently says "Breaking Changes: None". Align the PR description with the CHANGELOG so downstream Rust SDK consumers know a wildcard arm is needed, and prefer appending future variants (the newConsensusErrorswas inserted mid-enum, which is safe today only becauseProtocolErroris not platform-serialized).source: ['claude']
🤖 Fix this with AI agents