diff --git a/packages/rs-dpp/src/errors/protocol_error.rs b/packages/rs-dpp/src/errors/protocol_error.rs index 825bf832991..1bce8605821 100644 --- a/packages/rs-dpp/src/errors/protocol_error.rs +++ b/packages/rs-dpp/src/errors/protocol_error.rs @@ -89,6 +89,18 @@ pub enum ProtocolError { /// requested core height received: FeatureVersion, }, + + /// The method is not active at the current DPP version (the version field + /// is `None`). Mirrors drive-abci's `ExecutionError::VersionNotActive` so + /// the SDK can express the same condition without relying on an + /// `UnknownVersionMismatch` sentinel value. + #[error("{method} not active for dpp version")] + VersionNotActive { + /// method + method: String, + /// the versions of this method that exist (even if currently inactive) + known_versions: Vec, + }, #[error("current platform version not initialized")] CurrentProtocolVersionNotInitialized, #[error("unknown version error {0}")] @@ -131,6 +143,9 @@ pub enum ProtocolError { #[error(transparent)] ConsensusError(Box), + #[error("Multiple consensus errors: {0:?}")] + ConsensusErrors(Vec), + #[error(transparent)] Document(Box), @@ -344,6 +359,21 @@ impl From for ProtocolError { } } +impl From> for ProtocolError { + fn from(mut errors: Vec) -> Self { + debug_assert!( + !errors.is_empty(), + "ProtocolError::from(Vec) expects a non-empty error list" + ); + + match errors.len() { + 0 => ProtocolError::ConsensusErrors(errors), + 1 => ProtocolError::ConsensusError(Box::new(errors.remove(0))), + _ => ProtocolError::ConsensusErrors(errors), + } + } +} + impl From for ProtocolError { fn from(e: DocumentError) -> Self { ProtocolError::Document(Box::new(e)) diff --git a/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs b/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs index 8e978fbf383..9af4eda6eee 100644 --- a/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs +++ b/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs @@ -11,9 +11,9 @@ use dashcore::transaction::special_transaction::TransactionPayload; use dashcore::{InstantLock, OutPoint, Transaction, TxIn, TxOut}; use platform_value::{BinaryData, Value}; -#[cfg(feature = "validation")] +#[cfg(any(feature = "validation", feature = "state-transition-validation"))] use crate::identity::state_transition::asset_lock_proof::instant::methods; -#[cfg(feature = "validation")] +#[cfg(any(feature = "validation", feature = "state-transition-validation"))] use platform_version::version::PlatformVersion; use serde::de::Error as DeError; use serde::ser::Error as SerError; @@ -23,7 +23,7 @@ use crate::prelude::Identifier; #[cfg(feature = "cbor")] use crate::util::cbor_value::CborCanonicalMap; use crate::util::hash::hash_double; -#[cfg(feature = "validation")] +#[cfg(any(feature = "validation", feature = "state-transition-validation"))] use crate::validation::SimpleConsensusValidationResult; use crate::ProtocolError; @@ -176,7 +176,7 @@ impl InstantAssetLockProof { } /// Validate Instant Asset Lock Proof structure - #[cfg(feature = "validation")] + #[cfg(any(feature = "validation", feature = "state-transition-validation"))] pub fn validate_structure( &self, platform_version: &PlatformVersion, diff --git a/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/mod.rs b/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/mod.rs index 98e4fa5696b..c81f37cb53c 100644 --- a/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/mod.rs +++ b/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/mod.rs @@ -8,13 +8,13 @@ use bincode::{Decode, Encode}; pub use instant::*; use platform_value::Value; -#[cfg(feature = "validation")] +#[cfg(any(feature = "validation", feature = "state-transition-validation"))] use platform_version::version::PlatformVersion; use serde::de::Error; use crate::identity::state_transition::asset_lock_proof::chain::ChainAssetLockProof; use crate::prelude::Identifier; -#[cfg(feature = "validation")] +#[cfg(any(feature = "validation", feature = "state-transition-validation"))] use crate::validation::SimpleConsensusValidationResult; use crate::{ProtocolError, SerdeParsingError}; @@ -176,7 +176,7 @@ impl AssetLockProof { } /// Validate the structure of the asset lock proof - #[cfg(feature = "validation")] + #[cfg(any(feature = "validation", feature = "state-transition-validation"))] pub fn validate_structure( &self, platform_version: &PlatformVersion, diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/mod.rs index f5a7f789d41..1dc6b8aef49 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/mod.rs @@ -50,3 +50,81 @@ pub struct AddressCreditWithdrawalTransitionV0 { #[platform_signable(exclude_from_sig_hash)] pub input_witnesses: Vec, } + +#[cfg(all(test, feature = "state-transition-signing"))] +mod signing_tests { + use super::*; + use crate::address_funds::AddressFundsFeeStrategyStep; + use crate::consensus::basic::BasicError; + use crate::consensus::ConsensusError; + use crate::identity::signer::Signer; + use crate::state_transition::address_credit_withdrawal_transition::methods::AddressCreditWithdrawalTransitionMethodsV0; + use async_trait::async_trait; + use platform_value::BinaryData; + use platform_version::version::PlatformVersion; + + #[derive(Debug)] + struct UnreachableAddressSigner; + + #[async_trait] + impl Signer for UnreachableAddressSigner { + async fn sign( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!("sign should not run when protocol gating rejects the constructor") + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!( + "sign_create_witness should not run when protocol gating rejects the constructor" + ) + } + + fn can_sign_with(&self, _key: &PlatformAddress) -> bool { + false + } + } + + #[tokio::test] + async fn constructor_returns_not_active_before_structure_validation() { + let mut low_version = PlatformVersion::get(1) + .expect("platform version 1 exists") + .clone(); + low_version + .drive_abci + .validation_and_processing + .state_transitions + .address_credit_withdrawal + .basic_structure = None; + let mut inputs = BTreeMap::new(); + inputs.insert(PlatformAddress::P2pkh([1u8; 20]), (1, 1)); + + let result = AddressCreditWithdrawalTransitionV0::try_from_inputs_with_signer( + inputs, + None, + vec![AddressFundsFeeStrategyStep::DeductFromInput(99)], + 1, + crate::withdrawal::Pooling::Never, + CoreScript::default(), + &UnreachableAddressSigner, + 0, + &low_version, + ) + .await; + + assert!(matches!( + result, + Err(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::BasicError(BasicError::StateTransitionNotActiveError(_)) + ) + )); + } +} diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs index a49d1a1ec9e..c3092eaa6d0 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/state_transition_validation.rs @@ -21,8 +21,11 @@ use crate::withdrawal::Pooling; use platform_version::version::PlatformVersion; use std::collections::HashSet; -impl StateTransitionStructureValidation for AddressCreditWithdrawalTransitionV0 { - fn validate_structure( +impl AddressCreditWithdrawalTransitionV0 { + /// Validates all structural properties of the transition except for the + /// `input_witnesses` count. This is intended for client-side pre-signing + /// validation, where witnesses are not yet present. + pub fn validate_structure_without_input_witnesses( &self, platform_version: &PlatformVersion, ) -> SimpleConsensusValidationResult { @@ -44,17 +47,6 @@ impl StateTransitionStructureValidation for AddressCreditWithdrawalTransitionV0 ); } - // Validate input witnesses count matches inputs count - if self.inputs.len() != self.input_witnesses.len() { - return SimpleConsensusValidationResult::new_with_error( - BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( - self.inputs.len().min(u16::MAX as usize) as u16, - self.input_witnesses.len().min(u16::MAX as usize) as u16, - )) - .into(), - ); - } - // Validate output address is not also an input address if let Some((output_address, _)) = &self.output { if self.inputs.contains_key(output_address) { @@ -202,15 +194,14 @@ impl StateTransitionStructureValidation for AddressCreditWithdrawalTransitionV0 .inputs .values() .try_fold(0u64, |acc, (_, amount)| acc.checked_add(*amount)); - if input_sum.is_none() { + let Some(input_sum) = input_sum else { return SimpleConsensusValidationResult::new_with_error( BasicError::OverflowError(OverflowError::new("Input sum overflow".to_string())) .into(), ); - } + }; // Validate that input_sum > output_amount (withdrawal amount must be positive) - let input_sum = input_sum.unwrap(); // Safe: checked above let output_amount = self.output.as_ref().map_or(0, |(_, amount)| *amount); if input_sum <= output_amount { return SimpleConsensusValidationResult::new_with_error( @@ -240,6 +231,35 @@ impl StateTransitionStructureValidation for AddressCreditWithdrawalTransitionV0 SimpleConsensusValidationResult::new() } + + /// Validates that the number of `input_witnesses` matches the number of + /// inputs. Intended to be invoked after signing, once witnesses have been + /// produced by the signer. + pub fn validate_input_witnesses_count(&self) -> SimpleConsensusValidationResult { + if self.inputs.len() != self.input_witnesses.len() { + return SimpleConsensusValidationResult::new_with_error( + BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( + self.inputs.len().min(u16::MAX as usize) as u16, + self.input_witnesses.len().min(u16::MAX as usize) as u16, + )) + .into(), + ); + } + SimpleConsensusValidationResult::new() + } +} + +impl StateTransitionStructureValidation for AddressCreditWithdrawalTransitionV0 { + fn validate_structure( + &self, + platform_version: &PlatformVersion, + ) -> SimpleConsensusValidationResult { + let result = self.validate_structure_without_input_witnesses(platform_version); + if !result.is_valid() { + return result; + } + self.validate_input_witnesses_count() + } } #[cfg(test)] diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs index 696a54e7d80..5619f126e35 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_credit_withdrawal_transition/v0/v0_methods.rs @@ -14,6 +14,11 @@ 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::{ + address_funds_constructor_dispatch_error, consensus_errors_as_protocol_error, + verify_address_witnesses, StateTransitionType, +}; +#[cfg(feature = "state-transition-signing")] use crate::withdrawal::Pooling; #[cfg(feature = "state-transition-signing")] use crate::{ @@ -35,7 +40,7 @@ impl AddressCreditWithdrawalTransitionMethodsV0 for AddressCreditWithdrawalTrans output_script: CoreScript, signer: &S, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, ) -> Result { tracing::debug!("try_from_inputs_with_signer: Started"); tracing::debug!( @@ -47,7 +52,7 @@ impl AddressCreditWithdrawalTransitionMethodsV0 for AddressCreditWithdrawalTrans // Create the unsigned transition let mut address_credit_withdrawal_transition = AddressCreditWithdrawalTransitionV0 { - inputs: inputs.clone(), + inputs, output, fee_strategy, core_fee_per_byte, @@ -57,16 +62,52 @@ impl AddressCreditWithdrawalTransitionMethodsV0 for AddressCreditWithdrawalTrans input_witnesses: Vec::new(), }; + if let Some(error) = address_funds_constructor_dispatch_error( + StateTransitionType::AddressCreditWithdrawal, + platform_version, + ) { + return Err(error); + } + + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + // + // LOCKSTEP: this call is hard-coded to the v0 basic-structure check. + // If a future v1 basic-structure is introduced for this transition, + // both the drive-abci server dispatcher AND this SDK constructor must + // be updated together (e.g. by routing through a versioned + // `validate_basic_structure` wrapper as IdentityUpdate does). + let pre_validation_result = address_credit_withdrawal_transition + .validate_structure_without_input_witnesses(platform_version); + if let Some(error) = consensus_errors_as_protocol_error(pre_validation_result) { + return Err(error); + } + let state_transition: StateTransition = address_credit_withdrawal_transition.clone().into(); let signable_bytes = state_transition.signable_bytes()?; - let mut input_witnesses: Vec = Vec::with_capacity(inputs.len()); - for address in inputs.keys() { + let mut input_witnesses: Vec = + Vec::with_capacity(address_credit_withdrawal_transition.inputs.len()); + for address in address_credit_withdrawal_transition.inputs.keys() { input_witnesses.push(signer.sign_create_witness(address, &signable_bytes).await?); } + verify_address_witnesses( + address_credit_withdrawal_transition.inputs.keys(), + &input_witnesses, + &signable_bytes, + )?; address_credit_withdrawal_transition.input_witnesses = input_witnesses; + // After signing, only the witness count needs (re-)validation; the rest + // of the structure was already verified above. + let validation_result = + address_credit_withdrawal_transition.validate_input_witnesses_count(); + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + tracing::debug!("try_from_inputs_with_signer: Successfully created transition"); Ok(address_credit_withdrawal_transition.into()) } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/mod.rs index a251db91b44..717ada45817 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/mod.rs @@ -71,10 +71,19 @@ pub struct AddressFundingFromAssetLockTransitionV0 { mod tests { use super::*; use crate::address_funds::AddressFundsFeeStrategyStep; + use crate::consensus::basic::BasicError; + use crate::consensus::ConsensusError; + use crate::identity::signer::Signer; use crate::identity::state_transition::asset_lock_proof::chain::ChainAssetLockProof; use crate::identity::state_transition::AssetLockProved; + use crate::state_transition::address_funding_from_asset_lock_transition::methods::AddressFundingFromAssetLockTransitionMethodsV0; use crate::state_transition::{StateTransitionLike, StateTransitionType}; - use dashcore::OutPoint; + use crate::tests::fixtures::instant_asset_lock_proof_fixture; + use async_trait::async_trait; + use dashcore::secp256k1::SecretKey; + use dashcore::{Network, OutPoint, PrivateKey}; + use platform_version::version::PlatformVersion; + use std::str::FromStr; fn make_transition() -> AddressFundingFromAssetLockTransitionV0 { let mut inputs = BTreeMap::new(); @@ -224,4 +233,106 @@ mod tests { assert_eq!(t.user_fee_increase, 0); assert!(t.input_witnesses.is_empty()); } + + #[derive(Debug)] + struct UnreachableAddressSigner; + + #[async_trait] + impl Signer for UnreachableAddressSigner { + async fn sign( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!("sign should not run when protocol gating rejects the constructor") + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!( + "sign_create_witness should not run when protocol gating rejects the constructor" + ) + } + + fn can_sign_with(&self, _key: &PlatformAddress) -> bool { + false + } + } + + #[tokio::test] + async fn constructor_returns_not_active_before_structure_validation() { + let mut low_version = PlatformVersion::get(1) + .expect("platform version 1 exists") + .clone(); + low_version + .drive_abci + .validation_and_processing + .state_transitions + .address_funds_from_asset_lock + .basic_structure = None; + let mut inputs = BTreeMap::new(); + inputs.insert(PlatformAddress::P2pkh([1u8; 20]), (1, 1)); + let mut outputs = BTreeMap::new(); + outputs.insert(PlatformAddress::P2pkh([2u8; 20]), None); + + let result = + AddressFundingFromAssetLockTransitionV0::try_from_asset_lock_with_signer_and_private_key( + AssetLockProof::Chain(ChainAssetLockProof::new(42, [3u8; 36])), + &[7u8; 32], + inputs, + outputs, + vec![AddressFundsFeeStrategyStep::DeductFromInput(99)], + &UnreachableAddressSigner, + 0, + &low_version, + ) + .await; + + assert!(matches!( + result, + Err(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::BasicError(BasicError::StateTransitionNotActiveError(_)) + ) + )); + } + + #[tokio::test] + async fn constructor_rejects_wrong_asset_lock_private_key_locally() { + let mut inputs = BTreeMap::new(); + inputs.insert(PlatformAddress::P2pkh([1u8; 20]), (1, 1_000_000)); + let mut outputs = BTreeMap::new(); + outputs.insert(PlatformAddress::P2pkh([2u8; 20]), None); + + let correct_private_key = + PrivateKey::from_str("cSBnVM4xvxarwGQuAfQFwqDg9k5tErHUHzgWsEfD4zdwUasvqRVY") + .expect("fixture private key"); + let wrong_private_key = PrivateKey::new( + SecretKey::from_slice(&[2u8; 32]).expect("valid alternate private key"), + Network::Testnet, + ); + + let result = + AddressFundingFromAssetLockTransitionV0::try_from_asset_lock_with_signer_and_private_key( + instant_asset_lock_proof_fixture(Some(correct_private_key), None), + &wrong_private_key.inner.secret_bytes(), + inputs, + outputs, + vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + &UnreachableAddressSigner, + 0, + PlatformVersion::latest(), + ) + .await; + + assert!( + matches!(result, Err(ProtocolError::Generic(ref message)) if message.contains("does not match the locked output")), + "unexpected result: {:?}", + result + ); + } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/state_transition_validation.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/state_transition_validation.rs index 2bcbbe8eb57..b7157f193b6 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/state_transition_validation.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/state_transition_validation.rs @@ -13,8 +13,11 @@ use crate::validation::SimpleConsensusValidationResult; use platform_version::version::PlatformVersion; use std::collections::HashSet; -impl StateTransitionStructureValidation for AddressFundingFromAssetLockTransitionV0 { - fn validate_structure( +impl AddressFundingFromAssetLockTransitionV0 { + /// Validates all structural properties of the transition except for the + /// `input_witnesses` count. This is intended for client-side pre-signing + /// validation, where witnesses are not yet present. + pub fn validate_structure_without_input_witnesses( &self, platform_version: &PlatformVersion, ) -> SimpleConsensusValidationResult { @@ -62,17 +65,6 @@ impl StateTransitionStructureValidation for AddressFundingFromAssetLockTransitio ); } - // Validate input witnesses count matches inputs count (if there are inputs) - if self.inputs.len() != self.input_witnesses.len() { - return SimpleConsensusValidationResult::new_with_error( - BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( - self.inputs.len().min(u16::MAX as usize) as u16, - self.input_witnesses.len().min(u16::MAX as usize) as u16, - )) - .into(), - ); - } - // Validate no output address is also an input address for output_address in self.outputs.keys() { if self.inputs.contains_key(output_address) { @@ -202,6 +194,35 @@ impl StateTransitionStructureValidation for AddressFundingFromAssetLockTransitio SimpleConsensusValidationResult::new() } + + /// Validates that the number of `input_witnesses` matches the number of + /// inputs. Intended to be invoked after signing, once witnesses have been + /// produced by the signer. + pub fn validate_input_witnesses_count(&self) -> SimpleConsensusValidationResult { + if self.inputs.len() != self.input_witnesses.len() { + return SimpleConsensusValidationResult::new_with_error( + BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( + self.inputs.len().min(u16::MAX as usize) as u16, + self.input_witnesses.len().min(u16::MAX as usize) as u16, + )) + .into(), + ); + } + SimpleConsensusValidationResult::new() + } +} + +impl StateTransitionStructureValidation for AddressFundingFromAssetLockTransitionV0 { + fn validate_structure( + &self, + platform_version: &PlatformVersion, + ) -> SimpleConsensusValidationResult { + let result = self.validate_structure_without_input_witnesses(platform_version); + if !result.is_valid() { + return result; + } + self.validate_input_witnesses_count() + } } #[cfg(test)] diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/v0_methods.rs index 9397493b787..cbcb7d752d4 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/v0/v0_methods.rs @@ -14,12 +14,31 @@ 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::{ + address_funds_constructor_dispatch_error, consensus_errors_as_protocol_error, + verify_address_witnesses, StateTransitionType, +}; +#[cfg(feature = "state-transition-signing")] +use crate::util::hash::ripemd160_sha256; +#[cfg(feature = "state-transition-signing")] use crate::{prelude::UserFeeIncrease, state_transition::StateTransition, ProtocolError}; #[cfg(feature = "state-transition-signing")] +use dashcore::secp256k1::{Secp256k1, SecretKey}; +#[cfg(feature = "state-transition-signing")] use dashcore::signer; #[cfg(feature = "state-transition-signing")] +use dashcore::ScriptBuf; +#[cfg(feature = "state-transition-signing")] use platform_version::version::PlatformVersion; +#[cfg(feature = "state-transition-signing")] +fn p2pkh_pubkey_hash(script: &ScriptBuf) -> Option<[u8; 20]> { + script + .is_p2pkh() + .then(|| script.as_bytes().get(3..23)?.try_into().ok()) + .flatten() +} + impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetLockTransitionV0 { #[cfg(feature = "state-transition-signing")] async fn try_from_asset_lock_with_signer_and_private_key>( @@ -30,12 +49,12 @@ impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetL fee_strategy: AddressFundsFeeStrategy, signer: &S, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, ) -> Result { // Create the unsigned transition let mut address_funding_transition = AddressFundingFromAssetLockTransitionV0 { asset_lock_proof, - inputs: inputs.clone(), + inputs, outputs, fee_strategy, user_fee_increase, @@ -43,21 +62,113 @@ impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetL input_witnesses: Vec::new(), }; + if let Some(error) = address_funds_constructor_dispatch_error( + StateTransitionType::AddressFundingFromAssetLock, + platform_version, + ) { + return Err(error); + } + + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + // + // LOCKSTEP: this call is hard-coded to the v0 basic-structure check. + // If a future v1 basic-structure is introduced for this transition, + // both the drive-abci server dispatcher AND this SDK constructor must + // be updated together (e.g. by routing through a versioned + // `validate_basic_structure` wrapper as IdentityUpdate does). + let pre_validation_result = + address_funding_transition.validate_structure_without_input_witnesses(platform_version); + if let Some(error) = consensus_errors_as_protocol_error(pre_validation_result) { + return Err(error); + } + + // Validate the asset lock proof structure client-side before signing + // so malformed proofs are caught locally rather than being rejected by + // the network during basic-structure validation. Mirrors the symmetric + // check in IdentityCreateTransitionV0::try_from_identity_with_signer. + let asset_lock_validation_result = address_funding_transition + .asset_lock_proof + .validate_structure(platform_version)?; + if let Some(error) = consensus_errors_as_protocol_error(asset_lock_validation_result) { + return Err(error); + } + let state_transition: StateTransition = address_funding_transition.clone().into(); let signable_bytes = state_transition.signable_bytes()?; // Sign the asset lock proof let signature = signer::sign(&signable_bytes, asset_lock_proof_private_key)?; + if let Some(transaction) = address_funding_transition.asset_lock_proof.transaction() { + let output_index = address_funding_transition.asset_lock_proof.output_index() as usize; + let output = transaction + .special_transaction_payload + .as_ref() + .and_then(|payload| match payload { + dashcore::transaction::special_transaction::TransactionPayload::AssetLockPayloadType(payload) => { + payload.credit_outputs.get(output_index) + } + _ => None, + }) + .ok_or_else(|| { + ProtocolError::Generic(format!( + "asset lock proof output {output_index} is not available for local signature verification" + )) + })?; + if let Some(locked_pubkey_hash) = p2pkh_pubkey_hash(&output.script_pubkey) { + let secret_key = + SecretKey::from_slice(asset_lock_proof_private_key).map_err(|e| { + ProtocolError::Generic(format!("invalid asset lock proof private key: {e}")) + })?; + let secp = Secp256k1::new(); + let public_key = + dashcore::secp256k1::PublicKey::from_secret_key(&secp, &secret_key); + let compressed_pubkey_hash = ripemd160_sha256(&public_key.serialize()); + let uncompressed_pubkey_hash = + ripemd160_sha256(&public_key.serialize_uncompressed()); + + if locked_pubkey_hash != compressed_pubkey_hash + && locked_pubkey_hash != uncompressed_pubkey_hash + { + return Err(ProtocolError::Generic( + "asset lock proof private key does not match the locked output".to_string(), + )); + } + } + } else { + // Only Instant asset-lock proofs carry the full transaction needed for + // local script/private-key matching. Chain proofs are still validated + // structurally above, but this constructor cannot verify their locked + // output locally from just the outpoint. + tracing::debug!( + "skipping local asset-lock private-key verification because the proof does not carry a transaction" + ); + } address_funding_transition.signature = signature.to_vec().into(); // Sign with input witnesses - let mut input_witnesses: Vec = Vec::with_capacity(inputs.len()); - for address in inputs.keys() { + let mut input_witnesses: Vec = + Vec::with_capacity(address_funding_transition.inputs.len()); + for address in address_funding_transition.inputs.keys() { input_witnesses.push(signer.sign_create_witness(address, &signable_bytes).await?); } + verify_address_witnesses( + address_funding_transition.inputs.keys(), + &input_witnesses, + &signable_bytes, + )?; address_funding_transition.input_witnesses = input_witnesses; + // After signing, only the witness count needs (re-)validation; the rest + // of the structure was already verified above. + let validation_result = address_funding_transition.validate_input_witnesses_count(); + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + + tracing::debug!("try_from_asset_lock_with_signer: Successfully created transition"); Ok(address_funding_transition.into()) } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/signing_tests.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/signing_tests.rs index 52799e2c543..1e02f1943c6 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/signing_tests.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/signing_tests.rs @@ -10,6 +10,7 @@ use std::collections::{BTreeMap, HashMap}; +use async_trait::async_trait; use dashcore::blockdata::opcodes::all::*; use dashcore::blockdata::script::ScriptBuf; use dashcore::hashes::Hash; @@ -17,7 +18,9 @@ use dashcore::secp256k1::{PublicKey as RawPublicKey, Secp256k1, SecretKey as Raw use dashcore::PublicKey; use platform_value::BinaryData; -use crate::address_funds::{AddressWitness, PlatformAddress}; +use crate::address_funds::{AddressFundsFeeStrategyStep, AddressWitness, PlatformAddress}; +use crate::consensus::basic::BasicError; +use crate::consensus::ConsensusError; use crate::identity::signer::Signer; use crate::serialization::{PlatformDeserializable, PlatformSerializable, Signable}; use crate::state_transition::address_funds_transfer_transition::methods::AddressFundsTransferTransitionMethodsV0; @@ -130,7 +133,7 @@ impl TestAddressSigner { } } -#[async_trait::async_trait] +#[async_trait] impl Signer for TestAddressSigner { async fn sign(&self, key: &PlatformAddress, data: &[u8]) -> Result { match key { @@ -220,6 +223,59 @@ fn get_platform_version() -> &'static PlatformVersion { PlatformVersion::latest() } +#[derive(Debug)] +struct UnreachableAddressSigner; + +#[async_trait] +impl Signer for UnreachableAddressSigner { + async fn sign( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!("sign should not run when protocol gating rejects the constructor") + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!("sign_create_witness should not run when protocol gating rejects the constructor") + } + + fn can_sign_with(&self, _key: &PlatformAddress) -> bool { + false + } +} + +#[derive(Debug)] +struct WrongWitnessSigner<'a> { + inner: &'a TestAddressSigner, + replacement_address: PlatformAddress, +} + +#[async_trait] +impl Signer for WrongWitnessSigner<'_> { + async fn sign(&self, key: &PlatformAddress, data: &[u8]) -> Result { + self.inner.sign(key, data).await + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + data: &[u8], + ) -> Result { + self.inner + .sign_create_witness(&self.replacement_address, data) + .await + } + + fn can_sign_with(&self, key: &PlatformAddress) -> bool { + self.inner.can_sign_with(key) + } +} + /// Verifies all input witnesses against the transition's signable bytes fn verify_transition_signatures( transition: &AddressFundsTransferTransitionV0, @@ -268,16 +324,16 @@ async fn test_single_p2pkh_input_signing() { // Build inputs and outputs let mut inputs = BTreeMap::new(); - inputs.insert(input_address, (1u32, 1000u64)); // nonce: 1, credits: 1000 + inputs.insert(input_address.clone(), (1u32, 1_000_000u64)); // nonce: 1, credits: 1000 let mut outputs = BTreeMap::new(); - outputs.insert(output_address, 900u64); + outputs.insert(output_address, 1_000_000u64); // Create signed transition let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -303,6 +359,35 @@ async fn test_single_p2pkh_input_signing() { verify_transition_signatures(&transition).expect("signatures should be valid"); } +#[tokio::test] +async fn test_constructor_rejects_witness_for_wrong_input_address() { + let mut signer = TestAddressSigner::new(); + let input_address = signer.add_p2pkh([1u8; 32]); + let wrong_address = signer.add_p2pkh([2u8; 32]); + let output_address = PlatformAddress::P2pkh([99u8; 20]); + + let mut inputs = BTreeMap::new(); + inputs.insert(input_address, (1u32, 1_000_000u64)); + + let mut outputs = BTreeMap::new(); + outputs.insert(output_address, 1_000_000u64); + + let result = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( + inputs, + outputs, + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], + &WrongWitnessSigner { + inner: &signer, + replacement_address: wrong_address, + }, + 0, + get_platform_version(), + ) + .await; + + assert!(matches!(result, Err(ProtocolError::AddressWitnessError(_)))); +} + #[tokio::test] async fn test_multiple_p2pkh_inputs_signing() { let mut signer = TestAddressSigner::new(); @@ -317,18 +402,18 @@ async fn test_multiple_p2pkh_inputs_signing() { // Build inputs (multiple inputs) let mut inputs = BTreeMap::new(); - inputs.insert(input1, (1u32, 500u64)); - inputs.insert(input2, (1u32, 300u64)); - inputs.insert(input3, (1u32, 200u64)); + inputs.insert(input1.clone(), (1u32, 1_000_000u64)); + inputs.insert(input2.clone(), (1u32, 1_000_000u64)); + inputs.insert(input3.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 3_000_000u64); // Create signed transition let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -370,16 +455,16 @@ async fn test_single_p2sh_2_of_3_multisig_input_signing() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input_address, (1u32, 1000u64)); + inputs.insert(input_address.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); // Create signed transition let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -424,15 +509,15 @@ async fn test_p2sh_3_of_5_multisig_input_signing() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input_address, (1u32, 5000u64)); + inputs.insert(input_address.clone(), (1u32, 5_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 4500u64); + outputs.insert(output, 5_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -468,16 +553,16 @@ async fn test_multiple_p2sh_inputs_signing() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input1, (1u32, 1000u64)); - inputs.insert(input2, (1u32, 500u64)); + inputs.insert(input1.clone(), (1u32, 1_000_000u64)); + inputs.insert(input2.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 1400u64); + outputs.insert(output, 2_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -516,16 +601,16 @@ async fn test_mixed_p2pkh_and_p2sh_inputs() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(p2pkh_input, (1u32, 1000u64)); - inputs.insert(p2sh_input, (1u32, 2000u64)); + inputs.insert(p2pkh_input.clone(), (1u32, 1_000_000u64)); + inputs.insert(p2sh_input.clone(), (1u32, 2_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 2800u64); + outputs.insert(output, 3_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -575,19 +660,19 @@ async fn test_complex_mixed_inputs_multiple_outputs() { let output2 = PlatformAddress::P2sh([101u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(p2pkh1, (1u32, 1000u64)); - inputs.insert(p2pkh2, (1u32, 2000u64)); - inputs.insert(p2sh1, (1u32, 3000u64)); - inputs.insert(p2sh2, (1u32, 4000u64)); + inputs.insert(p2pkh1.clone(), (1u32, 1_000_000u64)); + inputs.insert(p2pkh2.clone(), (1u32, 2_000_000u64)); + inputs.insert(p2sh1.clone(), (1u32, 3_000_000u64)); + inputs.insert(p2sh2.clone(), (1u32, 4_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output1, 5000u64); - outputs.insert(output2, 4500u64); + outputs.insert(output1, 5_000_000u64); + outputs.insert(output2, 5_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -634,15 +719,15 @@ async fn test_signed_transition_serialization_roundtrip() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -680,15 +765,15 @@ async fn test_multisig_transition_serialization_roundtrip() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -722,16 +807,16 @@ async fn test_mixed_transition_serialization_roundtrip() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(p2pkh, (1u32, 1000u64)); - inputs.insert(p2sh, (1u32, 2000u64)); + inputs.insert(p2pkh.clone(), (1u32, 1_000_000u64)); + inputs.insert(p2sh.clone(), (1u32, 2_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 2800u64); + outputs.insert(output, 3_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -768,15 +853,15 @@ async fn test_tampered_inputs_verification_fails() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output.clone(), 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs.clone(), outputs.clone(), - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -793,7 +878,9 @@ async fn test_tampered_inputs_verification_fails() { // Tamper with the transition by modifying credits let original_witnesses = transition.input_witnesses.clone(); - transition.inputs.insert(input, (1u32, 2000u64)); // Changed credits + transition + .inputs + .insert(input.clone(), (1u32, 2_000_000u64)); // Changed credits // Re-add original witnesses (they were signed for different data) transition.input_witnesses = original_witnesses; @@ -814,15 +901,15 @@ async fn test_tampered_outputs_verification_fails() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output.clone(), 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -838,7 +925,7 @@ async fn test_tampered_outputs_verification_fails() { }; // Tamper with outputs - transition.outputs.insert(output, 950u64); // Changed output amount + transition.outputs.insert(output.clone(), 950_000u64); // Changed output amount // Verification should fail let result = verify_transition_signatures(&transition); @@ -857,15 +944,15 @@ async fn test_wrong_witness_for_address_fails() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input1, (1u32, 1000u64)); + inputs.insert(input1.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs.clone(), outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -882,7 +969,9 @@ async fn test_wrong_witness_for_address_fails() { // Replace input with a different address but keep the same witness transition.inputs.clear(); - transition.inputs.insert(input2, (1u32, 1000u64)); + transition + .inputs + .insert(input2.clone(), (1u32, 1_000_000u64)); // Verification should fail (witness public key doesn't match new address) let result = verify_transition_signatures(&transition); @@ -900,15 +989,15 @@ async fn test_missing_witness_fails() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -942,15 +1031,15 @@ async fn test_p2sh_insufficient_signatures_fails() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -999,15 +1088,15 @@ async fn test_1_of_1_multisig() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -1039,15 +1128,15 @@ async fn test_high_threshold_multisig() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 10000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 9500u64); + outputs.insert(output, 1_000_000u64); let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -1077,16 +1166,16 @@ async fn test_signer_cannot_sign_unknown_address() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(unknown_address, (1u32, 1000u64)); + inputs.insert(unknown_address.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); // Should fail because signer doesn't have the key let result = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, 0, get_platform_version(), @@ -1118,17 +1207,17 @@ async fn test_user_fee_increase_preserved() { let output = PlatformAddress::P2pkh([99u8; 20]); let mut inputs = BTreeMap::new(); - inputs.insert(input, (1u32, 1000u64)); + inputs.insert(input.clone(), (1u32, 1_000_000u64)); let mut outputs = BTreeMap::new(); - outputs.insert(output, 900u64); + outputs.insert(output, 1_000_000u64); let user_fee_increase = 50u16; let state_transition = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( inputs, outputs, - vec![], + vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], &signer, user_fee_increase, get_platform_version(), @@ -1155,9 +1244,9 @@ async fn test_different_nonces_produce_different_signable_bytes() { // First transition with nonce 1 let mut inputs1 = BTreeMap::new(); - inputs1.insert(input, (1u32, 1000u64)); + inputs1.insert(input.clone(), (1u32, 1000u64)); let mut outputs1 = BTreeMap::new(); - outputs1.insert(output, 900u64); + outputs1.insert(output.clone(), 900u64); let transition1 = AddressFundsTransferTransitionV0 { inputs: inputs1, @@ -1169,9 +1258,9 @@ async fn test_different_nonces_produce_different_signable_bytes() { // Second transition with nonce 2 let mut inputs2 = BTreeMap::new(); - inputs2.insert(input, (2u32, 1000u64)); // Different nonce + inputs2.insert(input.clone(), (2u32, 1000u64)); // Different nonce let mut outputs2 = BTreeMap::new(); - outputs2.insert(output, 900u64); + outputs2.insert(output.clone(), 900u64); let transition2 = AddressFundsTransferTransitionV0 { inputs: inputs2, @@ -1194,3 +1283,39 @@ async fn test_different_nonces_produce_different_signable_bytes() { "Different nonces should produce different signable bytes" ); } + +#[tokio::test] +async fn constructor_returns_not_active_before_structure_validation() { + let mut low_version = PlatformVersion::get(1) + .expect("platform version 1 exists") + .clone(); + low_version + .drive_abci + .validation_and_processing + .state_transitions + .address_funds_transfer + .basic_structure = None; + let mut inputs = BTreeMap::new(); + inputs.insert(PlatformAddress::P2pkh([1u8; 20]), (1, 1)); + let mut outputs = BTreeMap::new(); + outputs.insert(PlatformAddress::P2pkh([2u8; 20]), 1); + + let result = AddressFundsTransferTransitionV0::try_from_inputs_with_signer( + inputs, + outputs, + vec![AddressFundsFeeStrategyStep::DeductFromInput(99)], + &UnreachableAddressSigner, + 0, + &low_version, + ) + .await; + + assert!(matches!( + result, + Err(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::BasicError(BasicError::StateTransitionNotActiveError(_)) + ) + )); +} diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/state_transition_validation.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/state_transition_validation.rs index 4113e8ced54..9ab2dec5991 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/state_transition_validation.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/state_transition_validation.rs @@ -14,8 +14,11 @@ use crate::validation::SimpleConsensusValidationResult; use platform_version::version::PlatformVersion; use std::collections::HashSet; -impl StateTransitionStructureValidation for AddressFundsTransferTransitionV0 { - fn validate_structure( +impl AddressFundsTransferTransitionV0 { + /// Validates all structural properties of the transition except for the + /// `input_witnesses` count. This is intended for client-side pre-signing + /// validation, where witnesses are not yet present. + pub fn validate_structure_without_input_witnesses( &self, platform_version: &PlatformVersion, ) -> SimpleConsensusValidationResult { @@ -56,17 +59,6 @@ impl StateTransitionStructureValidation for AddressFundsTransferTransitionV0 { ); } - // Validate input witnesses count matches inputs count - if self.inputs.len() != self.input_witnesses.len() { - return SimpleConsensusValidationResult::new_with_error( - BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( - self.inputs.len().min(u16::MAX as usize) as u16, - self.input_witnesses.len().min(u16::MAX as usize) as u16, - )) - .into(), - ); - } - // Validate no output address is also an input address for output_address in self.outputs.keys() { if self.inputs.contains_key(output_address) { @@ -222,6 +214,35 @@ impl StateTransitionStructureValidation for AddressFundsTransferTransitionV0 { SimpleConsensusValidationResult::new() } + + /// Validates that the number of `input_witnesses` matches the number of + /// inputs. Intended to be invoked after signing, once witnesses have been + /// produced by the signer. + pub fn validate_input_witnesses_count(&self) -> SimpleConsensusValidationResult { + if self.inputs.len() != self.input_witnesses.len() { + return SimpleConsensusValidationResult::new_with_error( + BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( + self.inputs.len().min(u16::MAX as usize) as u16, + self.input_witnesses.len().min(u16::MAX as usize) as u16, + )) + .into(), + ); + } + SimpleConsensusValidationResult::new() + } +} + +impl StateTransitionStructureValidation for AddressFundsTransferTransitionV0 { + fn validate_structure( + &self, + platform_version: &PlatformVersion, + ) -> SimpleConsensusValidationResult { + let result = self.validate_structure_without_input_witnesses(platform_version); + if !result.is_valid() { + return result; + } + self.validate_input_witnesses_count() + } } #[cfg(test)] diff --git a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/v0_methods.rs index c1582ff5cf4..20f6186f674 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/v0/v0_methods.rs @@ -12,6 +12,11 @@ use crate::serialization::Signable; use crate::state_transition::address_funds_transfer_transition::methods::AddressFundsTransferTransitionMethodsV0; use crate::state_transition::address_funds_transfer_transition::v0::AddressFundsTransferTransitionV0; #[cfg(feature = "state-transition-signing")] +use crate::state_transition::{ + address_funds_constructor_dispatch_error, consensus_errors_as_protocol_error, + verify_address_witnesses, StateTransitionType, +}; +#[cfg(feature = "state-transition-signing")] use crate::{ prelude::{AddressNonce, UserFeeIncrease}, state_transition::StateTransition, @@ -28,7 +33,7 @@ impl AddressFundsTransferTransitionMethodsV0 for AddressFundsTransferTransitionV fee_strategy: AddressFundsFeeStrategy, signer: &S, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, ) -> Result { tracing::debug!("try_from_inputs_with_signer: Started"); tracing::debug!( @@ -39,23 +44,58 @@ impl AddressFundsTransferTransitionMethodsV0 for AddressFundsTransferTransitionV // Create the unsigned transition let mut address_funds_transition = AddressFundsTransferTransitionV0 { - inputs: inputs.clone(), + inputs, outputs, fee_strategy, user_fee_increase, input_witnesses: Vec::new(), }; + if let Some(error) = address_funds_constructor_dispatch_error( + StateTransitionType::AddressFundsTransfer, + platform_version, + ) { + return Err(error); + } + + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + // + // LOCKSTEP: this call is hard-coded to the v0 basic-structure check. + // If a future v1 basic-structure is introduced for this transition, + // both the drive-abci server dispatcher AND this SDK constructor must + // be updated together (e.g. by routing through a versioned + // `validate_basic_structure` wrapper as IdentityUpdate does). + let pre_validation_result = + address_funds_transition.validate_structure_without_input_witnesses(platform_version); + if let Some(error) = consensus_errors_as_protocol_error(pre_validation_result) { + return Err(error); + } + let state_transition: StateTransition = address_funds_transition.clone().into(); let signable_bytes = state_transition.signable_bytes()?; - let mut input_witnesses: Vec = Vec::with_capacity(inputs.len()); - for address in inputs.keys() { + let mut input_witnesses: Vec = + Vec::with_capacity(address_funds_transition.inputs.len()); + for address in address_funds_transition.inputs.keys() { input_witnesses.push(signer.sign_create_witness(address, &signable_bytes).await?); } + verify_address_witnesses( + address_funds_transition.inputs.keys(), + &input_witnesses, + &signable_bytes, + )?; address_funds_transition.input_witnesses = input_witnesses; + // After signing, only the witness count needs (re-)validation; the rest + // of the structure was already verified above. + let validation_result = address_funds_transition.validate_input_witnesses_count(); + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + tracing::debug!("try_from_inputs_with_signer: Successfully created transition"); Ok(address_funds_transition.into()) } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/mod.rs index 2d32a138cbc..3f120255f7e 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/mod.rs @@ -333,4 +333,462 @@ mod tests { IdentityCreateFromAddressesTransition::V0(_) )); } + + /// Verifies that `try_from_inputs_with_signer` rejects an identity whose + /// public keys violate purpose/security-level constraints (TRANSFER + HIGH) + /// via the structural public-key validation, returning + /// `ProtocolError::ConsensusError(InvalidIdentityPublicKeySecurityLevelError)` + /// before any signer is invoked. + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_inputs_with_signer_rejects_transfer_high_key() { + use crate::address_funds::{AddressFundsFeeStrategyStep, AddressWitness, PlatformAddress}; + use crate::consensus::ConsensusError; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, SecurityLevel}; + use crate::prelude::Identifier; + use crate::state_transition::identity_create_from_addresses_transition::methods::IdentityCreateFromAddressesTransitionMethodsV0; + use crate::version::PlatformVersion; + use async_trait::async_trait; + + /// A signer over `IdentityPublicKey` that should never be invoked. + #[derive(Debug)] + struct UnreachableIdentityKeySigner; + + #[async_trait] + impl Signer for UnreachableIdentityKeySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableIdentityKeySigner::sign must not be called when \ + pre-signing validation rejects the transition" + ); + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableIdentityKeySigner::sign_create_witness must not \ + be called when pre-signing validation rejects the transition" + ); + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + /// A signer over `PlatformAddress` that should never be invoked. + #[derive(Debug)] + struct UnreachableAddressSigner; + + #[async_trait] + impl Signer for UnreachableAddressSigner { + async fn sign( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableAddressSigner::sign must not be called when \ + pre-signing validation rejects the transition" + ); + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableAddressSigner::sign_create_witness must not be \ + called when pre-signing validation rejects the transition" + ); + } + + fn can_sign_with(&self, _key: &PlatformAddress) -> bool { + false + } + } + + let platform_version = PlatformVersion::latest(); + + // Required master key so that the identity satisfies the master-key + // presence requirement; the failure must come specifically from the + // invalid TRANSFER+HIGH key below. + let master_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 0, + purpose: crate::identity::Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![0u8; 33]), + disabled_at: None, + } + .into(); + + // Invalid combination: TRANSFER purpose only allows CRITICAL security level. + let invalid_transfer_high_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 1, + purpose: crate::identity::Purpose::TRANSFER, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![1u8; 33]), + disabled_at: None, + } + .into(); + + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::from([(0, master_key), (1, invalid_transfer_high_key)]), + balance: 0, + revision: 0, + } + .into(); + + // Inputs themselves are structurally valid; pre-signing validation must + // still fail because of the invalid public key on the identity. + let min_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + let min_funding = platform_version + .dpp + .state_transitions + .address_funds + .min_identity_funding_amount; + let mut inputs = BTreeMap::new(); + inputs.insert( + PlatformAddress::P2pkh([1u8; 20]), + (1u32, min_input.max(min_funding) * 2), + ); + + let result = IdentityCreateFromAddressesTransitionV0::try_from_inputs_with_signer( + &identity, + inputs, + None, + vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + &UnreachableIdentityKeySigner, + &UnreachableAddressSigner, + 0, + platform_version, + ) + .await; + + match result { + Err(ProtocolError::ConsensusError(boxed)) => match *boxed { + ConsensusError::BasicError( + BasicError::InvalidIdentityPublicKeySecurityLevelError(err), + ) => { + assert_eq!(err.purpose(), crate::identity::Purpose::TRANSFER); + assert_eq!(err.security_level(), SecurityLevel::HIGH); + } + other => panic!( + "expected InvalidIdentityPublicKeySecurityLevelError, got {:?}", + other + ), + }, + other => panic!( + "expected ConsensusError(InvalidIdentityPublicKeySecurityLevelError), got {:?}", + other + ), + } + } + + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_inputs_with_signer_rejects_bad_identity_public_key_signature_locally() { + use crate::address_funds::{AddressFundsFeeStrategyStep, AddressWitness, PlatformAddress}; + use crate::consensus::signature::SignatureError; + use crate::consensus::ConsensusError; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel}; + use crate::prelude::Identifier; + use crate::state_transition::identity_create_from_addresses_transition::methods::IdentityCreateFromAddressesTransitionMethodsV0; + use crate::version::PlatformVersion; + use async_trait::async_trait; + use dashcore::secp256k1::{PublicKey as RawPublicKey, Secp256k1, SecretKey}; + use std::collections::BTreeMap; + use std::sync::atomic::{AtomicUsize, Ordering}; + + #[derive(Debug)] + struct WrongIdentityKeySigner { + wrong_secret_key: SecretKey, + } + + #[async_trait] + impl Signer for WrongIdentityKeySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + data: &[u8], + ) -> Result { + Ok(BinaryData::new( + dashcore::signer::sign(data, &self.wrong_secret_key.secret_bytes()) + .expect("wrong-key signing should succeed") + .to_vec(), + )) + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("identity public key signer should not create address witnesses") + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + true + } + } + + #[derive(Debug, Default)] + struct RecordingAddressSigner { + sign_create_witness_calls: AtomicUsize, + } + + #[async_trait] + impl Signer for RecordingAddressSigner { + async fn sign( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + Err(ProtocolError::Generic( + "address signer should not be called before PoP verification".to_string(), + )) + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + self.sign_create_witness_calls + .fetch_add(1, Ordering::SeqCst); + Err(ProtocolError::Generic( + "address signer should not be called before PoP verification".to_string(), + )) + } + + fn can_sign_with(&self, _key: &PlatformAddress) -> bool { + true + } + } + + let secp = Secp256k1::new(); + let correct_secret_key = + SecretKey::from_slice(&[1u8; 32]).expect("valid identity secret key"); + let wrong_secret_key = + SecretKey::from_slice(&[2u8; 32]).expect("valid alternate secret key"); + let correct_public_key = RawPublicKey::from_secret_key(&secp, &correct_secret_key); + + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::from([( + 0, + IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(correct_public_key.serialize().to_vec()), + disabled_at: None, + } + .into(), + )]), + balance: 0, + revision: 0, + } + .into(); + + let platform_version = PlatformVersion::latest(); + let min_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + let min_funding = platform_version + .dpp + .state_transitions + .address_funds + .min_identity_funding_amount; + let mut inputs = BTreeMap::new(); + inputs.insert( + PlatformAddress::P2pkh([1u8; 20]), + (1u32, min_input.max(min_funding) * 2), + ); + + let identity_public_key_signer = WrongIdentityKeySigner { wrong_secret_key }; + let address_signer = RecordingAddressSigner::default(); + + let result = IdentityCreateFromAddressesTransitionV0::try_from_inputs_with_signer( + &identity, + inputs, + None, + vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + &identity_public_key_signer, + &address_signer, + 0, + platform_version, + ) + .await; + + assert_eq!( + address_signer + .sign_create_witness_calls + .load(Ordering::SeqCst), + 0, + "address signer should not be reached when PoP self-check fails" + ); + + match result { + Err(ProtocolError::ConsensusError(boxed)) => match *boxed { + ConsensusError::SignatureError(SignatureError::BasicECDSAError(_)) => {} + other => panic!("expected SignatureError(BasicECDSAError), got {:?}", other), + }, + Err(ProtocolError::ConsensusErrors(errors)) => { + assert_eq!(errors.len(), 1, "expected a single consensus error"); + assert!(matches!( + errors.as_slice(), + [ConsensusError::SignatureError( + SignatureError::BasicECDSAError(_) + )] + )); + } + other => panic!( + "expected ConsensusError/ConsensusErrors with BasicECDSAError, got {:?}", + other + ), + } + } + + #[tokio::test] + async fn try_from_inputs_with_signer_returns_not_active_before_structure_validation() { + use crate::address_funds::AddressFundsFeeStrategyStep; + use crate::address_funds::AddressWitness; + use crate::consensus::basic::BasicError; + use crate::consensus::ConsensusError; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey}; + use crate::state_transition::identity_create_from_addresses_transition::methods::IdentityCreateFromAddressesTransitionMethodsV0; + use async_trait::async_trait; + use platform_value::{BinaryData, Identifier}; + use platform_version::version::PlatformVersion; + use std::collections::BTreeMap; + + #[derive(Debug)] + struct UnreachableIdentityKeySigner; + + #[async_trait] + impl Signer for UnreachableIdentityKeySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("sign should not run when protocol gating rejects the constructor") + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "sign_create_witness should not run when protocol gating rejects the constructor" + ) + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + #[derive(Debug)] + struct UnreachableAddressSigner; + + #[async_trait] + impl Signer for UnreachableAddressSigner { + async fn sign( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!("sign should not run when protocol gating rejects the constructor") + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!( + "sign_create_witness should not run when protocol gating rejects the constructor" + ) + } + + fn can_sign_with(&self, _key: &PlatformAddress) -> bool { + false + } + } + + let mut low_version = PlatformVersion::get(1) + .expect("platform version 1 exists") + .clone(); + low_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_create_from_addresses_state_transition + .basic_structure = None; + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::new(), + balance: 0, + revision: 0, + } + .into(); + let mut inputs = BTreeMap::new(); + inputs.insert(PlatformAddress::P2pkh([1u8; 20]), (1u32, 1)); + + let result = IdentityCreateFromAddressesTransitionV0::try_from_inputs_with_signer( + &identity, + inputs, + None, + vec![AddressFundsFeeStrategyStep::DeductFromInput(99)], + &UnreachableIdentityKeySigner, + &UnreachableAddressSigner, + 0, + &low_version, + ) + .await; + + assert!(matches!( + result, + Err(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::BasicError(BasicError::StateTransitionNotActiveError(_)) + ) + )); + } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/state_transition_validation.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/state_transition_validation.rs index 70eaa1b720b..1fe455b04e1 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/state_transition_validation.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/state_transition_validation.rs @@ -15,8 +15,24 @@ use crate::validation::SimpleConsensusValidationResult; use platform_version::version::PlatformVersion; use std::collections::HashSet; -impl StateTransitionStructureValidation for IdentityCreateFromAddressesTransitionV0 { - fn validate_structure( +impl IdentityCreateFromAddressesTransitionV0 { + /// Narrow basic-structure check: validates all structural properties of + /// the transition except for the `input_witnesses` count, and **does NOT** + /// validate the public-key structure of `public_keys`. + /// + /// This is the same surface the server's basic-structure pipeline + /// exercises. Public-key structure validation is intentionally skipped + /// here so that submitting an invalid set of public keys reaches + /// drive-abci's `advanced_structure_v0`, where it attaches a + /// `BumpAddressInputNoncesAction` (penalty + processing fee) instead of + /// failing for free at basic-structure. See the NOTE at the bottom of this + /// function for the full rationale. + /// + /// SDK constructors that want to give callers pre-broadcast feedback on + /// public-key structure problems must call + /// `IdentityPublicKeyInCreation::validate_identity_public_keys_structure` + /// directly *in addition to* this method. + pub fn validate_structure_without_input_witnesses( &self, platform_version: &PlatformVersion, ) -> SimpleConsensusValidationResult { @@ -70,17 +86,6 @@ impl StateTransitionStructureValidation for IdentityCreateFromAddressesTransitio ); } - // Validate input witnesses count matches inputs count - if self.inputs.len() != self.input_witnesses.len() { - return SimpleConsensusValidationResult::new_with_error( - BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( - self.inputs.len().min(u16::MAX as usize) as u16, - self.input_witnesses.len().min(u16::MAX as usize) as u16, - )) - .into(), - ); - } - // Validate output address is not also an input address if let Some((output_address, _)) = &self.output { if self.inputs.contains_key(output_address) { @@ -243,6 +248,52 @@ impl StateTransitionStructureValidation for IdentityCreateFromAddressesTransitio ); } + // NOTE: Public-key structure validation (counts, duplicates, + // purpose/security level constraints) is intentionally NOT performed + // here. On the server, that validation lives in drive-abci's + // `advanced_structure_v0`, where invalid public keys attach a + // `BumpAddressInputNoncesAction` with a penalty + processing fee. + // Running it here on the basic-structure / trait surface would create + // a free failure mode, because basic-structure failures return only + // errors with no penalty action. Client-side constructors invoke + // `IdentityPublicKeyInCreation::validate_identity_public_keys_structure` + // directly for pre-broadcast feedback. SimpleConsensusValidationResult::new() } + + /// Validates that the number of `input_witnesses` matches the number of + /// inputs. Intended to be invoked after signing, once witnesses have been + /// produced by the signer. + pub fn validate_input_witnesses_count(&self) -> SimpleConsensusValidationResult { + if self.inputs.len() != self.input_witnesses.len() { + return SimpleConsensusValidationResult::new_with_error( + BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( + self.inputs.len().min(u16::MAX as usize) as u16, + self.input_witnesses.len().min(u16::MAX as usize) as u16, + )) + .into(), + ); + } + SimpleConsensusValidationResult::new() + } +} + +impl StateTransitionStructureValidation for IdentityCreateFromAddressesTransitionV0 { + /// Narrow basic-structure validation that mirrors what the server runs at + /// the basic-structure stage. As documented on + /// [`IdentityCreateFromAddressesTransitionV0::validate_structure_without_input_witnesses`], + /// this **does NOT** validate `public_keys` structure: that check is run + /// later in drive-abci's `advanced_structure_v0` so it can attach a + /// `BumpAddressInputNoncesAction` penalty. SDK constructors are expected + /// to call public-key validation separately for pre-broadcast UX. + fn validate_structure( + &self, + platform_version: &PlatformVersion, + ) -> SimpleConsensusValidationResult { + let result = self.validate_structure_without_input_witnesses(platform_version); + if !result.is_valid() { + return result; + } + self.validate_input_witnesses_count() + } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/v0_methods.rs index 63ad49a8ed5..b55afd714a6 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_from_addresses_transition/v0/v0_methods.rs @@ -19,6 +19,17 @@ use crate::state_transition::StateTransitionType; // ============================ // Crate: Feature-Gated (state-transition-signing) // ============================ +#[cfg(feature = "state-transition-signing")] +use crate::state_transition::{ + address_funds_constructor_dispatch_error, consensus_errors_as_protocol_error, + verify_address_witnesses, +}; +#[cfg(feature = "state-transition-signing")] +use crate::{ + serialization::PlatformMessageSignable, + state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters, +}; + #[cfg(feature = "state-transition-signing")] use crate::{ address_funds::AddressFundsFeeStrategy, @@ -49,12 +60,12 @@ impl IdentityCreateFromAddressesTransitionMethodsV0 for IdentityCreateFromAddres identity_public_key_signer: &S, address_signer: &WS, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, ) -> Result { // Create the unsigned transition let mut identity_create_from_addresses_transition = IdentityCreateFromAddressesTransitionV0 { - inputs: inputs.clone(), + inputs, output, fee_strategy, user_fee_increase, @@ -62,13 +73,51 @@ impl IdentityCreateFromAddressesTransitionMethodsV0 for IdentityCreateFromAddres ..Default::default() }; - let public_keys = identity + if let Some(error) = address_funds_constructor_dispatch_error( + StateTransitionType::IdentityCreateFromAddresses, + platform_version, + ) { + return Err(error); + } + + let public_keys: Vec = identity .public_keys() .values() .map(|public_key| public_key.clone().into()) .collect(); + + // Validate public key structure (purpose/security level compatibility) + // before broadcasting, so invalid combinations are caught client-side + // rather than being rejected by the network. + // + // LOCKSTEP: both this call and the + // `validate_structure_without_input_witnesses` call below are hard-coded + // to the v0 basic-structure checks. If a future v1 basic-structure is + // introduced for this transition, both the drive-abci server dispatcher + // AND this SDK constructor must be updated together (e.g. by routing + // through a versioned `validate_basic_structure` wrapper as + // IdentityUpdate does). + let validation_result = + IdentityPublicKeyInCreation::validate_identity_public_keys_structure( + &public_keys, + true, // in create_identity context + platform_version, + )?; + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + identity_create_from_addresses_transition.set_public_keys(public_keys); + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. See the LOCKSTEP note above. + let pre_validation_result = identity_create_from_addresses_transition + .validate_structure_without_input_witnesses(platform_version); + if let Some(error) = consensus_errors_as_protocol_error(pre_validation_result) { + return Err(error); + } + // Get signable bytes for the state transition let state_transition: StateTransition = identity_create_from_addresses_transition.clone().into(); @@ -88,17 +137,51 @@ impl IdentityCreateFromAddressesTransitionMethodsV0 for IdentityCreateFromAddres } } + // Verify proof-of-possession signatures we just produced before + // returning, matching the server-side + // `IdentityCreateFromAddressesStateTransitionSignaturesValidationV0` + // check. Only keys with unique types were signed above, so verify + // those exact keys here. + for public_key_with_witness in identity_create_from_addresses_transition.public_keys.iter() + { + if !public_key_with_witness.key_type().is_unique_key_type() { + continue; + } + let pop_result = signable_bytes.as_slice().verify_signature( + public_key_with_witness.key_type(), + public_key_with_witness.data().as_slice(), + public_key_with_witness.signature().as_slice(), + ); + if let Some(error) = consensus_errors_as_protocol_error(pop_result) { + return Err(error); + } + } + // Create witnesses for each input address - let mut input_witnesses = Vec::with_capacity(inputs.len()); - for address in inputs.keys() { + let mut input_witnesses = + Vec::with_capacity(identity_create_from_addresses_transition.inputs.len()); + for address in identity_create_from_addresses_transition.inputs.keys() { input_witnesses.push( address_signer .sign_create_witness(address, &signable_bytes) .await?, ); } + verify_address_witnesses( + identity_create_from_addresses_transition.inputs.keys(), + &input_witnesses, + &signable_bytes, + )?; identity_create_from_addresses_transition.input_witnesses = input_witnesses; + // After signing, only the witness count needs (re-)validation; the rest + // of the structure was already verified above. + let validation_result = + identity_create_from_addresses_transition.validate_input_witnesses_count(); + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + Ok(identity_create_from_addresses_transition.into()) } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/mod.rs index 5108230d276..d360a14a3b8 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/mod.rs @@ -373,4 +373,413 @@ mod test { .expect("try_from_identity"); assert!(t.public_keys.is_empty()); } + + /// Verifies that `try_from_identity_with_signer` rejects an identity that + /// contains an invalid TRANSFER+HIGH public key via the structural + /// public-key validation, returning + /// `ProtocolError::ConsensusError(InvalidIdentityPublicKeySecurityLevelError)` + /// before any signing or BLS work is attempted. + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_identity_with_signer_rejects_transfer_high_key() { + use crate::address_funds::AddressWitness; + use crate::consensus::basic::BasicError; + use crate::consensus::ConsensusError; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel}; + use crate::state_transition::identity_create_transition::methods::IdentityCreateTransitionMethodsV0; + use crate::version::PlatformVersion; + use crate::{BlsModule, ProtocolError, PublicKeyValidationError}; + use async_trait::async_trait; + use std::collections::BTreeMap; + + /// A signer that should never be invoked: pre-signing validation must + /// fail before any signing is attempted. + #[derive(Debug)] + struct UnreachableSigner; + + #[async_trait] + impl Signer for UnreachableSigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableSigner::sign must not be called when pre-signing \ + validation rejects the transition" + ); + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableSigner::sign_create_witness must not be called \ + when pre-signing validation rejects the transition" + ); + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + /// A BLS module that should never be invoked. + struct UnreachableBls; + + impl BlsModule for UnreachableBls { + fn validate_public_key(&self, _pk: &[u8]) -> Result<(), PublicKeyValidationError> { + panic!("UnreachableBls::validate_public_key must not be called"); + } + + fn verify_signature( + &self, + _signature: &[u8], + _data: &[u8], + _public_key: &[u8], + ) -> Result { + panic!("UnreachableBls::verify_signature must not be called"); + } + + fn private_key_to_public_key( + &self, + _private_key: &[u8], + ) -> Result, ProtocolError> { + panic!("UnreachableBls::private_key_to_public_key must not be called"); + } + + fn sign(&self, _data: &[u8], _private_key: &[u8]) -> Result, ProtocolError> { + panic!("UnreachableBls::sign must not be called"); + } + } + + let platform_version = PlatformVersion::latest(); + + // Required master key so that the identity satisfies the master-key + // presence requirement; the failure must come specifically from the + // invalid TRANSFER+HIGH key below. + let master_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![0u8; 33]), + disabled_at: None, + } + .into(); + + // Invalid combination: TRANSFER purpose only allows CRITICAL security level. + let invalid_transfer_high_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 1, + purpose: Purpose::TRANSFER, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![1u8; 33]), + disabled_at: None, + } + .into(); + + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::from([(0, master_key), (1, invalid_transfer_high_key)]), + balance: 0, + revision: 0, + } + .into(); + + let result = IdentityCreateTransitionV0::try_from_identity_with_signer_and_private_key( + &identity, + chain_proof(), + &[0u8; 32], + &UnreachableSigner, + &UnreachableBls, + 0, + platform_version, + ) + .await; + + match result { + Err(ProtocolError::ConsensusError(boxed)) => match *boxed { + ConsensusError::BasicError( + BasicError::InvalidIdentityPublicKeySecurityLevelError(err), + ) => { + assert_eq!(err.purpose(), Purpose::TRANSFER); + assert_eq!(err.security_level(), SecurityLevel::HIGH); + } + other => panic!( + "expected InvalidIdentityPublicKeySecurityLevelError, got {:?}", + other + ), + }, + other => panic!( + "expected ConsensusError(InvalidIdentityPublicKeySecurityLevelError), got {:?}", + other + ), + } + } + + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_identity_with_signer_rejects_wrong_instant_asset_lock_private_key_locally() { + use crate::address_funds::AddressWitness; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel}; + use crate::state_transition::identity_create_transition::methods::IdentityCreateTransitionMethodsV0; + use crate::tests::fixtures::instant_asset_lock_proof_fixture; + use crate::version::PlatformVersion; + use crate::{BlsModule, ProtocolError, PublicKeyValidationError}; + use async_trait::async_trait; + use dashcore::secp256k1::SecretKey; + use dashcore::{Network, PrivateKey}; + use std::collections::BTreeMap; + use std::str::FromStr; + + #[derive(Debug)] + struct UnreachableSigner; + + #[async_trait] + impl Signer for UnreachableSigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("UnreachableSigner::sign must not be called"); + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("UnreachableSigner::sign_create_witness must not be called"); + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + struct UnreachableBls; + + impl BlsModule for UnreachableBls { + fn validate_public_key(&self, _pk: &[u8]) -> Result<(), PublicKeyValidationError> { + panic!("UnreachableBls::validate_public_key must not be called"); + } + + fn verify_signature( + &self, + _signature: &[u8], + _data: &[u8], + _public_key: &[u8], + ) -> Result { + panic!("UnreachableBls::verify_signature must not be called"); + } + + fn private_key_to_public_key( + &self, + _private_key: &[u8], + ) -> Result, ProtocolError> { + panic!("UnreachableBls::private_key_to_public_key must not be called"); + } + + fn sign(&self, _data: &[u8], _private_key: &[u8]) -> Result, ProtocolError> { + panic!("UnreachableBls::sign must not be called"); + } + } + + let correct_private_key = + PrivateKey::from_str("cSBnVM4xvxarwGQuAfQFwqDg9k5tErHUHzgWsEfD4zdwUasvqRVY") + .expect("fixture private key"); + let wrong_private_key = PrivateKey::new( + SecretKey::from_slice(&[2u8; 32]).expect("valid alternate private key"), + Network::Testnet, + ); + + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::from([( + 0, + IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_HASH160, + read_only: false, + data: BinaryData::new(vec![0u8; 20]), + disabled_at: None, + } + .into(), + )]), + balance: 0, + revision: 0, + } + .into(); + + let result = IdentityCreateTransitionV0::try_from_identity_with_signer_and_private_key( + &identity, + instant_asset_lock_proof_fixture(Some(correct_private_key), None), + &wrong_private_key.inner.secret_bytes(), + &UnreachableSigner, + &UnreachableBls, + 0, + PlatformVersion::latest(), + ) + .await; + + assert!( + matches!(result, Err(ProtocolError::Generic(ref message)) if message.contains("does not match the locked output")), + "unexpected result: {:?}", + result + ); + } + + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_identity_with_signer_rejects_bad_identity_public_key_signature_locally() { + use crate::address_funds::AddressWitness; + use crate::consensus::signature::SignatureError; + use crate::consensus::ConsensusError; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel}; + use crate::state_transition::identity_create_transition::methods::IdentityCreateTransitionMethodsV0; + use crate::version::PlatformVersion; + use crate::{BlsModule, ProtocolError, PublicKeyValidationError}; + use async_trait::async_trait; + use dashcore::secp256k1::{PublicKey as RawPublicKey, Secp256k1, SecretKey}; + use std::collections::BTreeMap; + + #[derive(Debug)] + struct WrongIdentityKeySigner { + wrong_secret_key: SecretKey, + } + + #[async_trait] + impl Signer for WrongIdentityKeySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + data: &[u8], + ) -> Result { + Ok(BinaryData::new( + dashcore::signer::sign(data, &self.wrong_secret_key.secret_bytes()) + .expect("wrong-key signing should succeed") + .to_vec(), + )) + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("identity public key signer should not create address witnesses") + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + true + } + } + + struct UnreachableBls; + + impl BlsModule for UnreachableBls { + fn validate_public_key(&self, _pk: &[u8]) -> Result<(), PublicKeyValidationError> { + panic!("UnreachableBls::validate_public_key must not be called"); + } + + fn verify_signature( + &self, + _signature: &[u8], + _data: &[u8], + _public_key: &[u8], + ) -> Result { + panic!("UnreachableBls::verify_signature must not be called"); + } + + fn private_key_to_public_key( + &self, + _private_key: &[u8], + ) -> Result, ProtocolError> { + panic!("UnreachableBls::private_key_to_public_key must not be called"); + } + + fn sign(&self, _data: &[u8], _private_key: &[u8]) -> Result, ProtocolError> { + panic!("UnreachableBls::sign must not be called"); + } + } + + let secp = Secp256k1::new(); + let correct_secret_key = + SecretKey::from_slice(&[1u8; 32]).expect("valid identity secret key"); + let wrong_secret_key = + SecretKey::from_slice(&[2u8; 32]).expect("valid alternate secret key"); + let correct_public_key = RawPublicKey::from_secret_key(&secp, &correct_secret_key); + + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::from([( + 0, + IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(correct_public_key.serialize().to_vec()), + disabled_at: None, + } + .into(), + )]), + balance: 0, + revision: 0, + } + .into(); + + let result = IdentityCreateTransitionV0::try_from_identity_with_signer_and_private_key( + &identity, + chain_proof(), + &[3u8; 32], + &WrongIdentityKeySigner { wrong_secret_key }, + &UnreachableBls, + 0, + PlatformVersion::latest(), + ) + .await; + + match result { + Err(ProtocolError::ConsensusError(boxed)) => match *boxed { + ConsensusError::SignatureError(SignatureError::BasicECDSAError(_)) => {} + other => panic!("expected SignatureError(BasicECDSAError), got {:?}", other), + }, + Err(ProtocolError::ConsensusErrors(errors)) => { + assert_eq!(errors.len(), 1, "expected a single consensus error"); + assert!(matches!( + errors.as_slice(), + [ConsensusError::SignatureError( + SignatureError::BasicECDSAError(_) + )] + )); + } + other => panic!( + "expected ConsensusError/ConsensusErrors with BasicECDSAError, got {:?}", + other + ), + } + } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/v0_methods.rs index 794986570b5..edcb9b5b307 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_create_transition/v0/v0_methods.rs @@ -9,6 +9,8 @@ use crate::identity::identity_public_key::accessors::v0::IdentityPublicKeyGetter #[cfg(feature = "state-transition-signing")] use crate::identity::signer::Signer; #[cfg(feature = "state-transition-signing")] +use crate::identity::state_transition::AssetLockProved; +#[cfg(feature = "state-transition-signing")] use crate::identity::Identity; #[cfg(feature = "state-transition-signing")] use crate::identity::KeyType::ECDSA_HASH160; @@ -17,20 +19,39 @@ use crate::prelude::AssetLockProof; #[cfg(feature = "state-transition-signing")] use crate::prelude::UserFeeIncrease; #[cfg(feature = "state-transition-signing")] +use crate::serialization::PlatformMessageSignable; +#[cfg(feature = "state-transition-signing")] use crate::serialization::Signable; use crate::state_transition::identity_create_transition::accessors::IdentityCreateTransitionAccessorsV0; use crate::state_transition::identity_create_transition::methods::IdentityCreateTransitionMethodsV0; #[cfg(feature = "state-transition-signing")] +use crate::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; +#[cfg(feature = "state-transition-signing")] use crate::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Setters; +#[cfg(feature = "state-transition-signing")] +use crate::util::hash::ripemd160_sha256; #[cfg(feature = "state-transition-signing")] use crate::identity::IdentityPublicKey; use crate::state_transition::identity_create_transition::v0::IdentityCreateTransitionV0; use crate::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; #[cfg(feature = "state-transition-signing")] -use crate::state_transition::StateTransition; +use crate::state_transition::{consensus_errors_as_protocol_error, StateTransition}; #[cfg(feature = "state-transition-signing")] use crate::version::PlatformVersion; +#[cfg(feature = "state-transition-signing")] +use dashcore::secp256k1::{Secp256k1, SecretKey}; +#[cfg(feature = "state-transition-signing")] +use dashcore::ScriptBuf; + +#[cfg(feature = "state-transition-signing")] +fn p2pkh_pubkey_hash(script: &ScriptBuf) -> Option<[u8; 20]> { + script + .is_p2pkh() + .then(|| script.as_bytes().get(3..23)?.try_into().ok()) + .flatten() +} + impl IdentityCreateTransitionMethodsV0 for IdentityCreateTransitionV0 { #[cfg(feature = "state-transition-signing")] async fn try_from_identity_with_signer_and_private_key>( @@ -40,15 +61,34 @@ impl IdentityCreateTransitionMethodsV0 for IdentityCreateTransitionV0 { signer: &S, bls: &impl BlsModule, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, ) -> Result { - let public_keys = identity + let public_keys: Vec = identity .public_keys() .values() .map(|public_key| public_key.clone().into()) .collect(); let identity_id = asset_lock_proof.create_identifier()?; + // Validate public key structure (purpose/security level compatibility) + // before broadcasting, so invalid combinations are caught client-side + // rather than being rejected by the network. + // + // LOCKSTEP: this call is hard-coded to the v0 public-keys-structure + // check. If a future v1 basic-structure is introduced for this + // transition, both the drive-abci server dispatcher AND this SDK + // constructor must be updated together (e.g. by routing through a + // versioned `validate_basic_structure` wrapper as IdentityUpdate does). + let validation_result = + IdentityPublicKeyInCreation::validate_identity_public_keys_structure( + &public_keys, + true, // in create_identity context + platform_version, + )?; + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + let mut identity_create_transition = IdentityCreateTransitionV0 { public_keys, asset_lock_proof, @@ -57,6 +97,16 @@ impl IdentityCreateTransitionMethodsV0 for IdentityCreateTransitionV0 { ..Default::default() }; + // Validate the asset lock proof structure client-side before signing + // so malformed proofs are caught locally rather than being rejected by + // the network during basic-structure validation. + let asset_lock_validation_result = identity_create_transition + .asset_lock_proof() + .validate_structure(platform_version)?; + if let Some(error) = consensus_errors_as_protocol_error(asset_lock_validation_result) { + return Err(error); + } + //todo: remove clone let state_transition: StateTransition = identity_create_transition.clone().into(); @@ -73,8 +123,65 @@ impl IdentityCreateTransitionMethodsV0 for IdentityCreateTransitionV0 { } } - let mut state_transition: StateTransition = identity_create_transition.into(); + // Verify the proof-of-possession signatures we just produced before + // returning, mirroring the server-side identity_create signatures + // validator. Only keys with unique types were signed above, so verify + // those exact keys here. + for public_key_with_witness in identity_create_transition.public_keys.iter() { + if !public_key_with_witness.key_type().is_unique_key_type() { + continue; + } + let pop_result = key_signable_bytes.as_slice().verify_signature( + public_key_with_witness.key_type(), + public_key_with_witness.data().as_slice(), + public_key_with_witness.signature().as_slice(), + ); + if let Some(error) = consensus_errors_as_protocol_error(pop_result) { + return Err(error); + } + } + + if let Some(transaction) = identity_create_transition.asset_lock_proof().transaction() { + let output_index = + identity_create_transition.asset_lock_proof().output_index() as usize; + let output = transaction + .special_transaction_payload + .as_ref() + .and_then(|payload| match payload { + dashcore::transaction::special_transaction::TransactionPayload::AssetLockPayloadType(payload) => { + payload.credit_outputs.get(output_index) + } + _ => None, + }) + .ok_or_else(|| { + ProtocolError::Generic(format!( + "asset lock proof output {output_index} is not available for local signature verification" + )) + })?; + + if let Some(locked_pubkey_hash) = p2pkh_pubkey_hash(&output.script_pubkey) { + let secret_key = + SecretKey::from_slice(asset_lock_proof_private_key).map_err(|e| { + ProtocolError::Generic(format!("invalid asset lock proof private key: {e}")) + })?; + let secp = Secp256k1::new(); + let public_key = + dashcore::secp256k1::PublicKey::from_secret_key(&secp, &secret_key); + let compressed_pubkey_hash = ripemd160_sha256(&public_key.serialize()); + let uncompressed_pubkey_hash = + ripemd160_sha256(&public_key.serialize_uncompressed()); + if locked_pubkey_hash != compressed_pubkey_hash + && locked_pubkey_hash != uncompressed_pubkey_hash + { + return Err(ProtocolError::Generic( + "asset lock proof private key does not match the locked output".to_string(), + )); + } + } + } + + let mut state_transition: StateTransition = identity_create_transition.into(); state_transition.sign_by_private_key(asset_lock_proof_private_key, ECDSA_HASH160, bls)?; Ok(state_transition) @@ -101,11 +208,25 @@ impl IdentityCreateTransitionMethodsV0 for IdentityCreateTransitionV0 { IS: Signer, AS: ::key_wallet::signer::Signer, { - let public_keys = identity + let public_keys: Vec = identity .public_keys() .values() .map(|public_key| public_key.clone().into()) .collect(); + + // Validate public key structure (purpose/security level compatibility) + // before broadcasting, so invalid combinations are caught client-side + // rather than being rejected by the network. + let validation_result = + IdentityPublicKeyInCreation::validate_identity_public_keys_structure( + &public_keys, + true, // in create_identity context + _platform_version, + )?; + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + let identity_id = asset_lock_proof.create_identifier()?; let mut identity_create_transition = IdentityCreateTransitionV0 { diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/mod.rs index b1f611057ff..a17b18d1e40 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/mod.rs @@ -61,6 +61,19 @@ pub struct IdentityCreditTransferToAddressesTransitionV0 { #[cfg(test)] mod test { + use std::collections::BTreeMap; + + use crate::address_funds::PlatformAddress; + use crate::consensus::basic::BasicError; + use crate::consensus::ConsensusError; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey}; + use crate::state_transition::identity_credit_transfer_to_addresses_transition::methods::IdentityCreditTransferToAddressesTransitionMethodsV0; + use crate::ProtocolError; + use async_trait::async_trait; + use platform_value::BinaryData; + use platform_version::version::PlatformVersion; use crate::serialization::{PlatformDeserializable, PlatformSerializable}; @@ -179,4 +192,75 @@ mod test { StateTransition::IdentityCreditTransferToAddresses(_) )); } + + #[derive(Debug)] + struct UnreachableIdentitySigner; + + #[async_trait] + impl Signer for UnreachableIdentitySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("sign should not run when protocol gating rejects the constructor") + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "sign_create_witness should not run when protocol gating rejects the constructor" + ) + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + #[tokio::test] + async fn try_from_identity_returns_not_active_before_structure_validation() { + let mut low_version = PlatformVersion::get(1) + .expect("platform version 1 exists") + .clone(); + low_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_transfer_to_addresses_state_transition + .basic_structure = None; + let identity: Identity = IdentityV0 { + id: Identifier::random(), + public_keys: BTreeMap::::new(), + balance: 0, + revision: 0, + } + .into(); + let mut recipient_addresses = BTreeMap::new(); + recipient_addresses.insert(PlatformAddress::P2pkh([1u8; 20]), 1); + + let result = IdentityCreditTransferToAddressesTransitionV0::try_from_identity( + &identity, + recipient_addresses, + 0, + &UnreachableIdentitySigner, + None, + 1, + &low_version, + None, + ) + .await; + + assert!(matches!( + result, + Err(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::BasicError(BasicError::StateTransitionNotActiveError(_)) + ) + )); + } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/v0_methods.rs index bdb1697bf65..1bf588233c0 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_to_addresses_transition/v0/v0_methods.rs @@ -6,6 +6,11 @@ use crate::address_funds::PlatformAddress; #[cfg(feature = "state-transition-signing")] use crate::fee::Credits; #[cfg(feature = "state-transition-signing")] +use crate::state_transition::{ + address_funds_constructor_dispatch_error, consensus_errors_as_protocol_error, + StateTransitionStructureValidation, +}; +#[cfg(feature = "state-transition-signing")] use crate::{ identity::{ accessors::IdentityGettersV0, @@ -22,6 +27,8 @@ use crate::state_transition::identity_credit_transfer_to_addresses_transition::v #[cfg(feature = "state-transition-signing")] use crate::state_transition::GetDataContractSecurityLevelRequirementFn; #[cfg(feature = "state-transition-signing")] +use crate::state_transition::StateTransitionType; +#[cfg(feature = "state-transition-signing")] use platform_version::version::{FeatureVersion, PlatformVersion}; impl IdentityCreditTransferToAddressesTransitionMethodsV0 @@ -35,22 +42,43 @@ impl IdentityCreditTransferToAddressesTransitionMethodsV0 signer: &S, signing_withdrawal_key_to_use: Option<&IdentityPublicKey>, nonce: IdentityNonce, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, _version: Option, ) -> Result { tracing::debug!("try_from_identity: Started"); tracing::debug!(identity_id = %identity.id(), "try_from_identity"); tracing::debug!(recipient_addresses = ?to_recipient_addresses, has_signing_key = signing_withdrawal_key_to_use.is_some(), "try_from_identity inputs"); - let mut transition: StateTransition = IdentityCreditTransferToAddressesTransitionV0 { + let transition_v0 = IdentityCreditTransferToAddressesTransitionV0 { identity_id: identity.id(), recipient_addresses: to_recipient_addresses, nonce, user_fee_increase, signature_public_key_id: 0, signature: Default::default(), + }; + + if let Some(error) = address_funds_constructor_dispatch_error( + StateTransitionType::IdentityCreditTransferToAddresses, + platform_version, + ) { + return Err(error); + } + + // Validate structure before .into() conversion and signing, since this transition + // uses sign_external on the StateTransition rather than setting witnesses on the V0 struct. + // + // LOCKSTEP: this call resolves to the v0 structure check. If a future + // v1 basic-structure is introduced for this transition, both the + // drive-abci server dispatcher AND this SDK constructor must be + // updated together (e.g. by routing through a versioned + // `validate_basic_structure` wrapper as IdentityUpdate does). + let validation_result = transition_v0.validate_structure(platform_version); + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); } - .into(); + + let mut transition: StateTransition = transition_v0.into(); let identity_public_key = match signing_withdrawal_key_to_use { Some(key) => { diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/mod.rs index 9f15a4999fb..008d1d82a94 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/mod.rs @@ -33,6 +33,14 @@ use serde::{Deserialize, Serialize}; pub type IdentityCreditTransferTransitionLatest = IdentityCreditTransferTransitionV0; +/// Minimum credit-transfer amount enforced both client-side by the SDK +/// constructor and server-side by drive-abci's +/// `IdentityCreditTransferStateTransitionStructureValidationV0`. +/// +/// Defined once here so both sides agree on the value and any future change is +/// a single edit. +pub const MIN_TRANSFER_AMOUNT: u64 = 100_000; + #[cfg_attr( all(feature = "json-conversion", feature = "serde-conversion"), derive(JsonConvertible) @@ -83,6 +91,25 @@ impl IdentityCreditTransferTransition { } } +#[cfg(any( + feature = "state-transition-validation", + feature = "state-transition-signing" +))] +impl IdentityCreditTransferTransition { + /// Shared single source of truth for the v0 basic-structure rules of an + /// identity credit transfer. Delegates to + /// `IdentityCreditTransferTransitionV0::validate_basic_structure_v0` so + /// the client-side SDK constructor and drive-abci's structure validator + /// enforce the exact same self-transfer and `MIN_TRANSFER_AMOUNT` checks. + pub fn validate_basic_structure_v0( + &self, + ) -> crate::validation::SimpleConsensusValidationResult { + match self { + IdentityCreditTransferTransition::V0(v0) => v0.validate_basic_structure_v0(), + } + } +} + impl OptionallyAssetLockProved for IdentityCreditTransferTransition {} impl StateTransitionFieldTypes for IdentityCreditTransferTransition { diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/v0/v0_methods.rs index 7f8926efe70..fae31a82eb2 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_transfer_transition/v0/v0_methods.rs @@ -1,3 +1,15 @@ +#[cfg(any( + feature = "state-transition-validation", + feature = "state-transition-signing" +))] +use crate::consensus::basic::identity::{ + IdentityCreditTransferToSelfError, InvalidIdentityCreditTransferAmountError, +}; +#[cfg(any( + feature = "state-transition-validation", + feature = "state-transition-signing" +))] +use crate::validation::SimpleConsensusValidationResult; #[cfg(feature = "state-transition-signing")] use crate::{ identity::{ @@ -14,11 +26,47 @@ use platform_value::Identifier; use crate::state_transition::identity_credit_transfer_transition::methods::IdentityCreditTransferTransitionMethodsV0; use crate::state_transition::identity_credit_transfer_transition::v0::IdentityCreditTransferTransitionV0; +#[cfg(any( + feature = "state-transition-validation", + feature = "state-transition-signing" +))] +use crate::state_transition::identity_credit_transfer_transition::MIN_TRANSFER_AMOUNT; #[cfg(feature = "state-transition-signing")] -use crate::state_transition::GetDataContractSecurityLevelRequirementFn; +use crate::state_transition::{ + consensus_errors_as_protocol_error, GetDataContractSecurityLevelRequirementFn, +}; #[cfg(feature = "state-transition-signing")] use platform_version::version::{FeatureVersion, PlatformVersion}; +#[cfg(any( + feature = "state-transition-validation", + feature = "state-transition-signing" +))] +impl IdentityCreditTransferTransitionV0 { + /// Shared single source of truth for the v0 basic-structure rules of an + /// identity credit transfer: rejects self-transfers and amounts below + /// `MIN_TRANSFER_AMOUNT`. Used by both the client-side SDK constructor + /// (to fail fast before any signing work) and drive-abci's structure + /// validator (to enforce the same rules server-side), so the two cannot + /// drift. + pub fn validate_basic_structure_v0(&self) -> SimpleConsensusValidationResult { + if self.identity_id == self.recipient_id { + return SimpleConsensusValidationResult::new_with_error( + IdentityCreditTransferToSelfError::default().into(), + ); + } + + if self.amount < MIN_TRANSFER_AMOUNT { + return SimpleConsensusValidationResult::new_with_error( + InvalidIdentityCreditTransferAmountError::new(self.amount, MIN_TRANSFER_AMOUNT) + .into(), + ); + } + + SimpleConsensusValidationResult::new() + } +} + impl IdentityCreditTransferTransitionMethodsV0 for IdentityCreditTransferTransitionV0 { #[cfg(feature = "state-transition-signing")] async fn try_from_identity>( @@ -32,7 +80,7 @@ impl IdentityCreditTransferTransitionMethodsV0 for IdentityCreditTransferTransit _platform_version: &PlatformVersion, _version: Option, ) -> Result { - let mut transition: StateTransition = IdentityCreditTransferTransitionV0 { + let transition_v0 = IdentityCreditTransferTransitionV0 { identity_id: identity.id(), recipient_id: to_identity_with_identifier, amount, @@ -40,8 +88,23 @@ impl IdentityCreditTransferTransitionMethodsV0 for IdentityCreditTransferTransit user_fee_increase, signature_public_key_id: 0, signature: Default::default(), + }; + + // Pre-signing structure check that mirrors drive-abci's + // `IdentityCreditTransferStateTransitionStructureValidationV0`. This + // catches self-transfers and below-minimum amounts before any async + // signer work is performed. + // + // LOCKSTEP: hard-coded to the v0 basic-structure check. If a future v1 + // basic-structure is introduced for this transition, both the + // drive-abci server dispatcher AND this SDK constructor must be + // updated together. + let pre_validation_result = transition_v0.validate_basic_structure_v0(); + if let Some(error) = consensus_errors_as_protocol_error(pre_validation_result) { + return Err(error); } - .into(); + + let mut transition: StateTransition = transition_v0.into(); let identity_public_key = match signing_withdrawal_key_to_use { Some(key) => { diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs index 48f9e1915b1..8d1818f348a 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/mod.rs @@ -115,6 +115,103 @@ impl IdentityCreditWithdrawalTransition { } } +#[cfg(feature = "state-transition-validation")] +fn basic_structure_rules_v1_for_transition( + amount: u64, + pooling: crate::withdrawal::Pooling, + core_fee_per_byte: u32, + output_script: Option<&crate::identity::core_script::CoreScript>, + platform_version: &PlatformVersion, +) -> crate::validation::SimpleConsensusValidationResult { + use crate::consensus::basic::identity::{ + InvalidCreditWithdrawalTransitionCoreFeeError, + InvalidCreditWithdrawalTransitionOutputScriptError, + InvalidIdentityCreditWithdrawalTransitionAmountError, + NotImplementedCreditWithdrawalTransitionPoolingError, + }; + use crate::util::is_non_zero_fibonacci_number::is_non_zero_fibonacci_number; + use crate::validation::SimpleConsensusValidationResult; + use crate::withdrawal::Pooling; + + let mut result = SimpleConsensusValidationResult::default(); + let min_withdrawal_amount = platform_version.system_limits.min_withdrawal_amount; + + if amount < min_withdrawal_amount + || amount > platform_version.system_limits.max_withdrawal_amount + { + result.add_error(InvalidIdentityCreditWithdrawalTransitionAmountError::new( + amount, + min_withdrawal_amount, + platform_version.system_limits.max_withdrawal_amount, + )); + } + + if pooling != Pooling::Never { + result.add_error(NotImplementedCreditWithdrawalTransitionPoolingError::new( + pooling as u8, + )); + return result; + } + + if !is_non_zero_fibonacci_number(core_fee_per_byte as u64) { + result.add_error(InvalidCreditWithdrawalTransitionCoreFeeError::new( + core_fee_per_byte, + MIN_CORE_FEE_PER_BYTE, + )); + return result; + } + + if let Some(output_script) = output_script { + if !output_script.is_p2pkh() && !output_script.is_p2sh() { + result.add_error(InvalidCreditWithdrawalTransitionOutputScriptError::new( + output_script.clone(), + )); + } + } + + result +} + +#[cfg(feature = "state-transition-validation")] +impl IdentityCreditWithdrawalTransition { + /// Shared v1 basic-structure check. + /// + /// This is the single source of truth for v1 basic-structure validation: + /// the drive-abci server dispatcher + /// (`IdentityCreditWithdrawalStateTransitionStructureValidationV1`) and + /// the SDK constructor for [`v1::IdentityCreditWithdrawalTransitionV1`] + /// both delegate to it so the client cannot drift from the server. + /// + /// Operates on the enum via generic accessors, matching the prior + /// drive-abci impl semantics. When a future v2 basic-structure rule is + /// introduced, add a sibling `basic_structure_rules_v2` here and update + /// both dispatchers in lockstep. + /// + /// Named `basic_structure_rules_v1` (rather than the symmetric + /// `validate_basic_structure_v1`) to avoid colliding with drive-abci's + /// trait method of that name; drive-abci's trait wrapper delegates to + /// this rule function. + pub fn basic_structure_rules_v1( + &self, + platform_version: &PlatformVersion, + ) -> crate::validation::SimpleConsensusValidationResult { + match self { + IdentityCreditWithdrawalTransition::V0(transition) => { + basic_structure_rules_v1_for_transition( + transition.amount, + transition.pooling, + transition.core_fee_per_byte, + Some(&transition.output_script), + platform_version, + ) + } + IdentityCreditWithdrawalTransition::V1(transition) => { + transition.basic_structure_rules_v1(platform_version) + } + } + } +} + impl StateTransitionFieldTypes for IdentityCreditWithdrawalTransition { fn signature_property_paths() -> Vec<&'static str> { vec![SIGNATURE, SIGNATURE_PUBLIC_KEY_ID] @@ -134,6 +231,8 @@ impl OptionallyAssetLockProved for IdentityCreditWithdrawalTransition {} #[cfg(test)] mod test { use super::*; + use crate::consensus::basic::BasicError; + use crate::consensus::ConsensusError; use crate::identity::core_script::CoreScript; use crate::serialization::{PlatformDeserializable, PlatformSerializable}; use crate::state_transition::identity_credit_withdrawal_transition::accessors::IdentityCreditWithdrawalTransitionAccessorsV0; @@ -362,6 +461,31 @@ mod test { assert!(!result.is_valid()); } + #[test] + fn basic_structure_uses_versioned_min_withdrawal_amount_v12() { + let platform_version = PlatformVersion::get(12).expect("v12 exists"); + let mut t = make_withdrawal_v1(); + t.set_amount(999_999); + t.set_pooling(Pooling::Never); + + let result = t.basic_structure_rules_v1(platform_version); + + assert_eq!( + platform_version.system_limits.min_withdrawal_amount, + 1_000_000 + ); + assert_eq!(result.errors.len(), 1); + match &result.errors[0] { + ConsensusError::BasicError( + BasicError::InvalidIdentityCreditWithdrawalTransitionAmountError(error), + ) => { + assert_eq!(error.amount(), 999_999); + assert_eq!(error.min_amount(), 1_000_000); + } + error => panic!("unexpected error: {error:?}"), + } + } + #[test] #[allow(clippy::assertions_on_constants)] fn test_min_withdrawal_amount_constant() { diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/mod.rs index 7364334be17..315657c0fe3 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/mod.rs @@ -23,6 +23,8 @@ use crate::{ withdrawal::Pooling, ProtocolError, }; +#[cfg(feature = "state-transition-validation")] +use platform_version::version::PlatformVersion; #[cfg_attr(feature = "json-conversion", json_safe_fields)] #[derive(Debug, Clone, Encode, Decode, PlatformSignable, PartialEq)] @@ -51,6 +53,22 @@ pub struct IdentityCreditWithdrawalTransitionV1 { pub signature: BinaryData, } +#[cfg(feature = "state-transition-validation")] +impl IdentityCreditWithdrawalTransitionV1 { + pub fn basic_structure_rules_v1( + &self, + platform_version: &PlatformVersion, + ) -> crate::validation::SimpleConsensusValidationResult { + super::basic_structure_rules_v1_for_transition( + self.amount, + self.pooling, + self.core_fee_per_byte, + self.output_script.as_ref(), + platform_version, + ) + } +} + #[cfg(test)] mod test { use super::*; diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/v0_methods.rs index 6a8e5744094..22424bb3ac8 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_credit_withdrawal_transition/v1/v0_methods.rs @@ -1,5 +1,9 @@ #[cfg(feature = "state-transition-signing")] +use crate::state_transition::consensus_errors_as_protocol_error; +#[cfg(feature = "state-transition-signing")] use crate::{ + consensus::basic::state_transition::StateTransitionNotActiveError, + consensus::ConsensusError, identity::{ accessors::IdentityGettersV0, core_script::CoreScript, signer::Signer, Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel, @@ -18,6 +22,57 @@ use platform_version::version::{FeatureVersion, PlatformVersion}; use crate::state_transition::identity_credit_withdrawal_transition::methods::IdentityCreditWithdrawalTransitionMethodsV0; use crate::state_transition::identity_credit_withdrawal_transition::v1::IdentityCreditWithdrawalTransitionV1; +#[cfg(feature = "state-transition-signing")] +use crate::state_transition::StateTransitionType; + +#[cfg(feature = "state-transition-signing")] +fn validate_basic_structure_dispatch( + transition: &IdentityCreditWithdrawalTransitionV1, + platform_version: &PlatformVersion, +) -> Result { + match platform_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_withdrawal_state_transition + .basic_structure + { + Some(1) => Ok(transition.basic_structure_rules_v1(platform_version)), + None => { + let first_active_version = platform_version::version::PLATFORM_VERSIONS + .iter() + .find(|version| { + version + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_withdrawal_state_transition + .basic_structure + == Some(1) + }) + .map(|version| version.protocol_version) + .unwrap_or(platform_version.protocol_version); + + Err(ProtocolError::from(ConsensusError::from( + StateTransitionNotActiveError::new( + StateTransitionType::IdentityCreditWithdrawal.to_string(), + platform_version.protocol_version, + first_active_version, + ), + ))) + } + Some(0) => Err(ProtocolError::UnknownVersionMismatch { + method: "IdentityCreditWithdrawalTransitionV1::validate_basic_structure".to_string(), + known_versions: vec![1], + received: 0, + }), + Some(version) => Err(ProtocolError::UnknownVersionMismatch { + method: "IdentityCreditWithdrawalTransitionV1::validate_basic_structure".to_string(), + known_versions: vec![1], + received: version, + }), + } +} impl IdentityCreditWithdrawalTransitionMethodsV0 for IdentityCreditWithdrawalTransitionV1 { #[cfg(feature = "state-transition-signing")] @@ -32,10 +87,10 @@ impl IdentityCreditWithdrawalTransitionMethodsV0 for IdentityCreditWithdrawalTra signing_withdrawal_key_to_use: Option<&IdentityPublicKey>, preferred_key_purpose_for_signing_withdrawal: PreferredKeyPurposeForSigningWithdrawal, nonce: IdentityNonce, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, _version: Option, ) -> Result { - let mut transition: StateTransition = IdentityCreditWithdrawalTransitionV1 { + let transition_v1 = IdentityCreditWithdrawalTransitionV1 { identity_id: identity.id(), amount, core_fee_per_byte, @@ -45,8 +100,24 @@ impl IdentityCreditWithdrawalTransitionMethodsV0 for IdentityCreditWithdrawalTra user_fee_increase, signature_public_key_id: 0, signature: Default::default(), + }; + + // Pre-signing structure check that delegates to the shared DPP-owned + // v1 basic-structure validation. The drive-abci server dispatcher and + // the enum wrapper route through the same rule function, so client and + // server cannot drift. + // + // LOCKSTEP: hard-coded to the v1 basic-structure check. If a future v2 + // basic-structure rule is introduced for this transition, the DPP + // method, the drive-abci dispatcher, AND this SDK constructor must be + // updated together. + let pre_validation_result = + validate_basic_structure_dispatch(&transition_v1, platform_version)?; + if let Some(error) = consensus_errors_as_protocol_error(pre_validation_result) { + return Err(error); } - .into(); + + let mut transition: StateTransition = transition_v1.into(); let identity_public_key = match signing_withdrawal_key_to_use { Some(key) => { diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/mod.rs index 9b81be9dc55..6c45457fd49 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/mod.rs @@ -52,7 +52,13 @@ mod tests { use crate::address_funds::AddressFundsFeeStrategyStep; use crate::consensus::basic::BasicError; use crate::consensus::ConsensusError; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey}; + use crate::state_transition::identity_topup_from_addresses_transition::methods::IdentityTopUpFromAddressesTransitionMethodsV0; use crate::state_transition::StateTransitionStructureValidation; + use async_trait::async_trait; + use platform_value::BinaryData; use platform_value::Identifier; use platform_version::version::PlatformVersion; @@ -354,4 +360,73 @@ mod tests { t.set_output(Some((PlatformAddress::P2pkh([9u8; 20]), 1))); assert!(t.output().is_some()); } + + #[derive(Debug)] + struct UnreachableAddressSigner; + + #[async_trait] + impl Signer for UnreachableAddressSigner { + async fn sign( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!("sign should not run when protocol gating rejects the constructor") + } + + async fn sign_create_witness( + &self, + _key: &PlatformAddress, + _data: &[u8], + ) -> Result { + panic!( + "sign_create_witness should not run when protocol gating rejects the constructor" + ) + } + + fn can_sign_with(&self, _key: &PlatformAddress) -> bool { + false + } + } + + #[tokio::test] + async fn try_from_inputs_with_signer_returns_not_active_before_structure_validation() { + let mut low_version = PlatformVersion::get(1) + .expect("platform version 1 exists") + .clone(); + low_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_top_up_from_addresses_state_transition + .basic_structure = None; + let identity: Identity = IdentityV0 { + id: Identifier::random(), + public_keys: BTreeMap::::new(), + balance: 0, + revision: 0, + } + .into(); + let mut inputs = BTreeMap::new(); + inputs.insert(PlatformAddress::P2pkh([1u8; 20]), (1u32, 1)); + + let result = IdentityTopUpFromAddressesTransitionV0::try_from_inputs_with_signer( + &identity, + inputs, + &UnreachableAddressSigner, + 0, + &low_version, + None, + ) + .await; + + assert!(matches!( + result, + Err(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::BasicError(BasicError::StateTransitionNotActiveError(_)) + ) + )); + } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/state_transition_validation.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/state_transition_validation.rs index 80b2b649ef5..57dfd8e71bc 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/state_transition_validation.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/state_transition_validation.rs @@ -13,8 +13,11 @@ use crate::validation::SimpleConsensusValidationResult; use platform_version::version::PlatformVersion; use std::collections::HashSet; -impl StateTransitionStructureValidation for IdentityTopUpFromAddressesTransitionV0 { - fn validate_structure( +impl IdentityTopUpFromAddressesTransitionV0 { + /// Validates all structural properties of the transition except for the + /// `input_witnesses` count. This is intended for client-side pre-signing + /// validation, where witnesses are not yet present. + pub fn validate_structure_without_input_witnesses( &self, platform_version: &PlatformVersion, ) -> SimpleConsensusValidationResult { @@ -36,17 +39,6 @@ impl StateTransitionStructureValidation for IdentityTopUpFromAddressesTransition ); } - // Validate input witnesses count matches inputs count - if self.inputs.len() != self.input_witnesses.len() { - return SimpleConsensusValidationResult::new_with_error( - BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( - self.inputs.len().min(u16::MAX as usize) as u16, - self.input_witnesses.len().min(u16::MAX as usize) as u16, - )) - .into(), - ); - } - // Validate output address is not also an input address if let Some((output_address, _)) = &self.output { if self.inputs.contains_key(output_address) { @@ -211,4 +203,33 @@ impl StateTransitionStructureValidation for IdentityTopUpFromAddressesTransition SimpleConsensusValidationResult::new() } + + /// Validates that the number of `input_witnesses` matches the number of + /// inputs. Intended to be invoked after signing, once witnesses have been + /// produced by the signer. + pub fn validate_input_witnesses_count(&self) -> SimpleConsensusValidationResult { + if self.inputs.len() != self.input_witnesses.len() { + return SimpleConsensusValidationResult::new_with_error( + BasicError::InputWitnessCountMismatchError(InputWitnessCountMismatchError::new( + self.inputs.len().min(u16::MAX as usize) as u16, + self.input_witnesses.len().min(u16::MAX as usize) as u16, + )) + .into(), + ); + } + SimpleConsensusValidationResult::new() + } +} + +impl StateTransitionStructureValidation for IdentityTopUpFromAddressesTransitionV0 { + fn validate_structure( + &self, + platform_version: &PlatformVersion, + ) -> SimpleConsensusValidationResult { + let result = self.validate_structure_without_input_witnesses(platform_version); + if !result.is_valid() { + return result; + } + self.validate_input_witnesses_count() + } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/v0_methods.rs index 085f23837d8..b5cad6c0c36 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_topup_from_addresses_transition/v0/v0_methods.rs @@ -18,7 +18,10 @@ use { identity::{accessors::IdentityGettersV0, signer::Signer, Identity}, prelude::{AddressNonce, UserFeeIncrease}, serialization::Signable, - state_transition::StateTransition, + state_transition::{ + address_funds_constructor_dispatch_error, consensus_errors_as_protocol_error, + verify_address_witnesses, StateTransition, StateTransitionType, + }, version::FeatureVersion, ProtocolError, }, @@ -33,12 +36,12 @@ impl IdentityTopUpFromAddressesTransitionMethodsV0 for IdentityTopUpFromAddresse inputs: BTreeMap, signer: &S, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, _version: Option, ) -> Result { let mut identity_top_up_from_addresses_transition = IdentityTopUpFromAddressesTransitionV0 { - inputs: inputs.clone(), + inputs, output: None, identity_id: identity.id(), fee_strategy: vec![ @@ -48,17 +51,53 @@ impl IdentityTopUpFromAddressesTransitionMethodsV0 for IdentityTopUpFromAddresse input_witnesses: vec![], }; + if let Some(error) = address_funds_constructor_dispatch_error( + StateTransitionType::IdentityTopUpFromAddresses, + platform_version, + ) { + return Err(error); + } + + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + // + // LOCKSTEP: this call is hard-coded to the v0 basic-structure check. + // If a future v1 basic-structure is introduced for this transition, + // both the drive-abci server dispatcher AND this SDK constructor must + // be updated together (e.g. by routing through a versioned + // `validate_basic_structure` wrapper as IdentityUpdate does). + let pre_validation_result = identity_top_up_from_addresses_transition + .validate_structure_without_input_witnesses(platform_version); + if let Some(error) = consensus_errors_as_protocol_error(pre_validation_result) { + return Err(error); + } + let state_transition: StateTransition = identity_top_up_from_addresses_transition.clone().into(); let signable_bytes = state_transition.signable_bytes()?; - let mut input_witnesses: Vec = Vec::with_capacity(inputs.len()); - for address in inputs.keys() { + let mut input_witnesses: Vec = + Vec::with_capacity(identity_top_up_from_addresses_transition.inputs.len()); + for address in identity_top_up_from_addresses_transition.inputs.keys() { input_witnesses.push(signer.sign_create_witness(address, &signable_bytes).await?); } + verify_address_witnesses( + identity_top_up_from_addresses_transition.inputs.keys(), + &input_witnesses, + &signable_bytes, + )?; identity_top_up_from_addresses_transition.input_witnesses = input_witnesses; + // After signing, only the witness count needs (re-)validation; the rest + // of the structure was already verified above. + let validation_result = + identity_top_up_from_addresses_transition.validate_input_witnesses_count(); + if let Some(error) = consensus_errors_as_protocol_error(validation_result) { + return Err(error); + } + Ok(identity_top_up_from_addresses_transition.into()) } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/mod.rs index 8dc1da98ead..6a3cd85d890 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/mod.rs @@ -253,6 +253,340 @@ mod test { assert!(result.is_ok()); assert!(result.unwrap().is_empty()); } + + /// Verifies that `try_from_identity_with_signer` rejects an invalid TRANSFER+HIGH + /// added key client-side via the structural public-key validation, returning + /// `ProtocolError::ConsensusError(InvalidIdentityPublicKeySecurityLevelError)` + /// before any signing work is attempted. + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_identity_with_signer_rejects_transfer_high_added_key() { + use crate::address_funds::AddressWitness; + use crate::consensus::basic::BasicError; + use crate::consensus::ConsensusError; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel}; + use crate::state_transition::identity_update_transition::methods::IdentityUpdateTransitionMethodsV0; + use crate::version::PlatformVersion; + use crate::ProtocolError; + use async_trait::async_trait; + use std::collections::BTreeMap; + + /// A signer that should never be invoked: pre-signing validation must fail + /// before this signer is asked to sign anything. + #[derive(Debug)] + struct UnreachableSigner; + + #[async_trait] + impl Signer for UnreachableSigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableSigner::sign must not be called when pre-signing validation rejects the transition" + ); + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!( + "UnreachableSigner::sign_create_witness must not be called when pre-signing validation rejects the transition" + ); + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + let platform_version = PlatformVersion::latest(); + + // Master key on the existing identity (not used here, but the constructor expects + // an identity to read id/revision from). + let master_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![0u8; 33]), + disabled_at: None, + } + .into(); + + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::from([(0, master_key)]), + balance: 0, + revision: 0, + } + .into(); + + // Invalid combination: TRANSFER purpose only allows CRITICAL security level. + let invalid_transfer_high_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 1, + purpose: Purpose::TRANSFER, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![1u8; 33]), + disabled_at: None, + } + .into(); + + let result = IdentityUpdateTransitionV0::try_from_identity_with_signer( + &identity, + &0, + vec![invalid_transfer_high_key], + vec![], + 1, + 0, + &UnreachableSigner, + platform_version, + None, + ) + .await; + + match result { + Err(ProtocolError::ConsensusError(boxed)) => match *boxed { + ConsensusError::BasicError( + BasicError::InvalidIdentityPublicKeySecurityLevelError(err), + ) => { + assert_eq!(err.purpose(), Purpose::TRANSFER); + assert_eq!(err.security_level(), SecurityLevel::HIGH); + } + other => panic!( + "expected InvalidIdentityPublicKeySecurityLevelError, got {:?}", + other + ), + }, + other => panic!( + "expected ConsensusError(InvalidIdentityPublicKeySecurityLevelError), got {:?}", + other + ), + } + } + + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_identity_with_signer_checks_master_key_before_basic_structure() { + use crate::address_funds::AddressWitness; + use crate::consensus::signature::SignatureError; + use crate::consensus::ConsensusError; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel}; + use crate::state_transition::identity_update_transition::methods::IdentityUpdateTransitionMethodsV0; + use crate::version::PlatformVersion; + use crate::ProtocolError; + use async_trait::async_trait; + use std::collections::BTreeMap; + + #[derive(Debug)] + struct UnreachableSigner; + + #[async_trait] + impl Signer for UnreachableSigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("sign should not run when master key lookup fails") + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("sign_create_witness should not run when master key lookup fails") + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + let platform_version = PlatformVersion::latest(); + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::new(), + balance: 0, + revision: 0, + } + .into(); + let invalid_transfer_high_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 1, + purpose: Purpose::TRANSFER, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(vec![1u8; 33]), + disabled_at: None, + } + .into(); + + let result = IdentityUpdateTransitionV0::try_from_identity_with_signer( + &identity, + &999, + vec![invalid_transfer_high_key], + vec![], + 1, + 0, + &UnreachableSigner, + platform_version, + None, + ) + .await; + + assert!(matches!( + result, + Err(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::SignatureError(SignatureError::MissingPublicKeyError(_)) + ) + )); + } + + #[cfg(feature = "state-transition-signing")] + #[tokio::test] + async fn try_from_identity_with_signer_rejects_bad_added_public_key_signature_locally() { + use crate::address_funds::AddressWitness; + use crate::consensus::signature::SignatureError; + use crate::consensus::ConsensusError; + use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; + use crate::identity::signer::Signer; + use crate::identity::v0::IdentityV0; + use crate::identity::{Identity, IdentityPublicKey, KeyType, Purpose, SecurityLevel}; + use crate::state_transition::identity_update_transition::methods::IdentityUpdateTransitionMethodsV0; + use crate::version::PlatformVersion; + use crate::ProtocolError; + use async_trait::async_trait; + use dashcore::secp256k1::{PublicKey as RawPublicKey, Secp256k1, SecretKey}; + use std::collections::BTreeMap; + + #[derive(Debug)] + struct WrongIdentityKeySigner { + wrong_secret_key: SecretKey, + } + + #[async_trait] + impl Signer for WrongIdentityKeySigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + data: &[u8], + ) -> Result { + Ok(BinaryData::new( + dashcore::signer::sign(data, &self.wrong_secret_key.secret_bytes()) + .expect("wrong-key signing should succeed") + .to_vec(), + )) + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + panic!("identity public key signer should not create address witnesses") + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + true + } + } + + let secp = Secp256k1::new(); + let master_secret_key = + SecretKey::from_slice(&[1u8; 32]).expect("valid master key secret key"); + let correct_added_secret_key = + SecretKey::from_slice(&[2u8; 32]).expect("valid added key secret key"); + let wrong_secret_key = + SecretKey::from_slice(&[3u8; 32]).expect("valid alternate secret key"); + let master_public_key = RawPublicKey::from_secret_key(&secp, &master_secret_key); + let correct_added_public_key = + RawPublicKey::from_secret_key(&secp, &correct_added_secret_key); + + let identity: Identity = IdentityV0 { + id: Identifier::default(), + public_keys: BTreeMap::from([( + 0, + IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::MASTER, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(master_public_key.serialize().to_vec()), + disabled_at: None, + } + .into(), + )]), + balance: 0, + revision: 0, + } + .into(); + + let added_public_key: IdentityPublicKey = IdentityPublicKeyV0 { + id: 1, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: BinaryData::new(correct_added_public_key.serialize().to_vec()), + disabled_at: None, + } + .into(); + + let result = IdentityUpdateTransitionV0::try_from_identity_with_signer( + &identity, + &0, + vec![added_public_key], + vec![], + 1, + 0, + &WrongIdentityKeySigner { wrong_secret_key }, + PlatformVersion::latest(), + None, + ) + .await; + + match result { + Err(ProtocolError::ConsensusError(boxed)) => match *boxed { + ConsensusError::SignatureError(SignatureError::BasicECDSAError(_)) => {} + other => panic!("expected SignatureError(BasicECDSAError), got {:?}", other), + }, + Err(ProtocolError::ConsensusErrors(errors)) => { + assert_eq!(errors.len(), 1, "expected a single consensus error"); + assert!(matches!( + errors.as_slice(), + [ConsensusError::SignatureError( + SignatureError::BasicECDSAError(_) + )] + )); + } + other => panic!( + "expected ConsensusError/ConsensusErrors with BasicECDSAError, got {:?}", + other + ), + } + } } /// if the property isn't present the empty list is returned. If property is defined, the function diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/v0_methods.rs index a975040c95f..4e77e4d6e50 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/identity_update_transition/v0/v0_methods.rs @@ -1,43 +1,187 @@ #[cfg(feature = "state-transition-signing")] use crate::serialization::Signable; -#[cfg(feature = "state-transition-signing")] use platform_version::version::PlatformVersion; +use crate::consensus::basic::identity::{ + DisablingKeyIdAlsoBeingAddedInSameTransitionError, DuplicatedIdentityPublicKeyIdBasicError, + InvalidIdentityUpdateTransitionEmptyError, +}; #[cfg(feature = "state-transition-signing")] use crate::consensus::signature::{ InvalidSignaturePublicKeySecurityLevelError, MissingPublicKeyError, SignatureError, }; -#[cfg(feature = "state-transition-signing")] +use crate::consensus::state::identity::max_identity_public_key_limit_reached_error::MaxIdentityPublicKeyLimitReachedError; use crate::consensus::ConsensusError; #[cfg(feature = "state-transition-signing")] use crate::identity::signer::Signer; #[cfg(feature = "state-transition-signing")] use crate::identity::{Identity, IdentityPublicKey}; +#[cfg(feature = "state-transition-signing")] +use crate::serialization::PlatformMessageSignable; +use crate::validation::SimpleConsensusValidationResult; +use crate::ProtocolError; #[cfg(feature = "state-transition-signing")] use crate::identity::accessors::IdentityGettersV0; #[cfg(feature = "state-transition-signing")] use crate::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; +#[cfg(feature = "state-transition-signing")] +use crate::identity::SecurityLevel; use crate::prelude::IdentityNonce; #[cfg(feature = "state-transition-signing")] use crate::prelude::UserFeeIncrease; use crate::state_transition::identity_update_transition::accessors::IdentityUpdateTransitionAccessorsV0; use crate::state_transition::identity_update_transition::methods::IdentityUpdateTransitionMethodsV0; use crate::state_transition::identity_update_transition::v0::IdentityUpdateTransitionV0; +use crate::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; #[cfg(feature = "state-transition-signing")] use crate::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Setters; use crate::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; #[cfg(feature = "state-transition-signing")] -use crate::state_transition::{GetDataContractSecurityLevelRequirementFn, StateTransition}; +use crate::state_transition::{ + consensus_errors_as_protocol_error, GetDataContractSecurityLevelRequirementFn, StateTransition, +}; #[cfg(feature = "state-transition-signing")] use crate::version::FeatureVersion; use crate::{ identity::KeyID, prelude::{Identifier, Revision}, }; -#[cfg(feature = "state-transition-signing")] -use crate::{identity::SecurityLevel, ProtocolError}; + +/// Maximum number of identity public keys that may be disabled in a single +/// [`IdentityUpdateTransitionV0`]. Shared by client-side construction and +/// drive-abci basic-structure validation so both paths apply the same limit. +pub const MAX_IDENTITY_PUBLIC_KEYS_TO_DISABLE: usize = 10; + +impl IdentityUpdateTransitionV0 { + /// Dispatches basic-structure validation to the appropriate versioned + /// implementation based on the active [`PlatformVersion`]. + /// + /// The version source is the DPP-owned field + /// `platform_version.dpp.state_transitions.identities.identity_update.basic_structure` + /// — drive-abci's basic-structure dispatcher reads the same field, so the + /// client and server cannot drift apart. This intentionally avoids having + /// DPP depend on drive-abci-side version routing for a check whose + /// definition lives in DPP. + /// + /// IMPORTANT: when a future v1 basic-structure check is introduced, both + /// this wrapper and the drive-abci dispatcher must be updated in lockstep, + /// and the SDK constructor [`try_from_identity_with_signer`] (which calls + /// this method) must also be reviewed so it remains consistent with the + /// server. + pub fn validate_basic_structure( + &self, + platform_version: &PlatformVersion, + ) -> Result { + match platform_version + .dpp + .state_transitions + .identities + .identity_update + .basic_structure + { + Some(0) => self.validate_basic_structure_v0(platform_version), + Some(version) => Err(ProtocolError::UnknownVersionMismatch { + method: "IdentityUpdateTransitionV0::validate_basic_structure".to_string(), + known_versions: vec![0], + received: version, + }), + // `None` represents "basic-structure validation is not active at + // this PlatformVersion". Surface this with the dedicated + // [`ProtocolError::VersionNotActive`] variant, which mirrors + // drive-abci's `ExecutionError::VersionNotActive` semantics. + None => Err(ProtocolError::VersionNotActive { + method: "IdentityUpdateTransitionV0::validate_basic_structure".to_string(), + known_versions: vec![0], + }), + } + } + + /// Validates the basic structural invariants of this update transition. + /// + /// This mirrors the server-side basic-structure check used by drive-abci + /// (`IdentityUpdateStateTransitionStructureValidationV0::validate_basic_structure_v0`) + /// and is reused by the client-side constructor so that invalid transitions + /// are caught before any signing work is performed. + /// + /// Checks performed (matching the server's v0 behavior): + /// 1. Update is not empty (must add or disable at least one key); on + /// failure this returns immediately. + /// 2. If `disable_public_keys.len() > MAX_IDENTITY_PUBLIC_KEYS_TO_DISABLE`, + /// a [`MaxIdentityPublicKeyLimitReachedError`] is **accumulated** — + /// not short-circuited — so duplicate-id and disable-also-added checks + /// still run against the same input. Any errors collected from the + /// disable-keys block cause an early return before structural + /// validation of `add_public_keys`. + /// 3. Disabled key IDs are unique (first duplicate breaks the loop). + /// 4. No disabled key ID is also being added in the same transition + /// (first overlap breaks the loop). + /// 5. Public-key structure (counts, duplicates, purpose/security-level + /// constraints) via + /// [`IdentityPublicKeyInCreation::validate_identity_public_keys_structure`]. + pub fn validate_basic_structure_v0( + &self, + platform_version: &PlatformVersion, + ) -> Result { + let mut result = SimpleConsensusValidationResult::default(); + + // 1. Ensure that either disablePublicKeys or addPublicKeys is present + if self.disable_public_keys.is_empty() && self.add_public_keys.is_empty() { + result.add_error(ConsensusError::from( + InvalidIdentityUpdateTransitionEmptyError::new(), + )); + return Ok(result); + } + + // 2-4. Validate public keys to disable + if !self.disable_public_keys.is_empty() { + // 2. Ensure max items — accumulate (don't return) so we still + // surface duplicate-id and disable-also-added issues with the + // same input, matching the old server-side behavior. + if self.disable_public_keys.len() > MAX_IDENTITY_PUBLIC_KEYS_TO_DISABLE { + result.add_error(ConsensusError::from( + MaxIdentityPublicKeyLimitReachedError::new(MAX_IDENTITY_PUBLIC_KEYS_TO_DISABLE), + )); + } + + // 3-4. Check key id duplicates and overlap with added keys + let mut ids = std::collections::HashSet::new(); + for key_id in &self.disable_public_keys { + if ids.contains(key_id) { + result.add_error(ConsensusError::from( + DuplicatedIdentityPublicKeyIdBasicError::new(vec![*key_id]), + )); + break; + } + + if self + .add_public_keys + .iter() + .any(|public_key_in_creation| public_key_in_creation.id() == *key_id) + { + result.add_error(ConsensusError::from( + DisablingKeyIdAlsoBeingAddedInSameTransitionError::new(*key_id), + )); + break; + } + + ids.insert(key_id); + } + + if !result.is_valid() { + return Ok(result); + } + } + + // 5. Validate public-key structure (purpose/security level, duplicates, count) + IdentityPublicKeyInCreation::validate_identity_public_keys_structure( + &self.add_public_keys, + false, + platform_version, + ) + } +} impl IdentityUpdateTransitionMethodsV0 for IdentityUpdateTransitionV0 { #[cfg(feature = "state-transition-signing")] @@ -49,10 +193,10 @@ impl IdentityUpdateTransitionMethodsV0 for IdentityUpdateTransitionV0 { nonce: IdentityNonce, user_fee_increase: UserFeeIncrease, signer: &S, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, _version: Option, ) -> Result { - let add_public_keys_in_creation = add_public_keys + let add_public_keys_in_creation: Vec = add_public_keys .iter() .map(|public_key| public_key.into()) .collect(); @@ -68,6 +212,41 @@ impl IdentityUpdateTransitionMethodsV0 for IdentityUpdateTransitionV0 { user_fee_increase, }; + // Fail-fast: verify the master public key exists on the identity and + // has `SecurityLevel::MASTER` *before* doing any POP signing work for + // added unique keys. Catching this here matches the final signing + // contract and avoids spending signer cycles on a transition we + // already know cannot be signed. + let master_public_key = identity + .public_keys() + .get(master_public_key_id) + .ok_or::( + SignatureError::MissingPublicKeyError(MissingPublicKeyError::new( + *master_public_key_id, + )) + .into(), + )?; + if master_public_key.security_level() != SecurityLevel::MASTER { + return Err(ProtocolError::from(ConsensusError::from( + InvalidSignaturePublicKeySecurityLevelError::new( + master_public_key.security_level(), + vec![SecurityLevel::MASTER], + ), + ))); + } + + // Run the same basic-structure checks as the server-side + // `IdentityUpdateStateTransitionStructureValidationV0` impl, going + // through the shared version-dispatching wrapper so client and server + // pick the same versioned check. When a future v1 basic-structure + // check is introduced, the server dispatcher, the wrapper, and this + // constructor must be updated in lockstep. + let basic_structure_result = + identity_update_transition.validate_basic_structure(platform_version)?; + if let Some(error) = consensus_errors_as_protocol_error(basic_structure_result) { + return Err(error); + } + let state_transition: StateTransition = identity_update_transition.clone().into(); let key_signable_bytes = state_transition.signable_bytes()?; @@ -84,33 +263,34 @@ impl IdentityUpdateTransitionMethodsV0 for IdentityUpdateTransitionV0 { } } - let master_public_key = identity - .public_keys() - .get(master_public_key_id) - .ok_or::( - SignatureError::MissingPublicKeyError(MissingPublicKeyError::new( - *master_public_key_id, - )) - .into(), - )?; - if master_public_key.security_level() != SecurityLevel::MASTER { - Err(ProtocolError::InvalidSignaturePublicKeySecurityLevelError( - InvalidSignaturePublicKeySecurityLevelError::new( - master_public_key.security_level(), - vec![SecurityLevel::MASTER], - ), - )) - } else { - let mut state_transition: StateTransition = identity_update_transition.into(); - state_transition - .sign_external( - master_public_key, - signer, - None::, - ) - .await?; - Ok(state_transition) + // Verify proof-of-possession signatures we just produced before + // returning, matching the server-side + // `IdentityUpdateStateTransitionIdentityAndSignaturesValidationV0` + // check. Only keys with unique types were signed above, so verify + // those exact keys here. + for public_key_with_witness in identity_update_transition.add_public_keys.iter() { + if !public_key_with_witness.key_type().is_unique_key_type() { + continue; + } + let pop_result = key_signable_bytes.as_slice().verify_signature( + public_key_with_witness.key_type(), + public_key_with_witness.data().as_slice(), + public_key_with_witness.signature().as_slice(), + ); + if let Some(error) = consensus_errors_as_protocol_error(pop_result) { + return Err(error); + } } + + let mut state_transition: StateTransition = identity_update_transition.into(); + state_transition + .sign_external( + master_public_key, + signer, + None::, + ) + .await?; + Ok(state_transition) } } diff --git a/packages/rs-dpp/src/state_transition/state_transitions/lockstep_assertions.rs b/packages/rs-dpp/src/state_transition/state_transitions/lockstep_assertions.rs new file mode 100644 index 00000000000..9551bb174fb --- /dev/null +++ b/packages/rs-dpp/src/state_transition/state_transitions/lockstep_assertions.rs @@ -0,0 +1,268 @@ +//! Compile-time guards for SDK constructors that hard-code versioned +//! basic-structure checks. +//! +//! Several SDK constructors call a concrete structural validation directly +//! (search for `LOCKSTEP` comments in this directory). If the underlying +//! server basic_structure ever bumps to a higher version without the SDK +//! constructor also being updated, those constructors would silently keep +//! running the older check and could broadcast transitions the network +//! rejects. + +use platform_version::version::LATEST_PLATFORM_VERSION; + +macro_rules! const_assert_matches { + ($expr:expr, $pattern:pat) => { + const _: [(); 1] = [(); matches!($expr, $pattern) as usize]; + }; +} + +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .identity_create_state_transition + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .identity_create_from_addresses_state_transition + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .identity_top_up_from_addresses_state_transition + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_transfer_state_transition + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_transfer_to_addresses_state_transition + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .address_credit_withdrawal + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .address_funds_from_asset_lock + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .address_funds_transfer + .basic_structure, + Some(0) +); +const_assert_matches!( + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_withdrawal_state_transition + .basic_structure, + Some(1) +); +const_assert_matches!( + ( + LATEST_PLATFORM_VERSION + .dpp + .state_transitions + .identities + .identity_update + .basic_structure, + LATEST_PLATFORM_VERSION + .drive_abci + .validation_and_processing + .state_transitions + .identity_update_state_transition + .basic_structure + ), + (Some(0), Some(0)) +); + +#[cfg(test)] +mod tests { + use super::LATEST_PLATFORM_VERSION; + use platform_version::version::PLATFORM_VERSIONS; + + /// Constructors with `LOCKSTEP` notes that hard-code the v0 basic-structure + /// check. Each entry is `(label, basic_structure_field_value)`. + fn sdk_v0_lockstep_dispatch_fields( + v: &platform_version::version::PlatformVersion, + ) -> Vec<(&'static str, Option)> { + let st = &v.drive_abci.validation_and_processing.state_transitions; + vec![ + ( + "identity_create_state_transition", + st.identity_create_state_transition.basic_structure, + ), + ( + "identity_create_from_addresses_state_transition", + st.identity_create_from_addresses_state_transition + .basic_structure, + ), + ( + "identity_top_up_from_addresses_state_transition", + st.identity_top_up_from_addresses_state_transition + .basic_structure, + ), + ( + "identity_credit_transfer_state_transition", + st.identity_credit_transfer_state_transition.basic_structure, + ), + ( + "identity_credit_transfer_to_addresses_state_transition", + st.identity_credit_transfer_to_addresses_state_transition + .basic_structure, + ), + ( + "address_credit_withdrawal", + st.address_credit_withdrawal.basic_structure, + ), + ( + "address_funds_from_asset_lock", + st.address_funds_from_asset_lock.basic_structure, + ), + ( + "address_funds_transfer", + st.address_funds_transfer.basic_structure, + ), + ] + } + + #[test] + fn sdk_constructors_hardcoded_v0_dispatch_still_matches_all_supported_platform_versions() { + let mismatches: Vec<(u32, &'static str, Option)> = PLATFORM_VERSIONS + .iter() + .flat_map(|platform_version| { + sdk_v0_lockstep_dispatch_fields(platform_version) + .into_iter() + .filter(|(_, version)| !matches!(*version, None | Some(0))) + .map(|(label, version)| (platform_version.protocol_version, label, version)) + .collect::>() + }) + .collect(); + + assert!( + mismatches.is_empty(), + "SDK constructor(s) hard-code the v0 basic-structure check but the \ + supported PlatformVersion table no longer resolves their drive-abci \ + basic_structure to Some(0): {:?}. Update the constructor(s) \ + (search for `LOCKSTEP` in packages/rs-dpp/src/state_transition) \ + so they dispatch to the new version, or migrate them to a \ + versioned wrapper as IdentityUpdateTransitionV0 does.", + mismatches + ); + } + + #[test] + fn identity_credit_withdrawal_v1_constructor_dispatch_matches_supported_versions() { + let mismatches: Vec<(u32, Option)> = PLATFORM_VERSIONS + .iter() + .filter_map(|platform_version| { + let actual = platform_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_withdrawal_state_transition + .basic_structure; + (!matches!(actual, None | Some(0) | Some(1))) + .then_some((platform_version.protocol_version, actual)) + }) + .collect(); + + assert_eq!( + mismatches, + Vec::<(u32, Option)>::new(), + "drive-abci identity_credit_withdrawal_state_transition.basic_structure \ + is neither inactive/v0 nor the v1 constructor dispatch: {:?}. \ + Update the SDK constructor dispatch before bumping these versions.", + mismatches + ); + } + + #[test] + fn identity_update_dpp_and_drive_abci_basic_structure_move_together() { + let mismatches: Vec<(u32, Option, Option)> = PLATFORM_VERSIONS + .iter() + .filter_map(|platform_version| { + let dpp_field = platform_version + .dpp + .state_transitions + .identities + .identity_update + .basic_structure; + let drive_abci_field = platform_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_update_state_transition + .basic_structure; + + (dpp_field != drive_abci_field).then_some(( + platform_version.protocol_version, + dpp_field, + drive_abci_field, + )) + }) + .collect(); + + assert_eq!( + mismatches, + Vec::<(u32, Option, Option)>::new(), + "DPP-owned identity_update.basic_structure diverged from the drive-abci \ + identity_update_state_transition.basic_structure in supported versions: {:?}. \ + These fields must move together until the drive-abci field is removed/migrated.", + mismatches + ); + + let latest = LATEST_PLATFORM_VERSION; + assert_eq!( + latest + .dpp + .state_transitions + .identities + .identity_update + .basic_structure, + Some(0), + "DPP-owned identity_update.basic_structure changed from Some(0); \ + update both DPP `IdentityUpdateTransitionV0::validate_basic_structure` \ + and drive-abci's identity_update dispatcher to handle the new version." + ); + } +} diff --git a/packages/rs-dpp/src/state_transition/state_transitions/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/mod.rs index 1ee962d0587..1a66917df4a 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/mod.rs @@ -3,6 +3,7 @@ mod common_fields; mod contract; pub(crate) mod document; pub mod identity; +mod lockstep_assertions; pub mod signable_bytes_hasher; pub use address_funds::*; diff --git a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_transition/v0/v0_methods.rs b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_transition/v0/v0_methods.rs index 28940449e9f..d7432d498b4 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_transition/v0/v0_methods.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_transition/v0/v0_methods.rs @@ -38,7 +38,7 @@ impl ShieldTransitionMethodsV0 for ShieldTransitionV0 { ) -> Result { // Create the unsigned transition (empty witnesses) let mut shield_transition = ShieldTransitionV0 { - inputs: inputs.clone(), + inputs, actions, amount, anchor, @@ -54,8 +54,9 @@ impl ShieldTransitionMethodsV0 for ShieldTransitionV0 { let signable_bytes = state_transition.signable_bytes()?; // Sign each input address - let mut input_witnesses: Vec = Vec::with_capacity(inputs.len()); - for address in inputs.keys() { + let mut input_witnesses: Vec = + Vec::with_capacity(shield_transition.inputs.len()); + for address in shield_transition.inputs.keys() { input_witnesses.push(signer.sign_create_witness(address, &signable_bytes).await?); } shield_transition.input_witnesses = input_witnesses; diff --git a/packages/rs-dpp/src/state_transition/traits/state_transition_structure_validation.rs b/packages/rs-dpp/src/state_transition/traits/state_transition_structure_validation.rs index d8e44c89dfc..2b788299ec7 100644 --- a/packages/rs-dpp/src/state_transition/traits/state_transition_structure_validation.rs +++ b/packages/rs-dpp/src/state_transition/traits/state_transition_structure_validation.rs @@ -1,5 +1,17 @@ +#[cfg(feature = "state-transition-signing")] +use crate::address_funds::{AddressWitness, PlatformAddress}; +#[cfg(any(feature = "state-transition-signing", test))] +use crate::consensus::basic::state_transition::StateTransitionNotActiveError; +#[cfg(any(feature = "state-transition-signing", test))] +use crate::consensus::ConsensusError; +#[cfg(feature = "state-transition-signing")] +use crate::state_transition::StateTransitionType; use crate::validation::SimpleConsensusValidationResult; +use crate::ProtocolError; +use platform_version::version::feature_initial_protocol_versions::ADDRESS_FUNDS_INITIAL_PROTOCOL_VERSION; use platform_version::version::PlatformVersion; +#[cfg(feature = "state-transition-signing")] +use platform_version::version::PLATFORM_VERSIONS; /// Trait for validating the structure of a state transition pub trait StateTransitionStructureValidation { @@ -9,3 +21,248 @@ pub trait StateTransitionStructureValidation { platform_version: &PlatformVersion, ) -> SimpleConsensusValidationResult; } + +/// Converts a `SimpleConsensusValidationResult` into a `ProtocolError` when it +/// contains at least one consensus error, preserving the full error list. +/// +#[cfg(any(feature = "state-transition-signing", test))] +pub(crate) fn consensus_errors_as_protocol_error( + result: SimpleConsensusValidationResult, +) -> Option { + (!result.errors.is_empty()).then(|| result.errors.into()) +} + +#[cfg(feature = "state-transition-signing")] +pub(crate) fn verify_address_witnesses<'a, I>( + addresses: I, + witnesses: &[AddressWitness], + signable_bytes: &[u8], +) -> Result<(), ProtocolError> +where + I: IntoIterator, + I::IntoIter: ExactSizeIterator, +{ + let addresses = addresses.into_iter(); + if addresses.len() != witnesses.len() { + return Err(ProtocolError::AddressWitnessError(format!( + "input witness count mismatch: {} addresses but {} witnesses", + addresses.len(), + witnesses.len() + ))); + } + + for (address, witness) in addresses.zip(witnesses.iter()) { + address.verify_bytes_against_witness(witness, signable_bytes)?; + } + + Ok(()) +} + +#[cfg(feature = "state-transition-signing")] +fn address_funds_basic_structure_version( + state_transition_type: StateTransitionType, + platform_version: &PlatformVersion, +) -> Option { + match state_transition_type { + StateTransitionType::IdentityCreateFromAddresses => { + platform_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_create_from_addresses_state_transition + .basic_structure + } + StateTransitionType::IdentityTopUpFromAddresses => { + platform_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_top_up_from_addresses_state_transition + .basic_structure + } + StateTransitionType::IdentityCreditTransferToAddresses => { + platform_version + .drive_abci + .validation_and_processing + .state_transitions + .identity_credit_transfer_to_addresses_state_transition + .basic_structure + } + StateTransitionType::AddressFundsTransfer => { + platform_version + .drive_abci + .validation_and_processing + .state_transitions + .address_funds_transfer + .basic_structure + } + StateTransitionType::AddressFundingFromAssetLock => { + platform_version + .drive_abci + .validation_and_processing + .state_transitions + .address_funds_from_asset_lock + .basic_structure + } + StateTransitionType::AddressCreditWithdrawal => { + platform_version + .drive_abci + .validation_and_processing + .state_transitions + .address_credit_withdrawal + .basic_structure + } + StateTransitionType::DataContractCreate + | StateTransitionType::Batch + | StateTransitionType::IdentityCreate + | StateTransitionType::IdentityTopUp + | StateTransitionType::DataContractUpdate + | StateTransitionType::IdentityUpdate + | StateTransitionType::IdentityCreditWithdrawal + | StateTransitionType::IdentityCreditTransfer + | StateTransitionType::MasternodeVote + | StateTransitionType::Shield + | StateTransitionType::ShieldedTransfer + | StateTransitionType::Unshield + | StateTransitionType::ShieldFromAssetLock + | StateTransitionType::ShieldedWithdrawal + | StateTransitionType::IdentityCreateFromShieldedPool => unreachable!( + "{state_transition_type} is not an address-funds constructor dispatch target" + ), + } +} + +#[cfg(feature = "state-transition-signing")] +pub(crate) fn address_funds_constructor_dispatch_error( + state_transition_type: StateTransitionType, + platform_version: &PlatformVersion, +) -> Option { + if platform_version.protocol_version < ADDRESS_FUNDS_INITIAL_PROTOCOL_VERSION { + return Some(ProtocolError::from(ConsensusError::from( + StateTransitionNotActiveError::new( + state_transition_type.to_string(), + platform_version.protocol_version, + ADDRESS_FUNDS_INITIAL_PROTOCOL_VERSION, + ), + ))); + } + + let basic_structure = + address_funds_basic_structure_version(state_transition_type, platform_version); + + match basic_structure { + Some(0) => None, + Some(version) => Some(ProtocolError::UnknownVersionMismatch { + method: format!( + "{state_transition_type}::try_from_inputs_with_signer/try_from_identity" + ), + known_versions: vec![0], + received: version, + }), + None => { + let first_active_version = PLATFORM_VERSIONS + .iter() + .find(|version| { + address_funds_basic_structure_version(state_transition_type, version) == Some(0) + }) + .map(|version| version.protocol_version) + .unwrap_or(platform_version.protocol_version); + + Some(ProtocolError::from(ConsensusError::from( + StateTransitionNotActiveError::new( + state_transition_type.to_string(), + platform_version.protocol_version, + first_active_version, + ), + ))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[cfg(feature = "state-transition-signing")] + use crate::consensus::basic::BasicError; + #[cfg(feature = "state-transition-signing")] + use crate::state_transition::StateTransitionType; + #[cfg(feature = "state-transition-signing")] + use platform_version::version::PlatformVersion; + + #[test] + fn helper_preserves_all_consensus_errors_for_multiple_errors() { + let first = ConsensusError::from(StateTransitionNotActiveError::new("first", 1, 11)); + let second = ConsensusError::from(StateTransitionNotActiveError::new("second", 1, 11)); + let result = + SimpleConsensusValidationResult::new_with_errors(vec![first.clone(), second.clone()]); + + let protocol_error = consensus_errors_as_protocol_error(result); + + assert!(matches!( + protocol_error, + Some(ProtocolError::ConsensusErrors(errors)) + if errors == vec![first, second] + )); + } + + #[cfg(feature = "state-transition-signing")] + #[test] + fn address_funds_dispatch_is_not_active_before_protocol_v11_even_when_basic_structure_is_v0() { + let low_version = PlatformVersion::get(1) + .expect("platform version 1 exists") + .clone(); + + for state_transition_type in [ + StateTransitionType::IdentityCreateFromAddresses, + StateTransitionType::IdentityTopUpFromAddresses, + StateTransitionType::IdentityCreditTransferToAddresses, + StateTransitionType::AddressFundsTransfer, + StateTransitionType::AddressFundingFromAssetLock, + StateTransitionType::AddressCreditWithdrawal, + ] { + let result = + address_funds_constructor_dispatch_error(state_transition_type, &low_version); + + assert!(matches!( + result, + Some(ProtocolError::ConsensusError(boxed)) + if matches!( + *boxed, + ConsensusError::BasicError(BasicError::StateTransitionNotActiveError( + ref err + )) if err.current_protocol_version() == low_version.protocol_version + && err.required_protocol_version() + == ADDRESS_FUNDS_INITIAL_PROTOCOL_VERSION + ) + )); + } + } + + #[cfg(feature = "state-transition-signing")] + #[test] + fn address_funds_dispatch_still_reports_post_activation_version_mismatches() { + let mut active_version = PlatformVersion::get(11) + .expect("platform version 11 exists") + .clone(); + active_version + .drive_abci + .validation_and_processing + .state_transitions + .address_funds_transfer + .basic_structure = Some(1); + + let result = address_funds_constructor_dispatch_error( + StateTransitionType::AddressFundsTransfer, + &active_version, + ); + + assert!(matches!( + result, + Some(ProtocolError::UnknownVersionMismatch { + received: 1, + known_versions, + .. + }) if known_versions == vec![0] + )); + } +} diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_credit_withdrawal/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_credit_withdrawal/tests.rs index e2ea0840ec6..7bdc7362d72 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_credit_withdrawal/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_credit_withdrawal/tests.rs @@ -22,7 +22,7 @@ mod tests { use dpp::identity::signer::Signer; use dpp::platform_value::BinaryData; use dpp::prelude::AddressNonce; - use dpp::serialization::{PlatformDeserializable, PlatformSerializable}; + use dpp::serialization::{PlatformDeserializable, PlatformSerializable, Signable}; use dpp::state_transition::address_credit_withdrawal_transition::methods::AddressCreditWithdrawalTransitionMethodsV0; use dpp::state_transition::address_credit_withdrawal_transition::v0::AddressCreditWithdrawalTransitionV0; use dpp::state_transition::address_credit_withdrawal_transition::AddressCreditWithdrawalTransition; @@ -32,6 +32,9 @@ mod tests { use rand::prelude::StdRng; use rand::SeedableRng; use std::collections::BTreeMap; + use std::future::Future; + use std::sync::Arc; + use std::task::{Context, Poll, Wake, Waker}; use crate::execution::check_tx::CheckTxLevel; use crate::platform_types::platform::PlatformRef; @@ -40,6 +43,21 @@ mod tests { // Check TX Helper // ========================================== + fn run_immediate(future: F) -> F::Output { + struct NoopWake; + + impl Wake for NoopWake { + fn wake(self: Arc) {} + } + + let waker = Waker::from(Arc::new(NoopWake)); + let mut future = std::pin::pin!(future); + match future.as_mut().poll(&mut Context::from_waker(&waker)) { + Poll::Ready(output) => output, + Poll::Pending => panic!("expected signer future to complete immediately"), + } + } + /// Perform check_tx on a raw transaction and return whether it's valid /// This simulates what happens when a transaction is submitted to the mempool. /// - invalid_unpaid transactions should return false (rejected from mempool) @@ -173,6 +191,43 @@ mod tests { .expect("should create signed transition") } + /// Create a signed withdrawal transition without running constructor-time structure checks. + fn create_manually_signed_withdrawal_transition( + signer: &TestAddressSigner, + inputs: BTreeMap, + output: Option<(PlatformAddress, u64)>, + fee_strategy: AddressFundsFeeStrategy, + core_fee_per_byte: u32, + pooling: Pooling, + output_script: CoreScript, + user_fee_increase: u16, + ) -> StateTransition { + let mut transition = AddressCreditWithdrawalTransitionV0 { + inputs: inputs.clone(), + output, + fee_strategy, + core_fee_per_byte, + pooling, + output_script, + user_fee_increase, + input_witnesses: vec![], + }; + + let signable_bytes = StateTransition::from(transition.clone()) + .signable_bytes() + .expect("should get signable bytes"); + + transition.input_witnesses = inputs + .keys() + .map(|address| { + run_immediate(signer.sign_create_witness(address, &signable_bytes)) + .expect("should create witness") + }) + .collect(); + + AddressCreditWithdrawalTransition::V0(transition).into() + } + // ========================================== // STRUCTURE VALIDATION TESTS // These test basic structure validation (BasicError) @@ -1119,14 +1174,18 @@ mod tests { inputs.insert(input_address2, (1 as AddressNonce, dash_to_credits!(0.5))); inputs.insert(input_address3, (1 as AddressNonce, dash_to_credits!(0.5))); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, None, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -1339,14 +1398,18 @@ mod tests { let mut inputs = BTreeMap::new(); inputs.insert(input_address, (1 as AddressNonce, withdrawal_amount)); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, None, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -1444,14 +1507,18 @@ mod tests { let output = Some((output_address, output_amount)); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, output, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -1527,14 +1594,18 @@ mod tests { inputs.insert(input_address1, (1 as AddressNonce, dash_to_credits!(0.3))); inputs.insert(input_address2, (1 as AddressNonce, dash_to_credits!(0.3))); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, None, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -1630,14 +1701,18 @@ mod tests { let mut inputs = BTreeMap::new(); inputs.insert(input_address, (1 as AddressNonce, dash_to_credits!(0.5))); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, None, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -1862,14 +1937,18 @@ mod tests { inputs.insert(real_address, (1 as AddressNonce, dash_to_credits!(0.5))); // Sign with wrong signer - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &wrong_signer, inputs, None, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -3458,21 +3537,18 @@ mod tests { inputs.insert(input_address, (1 as AddressNonce, dash_to_credits!(0.5))); // Use Pooling::IfAvailable - let transition = AddressCreditWithdrawalTransitionV0::try_from_inputs_with_signer( + let transition = create_manually_signed_withdrawal_transition( + &signer, inputs, None, AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( 0, )]), 1, - Pooling::IfAvailable, // Different pooling mode + Pooling::IfAvailable, create_random_output_script(&mut rng), - &signer, 0, - platform_version, - ) - .await - .expect("should create signed transition"); + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -3518,21 +3594,18 @@ mod tests { inputs.insert(input_address, (1 as AddressNonce, dash_to_credits!(0.5))); // Use Pooling::Standard - let transition = AddressCreditWithdrawalTransitionV0::try_from_inputs_with_signer( + let transition = create_manually_signed_withdrawal_transition( + &signer, inputs, None, AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( 0, )]), 1, - Pooling::Standard, // Standard pooling + Pooling::Standard, create_random_output_script(&mut rng), - &signer, 0, - platform_version, - ) - .await - .expect("should create signed transition"); + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -3718,14 +3791,18 @@ mod tests { let mut inputs = BTreeMap::new(); inputs.insert(input_address, (1 as AddressNonce, withdrawal_amount)); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, None, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -4806,14 +4883,18 @@ mod tests { // Try to withdraw the tiny amount inputs.insert(input_address, (1 as AddressNonce, 5000)); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, None, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -5165,14 +5246,18 @@ mod tests { // Change output goes back to the same address (should fail) let output = Some((input_address, dash_to_credits!(0.5))); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, output, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -5216,14 +5301,18 @@ mod tests { // Change output goes to a different address let output = Some((change_address, dash_to_credits!(0.5))); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, output, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -5264,14 +5353,18 @@ mod tests { // Zero credits change output let output = Some((input_address, 0)); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, output, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -5312,14 +5405,18 @@ mod tests { // Change output exceeds remaining (after withdrawal + fees) let output = Some((input_address, dash_to_credits!(2.0))); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, output, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -5654,7 +5751,8 @@ mod tests { output_script: CoreScript, core_fee_per_byte: u32, ) -> StateTransition { - AddressCreditWithdrawalTransitionV0::try_from_inputs_with_signer( + create_manually_signed_withdrawal_transition( + signer, inputs, None, AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( @@ -5663,12 +5761,8 @@ mod tests { core_fee_per_byte, Pooling::Never, output_script, - signer, 0, - PlatformVersion::latest(), ) - .await - .expect("should create signed transition") } #[tokio::test] @@ -5998,14 +6092,18 @@ mod tests { // withdrawal_amount = 0.01 - 0.5 = UNDERFLOW let output_address = create_platform_address(2); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, Some((output_address, dash_to_credits!(0.5))), - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -6253,14 +6351,18 @@ mod tests { let output_address = create_platform_address(2); - let transition = create_signed_address_credit_withdrawal_transition( + let transition = create_manually_signed_withdrawal_transition( &signer, inputs, Some((output_address, output_amount)), - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + Pooling::Never, create_random_output_script(&mut rng), - ) - .await; + 0, + ); let result = transition.serialize_to_bytes().expect("should serialize"); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funding_from_asset_lock/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funding_from_asset_lock/tests.rs index 942da423aa5..4706729c0ea 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funding_from_asset_lock/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funding_from_asset_lock/tests.rs @@ -1692,7 +1692,6 @@ mod tests { .build_with_mock_rpc() .set_genesis_state(); - let signer = TestAddressSigner::new(); let mut rng = StdRng::seed_from_u64(567); let (asset_lock_proof, _) = create_asset_lock_proof_with_key(&mut rng); @@ -1704,15 +1703,14 @@ mod tests { outputs.insert(create_platform_address(1), Some(dash_to_credits!(0.5))); outputs.insert(create_platform_address(2), None); // Remainder - let transition = create_signed_address_funding_from_asset_lock_transition( + let transition = create_transition_with_custom_witnesses( asset_lock_proof, &wrong_private_key, - &signer, inputs, outputs, vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], - ) - .await; + vec![], + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -1767,19 +1765,6 @@ mod tests { // Create a different signer with a different key let mut wrong_signer = TestAddressSigner::new(); let wrong_address = wrong_signer.add_p2pkh([2u8; 32]); - // Add the real address hash to the wrong signer so it can "try" to sign for it - // but with the wrong key - wrong_signer.p2pkh_keys.insert( - match real_address { - PlatformAddress::P2pkh(h) => h, - _ => panic!("expected p2pkh"), - }, - wrong_signer.p2pkh_keys[&match wrong_address { - PlatformAddress::P2pkh(h) => h, - _ => panic!("expected p2pkh"), - }] - .clone(), - ); let mut rng = StdRng::seed_from_u64(567); let (asset_lock_proof, asset_lock_pk) = create_asset_lock_proof_with_key(&mut rng); @@ -1791,16 +1776,22 @@ mod tests { outputs.insert(create_platform_address(3), Some(dash_to_credits!(0.5))); outputs.insert(create_platform_address(4), None); // Remainder - // Sign with wrong signer - let transition = create_signed_address_funding_from_asset_lock_transition( + // Sign input witness bytes with the wrong key while keeping the asset lock signature valid. + let signable_bytes = + get_signable_bytes_for_transition(&asset_lock_proof, &inputs, &outputs); + let witness = wrong_signer + .sign_p2pkh(wrong_address, &signable_bytes) + .await + .expect("should sign"); + + let transition = create_transition_with_custom_witnesses( asset_lock_proof, &asset_lock_pk, - &wrong_signer, inputs, outputs, vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], - ) - .await; + vec![witness], + ); let result = transition.serialize_to_bytes().expect("should serialize"); @@ -5103,6 +5094,8 @@ mod tests { #[tokio::test] async fn test_self_transfer_same_input_output_address() { + use dpp::identity::signer::Signer; + // Input and output have the same address (though this should be blocked by structure validation) let platform_version = PlatformVersion::latest(); let platform_config = PlatformConfig { @@ -5132,15 +5125,21 @@ mod tests { let mut outputs = BTreeMap::new(); outputs.insert(address, None); // Same address as input (remainder recipient) - let state_transition = create_signed_address_funding_from_asset_lock_transition( + let signable_bytes = + get_signable_bytes_for_transition(&asset_lock_proof, &inputs, &outputs); + let witness = signer + .sign_create_witness(&address, &signable_bytes) + .await + .expect("should create witness"); + + let state_transition = create_transition_with_custom_witnesses( asset_lock_proof, &asset_lock_pk, - &signer, inputs, outputs, vec![AddressFundsFeeStrategyStep::ReduceOutput(0)], - ) - .await; + vec![witness], + ); let result = state_transition .serialize_to_bytes() @@ -9390,15 +9389,14 @@ mod tests { // Fee strategy targets ReduceOutput(2) — originally the remainder position. // After remainder is removed (outputs shrink from 3 to 2), index 2 is OOB. // Fee deduction may silently skip, giving a free transaction. - let transition = create_signed_address_funding_from_asset_lock_transition( + let transition = create_transition_with_custom_witnesses( asset_lock_proof, &asset_lock_pk, - &signer, BTreeMap::new(), // No address inputs outputs, vec![AddressFundsFeeStrategyStep::ReduceOutput(2)], - ) - .await; + vec![], + ); let result = transition.serialize_to_bytes().expect("should serialize"); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funds_transfer/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funds_transfer/tests.rs index 63e1a2f184e..5007ff59ce7 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funds_transfer/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/address_funds_transfer/tests.rs @@ -19,20 +19,39 @@ mod tests { use dpp::consensus::state::state_error::StateError; use dpp::consensus::ConsensusError; use dpp::dash_to_credits; + use dpp::identity::signer::Signer; use dpp::platform_value::BinaryData; use dpp::prelude::AddressNonce; - use dpp::serialization::PlatformSerializable; + use dpp::serialization::{PlatformSerializable, Signable}; use dpp::state_transition::address_funds_transfer_transition::methods::AddressFundsTransferTransitionMethodsV0; use dpp::state_transition::address_funds_transfer_transition::v0::AddressFundsTransferTransitionV0; use dpp::state_transition::address_funds_transfer_transition::AddressFundsTransferTransition; use dpp::state_transition::StateTransition; use platform_version::version::PlatformVersion; use std::collections::BTreeMap; + use std::future::Future; + use std::sync::Arc; + use std::task::{Context, Poll, Wake, Waker}; // ========================================== // Helper Functions // ========================================== + fn run_immediate(future: F) -> F::Output { + struct NoopWake; + + impl Wake for NoopWake { + fn wake(self: Arc) {} + } + + let waker = Waker::from(Arc::new(NoopWake)); + let mut future = std::pin::pin!(future); + match future.as_mut().poll(&mut Context::from_waker(&waker)) { + Poll::Ready(output) => output, + Poll::Pending => panic!("expected signer future to complete immediately"), + } + } + /// Perform check_tx on a raw transaction and return whether it's valid /// - valid transactions should return true (accepted to mempool) /// - invalid_unpaid transactions should return false (rejected from mempool) @@ -99,17 +118,47 @@ mod tests { fee_strategy: AddressFundsFeeStrategy, input_witnesses_count: usize, ) -> StateTransition { - let witnesses: Vec = (0..input_witnesses_count) - .map(|_| create_dummy_witness()) - .collect(); - AddressFundsTransferTransition::V0(AddressFundsTransferTransitionV0 { + let mut transition = AddressFundsTransferTransitionV0 { inputs, outputs, fee_strategy, user_fee_increase: 0, - input_witnesses: witnesses, - }) - .into() + input_witnesses: vec![], + }; + + let signable_bytes = StateTransition::AddressFundsTransfer( + AddressFundsTransferTransition::V0(transition.clone()), + ) + .signable_bytes() + .expect("should create signable bytes"); + + // Recover deterministic test keys (seeded as [i; 32]) and sign any matching inputs. + let mut deterministic_signer = TestAddressSigner::new(); + for i in 0u8..=255u8 { + deterministic_signer.add_p2pkh([i; 32]); + } + + let input_addresses: Vec = transition.inputs.keys().cloned().collect(); + let witnesses: Vec = (0..input_witnesses_count) + .map(|idx| { + input_addresses + .get(idx) + .and_then(|address| { + if deterministic_signer.can_sign_with(address) { + run_immediate( + deterministic_signer.sign_create_witness(address, &signable_bytes), + ) + .ok() + } else { + None + } + }) + .unwrap_or_else(create_dummy_witness) + }) + .collect(); + + transition.input_witnesses = witnesses; + AddressFundsTransferTransition::V0(transition).into() } /// Create a signed transition with custom inputs/outputs and fee strategy @@ -222,10 +271,13 @@ mod tests { inputs.insert(input_address, (1 as AddressNonce, dash_to_credits!(0.1))); let outputs = BTreeMap::new(); // Empty outputs - // Create transition with proper signature but empty outputs - let transition = - create_signed_transition_with_custom_outputs(&signer, inputs, outputs, vec![]) - .await; + // Create raw transition with empty outputs + let transition = create_raw_transition_with_dummy_witnesses( + inputs, + outputs, + AddressFundsFeeStrategy::from(vec![]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -285,13 +337,14 @@ mod tests { let mut outputs = BTreeMap::new(); outputs.insert(create_platform_address(100), dash_to_credits!(0.17)); - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], - ) - .await; + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 17, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -423,13 +476,14 @@ mod tests { let mut outputs = BTreeMap::new(); outputs.insert(same_address, dash_to_credits!(0.1)); // Same address as input - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], - ) - .await; + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -486,9 +540,12 @@ mod tests { outputs.insert(create_platform_address(2), dash_to_credits!(0.1)); // Empty fee strategy - let transition = - create_signed_transition_with_custom_outputs(&signer, inputs, outputs, vec![]) - .await; + let transition = create_raw_transition_with_dummy_witnesses( + inputs, + outputs, + AddressFundsFeeStrategy::from(vec![]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -545,19 +602,18 @@ mod tests { outputs.insert(create_platform_address(2), dash_to_credits!(0.1)); // 5 fee strategy steps (max is 4) - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![ + AddressFundsFeeStrategy::from(vec![ AddressFundsFeeStrategyStep::DeductFromInput(0), AddressFundsFeeStrategyStep::ReduceOutput(0), AddressFundsFeeStrategyStep::DeductFromInput(0), AddressFundsFeeStrategyStep::ReduceOutput(0), AddressFundsFeeStrategyStep::DeductFromInput(0), - ], - ) - .await; + ]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -614,16 +670,15 @@ mod tests { outputs.insert(create_platform_address(2), dash_to_credits!(0.1)); // Duplicate fee strategy steps - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![ + AddressFundsFeeStrategy::from(vec![ AddressFundsFeeStrategyStep::DeductFromInput(0), AddressFundsFeeStrategyStep::DeductFromInput(0), // Duplicate - ], - ) - .await; + ]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -680,13 +735,14 @@ mod tests { outputs.insert(create_platform_address(2), dash_to_credits!(0.1)); // Fee strategy references input index 5, but we only have 1 input - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![AddressFundsFeeStrategyStep::DeductFromInput(5)], - ) - .await; + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 5, + )]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -743,13 +799,12 @@ mod tests { outputs.insert(create_platform_address(2), dash_to_credits!(0.1)); // Fee strategy references output index 5, but we only have 1 output - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![AddressFundsFeeStrategyStep::ReduceOutput(5)], - ) - .await; + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::ReduceOutput(5)]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -806,13 +861,14 @@ mod tests { let mut outputs = BTreeMap::new(); outputs.insert(create_platform_address(2), 50_000); - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], - ) - .await; + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -869,13 +925,14 @@ mod tests { let mut outputs = BTreeMap::new(); outputs.insert(create_platform_address(2), 100_000); // Below minimum (500,000) - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], - ) - .await; + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); @@ -931,13 +988,14 @@ mod tests { let mut outputs = BTreeMap::new(); outputs.insert(create_platform_address(2), dash_to_credits!(0.5)); // Doesn't match input - let transition = create_signed_transition_with_custom_outputs( - &signer, + let transition = create_raw_transition_with_dummy_witnesses( inputs, outputs, - vec![AddressFundsFeeStrategyStep::DeductFromInput(0)], - ) - .await; + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 1, + ); let result = transition.serialize_to_bytes(); assert!(result.is_ok()); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_addresses/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_addresses/tests.rs index e3bd41dc828..1aca0df6171 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_addresses/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_addresses/tests.rs @@ -495,14 +495,17 @@ mod tests { let (identity, identity_signer) = create_identity_with_keys([50u8; 32], &mut rng, platform_version); - // Create signed transition with too many inputs - let transition = create_signed_identity_create_from_addresses_transition( + // Create signed transition with too many inputs (manual construction bypasses + // constructor-time structure validation so check_tx can assert the intended error). + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -651,14 +654,16 @@ mod tests { let (identity, identity_signer) = create_identity_with_keys([50u8; 32], &mut rng, platform_version); - // Create signed transition with input below minimum - let transition = create_signed_identity_create_from_addresses_transition( + // Manual construction keeps this test focused on check_tx validation behavior. + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -1021,13 +1026,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, input_amount)); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -1326,13 +1333,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, input_amount)); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -1445,13 +1454,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, input_amount)); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -1543,13 +1554,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, input_amount)); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -1659,13 +1672,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, dash_to_credits!(1.0))); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -1735,13 +1750,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, dash_to_credits!(1.0))); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -1813,13 +1830,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, input_amount)); // Wrong nonce // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -2307,13 +2326,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, input_amount)); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -2424,13 +2445,15 @@ mod tests { inputs.insert(address, (1 as AddressNonce, input_amount)); // Create signed transition - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -2963,13 +2986,15 @@ mod tests { (1 as AddressNonce, i64::MAX as u64 / 2 - 200_000_000), ); - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -3156,13 +3181,15 @@ mod tests { let mut inputs = BTreeMap::new(); inputs.insert(address, (1 as AddressNonce, min_input)); - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; @@ -3370,13 +3397,15 @@ mod tests { let mut inputs = BTreeMap::new(); inputs.insert(address, (1 as AddressNonce, min_input - 1)); // One below minimum - let transition = create_signed_identity_create_from_addresses_transition( + let transition = create_signed_identity_create_from_addresses_transition_full( &identity, &address_signer, &identity_signer, inputs, None, - None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), platform_version, ) .await; diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/mod.rs index 5148be630d7..37e2a7b0e0e 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/mod.rs @@ -74,8 +74,10 @@ impl StateTransitionBasicStructureValidationV0 for IdentityCreditTransferTransit .basic_structure { Some(0) => { - // There is nothing expensive here - self.validate_basic_structure_v0() + // There is nothing expensive here. + // Trait-qualified to avoid name collision with the inherent + // DPP method of the same name that this trait delegates to. + IdentityCreditTransferStateTransitionStructureValidationV0::validate_basic_structure_v0(self) } Some(version) => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "identity credit transfer transition: validate_basic_structure".to_string(), diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/structure/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/structure/v0/mod.rs index 2d19799dccd..1f5ad70ef35 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/structure/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer/structure/v0/mod.rs @@ -1,15 +1,7 @@ -use dpp::consensus::basic::identity::{ - IdentityCreditTransferToSelfError, InvalidIdentityCreditTransferAmountError, -}; - -// use dpp::platform_value:: use crate::error::Error; -use dpp::state_transition::identity_credit_transfer_transition::accessors::IdentityCreditTransferTransitionAccessorsV0; use dpp::state_transition::identity_credit_transfer_transition::IdentityCreditTransferTransition; use dpp::validation::SimpleConsensusValidationResult; -const MIN_TRANSFER_AMOUNT: u64 = 100000; - pub(in crate::execution::validation::state_transition::state_transitions::identity_credit_transfer) trait IdentityCreditTransferStateTransitionStructureValidationV0 { fn validate_basic_structure_v0(&self) -> Result; } @@ -18,21 +10,9 @@ impl IdentityCreditTransferStateTransitionStructureValidationV0 for IdentityCreditTransferTransition { fn validate_basic_structure_v0(&self) -> Result { - let result = SimpleConsensusValidationResult::new(); - - if self.identity_id() == self.recipient_id() { - return Ok(SimpleConsensusValidationResult::new_with_error( - IdentityCreditTransferToSelfError::default().into(), - )); - } - - if self.amount() < MIN_TRANSFER_AMOUNT { - return Ok(SimpleConsensusValidationResult::new_with_error( - InvalidIdentityCreditTransferAmountError::new(self.amount(), MIN_TRANSFER_AMOUNT) - .into(), - )); - } - - Ok(result) + // Delegate to the DPP-owned shared rule so client-side constructors + // and drive-abci enforce identical self-transfer and minimum-amount + // checks from a single source of truth. + Ok(IdentityCreditTransferTransition::validate_basic_structure_v0(self)) } } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer_to_addresses/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer_to_addresses/tests.rs index cacdbb9309b..261de176ca4 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer_to_addresses/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer_to_addresses/tests.rs @@ -14,10 +14,11 @@ mod tests { use dpp::dash_to_credits; use dpp::identity::accessors::IdentityGettersV0; use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + use dpp::identity::signer::Signer; use dpp::identity::{Identity, IdentityPublicKey, IdentityV0, KeyType, Purpose, SecurityLevel}; use dpp::platform_value::BinaryData; use dpp::prelude::IdentityNonce; - use dpp::serialization::PlatformSerializable; + use dpp::serialization::{PlatformSerializable, Signable}; use dpp::state_transition::identity_credit_transfer_to_addresses_transition::methods::IdentityCreditTransferToAddressesTransitionMethodsV0; use dpp::state_transition::identity_credit_transfer_to_addresses_transition::v0::IdentityCreditTransferToAddressesTransitionV0; use dpp::state_transition::identity_credit_transfer_to_addresses_transition::IdentityCreditTransferToAddressesTransition; @@ -4980,8 +4981,31 @@ mod tests { recipient_addresses.insert(create_platform_address(1), min_output); // Create the transition once - we'll reuse it for both runs - let transition = - create_signed_transition(&identity, &signer, recipient_addresses.clone(), 1).await; + let mut transition_v0 = IdentityCreditTransferToAddressesTransitionV0 { + identity_id: identity.id(), + recipient_addresses: recipient_addresses.clone(), + nonce: 1, + user_fee_increase: 0, + signature_public_key_id: 1, + signature: BinaryData::new(vec![]), + }; + let signable_bytes: Vec = StateTransition::from( + IdentityCreditTransferToAddressesTransition::V0(transition_v0.clone()), + ) + .signable_bytes() + .expect("should get signable bytes"); + let transfer_key = identity + .public_keys() + .get(&1) + .expect("transfer key should exist"); + transition_v0.signature = signer + .sign(transfer_key, &signable_bytes) + .await + .expect("should sign"); + + let transition = StateTransition::from( + IdentityCreditTransferToAddressesTransition::V0(transition_v0), + ); let transition_bytes = transition.serialize_to_bytes().expect("should serialize"); // First run in a transaction to measure actual fee (then rollback) @@ -5124,9 +5148,33 @@ mod tests { let total_outputs: u64 = recipient_addresses.values().sum(); // Create the transition once - we'll reuse it for both runs - let transition = - create_signed_transition(&identity, &signer, recipient_addresses.clone(), 1).await; - let transition_bytes = transition.serialize_to_bytes().expect("should serialize"); + let mut transition_v0 = IdentityCreditTransferToAddressesTransitionV0 { + identity_id: identity.id(), + recipient_addresses: recipient_addresses.clone(), + nonce: 1, + user_fee_increase: 0, + signature_public_key_id: 1, + signature: BinaryData::new(vec![]), + }; + let signable_bytes = StateTransition::from( + IdentityCreditTransferToAddressesTransition::V0(transition_v0.clone()), + ) + .signable_bytes() + .expect("should get signable bytes"); + let transfer_key = identity + .public_keys() + .get(&1) + .expect("transfer key should exist"); + transition_v0.signature = signer + .sign(transfer_key, &signable_bytes) + .await + .expect("should sign"); + + let transition_bytes = StateTransition::from( + IdentityCreditTransferToAddressesTransition::V0(transition_v0), + ) + .serialize_to_bytes() + .expect("should serialize"); // First run in a transaction to measure actual fee (then rollback) let platform_state = platform.state.load(); @@ -5251,9 +5299,32 @@ mod tests { recipient_addresses.insert(PlatformAddress::P2pkh(hash), min_output); } - let transition = - create_signed_transition(&identity, &signer, recipient_addresses.clone(), 1).await; - let transition_bytes = transition.serialize_to_bytes().expect("should serialize"); + let mut transition_v0 = IdentityCreditTransferToAddressesTransitionV0 { + identity_id: identity.id(), + recipient_addresses: recipient_addresses.clone(), + nonce: 1, + user_fee_increase: 0, + signature_public_key_id: 1, + signature: BinaryData::new(vec![]), + }; + let signable_bytes = StateTransition::from( + IdentityCreditTransferToAddressesTransition::V0(transition_v0.clone()), + ) + .signable_bytes() + .expect("should get signable bytes"); + let transfer_key = identity + .public_keys() + .get(&1) + .expect("transfer key should exist"); + transition_v0.signature = signer + .sign(transfer_key, &signable_bytes) + .await + .expect("should sign"); + let transition_bytes = StateTransition::from( + IdentityCreditTransferToAddressesTransition::V0(transition_v0), + ) + .serialize_to_bytes() + .expect("should serialize"); let platform_state = platform.state.load(); let transaction = platform.drive.grove.start_transaction(); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/mod.rs index cf9db2fbd55..1d4fa89303b 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/mod.rs @@ -114,13 +114,18 @@ mod tests { use dpp::consensus::basic::BasicError; use dpp::consensus::ConsensusError; use dpp::dash_to_credits; + use dpp::identity::accessors::IdentityGettersV0; use dpp::identity::core_script::CoreScript; use dpp::identity::KeyType::{ECDSA_HASH160, ECDSA_SECP256K1}; + use dpp::platform_value::BinaryData; use dpp::serialization::PlatformSerializable; use dpp::state_transition::identity_credit_withdrawal_transition::methods::{ IdentityCreditWithdrawalTransitionMethodsV0, PreferredKeyPurposeForSigningWithdrawal, }; + use dpp::state_transition::identity_credit_withdrawal_transition::v1::IdentityCreditWithdrawalTransitionV1; use dpp::state_transition::identity_credit_withdrawal_transition::IdentityCreditWithdrawalTransition; + use dpp::state_transition::GetDataContractSecurityLevelRequirementFn; + use dpp::state_transition::StateTransition; use dpp::withdrawal::Pooling; use platform_version::version::v1::PROTOCOL_VERSION_1; use platform_version::version::PlatformVersion; @@ -160,22 +165,28 @@ mod tests { let withdrawal_amount = dash_to_credits!(0.1); - let credit_withdrawal_transition = IdentityCreditWithdrawalTransition::try_from_identity( - &identity, - Some(CoreScript::random_p2pkh(&mut rng)), - withdrawal_amount, - Pooling::Never, - 1, - 0, - signer, - Some(&withdrawal_key), - PreferredKeyPurposeForSigningWithdrawal::Any, - 2, - platform_version, - Some(1), - ) - .await - .expect("expected a credit withdrawal transition"); + let credit_withdrawal_transition_v1 = IdentityCreditWithdrawalTransitionV1 { + identity_id: identity.id(), + amount: withdrawal_amount, + core_fee_per_byte: 1, + pooling: Pooling::Never, + output_script: Some(CoreScript::random_p2pkh(&mut rng)), + nonce: 2, + user_fee_increase: 0, + signature_public_key_id: 0, + signature: BinaryData::default(), + }; + let mut credit_withdrawal_transition: StateTransition = + credit_withdrawal_transition_v1.into(); + + credit_withdrawal_transition + .sign_external( + &withdrawal_key, + &signer, + None::, + ) + .await + .expect("expected to sign credit withdrawal transition"); let credit_withdrawal_transition_serialized_transition = credit_withdrawal_transition .serialize_to_bytes() diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs index e6131404b11..e645704852a 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/structure/v1/mod.rs @@ -1,20 +1,7 @@ -use dpp::consensus::basic::identity::{ - InvalidCreditWithdrawalTransitionCoreFeeError, - InvalidCreditWithdrawalTransitionOutputScriptError, - InvalidIdentityCreditWithdrawalTransitionAmountError, - NotImplementedCreditWithdrawalTransitionPoolingError, -}; -use dpp::consensus::ConsensusError; - use crate::error::Error; -use dpp::state_transition::identity_credit_withdrawal_transition::accessors::IdentityCreditWithdrawalTransitionAccessorsV0; -use dpp::state_transition::identity_credit_withdrawal_transition::{ - IdentityCreditWithdrawalTransition, MIN_CORE_FEE_PER_BYTE, -}; -use dpp::util::is_non_zero_fibonacci_number::is_non_zero_fibonacci_number; +use dpp::state_transition::identity_credit_withdrawal_transition::IdentityCreditWithdrawalTransition; use dpp::validation::SimpleConsensusValidationResult; use dpp::version::PlatformVersion; -use dpp::withdrawal::Pooling; pub(in crate::execution::validation::state_transition::state_transitions::identity_credit_withdrawal) trait IdentityCreditWithdrawalStateTransitionStructureValidationV1 { fn validate_basic_structure_v1(&self, platform_version: &PlatformVersion) -> Result; @@ -27,57 +14,11 @@ impl IdentityCreditWithdrawalStateTransitionStructureValidationV1 &self, platform_version: &PlatformVersion, ) -> Result { - let mut result = SimpleConsensusValidationResult::default(); - - let amount = self.amount(); - if amount < platform_version.system_limits.min_withdrawal_amount - || amount > platform_version.system_limits.max_withdrawal_amount - { - result.add_error(ConsensusError::from( - InvalidIdentityCreditWithdrawalTransitionAmountError::new( - self.amount(), - platform_version.system_limits.min_withdrawal_amount, - platform_version.system_limits.max_withdrawal_amount, - ), - )); - } - - // NOTE: the shielded-withdrawal path (v12) re-validates these same three Core-facing - // fields — `pooling`, `core_fee_per_byte`, `output_script` — in - // `dpp .../shielded/shielded_withdrawal_transition/v0/state_transition_validation.rs`, - // reusing the same error types and `MIN_CORE_FEE_PER_BYTE`. Keep the two in sync (or, if - // touching both, factor a shared helper). - - // currently we do not support pooling, so we must validate that pooling is `Never` - - if self.pooling() != Pooling::Never { - result.add_error(NotImplementedCreditWithdrawalTransitionPoolingError::new( - self.pooling() as u8, - )); - - return Ok(result); - } - - // validate core_fee is in fibonacci sequence - if !is_non_zero_fibonacci_number(self.core_fee_per_byte() as u64) { - result.add_error(InvalidCreditWithdrawalTransitionCoreFeeError::new( - self.core_fee_per_byte(), - MIN_CORE_FEE_PER_BYTE, - )); - - return Ok(result); - } - - if let Some(output_script) = self.output_script() { - // validate output_script types - if !output_script.is_p2pkh() && !output_script.is_p2sh() { - result.add_error(InvalidCreditWithdrawalTransitionOutputScriptError::new( - output_script.clone(), - )); - } - } - - Ok(result) + // Delegate to the shared DPP-owned v1 basic-structure rule so the + // server and the SDK constructor cannot drift apart. The DPP method + // owns the amount/pooling/core-fee/output-script logic; this trait + // method only adapts its return type for drive-abci's error chain. + Ok(self.basic_structure_rules_v1(platform_version)) } } @@ -86,10 +27,13 @@ mod tests { use super::*; use assert_matches::assert_matches; + use dpp::consensus::basic::identity::InvalidIdentityCreditWithdrawalTransitionAmountError; use dpp::consensus::basic::BasicError; + use dpp::consensus::ConsensusError; use dpp::dashcore::ScriptBuf; use dpp::identity::core_script::CoreScript; use dpp::state_transition::identity_credit_withdrawal_transition::v1::IdentityCreditWithdrawalTransitionV1; + use dpp::withdrawal::Pooling; use platform_version::version::v1::PLATFORM_V1; use rand::SeedableRng; diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_top_up_from_addresses/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_top_up_from_addresses/tests.rs index a8ce747c4a8..bb70d2a3556 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_top_up_from_addresses/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_top_up_from_addresses/tests.rs @@ -298,8 +298,18 @@ mod tests { inputs.insert(addr, (1 as AddressNonce, dash_to_credits!(0.01))); } - let transition = - create_signed_transition(&identity, &signer, inputs, platform_version).await; + let transition = create_signed_transition_with_options( + &identity, + &signer, + inputs, + None, + AddressFundsFeeStrategy::from(vec![AddressFundsFeeStrategyStep::DeductFromInput( + 0, + )]), + 0, + platform_version, + ) + .await; let result = transition.serialize_to_bytes(); assert!(result.is_ok()); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/basic_structure/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/basic_structure/v0/mod.rs index 2252d0192c4..4c652027b55 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/basic_structure/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/basic_structure/v0/mod.rs @@ -1,19 +1,7 @@ use crate::error::Error; -use dpp::consensus::basic::identity::{ - DisablingKeyIdAlsoBeingAddedInSameTransitionError, DuplicatedIdentityPublicKeyIdBasicError, - InvalidIdentityUpdateTransitionEmptyError, -}; -use dpp::consensus::state::identity::max_identity_public_key_limit_reached_error::MaxIdentityPublicKeyLimitReachedError; -use dpp::consensus::ConsensusError; -use dpp::state_transition::identity_update_transition::accessors::IdentityUpdateTransitionAccessorsV0; use dpp::state_transition::identity_update_transition::IdentityUpdateTransition; -use dpp::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; -use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dpp::validation::SimpleConsensusValidationResult; use dpp::version::PlatformVersion; -use std::collections::HashSet; - -const MAX_KEYS_TO_DISABLE: usize = 10; pub(in crate::execution::validation::state_transition::state_transitions::identity_update) trait IdentityUpdateStateTransitionStructureValidationV0 { @@ -28,62 +16,14 @@ impl IdentityUpdateStateTransitionStructureValidationV0 for IdentityUpdateTransi &self, platform_version: &PlatformVersion, ) -> Result { - let mut result = SimpleConsensusValidationResult::default(); - - // Ensure that either disablePublicKeys or addPublicKeys is present - if self.public_key_ids_to_disable().is_empty() && self.public_keys_to_add().is_empty() { - result.add_error(ConsensusError::from( - InvalidIdentityUpdateTransitionEmptyError::new(), - )); - } - - if !result.is_valid() { - return Ok(result); - } - - // Validate public keys to disable - if !self.public_key_ids_to_disable().is_empty() { - // Ensure max items - if self.public_key_ids_to_disable().len() > MAX_KEYS_TO_DISABLE { - result.add_error(ConsensusError::from( - MaxIdentityPublicKeyLimitReachedError::new(MAX_KEYS_TO_DISABLE), - )); - } - - // Check key id duplicates - let mut ids = HashSet::new(); - for key_id in self.public_key_ids_to_disable() { - if ids.contains(key_id) { - result.add_error(ConsensusError::from( - DuplicatedIdentityPublicKeyIdBasicError::new(vec![*key_id]), - )); - break; - } - - if self - .public_keys_to_add() - .iter() - .any(|public_key_in_creation| public_key_in_creation.id() == *key_id) - { - result.add_error(ConsensusError::from( - DisablingKeyIdAlsoBeingAddedInSameTransitionError::new(*key_id), - )); - break; - } - - ids.insert(key_id); - } + // Delegate to the DPP-owned implementation so both client-side + // construction and server-side validation apply identical checks + // (including the shared `MAX_IDENTITY_PUBLIC_KEYS_TO_DISABLE` limit + // and check ordering). + match self { + IdentityUpdateTransition::V0(v0) => v0 + .validate_basic_structure_v0(platform_version) + .map_err(Error::Protocol), } - - if !result.is_valid() { - return Ok(result); - } - - IdentityPublicKeyInCreation::validate_identity_public_keys_structure( - self.public_keys_to_add(), - false, - platform_version, - ) - .map_err(Error::Protocol) } } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs index 77e0d67c340..a95e0ca92b0 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs @@ -68,11 +68,16 @@ impl StateTransitionBasicStructureValidationV0 for IdentityUpdateTransition { _network_type: Network, platform_version: &PlatformVersion, ) -> Result { + // The version source for this dispatcher lives in DPP, not in + // `drive_abci.validation_and_processing.state_transitions...`. DPP + // exposes the same versioned basic-structure check on + // `IdentityUpdateTransitionV0::validate_basic_structure`, which reads + // the same field — keeping both paths in lockstep. match platform_version - .drive_abci - .validation_and_processing + .dpp .state_transitions - .identity_update_state_transition + .identities + .identity_update .basic_structure { Some(0) => self.validate_basic_structure_v0(platform_version), diff --git a/packages/rs-drive-abci/tests/strategy_tests/strategy.rs b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs index d7900326ce2..ea3294e5671 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/strategy.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs @@ -1508,15 +1508,40 @@ impl NetworkStrategy { "Expected to find sender identity in hardcoded start identities", ); - // Use the pre-specified outputs from transfer_info - let state_transition = create_identity_credit_transfer_to_addresses_transition_with_outputs( + // Use the pre-specified outputs from transfer_info. + // The helper returns `None` if any output is below the + // per-output minimum; skip in that case rather than + // panicking on a sub-minimum output. + if let Some(state_transition) = create_identity_credit_transfer_to_addresses_transition_with_outputs( sender, identity_nonce_counter, signer, transfer_info.outputs.clone(), platform_version, - ).await; - operations.push(state_transition); + ).await + { + // Stage recipient balances now that the transition + // was constructed successfully. Mirrors the + // random-output branch and the shared crate's + // explicit branch: existing addresses get their + // balance increased, new ones registered. Skipping + // this leaves the recipient balances out of the + // local view and breaks downstream operations. + for (recipient_address, amount) in &transfer_info.outputs { + if let Some((nonce, balance)) = current_addresses_with_balance + .get_effective(recipient_address) + { + let new_entry = (*nonce, balance + amount); + current_addresses_with_balance + .addresses_in_block_with_new_balance + .insert(*recipient_address, new_entry); + } else { + current_addresses_with_balance + .register_new_address(*recipient_address, *amount); + } + } + operations.push(state_transition); + } } else { // Handle the case where no sender/outputs are provided - generate random ones let identities_count = current_identities.len(); @@ -1533,7 +1558,7 @@ impl NetworkStrategy { rng.gen_range(output_count_range.clone()) as usize; let total_amount = rng.gen_range(amount_range.clone()); - let (state_transition, _recipient_addresses) = + if let Some((state_transition, _recipient_addresses)) = create_identity_credit_transfer_to_addresses_transition( sender, identity_nonce_counter, @@ -1544,8 +1569,10 @@ impl NetworkStrategy { rng, platform_version, ) - .await; - operations.push(state_transition); + .await + { + operations.push(state_transition); + } } } } @@ -2308,8 +2335,13 @@ impl NetworkStrategy { rng: &mut StdRng, platform_version: &PlatformVersion, ) -> Option { - let inputs = - current_addresses_with_balance.take_random_amounts_with_range(amount_range, rng)?; + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + let inputs = current_addresses_with_balance + .take_random_amounts_with_range_and_min_per_input(amount_range, min_per_input, rng)?; tracing::trace!( ?inputs, "Preparing identity top-up transition with addresses" @@ -2347,8 +2379,20 @@ impl NetworkStrategy { rng: &mut StdRng, platform_version: &PlatformVersion, ) -> Option<(Identity, StateTransition)> { - let inputs = - current_addresses_with_balance.take_random_amounts_with_range(amount_range, rng)?; + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + // Snapshot the staged map BEFORE picking inputs so any rollback + // restores the exact pre-staging state. Removing only `inputs.keys()` + // would be unsafe if the picker ever reuses same-block addresses or + // if other paths stage entries we shouldn't drop. + let staged_snapshot_before_inputs = current_addresses_with_balance + .addresses_in_block_with_new_balance + .clone(); + let inputs = current_addresses_with_balance + .take_random_amounts_with_range_and_min_per_input(amount_range, min_per_input, rng)?; tracing::debug!( ?inputs, "Preparing identity create from addresses transition" @@ -2388,14 +2432,104 @@ impl NetworkStrategy { .clone() .unwrap_or(vec![AddressFundsFeeStrategyStep::DeductFromInput(0)]); + // Calculate estimated fee for local balance tracking. + // Fee = identity_create_base_cost + identity_key_in_creation_cost * num_keys + // + address_funds_transfer_input_cost * num_inputs + // + address_funds_transfer_output_cost * num_outputs + let total_key_count = identity.public_keys().len() as u64; + let num_inputs = inputs.len() as u64; + let has_output = maybe_output_amount.is_some(); + let num_outputs: u64 = if has_output { 1 } else { 0 }; + + let min_fees = &platform_version.fee_version.state_transition_min_fees; + let estimated_fee = min_fees + .identity_create_base_cost + .saturating_add( + min_fees + .identity_key_in_creation_cost + .saturating_mul(total_key_count), + ) + .saturating_add( + min_fees + .address_funds_transfer_input_cost + .saturating_mul(num_inputs), + ) + .saturating_add( + min_fees + .address_funds_transfer_output_cost + .saturating_mul(num_outputs), + ); + + // Track fee deductions for balance tracking + let mut output_fee_deduction: Credits = 0; + let mut input_fee_deductions: BTreeMap = BTreeMap::new(); + + let input_addresses_in_order: Vec = inputs.keys().cloned().collect(); + + let mut remaining_fee = estimated_fee; + for step in &fee_strategy { + if remaining_fee == 0 { + break; + } + match step { + AddressFundsFeeStrategyStep::ReduceOutput(index) => { + if *index == 0 && has_output { + output_fee_deduction = output_fee_deduction.saturating_add(remaining_fee); + remaining_fee = 0; + } + } + AddressFundsFeeStrategyStep::DeductFromInput(index) => { + if (*index as usize) < inputs.len() { + let entry = input_fee_deductions.entry(*index as usize).or_insert(0); + *entry = entry.saturating_add(remaining_fee); + remaining_fee = 0; + } + } + } + } + + // Apply fee deductions to input balances (beyond the amount already staged) + for (idx, fee_deduction) in input_fee_deductions { + if let Some(address) = input_addresses_in_order.get(idx) { + if let Some((_, staged_balance)) = current_addresses_with_balance + .addresses_in_block_with_new_balance + .get_mut(address) + { + *staged_balance = staged_balance.saturating_sub(fee_deduction); + } + } + } + + // Sample the optional output amount and validate it against + // min_output_amount before registering any output address. The + // constructor rejects optional outputs below this minimum, so + // skip the candidate (rolling back staged input balances) instead + // of panicking on the expect below. + let sampled_output_amount = if let Some(output_range) = maybe_output_amount.as_ref() { + let amount = rng.gen_range(output_range.clone()); + let min_per_output = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + if amount < min_per_output { + current_addresses_with_balance.addresses_in_block_with_new_balance = + staged_snapshot_before_inputs; + return None; + } + Some(amount) + } else { + None + }; + // Create output if maybe_output_amount is provided - let output = maybe_output_amount.as_ref().map(|output_range| { - let output_amount = rng.gen_range(output_range.clone()); + let output = sampled_output_amount.map(|output_amount| { let output_address = signer.add_random_address_key(rng); - // Register the output address with balance + // Register the output address with balance minus fee deduction + let actual_output_amount = output_amount.saturating_sub(output_fee_deduction); current_addresses_with_balance.register_new_address_keep_only_highest( output_address.clone(), - output_amount, + actual_output_amount, self.max_addresses_to_choose_from_in_cache, ); (output_address, output_amount) @@ -2433,16 +2567,44 @@ impl NetworkStrategy { rng: &mut StdRng, platform_version: &PlatformVersion, ) -> Option { - let inputs = - current_addresses_with_balance.take_random_amounts_with_range(amount_range, rng)?; + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + let min_per_output = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + // Snapshot the staged map BEFORE picking inputs so any rollback + // restores the exact pre-staging state. Removing only `inputs.keys()` + // would be unsafe if the picker ever reuses same-block addresses or + // if other paths stage entries we shouldn't drop. + let staged_snapshot_before_inputs = current_addresses_with_balance + .addresses_in_block_with_new_balance + .clone(); + let inputs = current_addresses_with_balance + .take_random_amounts_with_range_and_min_per_input(amount_range, min_per_input, rng)?; tracing::debug!(?inputs, "Preparing address funds transfer transition"); // Calculate total input amount (we'll distribute this among outputs) let total_input: Credits = inputs.values().map(|(_, credits)| credits).sum(); - // Generate random number of outputs within the specified range - let output_count = rng.gen_range(output_count_range.clone()).max(1) as usize; + // If total_input is below min_output_amount, we cannot create even a single + // valid output. Roll back the staged input balances and skip this transition. + if total_input < min_per_output { + current_addresses_with_balance.addresses_in_block_with_new_balance = + staged_snapshot_before_inputs; + return None; + } + + // Generate random number of outputs within the specified range, + // but cap so each output gets at least min_output_amount + let max_outputs_by_amount = (total_input / min_per_output) as usize; + let output_count = + (rng.gen_range(output_count_range.clone()).max(1) as usize).min(max_outputs_by_amount); // Generate fee strategy: if not provided, reduce from outputs sequentially // Limited to 4 steps due to max_address_fee_strategies platform constraint @@ -2453,8 +2615,85 @@ impl NetworkStrategy { .collect() }); - // Create output addresses and distribute funds evenly + // Calculate estimated fee for local balance tracking. + // The transition must have inputs == outputs (balanced), but the chain + // will deduct fees according to fee_strategy. We need to track the + // actual credited amounts locally. + let min_fees = &platform_version.fee_version.state_transition_min_fees; + let estimated_fee = min_fees + .address_funds_transfer_input_cost + .saturating_mul(inputs.len() as u64) + .saturating_add( + min_fees + .address_funds_transfer_output_cost + .saturating_mul(output_count as u64), + ); + let amount_per_output = total_input / output_count as Credits; + let remainder = total_input - (amount_per_output * output_count as Credits); + + // Verify the configured fee_strategy can cover the fee given staged + // input/output amounts. If it can't, roll back staged input balances + // and skip this candidate. + let input_addresses_for_check: Vec = inputs.keys().cloned().collect(); + let mut fee_can_be_covered = false; + let mut remaining_fee_to_check = estimated_fee; + for step in &fee_strategy { + if remaining_fee_to_check == 0 { + fee_can_be_covered = true; + break; + } + match step { + AddressFundsFeeStrategyStep::ReduceOutput(index) => { + // Output 0 receives the remainder so the transition balances; + // other outputs get exactly amount_per_output. Use the actual + // gross amount for the targeted index when checking whether + // the fee can be absorbed. + let output_amount = if *index == 0 { + amount_per_output + remainder + } else { + amount_per_output + }; + if output_amount >= remaining_fee_to_check { + remaining_fee_to_check = 0; + fee_can_be_covered = true; + } else { + remaining_fee_to_check -= output_amount; + } + } + AddressFundsFeeStrategyStep::DeductFromInput(index) => { + if let Some(input_addr) = input_addresses_for_check.get(*index as usize) { + if let Some((_, remaining_balance)) = current_addresses_with_balance + .addresses_in_block_with_new_balance + .get(input_addr) + { + if *remaining_balance >= remaining_fee_to_check { + remaining_fee_to_check = 0; + fee_can_be_covered = true; + } else { + remaining_fee_to_check -= *remaining_balance; + } + } + } + } + } + } + + if !fee_can_be_covered { + tracing::warn!( + total_input = total_input, + estimated_fee = estimated_fee, + "AddressTransfer: insufficient remaining balance for fees, skipping" + ); + current_addresses_with_balance.addresses_in_block_with_new_balance = + staged_snapshot_before_inputs; + return None; + } + + // Create output addresses and distribute funds evenly. We defer + // staging recipient balances until fee deductions are computed so + // that staged balances reflect net post-fee credited amounts rather + // than gross transition outputs. let mut outputs = BTreeMap::new(); // Collect existing addresses that are not used as inputs (for potential reuse as outputs) @@ -2466,39 +2705,110 @@ impl NetworkStrategy { .cloned() .collect(); - for _ in 0..output_count { - // Check if we should use an existing address as output + let mut output_addresses_in_order: Vec = Vec::new(); + let mut existing_output_addresses: std::collections::HashSet = + std::collections::HashSet::new(); + + for outputs_created in 0..output_count { + // First output gets the remainder so input_sum == output_sum + let this_output_amount = if outputs_created == 0 { + amount_per_output + remainder + } else { + amount_per_output + }; + let use_existing = use_existing_outputs_chance .map(|chance| rng.gen::() < chance && !available_existing_addresses.is_empty()) .unwrap_or(false); let address = if use_existing { - // Pick a random existing address and remove it from available pool let idx = rng.gen_range(0..available_existing_addresses.len()); let existing_address = available_existing_addresses.swap_remove(idx); - // Update the balance for this existing address - if let Some((nonce, balance)) = current_addresses_with_balance - .addresses_with_balance - .get(&existing_address) + existing_output_addresses.insert(existing_address); + existing_address + } else { + signer.add_random_address_key(rng) + }; + + outputs.insert(address, this_output_amount); + output_addresses_in_order.push(address); + } + + // Calculate fee deductions based on fee_strategy. + // Track deductions for both outputs (ReduceOutput) and inputs (DeductFromInput) + let mut output_fee_deductions: BTreeMap = BTreeMap::new(); + let mut input_fee_deductions: BTreeMap = BTreeMap::new(); + let mut remaining_fee = estimated_fee; + + let input_addresses_in_order: Vec = inputs.keys().cloned().collect(); + + for step in &fee_strategy { + if remaining_fee == 0 { + break; + } + match step { + AddressFundsFeeStrategyStep::ReduceOutput(index) => { + let idx = *index as usize; + if idx < output_addresses_in_order.len() { + let output_addr = &output_addresses_in_order[idx]; + let output_amount = outputs.get(output_addr).copied().unwrap_or(0); + let deduction = remaining_fee.min(output_amount); + *output_fee_deductions.entry(idx).or_insert(0) += deduction; + remaining_fee = remaining_fee.saturating_sub(deduction); + } + } + AddressFundsFeeStrategyStep::DeductFromInput(index) => { + let idx = *index as usize; + if idx < input_addresses_in_order.len() { + let deduction = remaining_fee; + *input_fee_deductions.entry(idx).or_insert(0) += deduction; + remaining_fee = 0; + } + } + } + } + + // Apply fee deductions to INPUT balance tracking. + // The transfer amount was already deducted by take_random_amounts_with_range, + // but the fee is ALSO deducted from inputs when using DeductFromInput. + for (idx, fee_deduction) in input_fee_deductions { + if let Some(input_addr) = input_addresses_in_order.get(idx) { + if let Some((nonce, current_balance)) = current_addresses_with_balance + .addresses_in_block_with_new_balance + .get(input_addr) { + let adjusted_balance = current_balance.saturating_sub(fee_deduction); + let nonce = *nonce; current_addresses_with_balance .addresses_in_block_with_new_balance - .insert( - existing_address.clone(), - (*nonce, balance + amount_per_output), - ); + .insert(*input_addr, (nonce, adjusted_balance)); + } + } + } + + // Apply fee deductions to OUTPUT balance tracking and stage net + // post-fee credited amounts for recipients. + for (idx, address) in output_addresses_in_order.iter().enumerate() { + let transition_amount = outputs.get(address).copied().unwrap_or(0); + let fee_deduction = output_fee_deductions.get(&idx).copied().unwrap_or(0); + let actual_credited_amount = transition_amount.saturating_sub(fee_deduction); + + if existing_output_addresses.contains(address) { + // Use the effective state so any prior same-block delta + // (staged balance) is preserved before crediting. + if let Some((nonce, balance)) = + current_addresses_with_balance.get_effective(address) + { + let new_entry = (*nonce, balance + actual_credited_amount); + current_addresses_with_balance + .addresses_in_block_with_new_balance + .insert(*address, new_entry); } - existing_address } else { - // Create a new address - let new_address = signer.add_random_address_key(rng); current_addresses_with_balance .addresses_in_block_with_new_balance - .insert(new_address.clone(), (0, amount_per_output)); - new_address - }; - - outputs.insert(address, amount_per_output); + .insert(*address, (0, actual_credited_amount)); + } } let transfer_transition = AddressFundsTransferTransition::try_from_inputs_with_signer( @@ -2530,21 +2840,109 @@ impl NetworkStrategy { rng: &mut StdRng, platform_version: &PlatformVersion, ) -> Option { - let inputs = - current_addresses_with_balance.take_random_amounts_with_range(amount_range, rng)?; + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + + // Sample the optional change-output amount up front so we can reject + // sub-`min_output_amount` outputs before mutating any staged state. + // Mirror the shared crate: returning `None` here is the skip path, and + // we must avoid leaving phantom staged balances behind on that path. + let sampled_output_amount = if let Some(output_amount_range) = maybe_output_amount { + let output_amount = rng.gen_range(output_amount_range.clone()); + let min_output_amount = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + if output_amount < min_output_amount { + return None; + } + Some(output_amount) + } else { + None + }; + + let inputs = current_addresses_with_balance + .take_random_amounts_with_range_and_min_per_input(amount_range, min_per_input, rng)?; let fee_strategy = fee_strategy .clone() .unwrap_or(vec![AddressFundsFeeStrategyStep::DeductFromInput(0)]); tracing::debug!(?inputs, "Preparing address credit withdrawal transition"); - // Determine if we have an output (change address) and its amount - let output = if let Some(output_amount_range) = maybe_output_amount { - let output_amount = rng.gen_range(output_amount_range.clone()); + // Calculate estimated fee for local balance tracking. + // Fee = address_credit_withdrawal + input_cost * num_inputs + output_cost * num_outputs + let num_inputs = inputs.len() as u64; + let has_output = sampled_output_amount.is_some(); + let num_outputs: u64 = if has_output { 1 } else { 0 }; + + let min_fees = &platform_version.fee_version.state_transition_min_fees; + let estimated_fee = min_fees + .address_credit_withdrawal + .saturating_add( + min_fees + .address_funds_transfer_input_cost + .saturating_mul(num_inputs), + ) + .saturating_add( + min_fees + .address_funds_transfer_output_cost + .saturating_mul(num_outputs), + ); + + // Track fee deductions so the change output is staged at its + // post-fee credited amount and inputs absorb their share of the fee. + let mut output_fee_deduction: Credits = 0; + let mut input_fee_deductions: BTreeMap = BTreeMap::new(); + + let input_addresses_in_order: Vec = inputs.keys().cloned().collect(); + + let mut remaining_fee = estimated_fee; + for step in &fee_strategy { + if remaining_fee == 0 { + break; + } + match step { + AddressFundsFeeStrategyStep::ReduceOutput(index) => { + if *index == 0 && has_output { + output_fee_deduction = output_fee_deduction.saturating_add(remaining_fee); + remaining_fee = 0; + } + } + AddressFundsFeeStrategyStep::DeductFromInput(index) => { + if (*index as usize) < inputs.len() { + let entry = input_fee_deductions.entry(*index as usize).or_insert(0); + *entry = entry.saturating_add(remaining_fee); + remaining_fee = 0; + } + } + } + } + + // Apply fee deductions to input balances (beyond the amount already staged) + for (idx, fee_deduction) in input_fee_deductions { + if let Some(address) = input_addresses_in_order.get(idx) { + if let Some((_, staged_balance)) = current_addresses_with_balance + .addresses_in_block_with_new_balance + .get_mut(address) + { + *staged_balance = staged_balance.saturating_sub(fee_deduction); + } + } + } + + // Stage the change-output address with its post-fee credited amount + // now that inputs were taken successfully. Validation already + // happened up front. + let output = if let Some(output_amount) = sampled_output_amount { let output_address = signer.add_random_address_key(rng); + let actual_output_amount = output_amount.saturating_sub(output_fee_deduction); current_addresses_with_balance .addresses_in_block_with_new_balance - .insert(output_address.clone(), (0, output_amount)); + .insert(output_address.clone(), (0, actual_output_amount)); Some((output_address, output_amount)) } else { None @@ -2598,10 +2996,34 @@ impl NetworkStrategy { platform_version, ); + // With a single output and `ReduceOutput(0)`, the fee is taken from the + // output, so the address ends up with `funded_amount - fee` credits. + let min_fees = &platform_version.fee_version.state_transition_min_fees; + let estimated_fee = min_fees.address_funds_transfer_output_cost; // 1 output + + if funded_amount <= estimated_fee { + tracing::warn!( + "Asset lock amount {} is too small to cover fee {}", + funded_amount, + estimated_fee + ); + return None; + } + + let actual_funded_amount = funded_amount.saturating_sub(estimated_fee); + let address = signer.add_random_address_key(rng); + // Snapshot staged balances so we can fully restore them if the + // constructor fails. `register_new_address_keep_only_highest` may prune + // other staged entries when `max_addresses_to_choose_from_in_cache` is + // set, so removing only the new address would not be sufficient to + // reverse those side effects. + let staged_snapshot = current_addresses_with_balance + .addresses_in_block_with_new_balance + .clone(); current_addresses_with_balance.register_new_address_keep_only_highest( address, - funded_amount, + actual_funded_amount, self.max_addresses_to_choose_from_in_cache, ); let mut outputs = BTreeMap::new(); @@ -2609,7 +3031,7 @@ impl NetworkStrategy { tracing::debug!(?outputs, "Preparing funding transition"); let funding_transition = - AddressFundingFromAssetLockTransitionV0::try_from_asset_lock_with_signer_and_private_key( + match AddressFundingFromAssetLockTransitionV0::try_from_asset_lock_with_signer_and_private_key( asset_lock_proof, asset_lock_private_key.as_slice(), BTreeMap::new(), @@ -2620,7 +3042,19 @@ impl NetworkStrategy { platform_version, ) .await - .ok()?; + { + Ok(transition) => transition, + Err(e) => { + // Constructor failed: restore the staged map exactly as it + // was before registration so the tracker isn't left + // holding a phantom entry or missing a previously-pruned + // address. + current_addresses_with_balance.addresses_in_block_with_new_balance = + staged_snapshot; + tracing::error!("Error creating address funding transition: {:?}", e); + return None; + } + }; Some(funding_transition) } diff --git a/packages/rs-drive-abci/tests/strategy_tests/test_cases/address_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/test_cases/address_tests.rs index 5918016b0a6..55af4bd47b8 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/test_cases/address_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/test_cases/address_tests.rs @@ -881,11 +881,13 @@ mod tests { // The root hash should be valid (32 bytes) assert_eq!(root_hash.len(), 32, "root hash should be 32 bytes"); - // Verify trunk query results + // Verify trunk query results. + // Deterministic with seed=15 and the strategy above, after the + // client-side address validation constructor checks. assert_eq!( trunk_result.elements.len(), - 32, - "trunk query should return 32 elements" + 43, + "trunk query should return 43 elements" ); assert_eq!( trunk_result.leaf_keys.len(), @@ -894,8 +896,8 @@ mod tests { ); assert_eq!( trunk_result.chunk_depths, - vec![6], - "trunk query should have chunk_depths [6]" + vec![7], + "trunk query should have chunk_depths [7]" ); } @@ -1079,11 +1081,13 @@ mod tests { // The root hash should be valid (32 bytes) assert_eq!(root_hash.len(), 32, "root hash should be 32 bytes"); - // Verify trunk query results match expected values + // Verify trunk query results match expected values. + // Deterministic with seed=15 and the strategy above, after the + // client-side address validation constructor checks. assert_eq!( trunk_result.elements.len(), - 32, - "trunk query should return 32 elements after restart" + 43, + "trunk query should return 43 elements after restart" ); assert_eq!( trunk_result.leaf_keys.len(), @@ -1092,8 +1096,8 @@ mod tests { ); assert_eq!( trunk_result.chunk_depths, - vec![6], - "trunk query should have chunk_depths [6] after restart" + vec![7], + "trunk query should have chunk_depths [7] after restart" ); } @@ -2250,11 +2254,13 @@ mod tests { "trunk query should use checkpoint at height 12" ); - // Verify trunk query results + // Verify trunk query results. + // Deterministic with seed=15 and the strategy above, after the + // client-side address validation constructor checks. assert_eq!( trunk_result.elements.len(), - 32, - "trunk query should return 32 elements" + 43, + "trunk query should return 43 elements" ); assert_eq!( trunk_result.leaf_keys.len(), @@ -2263,8 +2269,8 @@ mod tests { ); assert_eq!( trunk_result.chunk_depths, - vec![6], - "trunk query should have chunk_depths [6]" + vec![7], + "trunk query should have chunk_depths [7]" ); // Verify the proof has valid quorum info @@ -3897,8 +3903,8 @@ mod tests { assert_eq!(root_hash.len(), 32, "root hash should be 32 bytes"); assert_eq!( trunk_data.elements.len(), - 32, - "phase 1: trunk should return 32 elements" + 49, + "phase 1: trunk should return 49 elements" ); // Record addresses known so far from state transitions diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/mod.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/mod.rs index 1bde330b4db..c6b6b438bde 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/mod.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/mod.rs @@ -1,4 +1,4 @@ -use versioned_feature_core::FeatureVersion; +use versioned_feature_core::{FeatureVersion, OptionalFeatureVersion}; pub mod v1; pub mod v2; @@ -20,10 +20,22 @@ pub struct IdentityTransitionVersions { pub max_public_keys_in_creation: u16, pub asset_locks: IdentityTransitionAssetLockVersions, pub credit_withdrawal: IdentityCreditWithdrawalTransitionVersions, + pub identity_update: IdentityUpdateTransitionVersions, pub calculate_min_required_fee_on_identity_create_transition: FeatureVersion, pub calculate_min_required_fee_on_identity_top_up_transition: i32, } +/// DPP-owned versioning for the identity-update state transition. +/// +/// `basic_structure` is the source of truth for which versioned basic-structure +/// check both DPP (client construction) and drive-abci (server validation) +/// should run. Keeping it in DPP avoids forcing DPP to depend on the +/// drive-abci-side version subtree just to know which structural check to use. +#[derive(Clone, Debug, Default)] +pub struct IdentityUpdateTransitionVersions { + pub basic_structure: OptionalFeatureVersion, +} + #[derive(Clone, Debug, Default)] pub struct ContractTransitionVersions { pub contract_create_transition_default_version: FeatureVersion, diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v1.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v1.rs index 07637973453..ac0478cadd4 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v1.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v1.rs @@ -3,6 +3,7 @@ use crate::version::dpp_versions::dpp_state_transition_versions::{ DocumentTransitionVersions, DocumentsBatchTransitionValidationVersions, DocumentsBatchTransitionVersions, IdentityCreditWithdrawalTransitionVersions, IdentityTransitionAssetLockVersions, IdentityTransitionVersions, + IdentityUpdateTransitionVersions, }; pub const STATE_TRANSITION_VERSIONS_V1: DPPStateTransitionVersions = DPPStateTransitionVersions { @@ -27,6 +28,9 @@ pub const STATE_TRANSITION_VERSIONS_V1: DPPStateTransitionVersions = DPPStateTra credit_withdrawal: IdentityCreditWithdrawalTransitionVersions { default_constructor: 0, }, + identity_update: IdentityUpdateTransitionVersions { + basic_structure: Some(0), + }, calculate_min_required_fee_on_identity_create_transition: 0, calculate_min_required_fee_on_identity_top_up_transition: 0, }, diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v2.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v2.rs index 8f63e97b8c6..bd832973e25 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v2.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v2.rs @@ -3,6 +3,7 @@ use crate::version::dpp_versions::dpp_state_transition_versions::{ DocumentTransitionVersions, DocumentsBatchTransitionValidationVersions, DocumentsBatchTransitionVersions, IdentityCreditWithdrawalTransitionVersions, IdentityTransitionAssetLockVersions, IdentityTransitionVersions, + IdentityUpdateTransitionVersions, }; pub const STATE_TRANSITION_VERSIONS_V2: DPPStateTransitionVersions = DPPStateTransitionVersions { @@ -27,6 +28,9 @@ pub const STATE_TRANSITION_VERSIONS_V2: DPPStateTransitionVersions = DPPStateTra credit_withdrawal: IdentityCreditWithdrawalTransitionVersions { default_constructor: 1, }, + identity_update: IdentityUpdateTransitionVersions { + basic_structure: Some(0), + }, calculate_min_required_fee_on_identity_create_transition: 0, calculate_min_required_fee_on_identity_top_up_transition: 0, }, diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v3.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v3.rs index cc43aa2771d..0b26fdf74ea 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v3.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v3.rs @@ -3,6 +3,7 @@ use crate::version::dpp_versions::dpp_state_transition_versions::{ DocumentTransitionVersions, DocumentsBatchTransitionValidationVersions, DocumentsBatchTransitionVersions, IdentityCreditWithdrawalTransitionVersions, IdentityTransitionAssetLockVersions, IdentityTransitionVersions, + IdentityUpdateTransitionVersions, }; pub const STATE_TRANSITION_VERSIONS_V3: DPPStateTransitionVersions = DPPStateTransitionVersions { @@ -27,6 +28,9 @@ pub const STATE_TRANSITION_VERSIONS_V3: DPPStateTransitionVersions = DPPStateTra credit_withdrawal: IdentityCreditWithdrawalTransitionVersions { default_constructor: 1, }, + identity_update: IdentityUpdateTransitionVersions { + basic_structure: Some(0), + }, calculate_min_required_fee_on_identity_create_transition: 1, // updated in v3 calculate_min_required_fee_on_identity_top_up_transition: 1, // updated in v3 }, diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs index 3acea0c769a..e3a27f0bb3f 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs @@ -74,6 +74,18 @@ pub struct DriveAbciStateTransitionValidationVersions { pub common_validation_methods: DriveAbciStateTransitionCommonValidationVersions, pub max_asset_lock_usage_attempts: u16, pub identity_create_state_transition: DriveAbciStateTransitionValidationVersion, + /// Legacy drive-abci validation versions for the identity-update state transition. + /// + /// The `basic_structure` subfield of this struct is legacy/deprecated for + /// identity-update basic-structure dispatch: drive-abci no longer reads it as the + /// dispatch source. The DPP-owned + /// `platform_version.dpp.state_transitions.identities.identity_update.basic_structure` + /// field is the source of truth used for dispatch. + /// + /// The remaining intentional consumer of this field is the rs-dpp lockstep assertion, + /// which cross-checks that the legacy drive-abci value matches the DPP-owned value until + /// the legacy field can be removed/migrated. Do not wire new dispatch logic to this + /// field. pub identity_update_state_transition: DriveAbciStateTransitionValidationVersion, pub identity_top_up_state_transition: DriveAbciStateTransitionValidationVersion, pub identity_credit_withdrawal_state_transition: DriveAbciStateTransitionValidationVersion, diff --git a/packages/strategy-tests/src/addresses_with_balance.rs b/packages/strategy-tests/src/addresses_with_balance.rs index a6486dafbee..e949442a8d2 100644 --- a/packages/strategy-tests/src/addresses_with_balance.rs +++ b/packages/strategy-tests/src/addresses_with_balance.rs @@ -385,6 +385,33 @@ impl AddressesWithBalance { &mut self, range: &AmountRange, rng: &mut R, + ) -> Option> { + self.take_random_amounts_with_range_and_min_per_input(range, 1, rng) + } + + /// Like `take_random_amounts_with_range`, but enforces a minimum amount per + /// individual input. This is needed when client-side validation rejects + /// inputs below `min_input_amount`. + /// + /// # Atomicity + /// + /// On any failure path (insufficient funds, picker exhaustion, inability to + /// satisfy `min_per_input` for a step) the staged map is restored to the + /// state it had on entry. Callers that treat `None` as "no transition + /// emitted" therefore never observe partially staged balances/nonces. + /// + /// # Availability + /// + /// The upfront availability check counts only the funds the picker is + /// allowed to draw from — committed addresses not already staged. Funds in + /// `addresses_in_block_with_new_balance` are deliberately excluded because + /// the picker cannot reuse those addresses within the same block (one + /// transaction per address per block, for nonce safety). + pub fn take_random_amounts_with_range_and_min_per_input( + &mut self, + range: &AmountRange, + min_per_input: Credits, + rng: &mut R, ) -> Option> { let range_min = *range.start(); let range_max = *range.end(); @@ -392,20 +419,20 @@ impl AddressesWithBalance { return None; } - // 1. Compute total available effective balance - let mut total_available: Credits = 0; + // Snapshot the staged map so we can roll back on any failure path, + // keeping this helper atomic from the caller's point of view. + let staged_snapshot = self.addresses_in_block_with_new_balance.clone(); - // in-block first (overrides) - for (_nonce, credits) in self.addresses_in_block_with_new_balance.values() { - total_available += *credits; - } - - // committed, skipping overridden - for (addr, (_nonce, credits)) in &self.addresses_with_balance { - if !self.addresses_in_block_with_new_balance.contains_key(addr) { - total_available += *credits; - } - } + // 1. Compute total available effective balance from the same set the + // picker will draw from: committed addresses not already staged. + // Staged funds are excluded because the picker won't reuse those + // addresses this block (nonce safety). + let total_available: Credits = self + .addresses_with_balance + .iter() + .filter(|(addr, _)| !self.addresses_in_block_with_new_balance.contains_key(*addr)) + .map(|(_, (_, credits))| *credits) + .fold(0, |acc, b| acc.saturating_add(b)); if total_available < range_min { return None; @@ -416,6 +443,7 @@ impl AddressesWithBalance { let mut taken_total: Credits = 0; let mut result: BTreeMap = BTreeMap::new(); + let mut failed = false; loop { // If we've hit the absolute upper bound, we must stop. @@ -426,30 +454,37 @@ impl AddressesWithBalance { // Remaining room we are allowed to take let remaining_max = global_max - taken_total; - // While we haven't reached the minimum yet, we must ensure we don't - // choose too tiny amounts. Once we hit range_min, we can be looser. + // Per-step min must be at least `min_per_input` (validation floor) + // AND at least enough that future picks can reach `range_min`. We + // never clamp it down to `remaining_max`: doing so would let the + // picker take a sub-`min_per_input` input, which the caller's + // client-side validation would reject. let remaining_to_min = range_min.saturating_sub(taken_total); - - // Per-step min: - // - at least 1 - // - at least enough so we can eventually reach range_min - // - but not more than remaining_max - let step_min = remaining_to_min.max(1).min(remaining_max); - - // Per-step max is whatever room is left - let step_max = remaining_max; - - if step_min == 0 || step_min > step_max { - // Can't take any more without violating bounds + let step_min = remaining_to_min.max(min_per_input); + + if step_min > remaining_max { + // We can't take another input without either dropping below + // `min_per_input` or exceeding `global_max`. If we already met + // `range_min` this is a clean stop; otherwise we cannot reach + // it from here and must fail. + if taken_total < range_min { + failed = true; + } break; } + let step_max = remaining_max; + // Use the internal bounded helper for this step let maybe = self.take_random_amount_with_bounds(step_min, step_max, rng); let (addr, new_nonce, taken) = match maybe { Some(triplet) => triplet, None => { - // No address can satisfy this step; bail out + // No address can satisfy this step. If we still haven't + // reached `range_min`, the overall call has failed. + if taken_total < range_min { + failed = true; + } break; } }; @@ -467,10 +502,10 @@ impl AddressesWithBalance { } } - if taken_total < range_min { - // We failed to reach the minimum; you could roll back changes here - // if you keep a snapshot of balances, but for now just signal None. - // NOTE: if you *need* strict atomicity, we should add snapshot/rollback. + if failed || taken_total < range_min { + // Restore staged state so the caller (which treats `None` as + // "no transition emitted") never sees corrupted balances/nonces. + self.addresses_in_block_with_new_balance = staged_snapshot; return None; } @@ -593,3 +628,113 @@ impl AddressesWithBalance { } } } + +#[cfg(test)] +mod tests { + use super::*; + use rand::prelude::StdRng; + use rand::SeedableRng; + + fn addr(byte: u8) -> PlatformAddress { + PlatformAddress::P2pkh([byte; 20]) + } + + /// `min_per_input` must always be honoured by the picker. Old code computed + /// `step_min = remaining_to_min.max(min_per_input).min(remaining_max)`, + /// which clamped `step_min` *below* `min_per_input` whenever + /// `remaining_max < min_per_input`. The picker would then happily return an + /// input below the validation floor. This test pins that down: with + /// `range_max < min_per_input` and an address barely above `range_max`, + /// old code would hand back a 70_000 input despite a 100_000 floor. + #[test] + fn min_per_input_is_never_violated_when_remaining_room_is_small() { + let mut tracker = AddressesWithBalance::new(); + tracker.addresses_with_balance.insert(addr(1), (0, 80_000)); + + let mut rng = StdRng::seed_from_u64(0xA11CE); + let range: AmountRange = 50_000..=70_000; + let result = + tracker.take_random_amounts_with_range_and_min_per_input(&range, 100_000, &mut rng); + + // No way to take >= 100_000 without exceeding range_max=70_000. + assert!(result.is_none()); + // Staged map must be untouched on failure. + assert!(tracker.addresses_in_block_with_new_balance.is_empty()); + // Committed map must also be untouched. + assert_eq!( + tracker.addresses_with_balance.get(&addr(1)), + Some(&(0, 80_000)) + ); + } + + /// When the first input does not by itself reach `range_min` and no other + /// committed address can satisfy `min_per_input`, the helper must roll back. + #[test] + fn failure_is_atomic_when_second_input_unavailable() { + let mut tracker = AddressesWithBalance::new(); + tracker.addresses_with_balance.insert(addr(1), (5, 150_000)); + tracker.addresses_with_balance.insert(addr(2), (7, 80_000)); + + let snapshot_committed = tracker.addresses_with_balance.clone(); + + let mut rng = StdRng::seed_from_u64(0xBEEF); + let range: AmountRange = 200_000..=300_000; + let result = + tracker.take_random_amounts_with_range_and_min_per_input(&range, 100_000, &mut rng); + + assert!(result.is_none()); + assert!(tracker.addresses_in_block_with_new_balance.is_empty()); + assert_eq!(tracker.addresses_with_balance, snapshot_committed); + } + + /// Availability accounting must match what the picker can actually draw + /// from. Staged-only funds must not make the upfront check pass when no + /// committed eligible funds remain. + #[test] + fn availability_excludes_staged_only_funds() { + let mut tracker = AddressesWithBalance::new(); + // Plenty of staged funds — but the picker won't reuse staged addresses + // this block (nonce safety), so they must not count toward availability. + tracker + .addresses_in_block_with_new_balance + .insert(addr(9), (3, 500_000)); + // Only committed eligible balance — far below `range_min`. + tracker.addresses_with_balance.insert(addr(1), (0, 50_000)); + + let staged_before = tracker.addresses_in_block_with_new_balance.clone(); + + let mut rng = StdRng::seed_from_u64(0xCAFE); + let range: AmountRange = 200_000..=300_000; + let result = + tracker.take_random_amounts_with_range_and_min_per_input(&range, 100_000, &mut rng); + + assert!(result.is_none()); + // Staged map untouched: no spurious mutations. + assert_eq!(tracker.addresses_in_block_with_new_balance, staged_before); + // Committed map untouched. + assert_eq!( + tracker.addresses_with_balance.get(&addr(1)), + Some(&(0, 50_000)) + ); + } + + /// Sanity check the success path is unaffected by the new atomicity logic. + #[test] + fn success_path_returns_inputs_above_min_per_input() { + let mut tracker = AddressesWithBalance::new(); + tracker.addresses_with_balance.insert(addr(1), (0, 200_000)); + tracker.addresses_with_balance.insert(addr(2), (0, 200_000)); + + let mut rng = StdRng::seed_from_u64(0xD00D); + let range: AmountRange = 150_000..=250_000; + let result = tracker + .take_random_amounts_with_range_and_min_per_input(&range, 100_000, &mut rng) + .expect("should succeed"); + + let total: Credits = result.values().map(|(_, c)| *c).sum(); + assert!(total >= 150_000 && total <= 250_000); + for (_, taken) in result.values() { + assert!(*taken >= 100_000, "input {} below min_per_input", taken); + } + } +} diff --git a/packages/strategy-tests/src/lib.rs b/packages/strategy-tests/src/lib.rs index fb3f104c226..0e8c90f8fd6 100644 --- a/packages/strategy-tests/src/lib.rs +++ b/packages/strategy-tests/src/lib.rs @@ -751,9 +751,27 @@ impl Strategy { AssetLockProof::Chain(_) => self.start_addresses.starting_balance, }; - // Create a new address for the funding + // Calculate estimated fee for local balance tracking. With a single + // output and `ReduceOutput(0)`, the fee is deducted from the output, + // so the address ends up with `funded_amount - fee` credits. + let min_fees = &platform_version.fee_version.state_transition_min_fees; + let estimated_fee = min_fees.address_funds_transfer_output_cost; // 1 output + + if funded_amount <= estimated_fee { + tracing::warn!( + "Asset lock amount {} is too small to cover fee {} for start_address", + funded_amount, + estimated_fee + ); + continue; + } + + let actual_funded_amount = funded_amount.saturating_sub(estimated_fee); + + // Create a new address for the funding. We only register it after the + // transition is successfully constructed, so a constructor failure does + // not leave a staged address with an over-credited balance. let address = signer.add_random_address_key(rng); - current_addresses_with_balance.register_new_address(address, funded_amount); let mut outputs = BTreeMap::new(); outputs.insert(address, None); @@ -772,7 +790,11 @@ impl Strategy { .await; match funding_transition { - Ok(transition) => state_transitions.push(transition), + Ok(transition) => { + current_addresses_with_balance + .register_new_address(address, actual_funded_amount); + state_transitions.push(transition); + } Err(e) => { tracing::error!( "Error creating address funding transition for start_address: {:?}", @@ -1478,8 +1500,17 @@ impl Strategy { .collect(); for random_identity in random_identities { + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; let Some(inputs) = current_addresses_with_balance - .take_random_amounts_with_range(amount_range, rng) + .take_random_amounts_with_range_and_min_per_input( + amount_range, + min_per_input, + rng, + ) else { // no funds left break; @@ -1875,18 +1906,40 @@ impl Strategy { "Expected to find sender identity in hardcoded start identities", ); - let (state_transition, _recipient_addresses) = - crate::transitions::create_identity_credit_transfer_to_addresses_transition( + // Use the pre-specified outputs from transfer_info so + // the caller's recipient addresses/amounts are honored + // instead of being replaced with random ones. + // The helper returns `None` if any output is below the + // per-output minimum; in that case skip without staging + // any balances to avoid phantom in-block state. + if let Some(state_transition) = + crate::transitions::create_identity_credit_transfer_to_addresses_transition_with_outputs( sender, identity_nonce_counter, - current_addresses_with_balance, signer, - transfer_info.amount, - transfer_info.outputs.len(), - rng, + transfer_info.outputs.clone(), platform_version, - ).await; - operations.push(state_transition); + ).await + { + // Stage recipient balances now that the transition + // was constructed successfully. Mirrors the + // random-output branch below: existing addresses + // get their balance increased, new ones registered. + for (recipient_address, amount) in &transfer_info.outputs { + if let Some((nonce, balance)) = current_addresses_with_balance + .get_effective(recipient_address) + { + let new_entry = (*nonce, balance + amount); + current_addresses_with_balance + .addresses_in_block_with_new_balance + .insert(*recipient_address, new_entry); + } else { + current_addresses_with_balance + .register_new_address(*recipient_address, *amount); + } + } + operations.push(state_transition); + } } else { // Handle the case where no specific sender is provided let identities_count = current_identities.len(); @@ -1901,9 +1954,24 @@ impl Strategy { // Get random amount within range let amount = rng.gen_range(amount_range.clone()); - // Get random output count within range + let min_per_output = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + + // Skip if the chosen amount cannot fund even a single + // valid output (constructor would otherwise reject it). + if amount < min_per_output { + continue; + } + + // Get random output count within range, capped so each + // output is at least min_output_amount. + let max_outputs_by_amount = (amount / min_per_output) as usize; let output_count = - rng.gen_range(output_count_range.clone()).max(1) as usize; + (rng.gen_range(output_count_range.clone()).max(1) as usize) + .min(max_outputs_by_amount); // Calculate amount per output let amount_per_output = amount / output_count as u64; @@ -1932,18 +2000,20 @@ impl Strategy { rng.gen_range(0..available_existing_addresses.len()); let existing_address = available_existing_addresses.swap_remove(idx); - // Update the balance for this existing address + // Update the balance for this existing address. + // Use the effective (committed + same-block staged) + // balance so subsequent recipients within the same + // block accumulate on top of prior deltas instead + // of overwriting them. Mirrors the explicit-output + // branch above. if let Some((nonce, balance)) = current_addresses_with_balance - .addresses_with_balance - .get(&existing_address) + .get_effective(&existing_address) { + let new_entry = (*nonce, balance + amount_per_output); current_addresses_with_balance .addresses_in_block_with_new_balance - .insert( - existing_address, - (*nonce, balance + amount_per_output), - ); + .insert(existing_address, new_entry); } existing_address } else { @@ -1957,15 +2027,17 @@ impl Strategy { recipient_addresses.insert(address, amount_per_output); } - let state_transition = + if let Some(state_transition) = crate::transitions::create_identity_credit_transfer_to_addresses_transition_with_outputs( sender, identity_nonce_counter, signer, recipient_addresses, platform_version, - ).await; - operations.push(state_transition); + ).await + { + operations.push(state_transition); + } } } } @@ -1979,8 +2051,25 @@ impl Strategy { extra_keys, ) => { for _i in 0..count { + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + // Snapshot the staged map BEFORE picking inputs so any + // rollback restores the exact pre-staging state. Removing + // only `inputs.keys()` would be unsafe if the picker ever + // reuses same-block addresses or if other paths stage + // entries we shouldn't drop. + let staged_snapshot_before_inputs = current_addresses_with_balance + .addresses_in_block_with_new_balance + .clone(); let Some(inputs) = current_addresses_with_balance - .take_random_amounts_with_range(amount_range, rng) + .take_random_amounts_with_range_and_min_per_input( + amount_range, + min_per_input, + rng, + ) else { // no funds left break; @@ -2097,9 +2186,32 @@ impl Strategy { } } + // Sample the optional output amount and validate it against + // min_output_amount before registering any output address. The + // constructor rejects optional outputs below this minimum, so + // skip the candidate (rolling back staged input balances) instead + // of panicking on the expect below. + let sampled_output_amount = + if let Some(output_range) = maybe_output_amount.as_ref() { + let amount = rng.gen_range(output_range.clone()); + let min_per_output = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + if amount < min_per_output { + current_addresses_with_balance + .addresses_in_block_with_new_balance = + staged_snapshot_before_inputs; + continue; + } + Some(amount) + } else { + None + }; + // Create output if maybe_output_amount is provided - let output = maybe_output_amount.as_ref().map(|output_range| { - let output_amount = rng.gen_range(output_range.clone()); + let output = sampled_output_amount.map(|output_amount| { let output_address = signer.add_random_address_key(rng); // Register the output address with balance minus fee deduction let actual_output_amount = @@ -2176,6 +2288,13 @@ impl Strategy { // Register with the actual amount after fee deduction (ReduceOutput(0)) let address = signer.add_random_address_key(rng); let actual_funded_amount = funded_amount.saturating_sub(estimated_fee); + // Snapshot staged balances so we can roll back if the + // constructor rejects the transition. Otherwise the + // freshly registered address would persist as a phantom + // staged balance that no on-chain state will ever match. + let staged_snapshot = current_addresses_with_balance + .addresses_in_block_with_new_balance + .clone(); current_addresses_with_balance .register_new_address(address, actual_funded_amount); @@ -2201,6 +2320,10 @@ impl Strategy { "Error creating address funding transition: {:?}", e ); + // Restore staged balances so the failed attempt + // does not leave a phantom entry behind. + current_addresses_with_balance + .addresses_in_block_with_new_balance = staged_snapshot; continue; } } @@ -2215,8 +2338,25 @@ impl Strategy { fee_strategy, ) => { for _i in 0..count { + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + // Snapshot the staged map BEFORE picking inputs so any + // rollback restores the exact pre-staging state. Removing + // only `inputs.keys()` would be unsafe if the picker ever + // reuses same-block addresses or if other paths stage + // entries we shouldn't drop. + let staged_snapshot_before_inputs = current_addresses_with_balance + .addresses_in_block_with_new_balance + .clone(); let Some(inputs) = current_addresses_with_balance - .take_random_amounts_with_range(amount_range, rng) + .take_random_amounts_with_range_and_min_per_input( + amount_range, + min_per_input, + rng, + ) else { eprintln!("no funds left on block {}", block_info.height); // no funds left @@ -2231,9 +2371,28 @@ impl Strategy { let total_input: Credits = inputs.values().map(|(_, credits)| credits).sum(); - // Generate random number of outputs within the specified range - let output_count = - rng.gen_range(output_count_range.clone()).max(1) as usize; + let min_per_output = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + + // If total_input is below min_output_amount, we cannot build + // even a single valid output. Roll back staged input balances + // and skip this candidate instead of producing an invalid one. + if total_input < min_per_output { + current_addresses_with_balance + .addresses_in_block_with_new_balance = + staged_snapshot_before_inputs; + continue; + } + + // Generate random number of outputs within the specified range, + // capped so each output is >= min_output_amount. + let max_outputs_by_amount = (total_input / min_per_output) as usize; + let output_count = (rng.gen_range(output_count_range.clone()).max(1) + as usize) + .min(max_outputs_by_amount); // Calculate estimated fee for local balance tracking // The transition must have inputs == outputs (balanced), but the chain @@ -2269,12 +2428,20 @@ impl Strategy { break; } match step { - AddressFundsFeeStrategyStep::ReduceOutput(_index) => { - // For ReduceOutput, the output amount must cover the fee - // Since we're distributing total_input evenly, check if - // the output amount is enough - let output_amount = - total_input / output_count.max(1) as Credits; + AddressFundsFeeStrategyStep::ReduceOutput(index) => { + // For ReduceOutput, the output amount at the targeted + // index must cover the fee. Outputs are distributed + // evenly with the remainder going to output 0, so the + // gross amount at index 0 differs from the rest when + // total_input is not evenly divisible. + let output_count_credits = output_count.max(1) as Credits; + let amount_per_output = total_input / output_count_credits; + let remainder = total_input % output_count_credits; + let output_amount = if *index == 0 { + amount_per_output + remainder + } else { + amount_per_output + }; if output_amount >= remaining_fee_to_check { remaining_fee_to_check = 0; fee_can_be_covered = true; @@ -2447,17 +2614,16 @@ impl Strategy { transition_amount.saturating_sub(fee_deduction); if existing_output_addresses.contains(address) { - // Update balance for existing address - if let Some((nonce, balance)) = current_addresses_with_balance - .addresses_with_balance - .get(address) + // Update balance for existing address using the + // effective state so any prior same-block delta + // (staged balance) is preserved before crediting. + if let Some((nonce, balance)) = + current_addresses_with_balance.get_effective(address) { + let new_entry = (*nonce, balance + actual_credited_amount); current_addresses_with_balance .addresses_in_block_with_new_balance - .insert( - *address, - (*nonce, balance + actual_credited_amount), - ); + .insert(*address, new_entry); } } else { // New address - track with fee-adjusted amount @@ -2514,8 +2680,25 @@ impl Strategy { fee_strategy, ) => { for _i in 0..count { + let min_per_input = platform_version + .dpp + .state_transitions + .address_funds + .min_input_amount; + // Snapshot the staged map BEFORE picking inputs so any + // rollback restores the exact pre-staging state. Removing + // only `inputs.keys()` would be unsafe if the picker ever + // reuses same-block addresses or if other paths stage + // entries we shouldn't drop. + let staged_snapshot_before_inputs = current_addresses_with_balance + .addresses_in_block_with_new_balance + .clone(); let Some(inputs) = current_addresses_with_balance - .take_random_amounts_with_range(amount_range, rng) + .take_random_amounts_with_range_and_min_per_input( + amount_range, + min_per_input, + rng, + ) else { // no funds left break; @@ -2593,9 +2776,32 @@ impl Strategy { } } + // Sample the optional change output amount and validate it + // against min_output_amount before staging an output address. + // The constructor rejects optional outputs below this minimum, + // so skip the candidate (rolling back staged input balances) + // instead of panicking on the expect below. + let sampled_output_amount = + if let Some(output_amount_range) = maybe_output_range { + let amount = rng.gen_range(output_amount_range.clone()); + let min_per_output = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + if amount < min_per_output { + current_addresses_with_balance + .addresses_in_block_with_new_balance = + staged_snapshot_before_inputs; + continue; + } + Some(amount) + } else { + None + }; + // Determine if we have an output (change address) and its amount - let output = if let Some(output_amount_range) = maybe_output_range { - let output_amount = rng.gen_range(output_amount_range.clone()); + let output = sampled_output_amount.map(|output_amount| { let output_address = signer.add_random_address_key(rng); // Register with actual amount after fee deduction let actual_output_amount = @@ -2603,10 +2809,8 @@ impl Strategy { current_addresses_with_balance .addresses_in_block_with_new_balance .insert(output_address, (0, actual_output_amount)); - Some((output_address, output_amount)) - } else { - None - }; + (output_address, output_amount) + }); // Generate a random output script for the withdrawal let output_script = if rng.gen_bool(0.5) { diff --git a/packages/strategy-tests/src/transitions.rs b/packages/strategy-tests/src/transitions.rs index ffe2b9bc227..834b67c49d4 100644 --- a/packages/strategy-tests/src/transitions.rs +++ b/packages/strategy-tests/src/transitions.rs @@ -924,6 +924,12 @@ pub async fn create_identity_credit_transfer_transition( /// This function transfers credits from the sender's identity to newly created addresses. /// The total amount is distributed evenly among the specified number of output addresses. /// +/// Each recipient output must be at least +/// `platform_version.dpp.state_transitions.address_funds.min_output_amount` +/// (client-side validation rejects smaller outputs). The requested `output_count` +/// is therefore capped by `total_amount / min_output_amount`, and `None` is +/// returned if `total_amount < min_output_amount` (no valid output is possible). +/// /// # Parameters /// - `identity`: The identity sending the credits. /// - `identity_nonce_counter`: A mutable reference to track nonces for each identity. @@ -933,9 +939,9 @@ pub async fn create_identity_credit_transfer_transition( /// - `rng`: A mutable reference to a random number generator. /// /// # Returns -/// A tuple containing: -/// 1. `StateTransition`: The signed state transition. -/// 2. `BTreeMap`: The recipient addresses and their amounts. +/// `Some((transition, recipient_addresses))` on success, or `None` if +/// `total_amount` is below the per-output minimum and no valid transition can +/// be constructed. /// /// # Panics /// This function may panic if: @@ -951,12 +957,27 @@ pub async fn create_identity_credit_transfer_to_addresses_transition( output_count: usize, rng: &mut StdRng, platform_version: &PlatformVersion, -) -> (StateTransition, BTreeMap) { +) -> Option<(StateTransition, BTreeMap)> { + let min_output_amount = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + + // Without enough credits to fund a single valid output, the constructor + // would reject the transition. Skip rather than build an invalid one. + if total_amount < min_output_amount { + return None; + } + + // Cap requested output count so each output is at least min_output_amount. + let max_outputs_by_amount = (total_amount / min_output_amount) as usize; + let output_count = output_count.max(1).min(max_outputs_by_amount); + let nonce = identity_nonce_counter.entry(identity.id()).or_default(); *nonce += 1; // Create output addresses and distribute funds evenly - let output_count = output_count.max(1); let amount_per_output = total_amount / output_count as u64; let mut recipient_addresses = BTreeMap::new(); @@ -979,13 +1000,20 @@ pub async fn create_identity_credit_transfer_to_addresses_transition( .await .expect("expected to create transfer to addresses transition"); - (transition, recipient_addresses) + Some((transition, recipient_addresses)) } /// Creates a state transition to transfer credits from an identity to specific addresses. /// /// This function transfers credits from the sender's identity to pre-specified addresses. /// +/// Each recipient output must be at least +/// `platform_version.dpp.state_transitions.address_funds.min_output_amount` +/// (client-side validation rejects smaller outputs). If `recipient_addresses` +/// is empty or any output amount is below the per-output minimum, this +/// function returns `None` rather than constructing an invalid transition +/// that the constructor would reject. +/// /// # Parameters /// - `identity`: The identity sending the credits. /// - `identity_nonce_counter`: A mutable reference to track nonces for each identity. @@ -994,7 +1022,8 @@ pub async fn create_identity_credit_transfer_to_addresses_transition( /// - `platform_version`: The platform version. /// /// # Returns -/// The signed state transition. +/// `Some(transition)` on success, or `None` if any output amount is below the +/// per-output minimum (and therefore no valid transition can be constructed). /// /// # Panics /// This function may panic if: @@ -1006,11 +1035,27 @@ pub async fn create_identity_credit_transfer_to_addresses_transition_with_output signer: &mut SimpleSigner, recipient_addresses: BTreeMap, platform_version: &PlatformVersion, -) -> StateTransition { +) -> Option { + let min_output_amount = platform_version + .dpp + .state_transitions + .address_funds + .min_output_amount; + + // Mirror the random-output helper: reject inputs the constructor would + // reject, returning `None` instead of panicking via `.expect(...)`. + if recipient_addresses.is_empty() + || recipient_addresses + .values() + .any(|amount| *amount < min_output_amount) + { + return None; + } + let nonce = identity_nonce_counter.entry(identity.id()).or_default(); *nonce += 1; - IdentityCreditTransferToAddressesTransition::try_from_identity( + let transition = IdentityCreditTransferToAddressesTransition::try_from_identity( identity, recipient_addresses, 0, // user_fee_increase @@ -1021,7 +1066,9 @@ pub async fn create_identity_credit_transfer_to_addresses_transition_with_output None, // version ) .await - .expect("expected to create transfer to addresses transition") + .expect("expected to create transfer to addresses transition"); + + Some(transition) } /// Generates a specified number of new identities and their corresponding state transitions. diff --git a/packages/wasm-dpp/src/errors/from.rs b/packages/wasm-dpp/src/errors/from.rs index ca7d7416f23..db588615060 100644 --- a/packages/wasm-dpp/src/errors/from.rs +++ b/packages/wasm-dpp/src/errors/from.rs @@ -9,12 +9,16 @@ use crate::document::errors::from_document_to_js_error; use crate::errors::value_error::PlatformValueErrorWasm; use super::data_contract_not_present_error::DataContractNotPresentNotConsensusErrorWasm; +use super::protocol_error::consensus_errors_to_js_error; use crate::errors::consensus::consensus_error::from_consensus_error; pub fn from_dpp_err(pe: ProtocolError) -> JsValue { match pe { // TODO(versioning): restore this ProtocolError::ConsensusError(consensus_error) => from_consensus_error(*consensus_error), + ProtocolError::ConsensusErrors(consensus_errors) => { + consensus_errors_to_js_error(consensus_errors) + } ProtocolError::DataContractError(e) => from_data_contract_to_js_error(e), ProtocolError::Document(e) => from_document_to_js_error(*e), diff --git a/packages/wasm-dpp/src/errors/protocol_error.rs b/packages/wasm-dpp/src/errors/protocol_error.rs index a91e8820a55..95d131fe418 100644 --- a/packages/wasm-dpp/src/errors/protocol_error.rs +++ b/packages/wasm-dpp/src/errors/protocol_error.rs @@ -1,12 +1,34 @@ +use js_sys::Array; +use js_sys::Error; +use js_sys::Reflect; use wasm_bindgen::JsValue; use crate::errors::consensus::consensus_error::from_consensus_error; +pub(crate) fn consensus_errors_to_js_error( + consensus_errors: Vec, +) -> JsValue { + let is_empty = consensus_errors.is_empty(); + let errors = Array::from_iter(consensus_errors.into_iter().map(from_consensus_error)); + let error = Error::new(if is_empty { + "Protocol error contained an empty consensus error list" + } else { + "Multiple consensus errors" + }); + error.set_name("ConsensusErrors"); + let error_value: JsValue = error.into(); + let _ = Reflect::set(&error_value, &JsValue::from_str("errors"), &errors.into()); + error_value +} + pub fn from_protocol_error(protocol_error: dpp::ProtocolError) -> JsValue { match protocol_error { dpp::ProtocolError::ConsensusError(consensus_error) => { from_consensus_error(*consensus_error) } + dpp::ProtocolError::ConsensusErrors(consensus_errors) => { + consensus_errors_to_js_error(consensus_errors) + } dpp::ProtocolError::Error(anyhow_error) => { format!("Non-protocol error: {}", anyhow_error).into() } diff --git a/packages/wasm-sdk/src/error.rs b/packages/wasm-sdk/src/error.rs index c8caa5e7608..c8ecf942dbf 100644 --- a/packages/wasm-sdk/src/error.rs +++ b/packages/wasm-sdk/src/error.rs @@ -1,7 +1,9 @@ +use dash_sdk::dpp::consensus::codes::ErrorWithCode; use dash_sdk::dpp::ProtocolError; use dash_sdk::{error::StateTransitionBroadcastError, Error as SdkError}; +use js_sys::{Array, Object, Reflect}; use rs_dapi_client::CanRetry; -use wasm_bindgen::prelude::wasm_bindgen; +use wasm_bindgen::{prelude::wasm_bindgen, JsValue}; use wasm_dpp2::error::WasmDppError; /// Structured error surfaced to JS consumers @@ -57,6 +59,8 @@ pub struct WasmSdkError { code: i32, /// Indicates if the operation can be retried safely. is_retriable: bool, + /// Optional machine-readable details for JS consumers. + details: JsValue, } // wasm-bindgen getters defined below in the second impl block @@ -73,6 +77,7 @@ impl WasmSdkError { message: message.into(), code: code.unwrap_or(-1), is_retriable, + details: JsValue::UNDEFINED, } } @@ -134,7 +139,7 @@ impl From for WasmSdkError { None, retriable, ), - Protocol(e) => Self::new(WasmSdkErrorKind::Protocol, e.to_string(), None, retriable), + Protocol(e) => WasmSdkError::from(e), Proof(e) => Self::new(WasmSdkErrorKind::Proof, e.to_string(), None, retriable), InvalidProvedResponse(msg) => Self::new( WasmSdkErrorKind::InvalidProvedResponse, @@ -238,7 +243,96 @@ impl From for WasmSdkError { } impl From for WasmSdkError { fn from(err: ProtocolError) -> Self { - Self::new(WasmSdkErrorKind::Protocol, err.to_string(), None, false) + match err { + ProtocolError::ConsensusError(error) => { + Self::protocol_with_consensus_errors(vec![*error]) + } + ProtocolError::ConsensusErrors(errors) => Self::protocol_with_consensus_errors(errors), + other => Self::new(WasmSdkErrorKind::Protocol, other.to_string(), None, false), + } + } +} + +impl WasmSdkError { + fn protocol_consensus_error_message(errors: &[T]) -> Option { + errors.first().map(ToString::to_string) + } + + fn protocol_with_consensus_errors( + errors: Vec, + ) -> Self { + if errors.is_empty() { + let details = Object::new(); + let _ = Reflect::set( + &details, + &JsValue::from_str("type"), + &JsValue::from_str("ConsensusErrors"), + ); + let _ = Reflect::set( + &details, + &JsValue::from_str("messages"), + &Array::new().into(), + ); + let _ = Reflect::set(&details, &JsValue::from_str("errors"), &Array::new().into()); + + let mut error = Self::new( + WasmSdkErrorKind::Protocol, + "Protocol error contained an empty consensus error list", + None, + false, + ); + error.details = details.into(); + return error; + } + + let details = Object::new(); + let messages = Array::new(); + let structured_errors = Array::new(); + + for error in &errors { + messages.push(&JsValue::from_str(&error.to_string())); + + let structured_error = Object::new(); + let _ = Reflect::set( + &structured_error, + &JsValue::from_str("message"), + &JsValue::from_str(&error.to_string()), + ); + let _ = Reflect::set( + &structured_error, + &JsValue::from_str("code"), + &JsValue::from_f64(error.code() as f64), + ); + structured_errors.push(&structured_error.into()); + } + + let kind = if errors.len() == 1 { + "ConsensusError" + } else { + "ConsensusErrors" + }; + let message = Self::protocol_consensus_error_message(&errors) + .expect("consensus errors should be non-empty after the empty-list guard"); + + let _ = Reflect::set( + &details, + &JsValue::from_str("type"), + &JsValue::from_str(kind), + ); + let _ = Reflect::set(&details, &JsValue::from_str("messages"), &messages.into()); + let _ = Reflect::set( + &details, + &JsValue::from_str("errors"), + &structured_errors.into(), + ); + + Self { + kind: WasmSdkErrorKind::Protocol, + message, + code: -1, + is_retriable: false, + details: details.into(), + } } } @@ -331,4 +425,35 @@ impl WasmSdkError { pub fn is_retriable(&self) -> bool { self.is_retriable } + + /// Optional machine-readable details for JS callers. + #[wasm_bindgen(getter)] + pub fn details(&self) -> JsValue { + self.details.clone() + } +} + +#[cfg(test)] +mod tests { + use super::WasmSdkError; + + #[test] + fn protocol_consensus_error_message_uses_first_error() { + let errors = ["first error", "second error"]; + + assert_eq!( + WasmSdkError::protocol_consensus_error_message(&errors), + Some("first error".to_string()) + ); + } + + #[test] + fn protocol_consensus_error_message_is_none_for_empty_input() { + let errors: [&str; 0] = []; + + assert_eq!( + WasmSdkError::protocol_consensus_error_message(&errors), + None + ); + } } diff --git a/packages/wasm-sdk/src/state_transitions/addresses.rs b/packages/wasm-sdk/src/state_transitions/addresses.rs index be9c2e9aa71..640b5a9e6e2 100644 --- a/packages/wasm-sdk/src/state_transitions/addresses.rs +++ b/packages/wasm-sdk/src/state_transitions/addresses.rs @@ -175,7 +175,7 @@ impl WasmSdk { .inner_sdk() .transfer_address_funds(inputs_map, outputs_map, fee_strategy, &signer, settings) .await - .map_err(|e| WasmSdkError::generic(format!("Failed to transfer funds: {}", e)))?; + .map_err(WasmSdkError::from)?; address_infos_to_js_map(address_infos, "transfer") } @@ -298,7 +298,7 @@ impl WasmSdk { let (address_infos, new_balance) = identity .top_up_from_addresses(self.inner_sdk(), inputs_map, &signer, settings) .await - .map_err(|e| WasmSdkError::generic(format!("Failed to top up identity: {}", e)))?; + .map_err(WasmSdkError::from)?; Ok(IdentityTopUpFromAddressesResultWasm { address_infos: address_infos_to_js_map(address_infos, "top up")?, @@ -464,7 +464,7 @@ impl WasmSdk { settings, ) .await - .map_err(|e| WasmSdkError::generic(format!("Failed to withdraw funds: {}", e)))?; + .map_err(WasmSdkError::from)?; address_infos_to_js_map(address_infos, "withdrawal") } @@ -516,9 +516,7 @@ impl WasmSdk { settings, ) .await - .map_err(|e| { - WasmSdkError::generic(format!("Failed to transfer credits to addresses: {}", e)) - })?; + .map_err(WasmSdkError::from)?; Ok(IdentityTransferToAddressesResultWasm { address_infos: address_infos_to_js_map(address_infos, "transfer")?, @@ -756,7 +754,7 @@ impl WasmSdk { settings, ) .await - .map_err(|e| WasmSdkError::generic(format!("Failed to fund addresses: {}", e)))?; + .map_err(WasmSdkError::from)?; address_infos_to_js_map(address_infos, "funding") } @@ -925,9 +923,7 @@ impl WasmSdk { settings, ) .await - .map_err(|e| { - WasmSdkError::generic(format!("Failed to create identity from addresses: {}", e)) - })?; + .map_err(WasmSdkError::from)?; Ok(IdentityCreateFromAddressesResultWasm { identity: created_identity.into(), diff --git a/packages/wasm-sdk/src/state_transitions/identity.rs b/packages/wasm-sdk/src/state_transitions/identity.rs index f03af7e37ce..188040c1ab4 100644 --- a/packages/wasm-sdk/src/state_transitions/identity.rs +++ b/packages/wasm-sdk/src/state_transitions/identity.rs @@ -122,7 +122,7 @@ impl WasmSdk { settings, ) .await - .map_err(|e| WasmSdkError::generic(format!("Failed to create identity: {}", e)))?; + .map_err(WasmSdkError::from)?; Ok(()) } @@ -684,7 +684,7 @@ impl WasmSdk { None, ) .await - .map_err(|e| WasmSdkError::generic(format!("Failed to create update transition: {}", e)))?; + .map_err(WasmSdkError::from)?; // Broadcast the transition use dash_sdk::dpp::state_transition::proof_result::StateTransitionProofResult;