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 b172e876d59..51788bda3f7 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 @@ -23,8 +23,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 { @@ -46,17 +49,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) { @@ -242,6 +234,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..f6cc0afb32f 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,8 @@ use crate::serialization::Signable; use crate::state_transition::address_credit_withdrawal_transition::methods::AddressCreditWithdrawalTransitionMethodsV0; use crate::state_transition::address_credit_withdrawal_transition::v0::AddressCreditWithdrawalTransitionV0; #[cfg(feature = "state-transition-signing")] +use crate::state_transition::first_consensus_error_as_protocol_error; +#[cfg(feature = "state-transition-signing")] use crate::withdrawal::Pooling; #[cfg(feature = "state-transition-signing")] use crate::{ @@ -35,7 +37,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!( @@ -57,6 +59,15 @@ impl AddressCreditWithdrawalTransitionMethodsV0 for AddressCreditWithdrawalTrans input_witnesses: Vec::new(), }; + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + let pre_validation_result = address_credit_withdrawal_transition + .validate_structure_without_input_witnesses(platform_version); + if let Some(error) = first_consensus_error_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()?; @@ -67,6 +78,14 @@ impl AddressCreditWithdrawalTransitionMethodsV0 for AddressCreditWithdrawalTrans } 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) = first_consensus_error_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/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 573f466aebc..446f7f13e21 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,6 +14,8 @@ use crate::serialization::Signable; use crate::state_transition::address_funding_from_asset_lock_transition::methods::AddressFundingFromAssetLockTransitionMethodsV0; use crate::state_transition::address_funding_from_asset_lock_transition::v0::AddressFundingFromAssetLockTransitionV0; #[cfg(feature = "state-transition-signing")] +use crate::state_transition::first_consensus_error_as_protocol_error; +#[cfg(feature = "state-transition-signing")] use crate::{prelude::UserFeeIncrease, state_transition::StateTransition, ProtocolError}; #[cfg(feature = "state-transition-signing")] use dashcore::signer; @@ -30,7 +32,7 @@ impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetL fee_strategy: AddressFundsFeeStrategy, signer: &S, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, ) -> Result { tracing::debug!("try_from_asset_lock_with_signer: Started"); tracing::debug!( @@ -50,6 +52,15 @@ impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetL input_witnesses: Vec::new(), }; + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + let pre_validation_result = + address_funding_transition.validate_structure_without_input_witnesses(platform_version); + if let Some(error) = first_consensus_error_as_protocol_error(pre_validation_result) { + return Err(error); + } + let state_transition: StateTransition = address_funding_transition.clone().into(); let signable_bytes = state_transition.signable_bytes()?; @@ -65,6 +76,13 @@ impl AddressFundingFromAssetLockTransitionMethodsV0 for AddressFundingFromAssetL } 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) = first_consensus_error_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..6c5d4d415f3 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,7 @@ 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::identity::signer::Signer; use crate::serialization::{PlatformDeserializable, PlatformSerializable, Signable}; use crate::state_transition::address_funds_transfer_transition::methods::AddressFundsTransferTransitionMethodsV0; @@ -130,7 +131,7 @@ impl TestAddressSigner { } } -#[async_trait::async_trait] +#[async_trait] impl Signer for TestAddressSigner { async fn sign(&self, key: &PlatformAddress, data: &[u8]) -> Result { match key { @@ -268,16 +269,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(), @@ -317,18 +318,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 +371,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 +425,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 +469,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 +517,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 +576,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 +635,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 +681,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 +723,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 +769,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 +794,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 +817,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 +841,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 +860,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 +885,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 +905,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 +947,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 +1004,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 +1044,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 +1082,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 +1123,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 +1160,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 +1174,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, 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..9b3677b5c0b 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,8 @@ 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::first_consensus_error_as_protocol_error; +#[cfg(feature = "state-transition-signing")] use crate::{ prelude::{AddressNonce, UserFeeIncrease}, state_transition::StateTransition, @@ -28,7 +30,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!( @@ -46,6 +48,15 @@ impl AddressFundsTransferTransitionMethodsV0 for AddressFundsTransferTransitionV input_witnesses: Vec::new(), }; + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + let pre_validation_result = + address_funds_transition.validate_structure_without_input_witnesses(platform_version); + if let Some(error) = first_consensus_error_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()?; @@ -56,6 +67,13 @@ impl AddressFundsTransferTransitionMethodsV0 for AddressFundsTransferTransitionV } 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) = first_consensus_error_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 23a610e17b6..e0808a24c5a 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 @@ -383,4 +383,177 @@ 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 + ), + } + } } 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..a8a36266dfc 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,11 @@ use crate::validation::SimpleConsensusValidationResult; use platform_version::version::PlatformVersion; use std::collections::HashSet; -impl StateTransitionStructureValidation for IdentityCreateFromAddressesTransitionV0 { - fn validate_structure( +impl IdentityCreateFromAddressesTransitionV0 { + /// 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 { @@ -70,17 +73,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) { @@ -245,4 +237,33 @@ impl StateTransitionStructureValidation for IdentityCreateFromAddressesTransitio 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 { + 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..b05da26a5cd 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 @@ -20,6 +20,8 @@ use crate::state_transition::StateTransitionType; // Crate: Feature-Gated (state-transition-signing) // ============================ #[cfg(feature = "state-transition-signing")] +use crate::state_transition::first_consensus_error_as_protocol_error; +#[cfg(feature = "state-transition-signing")] use crate::{ address_funds::AddressFundsFeeStrategy, identity::{ @@ -49,7 +51,7 @@ 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 = @@ -62,13 +64,36 @@ impl IdentityCreateFromAddressesTransitionMethodsV0 for IdentityCreateFromAddres ..Default::default() }; - 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) = first_consensus_error_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. + let pre_validation_result = identity_create_from_addresses_transition + .validate_structure_without_input_witnesses(platform_version); + if let Some(error) = first_consensus_error_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(); @@ -99,6 +124,14 @@ impl IdentityCreateFromAddressesTransitionMethodsV0 for IdentityCreateFromAddres } 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) = first_consensus_error_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..98589fd08a4 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,157 @@ 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( + &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 + ), + } + } } 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 2a342b2b8d1..499fd8f69b3 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 @@ -30,7 +30,7 @@ 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::{first_consensus_error_as_protocol_error, StateTransition}; #[cfg(feature = "state-transition-signing")] use crate::version::PlatformVersion; impl IdentityCreateTransitionMethodsV0 for IdentityCreateTransitionV0 { @@ -42,17 +42,31 @@ impl IdentityCreateTransitionMethodsV0 for IdentityCreateTransitionV0 { signer: &S, bls: &impl BlsModule, user_fee_increase: UserFeeIncrease, - _platform_version: &PlatformVersion, + platform_version: &PlatformVersion, ) -> Result { let mut identity_create_transition = IdentityCreateTransitionV0 { user_fee_increase, ..Default::default() }; - 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) = first_consensus_error_as_protocol_error(validation_result) { + return Err(error); + } + identity_create_transition.set_public_keys(public_keys); identity_create_transition.set_asset_lock_proof(asset_lock_proof)?; 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..f62c40c11fe 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,10 @@ use crate::address_funds::PlatformAddress; #[cfg(feature = "state-transition-signing")] use crate::fee::Credits; #[cfg(feature = "state-transition-signing")] +use crate::state_transition::{ + first_consensus_error_as_protocol_error, StateTransitionStructureValidation, +}; +#[cfg(feature = "state-transition-signing")] use crate::{ identity::{ accessors::IdentityGettersV0, @@ -35,22 +39,30 @@ 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(), + }; + + // Validate structure before .into() conversion and signing, since this transition + // uses sign_external on the StateTransition rather than setting witnesses on the V0 struct. + let validation_result = transition_v0.validate_structure(platform_version); + if let Some(error) = first_consensus_error_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_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..7bea58a8402 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,7 @@ use { identity::{accessors::IdentityGettersV0, signer::Signer, Identity}, prelude::{AddressNonce, UserFeeIncrease}, serialization::Signable, - state_transition::StateTransition, + state_transition::{first_consensus_error_as_protocol_error, StateTransition}, version::FeatureVersion, ProtocolError, }, @@ -33,7 +33,7 @@ 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 = @@ -48,6 +48,15 @@ impl IdentityTopUpFromAddressesTransitionMethodsV0 for IdentityTopUpFromAddresse input_witnesses: vec![], }; + // Pre-signing structure check: validate everything except the witness + // count, so structural errors fail fast before performing any async + // signer work. + let pre_validation_result = identity_top_up_from_addresses_transition + .validate_structure_without_input_witnesses(platform_version); + if let Some(error) = first_consensus_error_as_protocol_error(pre_validation_result) { + return Err(error); + } + let state_transition: StateTransition = identity_top_up_from_addresses_transition.clone().into(); @@ -59,6 +68,14 @@ impl IdentityTopUpFromAddressesTransitionMethodsV0 for IdentityTopUpFromAddresse } 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) = first_consensus_error_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..aea8aadcf36 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,124 @@ 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 + ), + } + } } /// 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..168bb9eb7b6 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 @@ -29,7 +29,10 @@ use crate::state_transition::identity_update_transition::v0::IdentityUpdateTrans 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::{ + first_consensus_error_as_protocol_error, GetDataContractSecurityLevelRequirementFn, + StateTransition, +}; #[cfg(feature = "state-transition-signing")] use crate::version::FeatureVersion; use crate::{ @@ -49,14 +52,27 @@ 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(); + // 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( + &add_public_keys_in_creation, + false, // not in create_identity context + platform_version, + )?; + if let Some(error) = first_consensus_error_as_protocol_error(validation_result) { + return Err(error); + } + let mut identity_update_transition = IdentityUpdateTransitionV0 { signature: Default::default(), signature_public_key_id: 0, 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..fe49169be55 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,4 +1,5 @@ use crate::validation::SimpleConsensusValidationResult; +use crate::ProtocolError; use platform_version::version::PlatformVersion; /// Trait for validating the structure of a state transition @@ -9,3 +10,18 @@ pub trait StateTransitionStructureValidation { platform_version: &PlatformVersion, ) -> SimpleConsensusValidationResult; } + +/// Converts a `SimpleConsensusValidationResult` into `Some(ProtocolError::ConsensusError)` +/// containing the first error if the result is invalid, or `None` otherwise. +/// +/// This avoids `unwrap` while still surfacing only the first consensus error, which is the +/// pattern used by client-side state transition constructors before/after signing. +pub(crate) fn first_consensus_error_as_protocol_error( + result: SimpleConsensusValidationResult, +) -> Option { + result + .errors + .into_iter() + .next() + .map(|error| ProtocolError::ConsensusError(Box::new(error))) +} 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 d93ef44dee9..c2fe0614993 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) @@ -1104,14 +1159,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"); @@ -1324,14 +1383,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"); @@ -1429,14 +1492,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"); @@ -1512,14 +1579,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"); @@ -1615,14 +1686,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"); @@ -3443,21 +3518,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"); @@ -3503,21 +3575,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"); @@ -3703,14 +3772,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"); @@ -4791,14 +4864,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"); @@ -5150,14 +5227,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"); @@ -5201,14 +5282,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"); @@ -5249,14 +5334,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"); @@ -5297,14 +5386,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"); @@ -5639,7 +5732,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( @@ -5648,12 +5742,8 @@ mod tests { core_fee_per_byte, Pooling::Never, output_script, - signer, 0, - PlatformVersion::latest(), ) - .await - .expect("should create signed transition") } #[tokio::test] @@ -5983,14 +6073,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"); @@ -6238,14 +6332,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 bb7f1a57f9c..b42f79cfb10 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 @@ -5088,6 +5088,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 { @@ -5117,15 +5119,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() @@ -9189,15 +9197,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 7f0823683ba..d277095f1a5 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 3a669499d72..1dae2cb8839 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; @@ -1311,13 +1318,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; @@ -1430,13 +1439,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; @@ -1528,13 +1539,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; @@ -1644,13 +1657,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; @@ -1720,13 +1735,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; @@ -1798,13 +1815,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; @@ -2292,13 +2311,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; @@ -2409,13 +2430,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; @@ -2948,13 +2971,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; @@ -3141,13 +3166,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; @@ -3355,13 +3382,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_to_addresses/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_transfer_to_addresses/tests.rs index 8b1698f952c..4d603d6e3f3 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; @@ -4965,8 +4966,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) @@ -5109,9 +5133,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(); @@ -5236,9 +5284,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_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 04781ed09dd..3c1fbcc8865 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/tests/strategy_tests/strategy.rs b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs index 82a7374d9d1..eff56fbe72c 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/strategy.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs @@ -1533,7 +1533,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 +1544,10 @@ impl NetworkStrategy { rng, platform_version, ) - .await; - operations.push(state_transition); + .await + { + operations.push(state_transition); + } } } } @@ -2308,8 +2310,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 +2354,13 @@ 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; + 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" @@ -2433,16 +2445,40 @@ 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; + 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 { + for addr in inputs.keys() { + current_addresses_with_balance + .addresses_in_block_with_new_balance + .remove(addr); + } + 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 @@ -2455,7 +2491,9 @@ impl NetworkStrategy { // Create output addresses and distribute funds evenly let amount_per_output = total_input / output_count as Credits; + let remainder = total_input - (amount_per_output * output_count as Credits); let mut outputs = BTreeMap::new(); + let mut first_output_address = None; // Collect existing addresses that are not used as inputs (for potential reuse as outputs) let input_addresses: std::collections::HashSet<_> = inputs.keys().cloned().collect(); @@ -2498,9 +2536,28 @@ impl NetworkStrategy { new_address }; + if first_output_address.is_none() { + first_output_address = Some(address.clone()); + } outputs.insert(address, amount_per_output); } + // Add remainder to the first output so input_sum == output_sum + if remainder > 0 { + if let Some(first_addr) = &first_output_address { + if let Some(amount) = outputs.get_mut(first_addr) { + *amount += remainder; + } + // Also update the balance tracking + if let Some((nonce, balance)) = current_addresses_with_balance + .addresses_in_block_with_new_balance + .get_mut(first_addr) + { + *balance += remainder; + } + } + } + let transfer_transition = AddressFundsTransferTransition::try_from_inputs_with_signer( inputs, outputs, @@ -2530,8 +2587,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)?; let fee_strategy = fee_strategy .clone() @@ -2598,10 +2660,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 +2695,7 @@ impl NetworkStrategy { tracing::debug!(?outputs, "Preparing funding transition"); let funding_transition = - AddressFundingFromAssetLockTransitionV0::try_from_asset_lock_with_signer( + match AddressFundingFromAssetLockTransitionV0::try_from_asset_lock_with_signer( asset_lock_proof, asset_lock_private_key.as_slice(), BTreeMap::new(), @@ -2620,7 +2706,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..2c7363561c1 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 validation min_per_input fix. assert_eq!( trunk_result.elements.len(), - 32, - "trunk query should return 32 elements" + 40, + "trunk query should return 40 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 validation min_per_input fix. assert_eq!( trunk_result.elements.len(), - 32, - "trunk query should return 32 elements after restart" + 40, + "trunk query should return 40 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 validation min_per_input fix. assert_eq!( trunk_result.elements.len(), - 32, - "trunk query should return 32 elements" + 40, + "trunk query should return 40 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" + 36, + "phase 1: trunk should return 36 elements" ); // Record addresses known so far from state transitions 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 d6318f55f0c..b8bd622551f 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,15 +1906,15 @@ 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. + let 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); @@ -1901,9 +1932,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; @@ -1979,8 +2025,17 @@ impl Strategy { extra_keys, ) => { for _i in 0..count { + 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; @@ -2097,9 +2152,34 @@ 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 { + for addr in inputs.keys() { + current_addresses_with_balance + .addresses_in_block_with_new_balance + .remove(addr); + } + 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 = @@ -2215,8 +2295,17 @@ impl Strategy { fee_strategy, ) => { for _i in 0..count { + 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 { eprintln!("no funds left on block {}", block_info.height); // no funds left @@ -2231,9 +2320,30 @@ 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 { + for addr in inputs.keys() { + current_addresses_with_balance + .addresses_in_block_with_new_balance + .remove(addr); + } + 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 @@ -2514,8 +2624,17 @@ impl Strategy { fee_strategy, ) => { for _i in 0..count { + 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; @@ -2593,9 +2712,34 @@ 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 { + for addr in inputs.keys() { + current_addresses_with_balance + .addresses_in_block_with_new_balance + .remove(addr); + } + 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 +2747,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 96e873e7ba7..f5f4bda36fd 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,7 +1000,7 @@ 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.