Skip to content

feat(sdk): add client-side validation to state transition construction methods #3096

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.

Sign up for GitHub

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

Draft
thepastaclaw wants to merge 10 commits into
base: v3.1-dev
Choose a base branch
from
Draft

feat(sdk): add client-side validation to state transition construction methods #3096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
854957b
fix(dpp): validate public key security levels client-side in Identity…
thepastaclaw Feb 17, 2026
01ccf22
fix(dpp): add client-side key validation to identity create transitions
thepastaclaw Feb 17, 2026
f86015e
fix(dpp): remove underscore prefix from now-used platform_version par…
thepastaclaw Feb 17, 2026
7f5fdbf
chore: remove underscore prefix from now-used platform_version params
thepastaclaw Feb 17, 2026
82f1577
feat(sdk): add client-side validate_structure() to remaining state tr…
thepastaclaw Feb 18, 2026
bfce46b
test(dpp,drive-abci): fix test fixtures for client-side validate_stru…
thepastaclaw Feb 21, 2026
1cd4c6b
test(drive-abci): adapt address transition tests to client-side valid…
thepastaclaw Feb 21, 2026
ed30f29
style(drive-abci): apply cargo fmt
thepastaclaw Feb 21, 2026
9cc4c85
fix(tests): adapt strategy tests for client-side validation
thepastaclaw Feb 21, 2026
465a55c
style: apply cargo fmt to strategy.rs
thepastaclaw Feb 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion ...ion/state_transitions/address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use crate::serialization::Signable;
use crate::state_transition::address_credit_withdrawal_transition::methods::AddressCreditWithdrawalTransitionMethodsV0;
use crate::state_transition::address_credit_withdrawal_transition::v0::AddressCreditWithdrawalTransitionV0;
#[cfg(feature = "state-transition-signing")]
use crate::state_transition::StateTransitionStructureValidation;
#[cfg(feature = "state-transition-signing")]
use crate::withdrawal::Pooling;
#[cfg(feature = "state-transition-signing")]
use crate::{
Expand All @@ -35,7 +37,7 @@ impl AddressCreditWithdrawalTransitionMethodsV0 for AddressCreditWithdrawalTrans
output_script: CoreScript,
signer: &S,
user_fee_increase: UserFeeIncrease,
_platform_version: &PlatformVersion,
platform_version: &PlatformVersion,
) -> Result<StateTransition, ProtocolError> {
tracing::debug!("try_from_inputs_with_signer: Started");
tracing::debug!(
Expand Down Expand Up @@ -66,6 +68,14 @@ impl AddressCreditWithdrawalTransitionMethodsV0 for AddressCreditWithdrawalTrans
.map(|address| signer.sign_create_witness(address, &signable_bytes))
.collect::<Result<Vec<AddressWitness>, ProtocolError>>()?;

// Validate the fully-constructed transition structure
let validation_result =
address_credit_withdrawal_transition.validate_structure(platform_version);
if !validation_result.is_valid() {
let first_error = validation_result.errors.into_iter().next().unwrap();
return Err(ProtocolError::ConsensusError(Box::new(first_error)));
}
Comment on lines +74 to +77
Copy link
Copy Markdown
Collaborator Author

@thepastaclaw thepastaclaw Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Replace unwrap() with if-let across 9 validation sites

The pattern validation_result.errors.into_iter().next().unwrap() appears 9 times across production code gated behind state-transition-signing. The unwrap is logically safe — !is_valid() guarantees errors is non-empty since is_valid() returns self.errors.is_empty(). However, it violates the project convention of no unwrap() in production code, and the panic-free alternative is trivial.

Affected files (all in packages/rs-dpp/src/state_transition/state_transitions/):

  • address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs
  • address_funds/address_funding_from_asset_lock_transition/v0/v0_methods.rs
  • address_funds/address_funds_transfer_transition/v0/v0_methods.rs
  • identity/identity_create_from_addresses_transition/v0/v0_methods.rs (×2)
  • identity/identity_create_transition/v0/v0_methods.rs
  • identity/identity_credit_transfer_to_addresses_transition/v0/v0_methods.rs
  • identity/identity_topup_from_addresses_transition/v0/v0_methods.rs
  • identity/identity_update_transition/v0/v0_methods.rs
Suggested change
if !validation_result.is_valid() {
let first_error = validation_result.errors.into_iter().next().unwrap();
return Err(ProtocolError::ConsensusError(Box::new(first_error)));
}
if let Some(first_error) = validation_result.errors.into_iter().next() {
return Err(ProtocolError::ConsensusError(Box::new(first_error)));
}

source: ['claude']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs`:
- [SUGGESTION] lines 74-77: Replace unwrap() with if-let across 9 validation sites
  The pattern `validation_result.errors.into_iter().next().unwrap()` appears 9 times across production code gated behind `state-transition-signing`. The unwrap is logically safe — `!is_valid()` guarantees `errors` is non-empty since `is_valid()` returns `self.errors.is_empty()`. However, it violates the project convention of no `unwrap()` in production code, and the panic-free alternative is trivial.

Affected files (all in `packages/rs-dpp/src/state_transition/state_transitions/`):
- `address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs`
- `address_funds/address_funding_from_asset_lock_transition/v0/v0_methods.rs`
- `address_funds/address_funds_transfer_transition/v0/v0_methods.rs`
- `identity/identity_create_from_addresses_transition/v0/v0_methods.rs` (×2)
- `identity/identity_create_transition/v0/v0_methods.rs`
- `identity/identity_credit_transfer_to_addresses_transition/v0/v0_methods.rs`
- `identity/identity_topup_from_addresses_transition/v0/v0_methods.rs`
- `identity/identity_update_transition/v0/v0_methods.rs`

Comment on lines +74 to +77
Copy link
Copy Markdown
Collaborator Author

@thepastaclaw thepastaclaw Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Only the first validation error is returned; remaining errors are discarded

All 9 validation sites extract only the first error from ValidationResult and discard the rest. In a client-side SDK context this forces users into a fix-one-resubmit cycle. For example, validate_identity_public_keys_structure specifically collects ALL invalid keys via filter_map, but only the first issue is surfaced. Consider whether ProtocolError could carry a Vec<ConsensusError> or a multi-error variant. This is a deliberate design choice (consistent across all call sites), so flagging for consideration rather than as a bug.

source: ['claude']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs`:
- [SUGGESTION] lines 74-77: Only the first validation error is returned; remaining errors are discarded
  All 9 validation sites extract only the first error from `ValidationResult` and discard the rest. In a client-side SDK context this forces users into a fix-one-resubmit cycle. For example, `validate_identity_public_keys_structure` specifically collects ALL invalid keys via `filter_map`, but only the first issue is surfaced. Consider whether `ProtocolError` could carry a `Vec<ConsensusError>` or a multi-error variant. This is a deliberate design choice (consistent across all call sites), so flagging for consideration rather than as a bug.


tracing::debug!("try_from_inputs_with_signer: Successfully created transition");
Ok(address_credit_withdrawal_transition.into())
}
Expand Down
11 changes: 10 additions & 1 deletion ...ate_transitions/address_funds/address_funding_from_asset_lock_transition/v0/v0_methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use crate::serialization::Signable;
use crate::state_transition::address_funding_from_asset_lock_transition::methods::AddressFundingFromAssetLockTransitionMethodsV0;
use crate::state_transition::address_funding_from_asset_lock_transition::v0::AddressFundingFromAssetLockTransitionV0;
#[cfg(feature = "state-transition-signing")]
use crate::state_transition::StateTransitionStructureValidation;
#[cfg(feature = "state-transition-signing")]
use crate::{prelude::UserFeeIncrease, state_transition::StateTransition, ProtocolError};
#[cfg(feature = "state-transition-signing")]
use dashcore::signer;
Expand All @@ -30,7 +32,7 @@ impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetL
fee_strategy: AddressFundsFeeStrategy,
signer: &S,
user_fee_increase: UserFeeIncrease,
_platform_version: &PlatformVersion,
platform_version: &PlatformVersion,
) -> Result<StateTransition, ProtocolError> {
tracing::debug!("try_from_asset_lock_with_signer: Started");
tracing::debug!(
Expand Down Expand Up @@ -64,6 +66,13 @@ impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetL
.map(|address| signer.sign_create_witness(address, &signable_bytes))
.collect::<Result<Vec<AddressWitness>, ProtocolError>>()?;

// Validate the fully-constructed transition structure
let validation_result = address_funding_transition.validate_structure(platform_version);
if !validation_result.is_valid() {
let first_error = validation_result.errors.into_iter().next().unwrap();
return Err(ProtocolError::ConsensusError(Box::new(first_error)));
}

tracing::debug!("try_from_asset_lock_with_signer: Successfully created transition");
Ok(address_funding_transition.into())
}
Expand Down
Loading
Loading