feat(sdk): add client-side validate_base_structure for document and token transitions#3133
Open
thepastaclaw wants to merge 7 commits intodashpay:v3.1-devfrom
Open
feat(sdk): add client-side validate_base_structure for document and token transitions#3133thepastaclaw wants to merge 7 commits intodashpay:v3.1-devfrom
thepastaclaw wants to merge 7 commits intodashpay:v3.1-devfrom
Conversation
thepastaclaw
requested review from
QuantumExplorer and
shumkov
as code owners
Contributor
📝 WalkthroughWalkthroughAdds runtime base-structure validation for Batch state transitions across many document and token transition builders, enables the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![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: 3
🧹 Nitpick comments (7)
packages/rs-sdk/src/platform/tokens/builders/unfreeze.rs (2)
188-202: Extract the repeated validation block into a shared helper to eliminate copy-paste across 17 files.The identical 14-line block (lines 188–202 here, and equivalent blocks in every other builder) is copy-pasted verbatim into all 17 token and document transition builders in this PR. This is a significant DRY violation that multiplies any future fix or change.
Consider adding a helper (e.g., in
packages/rs-sdk/src/platform/transition/mod.rsor a newvalidation.rs) and calling it from each builder:♻️ Proposed helper extraction
// In a shared module, e.g. packages/rs-sdk/src/platform/transition/validate.rs use crate::Error; use dpp::state_transition::StateTransition; use dpp::version::PlatformVersion; use dpp::state_transition::batch_transition::methods::v1::DocumentsBatchTransitionMethodsV1; pub fn validate_state_transition_is_valid_batch( state_transition: &StateTransition, platform_version: &PlatformVersion, ) -> Result<(), Error> { let validation_result = match state_transition { StateTransition::Batch(batch_transition) => { batch_transition.validate_base_structure(platform_version)? } _ => { return Err(Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType( "expected Batch transition".to_string(), ))); } }; if let Some(first_error) = validation_result.errors.into_iter().next() { return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); } Ok(()) }Each builder's
signmethod then becomes:- // Validate the transition structure before returning - let validation_result = match &state_transition { - StateTransition::Batch(batch_transition) => { - batch_transition.validate_base_structure(platform_version)? - } - _ => { - return Err(Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ))); - } - }; - if !validation_result.is_valid() { - let first_error = validation_result.errors.into_iter().next().unwrap(); - return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); - } + validate_state_transition_is_valid_batch(&state_transition, platform_version)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/unfreeze.rs` around lines 188 - 202, Extract the repeated 14-line validation into a shared helper function (e.g., validate_state_transition_is_valid_batch) placed in a common module such as platform/transition/validate.rs or platform/transition/mod.rs; the helper should accept &StateTransition and &PlatformVersion, perform the match on StateTransition::Batch, call batch_transition.validate_base_structure(platform_version) and convert any validation_result.errors.first() into Error::Protocol(dpp::ProtocolError::ConsensusError(...)) or return Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType("expected Batch transition".to_string())) for non-Batch variants; then replace the duplicated block in builders (e.g., in unfreeze.rs sign method referencing state_transition, validation_result, batch_transition.validate_base_structure) with a single call to this helper and propagate its Result.
199-201: Replaceunwrap()with the idiomaticif let Some(...)pattern.
validation_result.errors.into_iter().next().unwrap()relies on the implicit invariant thatis_valid() == falseimplieserrorsis non-empty. Idiomatic Rust makes this invariant explicit withoutunwrap(), and theis_valid()guard becomes redundant:♻️ Proposed fix (applies identically to all 17 builder files in this PR)
- if !validation_result.is_valid() { - let first_error = validation_result.errors.into_iter().next().unwrap(); - return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); - } + if let Some(first_error) = validation_result.errors.into_iter().next() { + return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/unfreeze.rs` around lines 199 - 201, Replace the unwrap() call on validation_result.errors.into_iter().next() with an explicit pattern match (if let Some(...) or match) to avoid panicking; extract the first_error via if let Some(first_error) = validation_result.errors.into_iter().next() { return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); } else { return Err(/* return a sensible Protocol/Consensus error for the no-error case */); } — update the code around validation_result and first_error (the Error::Protocol(dpp::ProtocolError::ConsensusError(...)) return) so it never calls unwrap()..gitignore (1)
87-88: Consider adding a comment or separate section forworktrees/.This entry is placed directly under the gRPC coverage report section without a grouping comment. A brief
# Git worktreescomment would make the intent clear to other contributors.✏️ Suggested addition
# gRPC coverage report grpc-coverage-report.txt + +# Git worktrees worktrees/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 87 - 88, The .gitignore has a standalone entry "worktrees/" placed under the grpc-coverage-report section—add a brief grouping comment (e.g. "# Git worktrees") or create a separate section header above the "worktrees/" entry so its purpose is clear to contributors; update the .gitignore near the existing "worktrees/" line to include that comment.packages/rs-sdk/src/platform/tokens/builders/mint.rs (1)
209-223: Extract the repeated validation block into a shared helper function.This exact 14-line validation snippet is duplicated verbatim across all token and document transition builders (~17 call sites per the PR description). A small helper would eliminate the repetition and ensure any future changes (e.g., reporting all errors, different error wrapping) are applied in one place.
♻️ Suggested helper (e.g., in a shared utility module)
/// Validates that `state_transition` is a `Batch` variant with a valid base structure. pub fn validate_batch_base_structure( state_transition: &StateTransition, platform_version: &PlatformVersion, ) -> Result<(), Error> { let validation_result = match state_transition { StateTransition::Batch(batch_transition) => { batch_transition.validate_base_structure(platform_version)? } _ => { return Err(Error::Protocol( dpp::ProtocolError::InvalidStateTransitionType( "expected Batch transition".to_string(), ), )); } }; if !validation_result.is_valid() { let first_error = validation_result.errors.into_iter().next().unwrap(); return Err(Error::Protocol(dpp::ProtocolError::ConsensusError( Box::new(first_error), ))); } Ok(()) }Each builder then collapses to a single call:
- // Validate the transition structure before returning - let validation_result = match &state_transition { - StateTransition::Batch(batch_transition) => { - batch_transition.validate_base_structure(platform_version)? - } - _ => { - return Err(Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ))); - } - }; - if !validation_result.is_valid() { - let first_error = validation_result.errors.into_iter().next().unwrap(); - return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); - } + validate_batch_base_structure(&state_transition, platform_version)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/mint.rs` around lines 209 - 223, Extract the duplicated 14-line validation into a shared helper named something like validate_batch_base_structure(&StateTransition, &PlatformVersion) -> Result<(), Error>; move the match on StateTransition::Batch, the call to batch_transition.validate_base_structure(platform_version) and the is_valid/error handling into that helper (preserving the InvalidStateTransitionType and ConsensusError wrapping), then replace the inline block in each builder (where the code matches StateTransition and checks validation_result) with a single call to validate_batch_base_structure(state_transition, platform_version). Ensure the helper returns Ok(()) on success and the same Error variants on failure so callers like the mint/builders can simply propagate the Result.packages/rs-sdk/src/platform/documents/transitions/delete.rs (1)
210-224: Identical duplicated validation block — same refactor applies.Same verbatim copy as
set_price.rslines 211–225. Once the shared helper from theset_price.rscomment is extracted, this block reduces to a single call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/delete.rs` around lines 210 - 224, This block duplicates the batch state transition validation used in set_price.rs; extract the shared validation logic into a helper (e.g., validate_batch_state_transition or reuse the helper you created for set_price.rs) that accepts &state_transition and platform_version and returns Result<(), ProtocolError>/ValidationResult; then replace the inline match/validation_result/error-unwrapping in delete.rs with a single call to that helper (handling the returned error by mapping it to Error::Protocol(dpp::ProtocolError::ConsensusError(...)) to preserve existing behavior), ensuring you still check the Batch variant and propagate protocol/consensus errors as before.packages/rs-sdk/src/platform/documents/transitions/create.rs (1)
170-184: Identical duplicated validation block — same refactor applies.Same verbatim copy as
set_price.rslines 211–225. Once the shared helper from theset_price.rscomment is extracted, this block reduces to a single call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/create.rs` around lines 170 - 184, Duplicate validation logic: extract the repeated block that checks for StateTransition::Batch and calls batch_transition.validate_base_structure into a shared helper (e.g., validate_batch_base_structure) that returns Result<(), Error>; replace the block in create.rs (the state_transition match/validation_result handling referencing StateTransition::Batch, batch_transition.validate_base_structure, validation_result, and the Error::Protocol/Dpp errors) with a single call to that helper, and update the analogous block in set_price.rs to call the same helper so both files reuse the extracted validation logic.packages/rs-sdk/src/platform/documents/transitions/set_price.rs (1)
211-225: Create a shared helper for Batch transition validation in builder modules.There are 17+ builder files across
packages/rs-sdk/src/platform/documents/transitions/andpackages/rs-sdk/src/platform/tokens/builders/duplicating this exact 14-line block:let validation_result = match &state_transition { StateTransition::Batch(batch_transition) => { batch_transition.validate_base_structure(platform_version)? } _ => { return Err(Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType( "expected Batch transition".to_string(), ))); } }; if !validation_result.is_valid() { let first_error = validation_result.errors.into_iter().next().unwrap(); return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); }Extract this to a shared helper function in the appropriate module (e.g.,
packages/rs-sdk/src/platform/helpers/or within the builders/transitions modules). Note:ensure_valid_state_transition_structureexists in the transition module and serves a different purpose (generic state transition validation with UnsupportedFeatureError handling), so this extraction should be specific to Batch transition validation in builders.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/set_price.rs` around lines 211 - 225, Extract the duplicated 14-line Batch transition validation into a shared helper named something like ensure_batch_transition_structure that takes a &StateTransition and platform_version and returns Result<ValidationResult, Error> (or Result<(), Error> after converting errors), locate and call it from builder modules instead of repeating the match; inside the helper perform the match on StateTransition::Batch, call batch_transition.validate_base_structure(platform_version) and map non-valid results to Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error))) and non-Batch cases to Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType("expected Batch transition".to_string())); do not change ensure_valid_state_transition_structure (it serves a different purpose).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-sdk/src/platform/documents/transitions/create.rs`:
- Around line 181-183: Replace the unsafe unwrap when extracting the first
validation error from validation_result.errors with an if let pattern: attempt
to get the first error via if let Some(first_error) =
validation_result.errors.into_iter().next() { return
Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error))));
} else { return
Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(<create a new
error/message indicating "no validation errors found" or similar>)))); } This
change should be applied to the block using validation_result and first_error in
create.rs to avoid a potential panic.
In `@packages/rs-sdk/src/platform/documents/transitions/delete.rs`:
- Around line 221-223: Replace the unwrap() on
validation_result.errors.into_iter().next() with an if let Some(...) pattern: if
let Some(first_error) = validation_result.errors.into_iter().next() { return
Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error))));
} else { return
Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(/* create a
small fallback/generic consensus error indicating validation failed with no
details */)))); } This removes the panic risk in the validation branch that uses
validation_result.is_valid() and mirrors the safe handling used in set_price.rs.
In `@packages/rs-sdk/src/platform/documents/transitions/set_price.rs`:
- Around line 222-224: The code uses unwrap() when pulling the first error from
validation_result (validation_result.errors.into_iter().next().unwrap()), which
can panic if the invariant is ever broken; change this to a safe pattern: check
for Some(err) with if let Some(first_error) =
validation_result.errors.into_iter().next() { return
Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error))));
} else { return
Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(/* create a
fallback error or use a generic consensus error here */)))); }—apply the same
if-let pattern to all similar occurrences in set_price.rs and other builder
files to avoid unwrap panics and provide a sensible fallback error when no items
are present.
---
Duplicate comments:
In `@packages/rs-sdk/src/platform/documents/transitions/purchase.rs`:
- Around line 224-238: This block duplicates validation logic from mint.rs;
factor it into a shared helper (e.g., validate_batch_state_transition_structure)
and call it from purchase.rs instead of repeating the match. The helper should
accept the StateTransition reference and platform_version, perform the match
against StateTransition::Batch, call
batch_transition.validate_base_structure(platform_version), check
validation_result.is_valid(), and return a Result<(), dpp::ProtocolError>
(wrapping the first consensus error when invalid). Replace the existing
match/validation in purchase.rs (the state_transition / StateTransition::Batch
branch and subsequent error mapping) with a single call to that new helper.
In `@packages/rs-sdk/src/platform/documents/transitions/replace.rs`:
- Around line 163-177: The validation block duplicating logic (matching
StateTransition::Batch, calling
batch_transition.validate_base_structure(platform_version), checking
validation_result.is_valid(), and converting the first error into
Error::Protocol(dpp::ProtocolError::ConsensusError(...))) should be extracted
into a shared helper (e.g., validate_state_transition_base_structure) and used
here instead of duplicating; update replace.rs to call that helper with the
state_transition and platform_version, remove the inline match/validation_result
logic, and ensure the helper returns the same Result<(), Error> so both
replace.rs and mint.rs can reuse it (reference symbols: StateTransition::Batch,
batch_transition.validate_base_structure, validation_result.is_valid()).
In `@packages/rs-sdk/src/platform/documents/transitions/transfer.rs`:
- Around line 210-224: This duplicate validation block should be replaced with
the shared helper introduced for mint.rs: extract the common logic that matches
on StateTransition, calls validate_base_structure(platform_version) for
StateTransition::Batch, and converts an invalid ValidationResult into
Error::Protocol(ProtocolError::ConsensusError(...)); then call that helper here
instead of duplicating the match/validation logic in transfer.rs (refer to
symbols StateTransition, Batch, validate_base_structure, ValidationResult, and
the Error::Protocol/ProtocolError::ConsensusError conversion to implement the
helper and invoke it from this function).
In `@packages/rs-sdk/src/platform/tokens/builders/burn.rs`:
- Around line 182-196: The validation block for state_transition duplicates
logic and uses unwrap() unsafely; extract a small helper (e.g.,
validate_batch_base_structure or validate_state_transition_base) and replace the
duplicated code in burn.rs (and the identical block in unfreeze.rs) to call that
helper; have the helper match on state_transition (StateTransition::Batch ->
call batch_transition.validate_base_structure(platform_version) otherwise return
ProtocolError::InvalidStateTransitionType) and then check
validation_result.is_valid(), returning a clear ConsensusError with the first
error if any, but avoid unwrap() by handling the Option from
errors.into_iter().next() (return a generic ConsensusError if none) so no panic
occurs.
In `@packages/rs-sdk/src/platform/tokens/builders/claim.rs`:
- Around line 168-182: The validation block for StateTransition::Batch
duplicates logic and uses unwrap() unsafely; refactor to remove duplication and
avoid panics by handling empty error lists. Replace the direct match and
subsequent unwrap with a helper function (e.g., validate_batch_structure) that
accepts &state_transition, matches StateTransition::Batch, calls
batch_transition.validate_base_structure(platform_version) and returns
Result<(), Error>, and ensure when validation_result.is_valid() is false you
safely extract the first error without unwrap (e.g., using
validation_result.errors.into_iter().next().ok_or_else(...) to produce a
ProtocolError) so StateTransition::Batch matching, calling
validate_base_structure, and error conversion are centralized and safe.
In `@packages/rs-sdk/src/platform/tokens/builders/config_update.rs`:
- Around line 190-204: Extract the duplicated validation logic into a shared
helper, e.g. a function validate_batch_base_structure(state_transition:
&StateTransition, platform_version: PlatformVersion) -> Result<(), Error> that
matches StateTransition::Batch and calls
batch_transition.validate_base_structure(platform_version)? and maps
validation_result errors into the same
Error::Protocol(dpp::ProtocolError::ConsensusError(...)) or
InvalidStateTransitionType as needed; then replace the duplicated block in
config_update.rs (the match on StateTransition::Batch + validation_result
handling) with a call to validate_batch_base_structure(...) and do the same in
mint.rs so both files reuse the helper and keep identical error behavior.
In `@packages/rs-sdk/src/platform/tokens/builders/destroy.rs`:
- Around line 188-202: This block repeats the same validate-and-unwrap pattern
and unsafely calls unwrap() on validation_result.errors.next(); replace it with
a safe-option-to-error conversion (e.g., use .into_iter().next().ok_or(...) or
match to produce a ProtocolError when there are no errors) and propagate that
error instead of panicking, and remove duplication by extracting the logic into
a shared helper (e.g., a function named validate_batch_base_structure that
accepts a &StateTransition, calls
batch_transition.validate_base_structure(platform_version), checks is_valid(),
and returns Result<(), Error::Protocol(...)>) and call that helper from both
destroy.rs and unfreeze.rs; reference symbols: StateTransition::Batch,
batch_transition.validate_base_structure, validation_result, and the
construction of Error::Protocol(dpp::ProtocolError::ConsensusError(...)).
In `@packages/rs-sdk/src/platform/tokens/builders/emergency_action.rs`:
- Around line 216-230: The validation block for StateTransition::Batch in
emergency_action.rs is a duplicate of the one in mint.rs; extract this logic
into a shared helper (e.g., validate_batch_state_transition or
validate_state_transition_base_structure) that accepts a StateTransition
reference and platform_version and returns a validated result or maps errors to
Error::Protocol (preserving the dpp::ProtocolError::InvalidStateTransitionType
and ConsensusError wrapping behavior). Replace the inline match/validation in
emergency_action.rs (the StateTransition::Batch match, validate_base_structure
call, and subsequent invalid-result error mapping) with a call to the new helper
so both emergency_action.rs and mint.rs reuse the common function. Ensure the
helper returns the same error types and messages so callers' error handling
remains unchanged.
In `@packages/rs-sdk/src/platform/tokens/builders/freeze.rs`:
- Around line 188-202: This block duplicates validation logic found in mint.rs:
replace the inline match/validation in freeze.rs (around StateTransition::Batch
/ batch_transition.validate_base_structure) with the shared helper function used
by mint.rs (the common validator that takes a StateTransition and
platform_version and returns validation_result or an error); call that helper
instead of duplicating the match, preserve the same error mapping to
Error::Protocol / dpp::ProtocolError::InvalidStateTransitionType and
ConsensusError when validation_result.is_valid() is false, and remove the
duplicated code block so both mint.rs and freeze.rs use the single helper.
In `@packages/rs-sdk/src/platform/tokens/builders/purchase.rs`:
- Around line 156-170: The validation block for StateTransition::Batch
duplicates logic elsewhere and uses unwrap() unsafely; refactor by extracting a
helper (e.g., validate_batch_base_structure or
validate_transition_base_structure) that accepts a &StateTransition, matches
StateTransition::Batch, calls
batch_transition.validate_base_structure(platform_version) and returns
Result<(), Error>, then replace the repeated blocks in purchase.rs and
unfreeze.rs with calls to that helper; also remove the unwrap() by explicitly
handling the Option from validation_result.errors.into_iter().next()—return an
appropriate Protocol::ConsensusError when no error is present (instead of
panicking).
In `@packages/rs-sdk/src/platform/tokens/builders/set_price.rs`:
- Around line 233-247: This block duplicates validation logic from mint.rs —
replace the inline match on state_transition and the subsequent
validation_result handling with the shared helper used in mint.rs (e.g., call
the common validate_state_transition_base_structure helper/function), keeping
the same semantics: accept only StateTransition::Batch, call
batch_transition.validate_base_structure(platform_version) via the helper, and
convert invalid ValidationResult into
Error::Protocol(dpp::ProtocolError::ConsensusError(...)) or the existing
InvalidStateTransitionType error; update references to StateTransition::Batch
and batch_transition.validate_base_structure to use the shared function to avoid
duplication.
In `@packages/rs-sdk/src/platform/tokens/builders/transfer.rs`:
- Around line 214-228: The validation block for state_transition duplicates code
and uses unwrap() unsafely; in the StateTransition::Batch match arm (and
surrounding logic referencing state_transition, StateTransition::Batch,
batch_transition.validate_base_structure, validation_result, first_error)
replace the duplicated pattern with a shared helper or common function that
returns a Result<ValidationResult, Error> and avoid unwrap() by handling the
Option from validation_result.errors.into_iter().next() — if no error exists
return a sensible Protocol Error (or map to an existing ConsensusError) instead
of panicking; keep the same error wrapping with
Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(...))) when an
actual error is present.
---
Nitpick comments:
In @.gitignore:
- Around line 87-88: The .gitignore has a standalone entry "worktrees/" placed
under the grpc-coverage-report section—add a brief grouping comment (e.g. "# Git
worktrees") or create a separate section header above the "worktrees/" entry so
its purpose is clear to contributors; update the .gitignore near the existing
"worktrees/" line to include that comment.
In `@packages/rs-sdk/src/platform/documents/transitions/create.rs`:
- Around line 170-184: Duplicate validation logic: extract the repeated block
that checks for StateTransition::Batch and calls
batch_transition.validate_base_structure into a shared helper (e.g.,
validate_batch_base_structure) that returns Result<(), Error>; replace the block
in create.rs (the state_transition match/validation_result handling referencing
StateTransition::Batch, batch_transition.validate_base_structure,
validation_result, and the Error::Protocol/Dpp errors) with a single call to
that helper, and update the analogous block in set_price.rs to call the same
helper so both files reuse the extracted validation logic.
In `@packages/rs-sdk/src/platform/documents/transitions/delete.rs`:
- Around line 210-224: This block duplicates the batch state transition
validation used in set_price.rs; extract the shared validation logic into a
helper (e.g., validate_batch_state_transition or reuse the helper you created
for set_price.rs) that accepts &state_transition and platform_version and
returns Result<(), ProtocolError>/ValidationResult; then replace the inline
match/validation_result/error-unwrapping in delete.rs with a single call to that
helper (handling the returned error by mapping it to
Error::Protocol(dpp::ProtocolError::ConsensusError(...)) to preserve existing
behavior), ensuring you still check the Batch variant and propagate
protocol/consensus errors as before.
In `@packages/rs-sdk/src/platform/documents/transitions/set_price.rs`:
- Around line 211-225: Extract the duplicated 14-line Batch transition
validation into a shared helper named something like
ensure_batch_transition_structure that takes a &StateTransition and
platform_version and returns Result<ValidationResult, Error> (or Result<(),
Error> after converting errors), locate and call it from builder modules instead
of repeating the match; inside the helper perform the match on
StateTransition::Batch, call
batch_transition.validate_base_structure(platform_version) and map non-valid
results to
Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error))) and
non-Batch cases to
Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType("expected Batch
transition".to_string())); do not change ensure_valid_state_transition_structure
(it serves a different purpose).
In `@packages/rs-sdk/src/platform/tokens/builders/mint.rs`:
- Around line 209-223: Extract the duplicated 14-line validation into a shared
helper named something like validate_batch_base_structure(&StateTransition,
&PlatformVersion) -> Result<(), Error>; move the match on
StateTransition::Batch, the call to
batch_transition.validate_base_structure(platform_version) and the
is_valid/error handling into that helper (preserving the
InvalidStateTransitionType and ConsensusError wrapping), then replace the inline
block in each builder (where the code matches StateTransition and checks
validation_result) with a single call to
validate_batch_base_structure(state_transition, platform_version). Ensure the
helper returns Ok(()) on success and the same Error variants on failure so
callers like the mint/builders can simply propagate the Result.
In `@packages/rs-sdk/src/platform/tokens/builders/unfreeze.rs`:
- Around line 188-202: Extract the repeated 14-line validation into a shared
helper function (e.g., validate_state_transition_is_valid_batch) placed in a
common module such as platform/transition/validate.rs or
platform/transition/mod.rs; the helper should accept &StateTransition and
&PlatformVersion, perform the match on StateTransition::Batch, call
batch_transition.validate_base_structure(platform_version) and convert any
validation_result.errors.first() into
Error::Protocol(dpp::ProtocolError::ConsensusError(...)) or return
Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType("expected Batch
transition".to_string())) for non-Batch variants; then replace the duplicated
block in builders (e.g., in unfreeze.rs sign method referencing
state_transition, validation_result, batch_transition.validate_base_structure)
with a single call to this helper and propagate its Result.
- Around line 199-201: Replace the unwrap() call on
validation_result.errors.into_iter().next() with an explicit pattern match (if
let Some(...) or match) to avoid panicking; extract the first_error via if let
Some(first_error) = validation_result.errors.into_iter().next() { return
Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error))));
} else { return Err(/* return a sensible Protocol/Consensus error for the
no-error case */); } — update the code around validation_result and first_error
(the Error::Protocol(dpp::ProtocolError::ConsensusError(...)) return) so it
never calls unwrap().
packages/rs-sdk/src/platform/documents/transitions/set_price.rs
Outdated
Show resolved
Hide resolved
This was referenced Feb 20, 2026
CI: dash-sdk dead_code lint failure (parse_proof) on PRs #3131, #3132, #3133
thepastaclaw/tracker#43
Closed
thepastaclaw
force-pushed
the
feat/sdk-validate-all-transitions
branch
from
0f7b279 to
3b7eb4d
Compare
…oken transitions Add structural validation to all document and token SDK transition builders, matching the pattern from PR dashpay#3096 (identity/address transitions). Calls validate_base_structure() on BatchTransition after construction but before broadcast, catching invalid transitions early. Applied to: - Document transitions: create, delete, replace, purchase, set_price, transfer - Token builders: burn, claim, config_update, destroy, purchase, emergency_action, freeze, mint, set_price, transfer, unfreeze - Enabled dpp 'validation' feature for dash-sdk crate
CodeRabbit correctly flagged that validation_result.errors.into_iter() .next().unwrap() could panic if is_valid() returns false but the errors vec is somehow empty. Use if-let pattern instead for safe handling.
The validation feature added to rs-sdk's dpp dependency now enforces that indexed string properties must have maxLength <= 63. The rarity field in the crypto card game test fixture was missing this constraint, causing all DataContract tests to fail.
thepastaclaw
force-pushed
the
feat/sdk-validate-all-transitions
branch
from
3b7eb4d to
47269ca
Compare
…and token builders Tests cover: - InvalidTokenAmountError via TokenMintTransitionBuilder with amount=0 - InvalidActionIdError via TokenMintTransitionBuilder with mismatched group action ID - Document nonce masking (validates nonce out-of-bounds is unreachable via builder API) Documents that several error paths (empty transitions, max exceeded, duplicate transitions, invalid token ID) are unreachable through the single-transition builder API by design.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rs-sdk/src/platform/tokens/builders/freeze.rs (1)
32-32:⚠️ Potential issue | 🟡 MinorCopy-paste doc comment error: "mint" should be "freeze".
The doc comment for
newwas copied frommint.rsand still reads "Start building a mint tokens request."📝 Proposed fix
- /// Start building a mint tokens request for the provided DataContract. + /// Start building a freeze tokens request for the provided DataContract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/freeze.rs` at line 32, The doc comment for the constructor in freeze.rs was copied from mint.rs and incorrectly says "Start building a mint tokens request"; update the documentation for the function/impl named `new` (or the builder type in freeze.rs) to describe that it "Start building a freeze tokens request" (or equivalent phrasing mentioning "freeze tokens") so the comment matches the freeze builder's purpose; ensure the doc comment above the `new` function/struct is changed accordingly.
🧹 Nitpick comments (16)
.gitignore (1)
86-88: Misplaced entry and consider anchoring the pattern
worktrees/is a git worktree artefact and is unrelated to gRPC coverage reports, so placing it immediately under the# gRPC coverage reportcomment is misleading. Additionally, without a leading/the pattern matches any directory namedworktreesat any depth in the tree; if worktrees are only ever created at the repository root (the typicalgit worktree add worktrees/<name>convention), anchoring with a leading/makes the intent explicit.♻️ Proposed fix
# gRPC coverage report grpc-coverage-report.txt -worktrees/ + +# Git worktrees +/worktrees/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 86 - 88, The .gitignore currently places the worktrees/ entry under the "# gRPC coverage report" comment and leaves it unanchored; move the worktrees/ line out from beneath the gRPC comment into a more appropriate section (e.g., a worktree or local dev artifacts section) and anchor it by prepending a slash so the pattern reads "/worktrees/" to ensure it only matches a top-level worktrees directory; update the comment grouping so the grpc-coverage-report.txt stays under the gRPC section and "/worktrees/" is clearly associated with worktree artifacts.packages/wasm-sdk/tests/unit/fixtures/data-contract-v0-crypto-card-game.ts (1)
52-63:maxLength: 9is redundant alongside theenumconstraint but harmless.All four enum values already fit within 9 characters (
'legendary'is exactly 9). Since theenumconstraint already fully restricts the field to those values,maxLengthadds no additional restriction at runtime. If the intent is to exercise themaxLengthvalidation path in the new structural validation tests, that is fine — just worth a brief inline comment so future readers understand why both exist together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-sdk/tests/unit/fixtures/data-contract-v0-crypto-card-game.ts` around lines 52 - 63, The rarity property currently has both enum and maxLength (maxLength: 9) which is redundant because each enum value already fits within 9 chars; either remove the maxLength field from the rarity definition (in the rarity object) or, if the intent is to explicitly exercise the maxLength validation path in tests, add a short inline comment next to the rarity/maxLength entry explaining that maxLength is intentionally kept to trigger structural validation tests (reference symbols: rarity, maxLength, enum).packages/rs-sdk/src/platform/tokens/builders/claim.rs (1)
168-185: Consider extracting the validation block into a shared helper to reduce duplication across all builders.This exact validation pattern (match on
Batch, callvalidate_base_structure, return first error asConsensusError) is repeated identically across ~17 builder files (all document transitions + all token builders). The test modules already definevalidate_transition_like_builderwhich does exactly this. A production-side counterpart would eliminate the duplication:♻️ Suggested shared helper (e.g., in a common transitions utility module)
pub(crate) fn validate_batch_base_structure( state_transition: &StateTransition, platform_version: &PlatformVersion, ) -> Result<(), Error> { let validation_result = match state_transition { StateTransition::Batch(batch_transition) => { batch_transition.validate_base_structure(platform_version)? } _ => { return Err(Error::Protocol( dpp::ProtocolError::InvalidStateTransitionType( "expected Batch transition".to_string(), ), )); } }; if let Some(first_error) = validation_result.errors.into_iter().next() { return Err(Error::Protocol(dpp::ProtocolError::ConsensusError( Box::new(first_error), ))); } Ok(()) }Each builder then reduces to a single call:
- // Validate the transition structure before returning - let validation_result = match &state_transition { - StateTransition::Batch(batch_transition) => { - batch_transition.validate_base_structure(platform_version)? - } - _ => { - return Err(Error::Protocol( - dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ), - )); - } - }; - if let Some(first_error) = validation_result.errors.into_iter().next() { - return Err(Error::Protocol(dpp::ProtocolError::ConsensusError( - Box::new(first_error), - ))); - } + validate_batch_base_structure(&state_transition, platform_version)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/claim.rs` around lines 168 - 185, Extract the repeated validation into a shared helper (e.g., validate_batch_base_structure) that takes &StateTransition and &PlatformVersion and performs the match on StateTransition::Batch, calls batch_transition.validate_base_structure(platform_version) and converts the first validation error into Error::Protocol(dpp::ProtocolError::ConsensusError(...)) or returns Ok(()); then replace the inline block in claim.rs (the match on StateTransition::Batch, validate_base_structure, and the first_error handling) with a single call to that helper; ensure the helper is placed in a common transitions utility module and update all other builders to call this helper instead of duplicating the logic.packages/rs-sdk/src/platform/documents/transitions/tests.rs (2)
28-62: Test utilities (TestSigner,test_identity_public_key,test_data_contract,validate_transition_like_builder,new_mock_sdk_with_contract_nonce) are duplicated verbatim inpackages/rs-sdk/src/platform/tokens/builders/tests.rs.Consider extracting these shared helpers into a common test utility module (e.g.,
packages/rs-sdk/src/test_utils.rsgated with#[cfg(test)]) to avoid maintaining two copies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/tests.rs` around lines 28 - 62, Extract the duplicated test helpers (TestSigner, test_identity_public_key, test_data_contract, validate_transition_like_builder, new_mock_sdk_with_contract_nonce) into a single test utility module annotated with #[cfg(test)] (e.g., a new test_utils.rs) and make the helper symbols pub(crate) so both tests can import them; replace the duplicated definitions in both packages/rs-sdk/src/platform/documents/transitions/tests.rs and packages/rs-sdk/src/platform/tokens/builders/tests.rs with imports from the new module (use crate::test_utils::* or appropriate path) and run tests to ensure visibility and imports are correct.
392-407: The second assertion is redundant afterassert!(result.is_ok(), ...).If the
is_ok()assertion on line 392 passes,resultisOk(...)and the!matches!(result, Err(...))on line 398 can never fail. This pattern repeats in all six nonce-masking tests in this file.You could safely remove the second assertions, or alternatively, remove the first and keep only the more specific second assertion (if you want to test specifically that the nonce error variant doesn't occur, even if other errors are acceptable).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/tests.rs` around lines 392 - 407, The second assertion that checks !matches!(result, Err(Error::Protocol(ProtocolError::ConsensusError(consensus_error))) ...) is redundant because the earlier assert!(result.is_ok(), ...) already guarantees result is Ok; remove the redundant matches-based assertion (the block starting with assert!(!matches!(result, Err(...))) in the nonce-masking tests) in each of the six tests, or if you prefer to assert specifically that the NonceOutOfBoundsError never occurs even when other errors might, replace the initial assert!(result.is_ok(), ...) with only the more specific matches-based assertion in those tests; update the tests referencing result and the consensus_error pattern accordingly to keep one clear assertion per test.packages/rs-sdk/src/platform/tokens/builders/transfer.rs (1)
237-243: Test module name deviates from Rust convention.The module is named
validation_tests; idiomatic Rust names inline test modulestests. There is no functional difference, buttestsis whatcargo testdiscoverers and tooling (e.g.,cargo-tarpaulin, IDEs) typically highlight.♻️ Suggested rename
#[cfg(test)] -mod validation_tests { +mod tests { #[test] fn validate_base_structure_error_case() { super::super::tests::assert_token_transfer_validate_base_structure_error(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/transfer.rs` around lines 237 - 243, Rename the test module from validation_tests to the conventional tests to match Rust tooling expectations: change the module declaration mod validation_tests { ... } to mod tests { ... } in transfer.rs and keep the inner test function validate_base_structure_error_case unchanged; ensure any references to validation_tests (if any) are updated to tests so cargo test and tools like cargo-tarpaulin/IDEs recognize the inline tests.packages/rs-sdk/src/platform/tokens/builders/burn.rs (2)
187-193:_ =>arm is unreachable — preferunreachable!()over a silentErr
BatchTransition::new_token_burn_transitionis aBatchTransitionconstructor and always wraps intoStateTransition::Batch, so the wildcard arm can never be reached. Returning a gracefulErrhere masks a programming invariant violation;unreachable!()would surface it loudly during development and testing. This pattern is duplicated across all ~17 builder files, so the suggestion applies uniformly.♻️ Proposed refactor
- _ => { - return Err(Error::Protocol( - dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ), - )); - } + _ => unreachable!( + "new_token_burn_transition always produces StateTransition::Batch" + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/burn.rs` around lines 187 - 193, The wildcard match arm currently returning Err(...) should be replaced with an explicit unreachable!() to reflect the invariant that BatchTransition::new_token_burn_transition always yields StateTransition::Batch; locate the match handling the StateTransition (the arm returning Err::Protocol with InvalidStateTransitionType) within the builder (e.g., new_token_burn_transition in burn.rs) and change that `_ =>` branch to call unreachable!() so failures surface loudly during development and testing; apply the same change to the corresponding wildcard arms in the other builder files following the same pattern.
207-210: Test bypasses thesign()builder path
assert_token_burn_validate_base_structure_errorcallsBatchTransition::new_token_burn_transitiondirectly and invokesvalidate_transition_like_builder, so the new validation block insideTokenBurnTransitionBuilder::sign()(lines 182-199) is never exercised by this test. Valid transitions through the builder are also untested here.Consider adding an async integration test (or extending an existing one) that drives
TokenBurnTransitionBuilder::sign()end-to-end with a mock SDK, to confirm validation is correctly wired into the builder path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/burn.rs` around lines 207 - 210, The test currently bypasses TokenBurnTransitionBuilder::sign() by calling BatchTransition::new_token_burn_transition directly (via assert_token_burn_validate_base_structure_error), so the new validation added inside TokenBurnTransitionBuilder::sign() (lines ~182-199) is never executed; add or extend an async integration test that constructs a TokenBurnTransition through the builder path (use TokenBurnTransitionBuilder::new/... then call TokenBurnTransitionBuilder::sign()) and wire a mock SDK/crypto provider to exercise the signing flow, then call validate_transition_like_builder (or assert_token_burn_validate_base_structure_error behavior) to assert the validation error is raised; ensure the test uses the same inputs (invalid base structure) as the existing helper so you assert the builder-based sign path triggers the same validation failure.packages/rs-sdk/src/platform/tokens/builders/freeze.rs (1)
188-205: Validation block is correct; only the first error is surfaced.The match/validate/early-return pattern is consistent with the rest of the PR and handles both the impossible non-Batch case and validation failures cleanly. One trade-off worth noting:
validation_result.errors.into_iter().next()silently discards every error after the first, so callers have no way to see the full set of validation failures from a single call. If multi-error diagnostics ever matter at the SDK boundary, consider accumulating all errors (or at least including a count in the message).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/freeze.rs` around lines 188 - 205, Currently the code only returns the first validation error from validation_result.errors in the StateTransition::Batch branch, so modify the handling in the block using state_transition / StateTransition::Batch / batch_transition.validate_base_structure to aggregate all validation errors (or at minimum include the total count) into a single ProtocolError; collect validation_result.errors into a Vec, format them (or their count) and return Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(...))) containing either a combined error that wraps all errors or a message that includes the number of errors and the joined messages so callers can see full diagnostics instead of only the first error.packages/rs-sdk/src/platform/documents/transitions/delete.rs (1)
308-314: Test delegates to a helper that bypassessign— new validation branches not directly exercised.
assert_document_delete_validate_base_structure_errorconstructs theBatchTransitiondirectly viaBatchTransition::new_document_deletion_transition_from_documentand callsvalidate_transition_like_builder, never enteringsign. The two new code paths insign— the_ =>early return (lines 215–221) and theif let Some(first_error)error return (lines 223–227) — are not covered by this test.Consider an async integration test (or a mock-SDK unit test) that calls
signwith an invalid nonce so it walks the validation block in-situ.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/delete.rs` around lines 308 - 314, The existing unit test uses BatchTransition::new_document_deletion_transition_from_document and validate_transition_like_builder which bypasses BatchTransition::sign, so the new branches in sign (the `_ =>` early return and the `if let Some(first_error)` error return) are not exercised; add a test that constructs a BatchTransition for deletion and calls the sign method directly (or via an async integration/mock-SDK test) using an intentionally invalid nonce to force the validation path inside sign to run, asserting that the `_ =>` branch and the `first_error` early return produce the expected errors; target the sign function on the BatchTransition type and reuse or adapt assert_document_delete_validate_base_structure_error to invoke sign rather than constructing the transition via new_document_deletion_transition_from_document alone.packages/rs-sdk/src/platform/documents/transitions/set_price.rs (2)
309-315: Test module only covers the error path — consider adding a happy-path assertion.
validate_base_structure_error_caseconfirms that an invalid transition is rejected, but there is no test asserting that a structurally-valid transition passes throughsign(or the equivalent builder path) without error. A regression that makesvalidate_base_structurereject valid transitions would go undetected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/set_price.rs` around lines 309 - 315, Add a complementary happy-path test next to validate_base_structure_error_case inside the validation_tests module that builds a structurally-valid SetPrice transition via the same test helpers (instead of the error helper) and asserts validate_base_structure accepts it and that the subsequent sign/builder step (call the sign method or the builder function used in other tests) completes without error; locate validate_base_structure_error_case and replace-or-extend its usage of super::super::tests::assert_document_set_price_validate_base_structure_error() with a new test that calls the positive helper (or constructs a valid transition), then invokes validate_base_structure and sign (or the transition builder) and asserts success.
211-228: Validation logic is correct; consider extracting the repeated block into a shared helper.The
if let Somepattern is properly used (previousunwrap()concern resolved), the borrow/ownership split is correct, and the early-return on the first consensus error is consistent with the PR pattern.However, this exact match-validate-propagate block is duplicated across all ~17 builder files in this PR. Extracting it into a module-level or crate-internal helper would remove the duplication and make future changes (e.g., switching to returning all errors, or changing error types) a single-site edit:
♻️ Suggested helper (applicable to all builder files)
+/// Validates a freshly-built state transition's base structure and returns the +/// first consensus error, if any. +pub(crate) fn validate_batch_base_structure( + state_transition: &StateTransition, + platform_version: &PlatformVersion, +) -> Result<(), Error> { + let validation_result = match state_transition { + StateTransition::Batch(batch_transition) => { + batch_transition.validate_base_structure(platform_version)? + } + _ => { + return Err(Error::Protocol( + dpp::ProtocolError::InvalidStateTransitionType( + "expected Batch transition".to_string(), + ), + )); + } + }; + if let Some(first_error) = validation_result.errors.into_iter().next() { + return Err(Error::Protocol(dpp::ProtocolError::ConsensusError( + Box::new(first_error), + ))); + } + Ok(()) +}Then in each
sign/ build method:- let validation_result = match &state_transition { - StateTransition::Batch(batch_transition) => { - batch_transition.validate_base_structure(platform_version)? - } - _ => { - return Err(Error::Protocol( - dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ), - )); - } - }; - if let Some(first_error) = validation_result.errors.into_iter().next() { - return Err(Error::Protocol(dpp::ProtocolError::ConsensusError( - Box::new(first_error), - ))); - } + validate_batch_base_structure(&state_transition, platform_version)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/set_price.rs` around lines 211 - 228, The match-validate-propagate block for StateTransition::Batch (matching on state_transition, calling batch_transition.validate_base_structure(platform_version), checking validation_result.errors and returning Error::Protocol(dpp::ProtocolError::ConsensusError(...))) is duplicated across many builders; extract it into a shared helper (e.g., a crate- or module-level function like validate_batch_state_transition(state_transition: &StateTransition, platform_version: ...) -> Result<(), Error>) that performs the match, calls batch_transition.validate_base_structure(platform_version) and maps the first validation error into the same Error::Protocol(dpp::ProtocolError::ConsensusError(...)) return, then replace the duplicated blocks in each sign/build method to call that helper (keeping the same error mapping and early-return behavior).packages/rs-sdk/src/platform/tokens/builders/config_update.rs (1)
191-207: Extract the repeated validation block into a shared helper to eliminate cross-file duplication.The same ~10-line block:
let validation_result = match &state_transition { StateTransition::Batch(batch_transition) => { batch_transition.validate_base_structure(platform_version)? } _ => { return Err(Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType(...))); } }; if let Some(first_error) = validation_result.errors.into_iter().next() { return Err(Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(first_error)))); }is copy-pasted verbatim across all 17 builder files touched in this PR (11 token builders + 6 document builders). Any future change to the validation strategy (e.g., collecting all errors instead of the first, or changing the error type) requires 17 coordinated edits.
Consider adding a free function in the shared
tokens/builders(or a commontransitionutility module), e.g.:♻️ Proposed helper extraction
// In e.g. packages/rs-sdk/src/platform/transition/validate.rs (new file) // or packages/rs-sdk/src/platform/tokens/builders/mod.rs pub(crate) fn validate_batch_transition_base_structure( state_transition: &StateTransition, platform_version: &PlatformVersion, ) -> Result<(), Error> { let validation_result = match state_transition { StateTransition::Batch(batch_transition) => { batch_transition.validate_base_structure(platform_version)? } _ => { return Err(Error::Protocol( dpp::ProtocolError::InvalidStateTransitionType( "expected Batch transition".to_string(), ), )); } }; if let Some(first_error) = validation_result.errors.into_iter().next() { return Err(Error::Protocol(dpp::ProtocolError::ConsensusError( Box::new(first_error), ))); } Ok(()) }Each
sign()method then reduces to a single call:- let validation_result = match &state_transition { - StateTransition::Batch(batch_transition) => { - batch_transition.validate_base_structure(platform_version)? - } - _ => { - return Err(Error::Protocol( - dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ), - )); - } - }; - if let Some(first_error) = validation_result.errors.into_iter().next() { - return Err(Error::Protocol(dpp::ProtocolError::ConsensusError( - Box::new(first_error), - ))); - } + validate_batch_transition_base_structure(&state_transition, platform_version)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/config_update.rs` around lines 191 - 207, Extract the duplicated validation block into a single helper function (e.g., validate_batch_transition_base_structure) that accepts &StateTransition and &PlatformVersion and returns Result<(), Error>; move the match on StateTransition::Batch and the check for the first validation_result.errors.into_iter().next() into that function, returning the same Protocol errors on failure, and then replace the duplicated block in each sign() method (and other builders) with a single call to this helper to centralize validation logic and avoid repetition.packages/rs-sdk/src/platform/tokens/builders/set_price.rs (1)
234-244: The_wildcard arm is unreachable dead code.
new_token_change_direct_purchase_price_transition(Line 217) always returns aStateTransition::Batch, so the_branch can never be reached. While the defensive guard is consistent with the PR's pattern across all builders, it's still dead code and may trigger aclippy::unreachable_patternslint. Using anif let/else unreachable!()pattern is idiomatic and makes the invariant explicit.♻️ Proposed refactor using
if let+unreachable!- let validation_result = match &state_transition { - StateTransition::Batch(batch_transition) => { - batch_transition.validate_base_structure(platform_version)? - } - _ => { - return Err(Error::Protocol( - dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ), - )); - } - }; + let StateTransition::Batch(batch_transition) = &state_transition else { + unreachable!("new_token_change_direct_purchase_price_transition always returns a Batch transition"); + }; + let validation_result = batch_transition.validate_base_structure(platform_version)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/set_price.rs` around lines 234 - 244, The match on state_transition producing validation_result contains an unreachable `_` arm because new_token_change_direct_purchase_price_transition always returns StateTransition::Batch; replace the match with an if let to extract Batch and call batch_transition.validate_base_structure(platform_version), and in the else branch use unreachable!() (or panic with an explanatory message) to make the invariant explicit; reference StateTransition::Batch, validation_result, and new_token_change_direct_purchase_price_transition when locating and updating the code.packages/rs-sdk/src/platform/tokens/builders/mint.rs (2)
232-238: Test does not exerciseTokenMintTransitionBuilder::sign, so the added validation path (lines 209–226) is untested.
assert_token_mint_validate_base_structure_errorconstructs a transition directly viaBatchTransition::new_token_mint_transitionand callsvalidate_transition_like_builder— it never invokes the builder'ssignmethod. A regression that removed or mis-wired the validation block insignwould not be caught by this test. Consider adding a test that callsTokenMintTransitionBuilder::signwith an invalid input and asserts the expectedProtocolError::ConsensusErroris returned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/mint.rs` around lines 232 - 238, Add a test that exercises TokenMintTransitionBuilder::sign so the validation path in sign (lines ~209–226) is covered: create the same invalid token mint input used by assert_token_mint_validate_base_structure_error, build a TokenMintTransitionBuilder for it, call TokenMintTransitionBuilder::sign (instead of constructing via BatchTransition::new_token_mint_transition), and assert that the call returns the expected ProtocolError::ConsensusError (or the same error type asserted by validate_transition_like_builder); reference the existing helper assert_token_mint_validate_base_structure_error and validate_transition_like_builder to reuse input construction and error expectations when writing this new test.
210-221: Consider replacing the unreachable_arm withlet…else.
BatchTransition::new_token_mint_transitionalways producesStateTransition::Batch, so the wildcard arm is dead code. The idiomatic Rust replacement (stable since 1.65) flattens the nesting and makes the single expected variant explicit:♻️ Proposed refactor using
let…else- let validation_result = match &state_transition { - StateTransition::Batch(batch_transition) => { - batch_transition.validate_base_structure(platform_version)? - } - _ => { - return Err(Error::Protocol( - dpp::ProtocolError::InvalidStateTransitionType( - "expected Batch transition".to_string(), - ), - )); - } - }; + let StateTransition::Batch(batch_transition) = &state_transition else { + return Err(Error::Protocol( + dpp::ProtocolError::InvalidStateTransitionType( + "expected Batch transition".to_string(), + ), + )); + }; + let validation_result = batch_transition.validate_base_structure(platform_version)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/tokens/builders/mint.rs` around lines 210 - 221, The match on state_transition that handles StateTransition::Batch with a wildcard `_` arm is dead code; replace it with a let…else to make the single expected variant explicit: destructure the reference using let StateTransition::Batch(batch_transition) = &state_transition else { return Err(Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType("expected Batch transition".to_string()))); }; then call batch_transition.validate_base_structure(platform_version)? to produce validation_result. This removes the unreachable `_` arm and flattens the nesting while keeping the same error return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/rs-sdk/src/platform/tokens/builders/freeze.rs`:
- Line 32: The doc comment for the constructor in freeze.rs was copied from
mint.rs and incorrectly says "Start building a mint tokens request"; update the
documentation for the function/impl named `new` (or the builder type in
freeze.rs) to describe that it "Start building a freeze tokens request" (or
equivalent phrasing mentioning "freeze tokens") so the comment matches the
freeze builder's purpose; ensure the doc comment above the `new` function/struct
is changed accordingly.
---
Duplicate comments:
In `@packages/rs-sdk/src/platform/documents/transitions/transfer.rs`:
- Around line 210-227: The duplicated validation block for ensuring a Batch
StateTransition (matching on StateTransition::Batch and calling
batch_transition.validate_base_structure(platform_version) and mapping any first
error into Error::Protocol) should be extracted into a shared helper function
(e.g., validate_batch_base_structure or validate_state_transition_base) and
reused from both transfer.rs and claim.rs; implement a function that accepts a
&StateTransition and &PlatformVersion (or the exact types used), performs the
match on StateTransition::Batch, calls
batch_transition.validate_base_structure(platform_version) and converts the
first consensus error into Error::Protocol as done now, then replace the
duplicate block in transfer.rs (the state_transition match, validation_result
handling, and error mapping) with a call to that helper.
In `@packages/rs-sdk/src/platform/tokens/builders/tests.rs`:
- Around line 40-153: The tests duplicate utilities (TestSigner,
test_identity_public_key, test_data_contract, validate_transition_like_builder,
and new_mock_sdk_with_contract_nonce) already defined in documents transitions
tests; refactor by moving these shared helpers into a common test-utils module
(e.g., a new crate/module under packages/rs-sdk/src/platform/test_utils or
tests/common) and update the current file to import and reuse TestSigner,
test_identity_public_key, test_data_contract, validate_transition_like_builder,
and new_mock_sdk_with_contract_nonce instead of redefining them; ensure
visibility (pub) and adjust imports in both tests to use the shared helpers and
run cargo test to confirm no symbol or visibility regressions.
---
Nitpick comments:
In @.gitignore:
- Around line 86-88: The .gitignore currently places the worktrees/ entry under
the "# gRPC coverage report" comment and leaves it unanchored; move the
worktrees/ line out from beneath the gRPC comment into a more appropriate
section (e.g., a worktree or local dev artifacts section) and anchor it by
prepending a slash so the pattern reads "/worktrees/" to ensure it only matches
a top-level worktrees directory; update the comment grouping so the
grpc-coverage-report.txt stays under the gRPC section and "/worktrees/" is
clearly associated with worktree artifacts.
In `@packages/rs-sdk/src/platform/documents/transitions/delete.rs`:
- Around line 308-314: The existing unit test uses
BatchTransition::new_document_deletion_transition_from_document and
validate_transition_like_builder which bypasses BatchTransition::sign, so the
new branches in sign (the `_ =>` early return and the `if let Some(first_error)`
error return) are not exercised; add a test that constructs a BatchTransition
for deletion and calls the sign method directly (or via an async
integration/mock-SDK test) using an intentionally invalid nonce to force the
validation path inside sign to run, asserting that the `_ =>` branch and the
`first_error` early return produce the expected errors; target the sign function
on the BatchTransition type and reuse or adapt
assert_document_delete_validate_base_structure_error to invoke sign rather than
constructing the transition via new_document_deletion_transition_from_document
alone.
In `@packages/rs-sdk/src/platform/documents/transitions/set_price.rs`:
- Around line 309-315: Add a complementary happy-path test next to
validate_base_structure_error_case inside the validation_tests module that
builds a structurally-valid SetPrice transition via the same test helpers
(instead of the error helper) and asserts validate_base_structure accepts it and
that the subsequent sign/builder step (call the sign method or the builder
function used in other tests) completes without error; locate
validate_base_structure_error_case and replace-or-extend its usage of
super::super::tests::assert_document_set_price_validate_base_structure_error()
with a new test that calls the positive helper (or constructs a valid
transition), then invokes validate_base_structure and sign (or the transition
builder) and asserts success.
- Around line 211-228: The match-validate-propagate block for
StateTransition::Batch (matching on state_transition, calling
batch_transition.validate_base_structure(platform_version), checking
validation_result.errors and returning
Error::Protocol(dpp::ProtocolError::ConsensusError(...))) is duplicated across
many builders; extract it into a shared helper (e.g., a crate- or module-level
function like validate_batch_state_transition(state_transition:
&StateTransition, platform_version: ...) -> Result<(), Error>) that performs the
match, calls batch_transition.validate_base_structure(platform_version) and maps
the first validation error into the same
Error::Protocol(dpp::ProtocolError::ConsensusError(...)) return, then replace
the duplicated blocks in each sign/build method to call that helper (keeping the
same error mapping and early-return behavior).
In `@packages/rs-sdk/src/platform/documents/transitions/tests.rs`:
- Around line 28-62: Extract the duplicated test helpers (TestSigner,
test_identity_public_key, test_data_contract, validate_transition_like_builder,
new_mock_sdk_with_contract_nonce) into a single test utility module annotated
with #[cfg(test)] (e.g., a new test_utils.rs) and make the helper symbols
pub(crate) so both tests can import them; replace the duplicated definitions in
both packages/rs-sdk/src/platform/documents/transitions/tests.rs and
packages/rs-sdk/src/platform/tokens/builders/tests.rs with imports from the new
module (use crate::test_utils::* or appropriate path) and run tests to ensure
visibility and imports are correct.
- Around line 392-407: The second assertion that checks !matches!(result,
Err(Error::Protocol(ProtocolError::ConsensusError(consensus_error))) ...) is
redundant because the earlier assert!(result.is_ok(), ...) already guarantees
result is Ok; remove the redundant matches-based assertion (the block starting
with assert!(!matches!(result, Err(...))) in the nonce-masking tests) in each of
the six tests, or if you prefer to assert specifically that the
NonceOutOfBoundsError never occurs even when other errors might, replace the
initial assert!(result.is_ok(), ...) with only the more specific matches-based
assertion in those tests; update the tests referencing result and the
consensus_error pattern accordingly to keep one clear assertion per test.
In `@packages/rs-sdk/src/platform/tokens/builders/burn.rs`:
- Around line 187-193: The wildcard match arm currently returning Err(...)
should be replaced with an explicit unreachable!() to reflect the invariant that
BatchTransition::new_token_burn_transition always yields StateTransition::Batch;
locate the match handling the StateTransition (the arm returning Err::Protocol
with InvalidStateTransitionType) within the builder (e.g.,
new_token_burn_transition in burn.rs) and change that `_ =>` branch to call
unreachable!() so failures surface loudly during development and testing; apply
the same change to the corresponding wildcard arms in the other builder files
following the same pattern.
- Around line 207-210: The test currently bypasses
TokenBurnTransitionBuilder::sign() by calling
BatchTransition::new_token_burn_transition directly (via
assert_token_burn_validate_base_structure_error), so the new validation added
inside TokenBurnTransitionBuilder::sign() (lines ~182-199) is never executed;
add or extend an async integration test that constructs a TokenBurnTransition
through the builder path (use TokenBurnTransitionBuilder::new/... then call
TokenBurnTransitionBuilder::sign()) and wire a mock SDK/crypto provider to
exercise the signing flow, then call validate_transition_like_builder (or
assert_token_burn_validate_base_structure_error behavior) to assert the
validation error is raised; ensure the test uses the same inputs (invalid base
structure) as the existing helper so you assert the builder-based sign path
triggers the same validation failure.
In `@packages/rs-sdk/src/platform/tokens/builders/claim.rs`:
- Around line 168-185: Extract the repeated validation into a shared helper
(e.g., validate_batch_base_structure) that takes &StateTransition and
&PlatformVersion and performs the match on StateTransition::Batch, calls
batch_transition.validate_base_structure(platform_version) and converts the
first validation error into
Error::Protocol(dpp::ProtocolError::ConsensusError(...)) or returns Ok(()); then
replace the inline block in claim.rs (the match on StateTransition::Batch,
validate_base_structure, and the first_error handling) with a single call to
that helper; ensure the helper is placed in a common transitions utility module
and update all other builders to call this helper instead of duplicating the
logic.
In `@packages/rs-sdk/src/platform/tokens/builders/config_update.rs`:
- Around line 191-207: Extract the duplicated validation block into a single
helper function (e.g., validate_batch_transition_base_structure) that accepts
&StateTransition and &PlatformVersion and returns Result<(), Error>; move the
match on StateTransition::Batch and the check for the first
validation_result.errors.into_iter().next() into that function, returning the
same Protocol errors on failure, and then replace the duplicated block in each
sign() method (and other builders) with a single call to this helper to
centralize validation logic and avoid repetition.
In `@packages/rs-sdk/src/platform/tokens/builders/freeze.rs`:
- Around line 188-205: Currently the code only returns the first validation
error from validation_result.errors in the StateTransition::Batch branch, so
modify the handling in the block using state_transition / StateTransition::Batch
/ batch_transition.validate_base_structure to aggregate all validation errors
(or at minimum include the total count) into a single ProtocolError; collect
validation_result.errors into a Vec, format them (or their count) and return
Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(...))) containing
either a combined error that wraps all errors or a message that includes the
number of errors and the joined messages so callers can see full diagnostics
instead of only the first error.
In `@packages/rs-sdk/src/platform/tokens/builders/mint.rs`:
- Around line 232-238: Add a test that exercises
TokenMintTransitionBuilder::sign so the validation path in sign (lines ~209–226)
is covered: create the same invalid token mint input used by
assert_token_mint_validate_base_structure_error, build a
TokenMintTransitionBuilder for it, call TokenMintTransitionBuilder::sign
(instead of constructing via BatchTransition::new_token_mint_transition), and
assert that the call returns the expected ProtocolError::ConsensusError (or the
same error type asserted by validate_transition_like_builder); reference the
existing helper assert_token_mint_validate_base_structure_error and
validate_transition_like_builder to reuse input construction and error
expectations when writing this new test.
- Around line 210-221: The match on state_transition that handles
StateTransition::Batch with a wildcard `_` arm is dead code; replace it with a
let…else to make the single expected variant explicit: destructure the reference
using let StateTransition::Batch(batch_transition) = &state_transition else {
return
Err(Error::Protocol(dpp::ProtocolError::InvalidStateTransitionType("expected
Batch transition".to_string()))); }; then call
batch_transition.validate_base_structure(platform_version)? to produce
validation_result. This removes the unreachable `_` arm and flattens the nesting
while keeping the same error return.
In `@packages/rs-sdk/src/platform/tokens/builders/set_price.rs`:
- Around line 234-244: The match on state_transition producing validation_result
contains an unreachable `_` arm because
new_token_change_direct_purchase_price_transition always returns
StateTransition::Batch; replace the match with an if let to extract Batch and
call batch_transition.validate_base_structure(platform_version), and in the else
branch use unreachable!() (or panic with an explanatory message) to make the
invariant explicit; reference StateTransition::Batch, validation_result, and
new_token_change_direct_purchase_price_transition when locating and updating the
code.
In `@packages/rs-sdk/src/platform/tokens/builders/transfer.rs`:
- Around line 237-243: Rename the test module from validation_tests to the
conventional tests to match Rust tooling expectations: change the module
declaration mod validation_tests { ... } to mod tests { ... } in transfer.rs and
keep the inner test function validate_base_structure_error_case unchanged;
ensure any references to validation_tests (if any) are updated to tests so cargo
test and tools like cargo-tarpaulin/IDEs recognize the inline tests.
In `@packages/wasm-sdk/tests/unit/fixtures/data-contract-v0-crypto-card-game.ts`:
- Around line 52-63: The rarity property currently has both enum and maxLength
(maxLength: 9) which is redundant because each enum value already fits within 9
chars; either remove the maxLength field from the rarity definition (in the
rarity object) or, if the intent is to explicitly exercise the maxLength
validation path in tests, add a short inline comment next to the
rarity/maxLength entry explaining that maxLength is intentionally kept to
trigger structural validation tests (reference symbols: rarity, maxLength,
enum).
All 6 document_*_sign_masks_nonce tests tested the same underlying behavior (Sdk::get_identity_contract_nonce masks out-of-bounds bits). Keep one representative test, remove the other 5. Also removes dead second assert that was always true (result was Ok) and unused test_document_for_create helper.
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
PastaPastaPasta
approved these changes
Feb 22, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rs-sdk/src/platform/documents/transitions/tests.rs (1)
156-311: Extract a shared test-context helper to eliminate the repeated setup.All six
assert_document_*helpers repeat the same four-line setup (platform_version,data_contract,owner_id,document). Extracting it reduces noise and makes future changes easier.♻️ Suggested refactor
+struct TestContext { + platform_version: &'static dpp::version::PlatformVersion, + data_contract: Arc<dpp::data_contract::DataContract>, + document: Document, + document_type: &'static dpp::data_contract::document_type::DocumentType, +} + +fn make_test_context() -> ( + &'static dpp::version::PlatformVersion, + Arc<dpp::data_contract::DataContract>, + Document, +) { + let platform_version = dpp::version::PlatformVersion::latest(); + let data_contract = test_data_contract(TEST_DOCUMENT_TYPE_NAME); + let owner_id = Identifier::random(); + let document = test_document(owner_id); + (platform_version, data_contract, document) +}Each
assert_document_*function can then open with:let (platform_version, data_contract, document) = make_test_context(); let document_type = data_contract .document_type_for_name(TEST_DOCUMENT_TYPE_NAME) .expect("expected test document type");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/documents/transitions/tests.rs` around lines 156 - 311, Multiple test helpers repeat the same setup (platform_version, data_contract, owner_id, document); extract a shared helper (e.g., make_test_context) that returns (platform_version, data_contract, document) and use it from each assert_document_* function (assert_document_create_validate_base_structure_error, assert_document_delete_validate_base_structure_error, assert_document_purchase_validate_base_structure_error, assert_document_replace_validate_base_structure_error, assert_document_set_price_validate_base_structure_error, assert_document_transfer_validate_base_structure_error) so you replace the four repeated lines with let (platform_version, data_contract, document) = make_test_context(); and keep the existing document_type lookup and rest of each test unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-sdk/src/platform/documents/transitions/tests.rs`:
- Line 337: Replace the hardcoded "testDoc" literal with the existing
TEST_DOCUMENT_TYPE_NAME constant used elsewhere in the test suite; locate the
assignment where document_type_name is set (currently let document_type_name =
"testDoc";) and change it to reference TEST_DOCUMENT_TYPE_NAME so the test stays
in sync with the canonical constant.
---
Nitpick comments:
In `@packages/rs-sdk/src/platform/documents/transitions/tests.rs`:
- Around line 156-311: Multiple test helpers repeat the same setup
(platform_version, data_contract, owner_id, document); extract a shared helper
(e.g., make_test_context) that returns (platform_version, data_contract,
document) and use it from each assert_document_* function
(assert_document_create_validate_base_structure_error,
assert_document_delete_validate_base_structure_error,
assert_document_purchase_validate_base_structure_error,
assert_document_replace_validate_base_structure_error,
assert_document_set_price_validate_base_structure_error,
assert_document_transfer_validate_base_structure_error) so you replace the four
repeated lines with let (platform_version, data_contract, document) =
make_test_context(); and keep the existing document_type lookup and rest of each
test unchanged.
| // which masks out-of-bounds bits. This makes `validate_base_structure` | ||
| // nonce-out-of-bounds errors unreachable through the builder API. | ||
| // One test suffices since all document builders use the same SDK nonce path. | ||
| let document_type_name = "testDoc"; |
Contributor
There was a problem hiding this comment.
Use TEST_DOCUMENT_TYPE_NAME constant instead of the hardcoded literal.
Line 337 duplicates the "testDoc" string already defined as TEST_DOCUMENT_TYPE_NAME at Line 112. If the constant is ever renamed or changed, this test silently diverges.
🔧 Proposed fix
- let document_type_name = "testDoc";
+ let document_type_name = TEST_DOCUMENT_TYPE_NAME;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let document_type_name = "testDoc"; | |
| let document_type_name = TEST_DOCUMENT_TYPE_NAME; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rs-sdk/src/platform/documents/transitions/tests.rs` at line 337,
Replace the hardcoded "testDoc" literal with the existing
TEST_DOCUMENT_TYPE_NAME constant used elsewhere in the test suite; locate the
assignment where document_type_name is set (currently let document_type_name =
"testDoc";) and change it to reference TEST_DOCUMENT_TYPE_NAME so the test stays
in sync with the canonical constant.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Follow-up to #3096 per shumkov's review: add client-side structural validation to all remaining SDK state transition construction methods for consistent behavior.
PR #3096 added validation to identity and address transitions. This PR extends the same pattern to document and token transitions.
What was done?
Added
validate_base_structure()calls to all document and token SDK transition builders. After theBatchTransitionis constructed but before it's returned to the caller, structural validation catches invalid transitions early with clear errors instead of confusing network rejections.Document transitions (in
packages/rs-sdk/src/platform/documents/transitions/):create.rs,delete.rs,replace.rs,purchase.rs,set_price.rs,transfer.rsToken builders (in
packages/rs-sdk/src/platform/tokens/builders/):burn.rs,claim.rs,config_update.rs,destroy.rs,purchase.rs,emergency_action.rs,freeze.rs,mint.rs,set_price.rs,transfer.rs,unfreeze.rsContract transitions:
put_contract.rsalready had structure validation viaensure_valid_state_transition_structure, so no changes needed.Also enabled the
dppvalidationfeature fordash-sdkinCargo.tomlto makeBatchTransition::validate_base_structure()available.Pattern used:
How Has This Been Tested?
cargo check -p dash-sdk— compiles cleancargo test -p dash-sdk— all tests passBreaking Changes
None. Existing valid transitions are unaffected. Only invalid transitions that would have been rejected by the network will now fail earlier with clearer errors.
Summary by CodeRabbit
New Features
Tests
Chores