From f582e56f82b5a9e2f4534bd107d18377a0074652 Mon Sep 17 00:00:00 2001 From: Pawel Iwan Date: Fri, 31 Mar 2023 12:20:12 +0200 Subject: [PATCH 1/4] fix overflows --- .../validate_time_in_block_time_window.rs | 20 +++++-- ...e_data_contract_update_transition_basic.rs | 12 +++- ...lidate_documents_batch_transition_state.rs | 23 ++++---- .../rs-dpp/src/errors/non_consensus_error.rs | 4 ++ .../rs-dpp/src/identity/credits_converter.rs | 20 ++++--- .../apply_identity_create_transition.rs | 2 +- .../apply_identity_topup_transition.rs | 2 +- ...lidate_identity_update_transition_state.rs | 9 ++- .../fee/calculate_operation_fees.rs | 29 +++++++--- .../calculate_state_transition_fee_factory.rs | 6 +- ..._transition_fee_from_operations_factory.rs | 24 ++++---- .../state_transition/fee/operations/mod.rs | 10 ++-- .../fee/operations/precalculated_operation.rs | 13 +++-- .../fee/operations/read_operation.rs | 31 ++++++++--- .../signature_verification_operation.rs | 9 +-- .../validate_state_transition_fee.rs | 55 +++++++++++++------ .../src/identity_credit_withdrawal/mod.rs | 2 +- .../fee/calculate_operation_fees.rs | 14 +++-- .../fee/calculate_state_transition_fee.rs | 11 +++- .../operations/pre_calculated_operation.rs | 27 ++++++--- .../fee/operations/read_operation.rs | 23 ++++++-- .../signature_verification_operation.rs | 19 +++++-- 22 files changed, 251 insertions(+), 114 deletions(-) diff --git a/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs b/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs index 65f18fcd3ac..759c085648e 100644 --- a/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs +++ b/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs @@ -1,4 +1,4 @@ -use crate::prelude::TimestampMillis; +use crate::{prelude::TimestampMillis, NonConsensusError, ProtocolError}; use super::validation_result::TimeWindowValidationResult; @@ -8,16 +8,24 @@ pub const BLOCK_TIME_WINDOW_MILLIS: u64 = BLOCK_TIME_WINDOW_MINUTES * 60 * 1000; pub fn validate_time_in_block_time_window( last_block_header_time_millis: TimestampMillis, time_to_check_millis: TimestampMillis, -) -> TimeWindowValidationResult { - let time_window_start = last_block_header_time_millis - BLOCK_TIME_WINDOW_MILLIS; - let time_window_end = last_block_header_time_millis + BLOCK_TIME_WINDOW_MILLIS; +) -> Result { + let time_window_start = last_block_header_time_millis + .checked_sub(BLOCK_TIME_WINDOW_MILLIS) + .ok_or(NonConsensusError::Overflow( + "calculation of start window failed", + ))?; + let time_window_end = last_block_header_time_millis + .checked_add(BLOCK_TIME_WINDOW_MILLIS) + .ok_or(NonConsensusError::Overflow( + "calculation of end window failed", + ))?; let valid = time_to_check_millis >= time_window_start && time_to_check_millis <= time_window_end; - TimeWindowValidationResult { + Ok(TimeWindowValidationResult { time_window_start, time_window_end, valid, - } + }) } diff --git a/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_data_contract_update_transition_basic.rs b/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_data_contract_update_transition_basic.rs index 99a083101a7..f7a9798982e 100644 --- a/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_data_contract_update_transition_basic.rs +++ b/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_data_contract_update_transition_basic.rs @@ -144,9 +144,17 @@ where } }; - let new_version = new_data_contract_object.get_integer(contract_property_names::VERSION)?; + let new_version: u32 = + new_data_contract_object.get_integer(contract_property_names::VERSION)?; let old_version = existing_data_contract.version; - if (new_version - old_version) != 1 { + + if (new_version + .checked_sub(old_version) + .ok_or(ProtocolError::Overflow( + "comparing protocol versions failed", + ))?) + != 1 + { validation_result.add_error(BasicError::InvalidDataContractVersionError( InvalidDataContractVersionError::new(old_version + 1, new_version), )) diff --git a/packages/rs-dpp/src/document/state_transition/documents_batch_transition/validation/state/validate_documents_batch_transition_state.rs b/packages/rs-dpp/src/document/state_transition/documents_batch_transition/validation/state/validate_documents_batch_transition_state.rs index 97af008ff72..98c63491992 100644 --- a/packages/rs-dpp/src/document/state_transition/documents_batch_transition/validation/state/validate_documents_batch_transition_state.rs +++ b/packages/rs-dpp/src/document/state_transition/documents_batch_transition/validation/state/validate_documents_batch_transition_state.rs @@ -14,6 +14,7 @@ use crate::document::state_transition::documents_batch_transition::{ }; use crate::document::Document; use crate::validation::{AsyncDataValidator, SimpleValidationResult}; +use crate::NonConsensusError; use crate::{ block_time_window::validate_time_in_block_time_window::validate_time_in_block_time_window, consensus::ConsensusError, @@ -248,11 +249,11 @@ fn validate_transition( result.merge(validation_result); let validation_result = - check_created_inside_time_window(transition, last_header_block_time_millis); + check_created_inside_time_window(transition, last_header_block_time_millis)?; result.merge(validation_result); let validation_result = - check_updated_inside_time_window(transition, last_header_block_time_millis); + check_updated_inside_time_window(transition, last_header_block_time_millis)?; result.merge(validation_result); let validation_result = @@ -273,7 +274,7 @@ fn validate_transition( DocumentTransitionAction::ReplaceAction(DocumentReplaceTransitionAction::default()), ); let validation_result = - check_updated_inside_time_window(transition, last_header_block_time_millis); + check_updated_inside_time_window(transition, last_header_block_time_millis)?; result.merge(validation_result); let validation_result = check_revision(transition, fetched_documents); @@ -445,14 +446,14 @@ fn check_if_timestamps_are_equal( fn check_created_inside_time_window( document_transition: &DocumentTransition, last_block_ts_millis: TimestampMillis, -) -> SimpleValidationResult { +) -> Result { let mut result = SimpleValidationResult::default(); let created_at = match document_transition.get_created_at() { Some(t) => t, - None => return result, + None => return Ok(result), }; - let window_validation = validate_time_in_block_time_window(last_block_ts_millis, created_at); + let window_validation = validate_time_in_block_time_window(last_block_ts_millis, created_at)?; if !window_validation.is_valid() { result.add_error(ConsensusError::StateError(Box::new( StateError::DocumentTimestampWindowViolationError { @@ -464,20 +465,20 @@ fn check_created_inside_time_window( }, ))); } - result + Ok(result) } fn check_updated_inside_time_window( document_transition: &DocumentTransition, last_block_ts_millis: TimestampMillis, -) -> SimpleValidationResult { +) -> Result { let mut result = SimpleValidationResult::default(); let updated_at = match document_transition.get_updated_at() { Some(t) => t, - None => return result, + None => return Ok(result), }; - let window_validation = validate_time_in_block_time_window(last_block_ts_millis, updated_at); + let window_validation = validate_time_in_block_time_window(last_block_ts_millis, updated_at)?; if !window_validation.is_valid() { result.add_error(ConsensusError::StateError(Box::new( StateError::DocumentTimestampWindowViolationError { @@ -489,5 +490,5 @@ fn check_updated_inside_time_window( }, ))); } - result + Ok(result) } diff --git a/packages/rs-dpp/src/errors/non_consensus_error.rs b/packages/rs-dpp/src/errors/non_consensus_error.rs index 015098cd6e8..31e10d6d193 100644 --- a/packages/rs-dpp/src/errors/non_consensus_error.rs +++ b/packages/rs-dpp/src/errors/non_consensus_error.rs @@ -46,6 +46,10 @@ pub enum NonConsensusError { #[error(transparent)] Error(#[from] anyhow::Error), + + /// Error + #[error("overflow error: {0}")] + Overflow(&'static str), } pub mod object_names { diff --git a/packages/rs-dpp/src/identity/credits_converter.rs b/packages/rs-dpp/src/identity/credits_converter.rs index beb411ed613..98ff23a815e 100644 --- a/packages/rs-dpp/src/identity/credits_converter.rs +++ b/packages/rs-dpp/src/identity/credits_converter.rs @@ -1,11 +1,17 @@ +use crate::{state_transition::fee::Credits, ProtocolError}; + pub const RATIO: u64 = 1000; -pub fn convert_satoshi_to_credits(amount: u64) -> u64 { - amount * RATIO +pub fn convert_satoshi_to_credits(amount: u64) -> Result { + amount.checked_mul(RATIO).ok_or(ProtocolError::Overflow( + "converting satoshi to credits failed", + )) } -pub fn convert_credits_to_satoshi(amount: u64) -> u64 { - amount / RATIO +pub fn convert_credits_to_satoshi(amount: Credits) -> Result { + amount.checked_div(RATIO).ok_or(ProtocolError::Overflow( + "converting credits to satoshi failed", + )) } #[cfg(test)] @@ -15,7 +21,7 @@ mod test { #[test] fn test_should_convert_satoshi_to_credits() { let amount = 42; - let converted = convert_satoshi_to_credits(amount); + let converted = convert_satoshi_to_credits(amount).unwrap(); assert_eq!(converted, amount * RATIO); } @@ -23,14 +29,14 @@ mod test { #[test] fn test_should_convert_credits_to_satoshi() { let amount = 10000; - let converted = convert_credits_to_satoshi(amount); + let converted = convert_credits_to_satoshi(amount).unwrap(); assert_eq!(converted, amount / RATIO); } #[test] fn test_convert_to_0_satoshi_if_amount_lower_than_ratio() { let amount = RATIO - 1; - let converted = convert_credits_to_satoshi(amount); + let converted = convert_credits_to_satoshi(amount).unwrap(); assert_eq!(converted, 0); } } diff --git a/packages/rs-dpp/src/identity/state_transition/identity_create_transition/apply_identity_create_transition.rs b/packages/rs-dpp/src/identity/state_transition/identity_create_transition/apply_identity_create_transition.rs index be81e19c394..82832b227f5 100644 --- a/packages/rs-dpp/src/identity/state_transition/identity_create_transition/apply_identity_create_transition.rs +++ b/packages/rs-dpp/src/identity/state_transition/identity_create_transition/apply_identity_create_transition.rs @@ -43,7 +43,7 @@ where ) .await?; - let credits_amount = convert_satoshi_to_credits(output.value); + let credits_amount = convert_satoshi_to_credits(output.value)?; let identity = Identity { protocol_version: state_transition.get_protocol_version(), diff --git a/packages/rs-dpp/src/identity/state_transition/identity_topup_transition/apply_identity_topup_transition.rs b/packages/rs-dpp/src/identity/state_transition/identity_topup_transition/apply_identity_topup_transition.rs index 571f3d7d0b1..55f881c9488 100644 --- a/packages/rs-dpp/src/identity/state_transition/identity_topup_transition/apply_identity_topup_transition.rs +++ b/packages/rs-dpp/src/identity/state_transition/identity_topup_transition/apply_identity_topup_transition.rs @@ -39,7 +39,7 @@ where ) .await?; - let mut credits_amount = convert_satoshi_to_credits(output.value); + let mut credits_amount = convert_satoshi_to_credits(output.value)?; let out_point = state_transition .get_asset_lock_proof() diff --git a/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs b/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs index a5e75041162..3a10e1118a5 100644 --- a/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs +++ b/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use crate::consensus::signature::{IdentityNotFoundError, SignatureError}; use crate::identity::state_transition::identity_update_transition::IdentityUpdateTransitionAction; +use crate::ProtocolError; use crate::{ block_time_window::validate_time_in_block_time_window::validate_time_in_block_time_window, identity::validation::{RequiredPurposeAndSecurityLevelValidator, TPublicKeysValidator}, @@ -75,7 +76,11 @@ where let mut identity = stored_identity.clone(); // Check revision - if identity.get_revision() != (state_transition.get_revision() - 1) { + if identity.get_revision() + != (state_transition.get_revision().checked_sub(1).ok_or( + NonConsensusError::Overflow("unable subtract 1 from revision"), + )?) + { validation_result.add_error(StateError::InvalidIdentityRevisionError { identity_id: state_transition.get_identity_id().to_owned(), current_revision: identity.get_revision(), @@ -125,7 +130,7 @@ where }, )?; let window_validation_result = - validate_time_in_block_time_window(last_block_header_time, disabled_at_ms); + validate_time_in_block_time_window(last_block_header_time, disabled_at_ms)?; if !window_validation_result.is_valid() { validation_result.add_error( diff --git a/packages/rs-dpp/src/state_transition/fee/calculate_operation_fees.rs b/packages/rs-dpp/src/state_transition/fee/calculate_operation_fees.rs index d6fe927df83..3656777fa8e 100644 --- a/packages/rs-dpp/src/state_transition/fee/calculate_operation_fees.rs +++ b/packages/rs-dpp/src/state_transition/fee/calculate_operation_fees.rs @@ -1,16 +1,24 @@ +use crate::NonConsensusError; + use super::{ operations::{Operation, OperationLike}, - DummyFeesResult, Refunds, + Credits, DummyFeesResult, Refunds, }; -pub fn calculate_operation_fees(operations: &[Operation]) -> DummyFeesResult { - let mut storage_fee = 0; - let mut processing_fee = 0; +pub fn calculate_operation_fees( + operations: &[Operation], +) -> Result { + let mut storage_fee: Credits = 0; + let mut processing_fee: Credits = 0; let mut fee_refunds: Vec = Vec::new(); for operation in operations { - storage_fee += operation.get_storage_cost(); - processing_fee += operation.get_processing_cost(); + storage_fee = storage_fee + .checked_add(operation.get_storage_cost()?) + .ok_or(NonConsensusError::Overflow("storage cost is too big"))?; + processing_fee = processing_fee + .checked_add(operation.get_processing_cost()?) + .ok_or(NonConsensusError::Overflow("processing cost is too big"))?; // Merge refunds if let Some(operation_refunds) = operation.get_refunds() { @@ -30,16 +38,19 @@ pub fn calculate_operation_fees(operations: &[Operation]) -> DummyFeesResult { .credits_per_epoch .entry(epoch_index.to_string()) .or_default(); - *epoch += credits + + *epoch = epoch + .checked_add(*credits) + .ok_or(NonConsensusError::Overflow("credits per epoch are too big"))? } } } } } - DummyFeesResult { + Ok(DummyFeesResult { storage: storage_fee, processing: processing_fee, fee_refunds, - } + }) } diff --git a/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs b/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs index 01d0583746c..64cd798b1e4 100644 --- a/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs +++ b/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs @@ -1,11 +1,11 @@ -use crate::state_transition::{ +use crate::{state_transition::{ fee::calculate_state_transition_fee_from_operations_factory::calculate_state_transition_fee_from_operations, StateTransition, StateTransitionLike, -}; +}, NonConsensusError}; use super::FeeResult; -pub fn calculate_state_transition_fee(state_transition: &StateTransition) -> FeeResult { +pub fn calculate_state_transition_fee(state_transition: &StateTransition) -> Result { let execution_context = state_transition.get_execution_context(); calculate_state_transition_fee_from_operations( diff --git a/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_from_operations_factory.rs b/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_from_operations_factory.rs index a5a7adf92f6..89eee9ed9b1 100644 --- a/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_from_operations_factory.rs +++ b/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_from_operations_factory.rs @@ -1,4 +1,4 @@ -use crate::prelude::Identifier; +use crate::{prelude::Identifier, NonConsensusError}; use super::{ calculate_operation_fees::calculate_operation_fees, constants::DEFAULT_USER_TIP, @@ -8,7 +8,7 @@ use super::{ pub fn calculate_state_transition_fee_from_operations( operations: &[Operation], identity_id: &Identifier, -) -> FeeResult { +) -> Result { calculate_state_transition_fee_from_operations_with_custom_calculator( operations, identity_id, @@ -19,9 +19,9 @@ pub fn calculate_state_transition_fee_from_operations( fn calculate_state_transition_fee_from_operations_with_custom_calculator( operations: &[Operation], identity_id: &Identifier, - calculate_operation_fees_fn: impl FnOnce(&[Operation]) -> DummyFeesResult, -) -> FeeResult { - let calculated_fees = calculate_operation_fees_fn(operations); + calculate_operation_fees_fn: impl FnOnce(&[Operation]) -> Result, +) -> Result { + let calculated_fees = calculate_operation_fees_fn(operations)?; let storage_fee = calculated_fees.storage; let processing_fee = calculated_fees.processing; @@ -43,14 +43,14 @@ fn calculate_state_transition_fee_from_operations_with_custom_calculator( let required_amount = (storage_fee - total_refunds) + DEFAULT_USER_TIP; let desired_amount = (storage_fee + processing_fee - total_refunds) + DEFAULT_USER_TIP; - FeeResult { + Ok(FeeResult { storage_fee, processing_fee, fee_refunds, total_refunds, required_amount, desired_amount, - } + }) } #[cfg(test)] @@ -62,6 +62,7 @@ mod test { operations::Operation, Credits, DummyFeesResult, FeeResult, Refunds, }, tests::utils::generate_random_identifier_struct, + NonConsensusError, }; use super::calculate_state_transition_fee_from_operations_with_custom_calculator; @@ -84,19 +85,20 @@ mod test { credits_per_epoch, }; - let mock = |_operations: &[Operation]| -> DummyFeesResult { - DummyFeesResult { + let mock = |_operations: &[Operation]| -> Result { + Ok(DummyFeesResult { storage: storage_fee, processing: processing_fee, fee_refunds: vec![refunds.clone()], - } + }) }; let result = calculate_state_transition_fee_from_operations_with_custom_calculator( &[], &identifier, mock, - ); + ) + .expect("result should be returned"); let expected = FeeResult { storage_fee, processing_fee, diff --git a/packages/rs-dpp/src/state_transition/fee/operations/mod.rs b/packages/rs-dpp/src/state_transition/fee/operations/mod.rs index 59430db494a..63a1b8563b4 100644 --- a/packages/rs-dpp/src/state_transition/fee/operations/mod.rs +++ b/packages/rs-dpp/src/state_transition/fee/operations/mod.rs @@ -10,6 +10,8 @@ use serde_json::Value; mod signature_verification_operation; pub use signature_verification_operation::*; +use crate::NonConsensusError; + use super::{Credits, Refunds}; pub const STORAGE_CREDIT_PER_BYTE: i64 = 5000; @@ -25,9 +27,9 @@ pub enum Operation { pub trait OperationLike { /// Get CPU cost of the operation - fn get_processing_cost(&self) -> Credits; + fn get_processing_cost(&self) -> Result; /// Get storage cost of the operation - fn get_storage_cost(&self) -> Credits; + fn get_storage_cost(&self) -> Result; /// Get refunds fn get_refunds(&self) -> Option<&Vec>; @@ -44,11 +46,11 @@ macro_rules! call_method { } impl OperationLike for Operation { - fn get_processing_cost(&self) -> Credits { + fn get_processing_cost(&self) -> Result { call_method!(self, get_processing_cost) } - fn get_storage_cost(&self) -> Credits { + fn get_storage_cost(&self) -> Result { call_method!(self, get_storage_cost) } diff --git a/packages/rs-dpp/src/state_transition/fee/operations/precalculated_operation.rs b/packages/rs-dpp/src/state_transition/fee/operations/precalculated_operation.rs index b7cdc272502..227cb47475a 100644 --- a/packages/rs-dpp/src/state_transition/fee/operations/precalculated_operation.rs +++ b/packages/rs-dpp/src/state_transition/fee/operations/precalculated_operation.rs @@ -1,6 +1,9 @@ use serde::{Deserialize, Serialize}; -use crate::state_transition::fee::{Credits, DummyFeesResult, Refunds}; +use crate::{ + state_transition::fee::{Credits, DummyFeesResult, Refunds}, + NonConsensusError, +}; use super::OperationLike; @@ -35,12 +38,12 @@ impl PreCalculatedOperation { } impl OperationLike for PreCalculatedOperation { - fn get_processing_cost(&self) -> Credits { - self.processing_cost + fn get_processing_cost(&self) -> Result { + Ok(self.processing_cost) } - fn get_storage_cost(&self) -> Credits { - self.storage_cost + fn get_storage_cost(&self) -> Result { + Ok(self.storage_cost) } fn get_refunds(&self) -> Option<&Vec> { diff --git a/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs b/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs index 007be2e941e..4a74727bd3c 100644 --- a/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs +++ b/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs @@ -2,11 +2,17 @@ use serde::{Deserialize, Serialize}; use super::OperationLike; -use crate::state_transition::fee::{ - constants::{PROCESSING_CREDIT_PER_BYTE, READ_BASE_PROCESSING_COST}, - Credits, Refunds, +use crate::{ + state_transition::fee::{ + constants::{PROCESSING_CREDIT_PER_BYTE, READ_BASE_PROCESSING_COST}, + Credits, Refunds, + }, + NonConsensusError, }; +const ProcessingCostOverflowError: NonConsensusError = + NonConsensusError::Overflow("the processing cost is too big"); + #[derive(Default, Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] pub struct ReadOperation { @@ -20,12 +26,23 @@ impl ReadOperation { } impl OperationLike for ReadOperation { - fn get_processing_cost(&self) -> Credits { - READ_BASE_PROCESSING_COST + (self.value_size * PROCESSING_CREDIT_PER_BYTE) + fn get_processing_cost(&self) -> Result { + let value_byte_processing_cost = self + .value_size + .checked_mul(PROCESSING_CREDIT_PER_BYTE) + .ok_or(NonConsensusError::Overflow( + "value processing cost is too big", + ))?; + + READ_BASE_PROCESSING_COST + .checked_add(value_byte_processing_cost) + .ok_or(NonConsensusError::Overflow( + "can't add read base processing cost", + )) } - fn get_storage_cost(&self) -> Credits { - 0 + fn get_storage_cost(&self) -> Result { + Ok(0) } fn get_refunds(&self) -> Option<&Vec> { diff --git a/packages/rs-dpp/src/state_transition/fee/operations/signature_verification_operation.rs b/packages/rs-dpp/src/state_transition/fee/operations/signature_verification_operation.rs index 59d43afb249..299d6a2564c 100644 --- a/packages/rs-dpp/src/state_transition/fee/operations/signature_verification_operation.rs +++ b/packages/rs-dpp/src/state_transition/fee/operations/signature_verification_operation.rs @@ -4,6 +4,7 @@ use super::OperationLike; use crate::{ identity::KeyType, state_transition::fee::{constants::signature_verify_cost, Credits, Refunds}, + NonConsensusError, }; #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] @@ -19,12 +20,12 @@ impl SignatureVerificationOperation { } impl OperationLike for SignatureVerificationOperation { - fn get_processing_cost(&self) -> Credits { - signature_verify_cost(self.signature_type) + fn get_processing_cost(&self) -> Result { + Ok(signature_verify_cost(self.signature_type)) } - fn get_storage_cost(&self) -> Credits { - 0 + fn get_storage_cost(&self) -> Result { + Ok(0) } fn get_refunds(&self) -> Option<&Vec> { diff --git a/packages/rs-dpp/src/state_transition/validation/validate_state_transition_fee.rs b/packages/rs-dpp/src/state_transition/validation/validate_state_transition_fee.rs index 994857e72f7..1b11d244732 100644 --- a/packages/rs-dpp/src/state_transition/validation/validate_state_transition_fee.rs +++ b/packages/rs-dpp/src/state_transition/validation/validate_state_transition_fee.rs @@ -6,6 +6,7 @@ use crate::data_contract::errors::IdentityNotPresentError; use crate::state_transition::fee::calculate_state_transition_fee_factory::calculate_state_transition_fee; use crate::state_transition::fee::{Credits, FeeResult}; use crate::state_transition::StateTransitionType; +use crate::NonConsensusError; use crate::{ consensus::fee::FeeError, identity::{ @@ -48,12 +49,14 @@ where async fn validate_with_custom_calculator( &self, state_transition: &StateTransition, - calculate_state_transition_fee_fn: impl Fn(&StateTransition) -> FeeResult, + calculate_state_transition_fee_fn: impl Fn( + &StateTransition, + ) -> Result, ) -> Result { let mut result = SimpleValidationResult::default(); let execution_context = state_transition.get_execution_context(); - let required_fee = calculate_state_transition_fee_fn(state_transition); + let required_fee = calculate_state_transition_fee_fn(state_transition)?; let balance = match state_transition { StateTransition::IdentityCreate(st) => { @@ -67,7 +70,7 @@ where st.get_asset_lock_proof() ) })?; - convert_satoshi_to_credits(output.value) + convert_satoshi_to_credits(output.value)? } StateTransition::IdentityTopUp(st) => { let output = self @@ -80,7 +83,7 @@ where st.get_asset_lock_proof() ) })?; - let balance = convert_satoshi_to_credits(output.value); + let balance = convert_satoshi_to_credits(output.value)?; let identity_id = st.get_owner_id(); let identity_balance: i64 = self .state_repository @@ -105,9 +108,17 @@ where } if identity_balance.is_negative() { - balance - identity_balance.unsigned_abs() + balance.checked_sub(identity_balance.unsigned_abs()).ok_or( + ProtocolError::Overflow( + "can't subtract identity balance from the state transition balance", + ), + )? } else { - balance + identity_balance as Credits + balance.checked_add(identity_balance as Credits).ok_or( + ProtocolError::Overflow( + "can't add identity balance to state transition balance", + ), + )? } } StateTransition::DataContractCreate(st) => { @@ -406,9 +417,11 @@ mod test { IdentityCreateTransition::new(identity_create_transition_fixture(None)).unwrap(); let output_amount = get_output_amount_from_identity_transition!(identity_create_transition); let state_repository_mock = MockStateRepositoryLike::new(); - let calculate_state_transition_fee_mock = |_: &StateTransition| FeeResult { - desired_amount: output_amount + 1, - ..Default::default() + let calculate_state_transition_fee_mock = |_: &StateTransition| { + Ok(FeeResult { + desired_amount: output_amount + 1, + ..Default::default() + }) }; let validator = StateTransitionFeeValidator::new(Arc::new(state_repository_mock)); @@ -434,9 +447,11 @@ mod test { IdentityCreateTransition::new(identity_create_transition_fixture(None)).unwrap(); let output_amount = get_output_amount_from_identity_transition!(identity_create_transition); let state_repository_mock = MockStateRepositoryLike::new(); - let calculate_state_transition_fee_mock = |_: &StateTransition| FeeResult { - desired_amount: output_amount, - ..Default::default() + let calculate_state_transition_fee_mock = |_: &StateTransition| { + Ok(FeeResult { + desired_amount: output_amount, + ..Default::default() + }) }; let validator = StateTransitionFeeValidator::new(Arc::new(state_repository_mock)); let result = validator @@ -460,9 +475,11 @@ mod test { IdentityTopUpTransition::new(identity_topup_transition_fixture(None)).unwrap(); let output_amount = get_output_amount_from_identity_transition!(identity_topup_transition); - let calculate_state_transition_fee_mock = |_: &StateTransition| FeeResult { - desired_amount: output_amount + 2, - ..Default::default() + let calculate_state_transition_fee_mock = |_: &StateTransition| { + Ok(FeeResult { + desired_amount: output_amount + 2, + ..Default::default() + }) }; let validator = StateTransitionFeeValidator::new(Arc::new(state_repository_mock)); @@ -493,9 +510,11 @@ mod test { IdentityTopUpTransition::new(identity_topup_transition_fixture(None)).unwrap(); let output_amount = get_output_amount_from_identity_transition!(identity_topup_transition); - let calculation_mock = |_: &StateTransition| FeeResult { - desired_amount: output_amount - 1, - ..Default::default() + let calculation_mock = |_: &StateTransition| { + Ok(FeeResult { + desired_amount: output_amount - 1, + ..Default::default() + }) }; let validator = StateTransitionFeeValidator::new(Arc::new(state_repository_mock)); diff --git a/packages/rs-drive-abci/src/identity_credit_withdrawal/mod.rs b/packages/rs-drive-abci/src/identity_credit_withdrawal/mod.rs index 76af007c702..c4949c7d85a 100644 --- a/packages/rs-drive-abci/src/identity_credit_withdrawal/mod.rs +++ b/packages/rs-drive-abci/src/identity_credit_withdrawal/mod.rs @@ -492,7 +492,7 @@ impl Platform { let output_script: Script = Script::from(output_script_bytes); let tx_out = TxOut { - value: convert_credits_to_satoshi(amount), + value: convert_credits_to_satoshi(amount)?, script_pubkey: output_script, }; diff --git a/packages/wasm-dpp/src/state_transition/fee/calculate_operation_fees.rs b/packages/wasm-dpp/src/state_transition/fee/calculate_operation_fees.rs index 91625c269d8..8a270bbede5 100644 --- a/packages/wasm-dpp/src/state_transition/fee/calculate_operation_fees.rs +++ b/packages/wasm-dpp/src/state_transition/fee/calculate_operation_fees.rs @@ -1,11 +1,14 @@ -use dpp::state_transition::fee::{ - calculate_operation_fees::calculate_operation_fees, operations::Operation, +use dpp::{ + state_transition::fee::{ + calculate_operation_fees::calculate_operation_fees, operations::Operation, + }, + ProtocolError, }; use wasm_bindgen::prelude::*; use crate::{ fee::dummy_fee_result::DummyFeesResultWasm, - utils::{Inner, IntoWasm}, + utils::{Inner, IntoWasm, WithJsError}, }; use super::OperationWasm; @@ -20,5 +23,8 @@ pub fn calculate_operation_fees_wasm( inner_operations.push(operation.into_inner()) } - Ok(calculate_operation_fees(&inner_operations).into()) + Ok(calculate_operation_fees(&inner_operations) + .map_err(ProtocolError::from) + .with_js_error()? + .into()) } diff --git a/packages/wasm-dpp/src/state_transition/fee/calculate_state_transition_fee.rs b/packages/wasm-dpp/src/state_transition/fee/calculate_state_transition_fee.rs index f6bcddef07d..777fd762fc8 100644 --- a/packages/wasm-dpp/src/state_transition/fee/calculate_state_transition_fee.rs +++ b/packages/wasm-dpp/src/state_transition/fee/calculate_state_transition_fee.rs @@ -1,8 +1,12 @@ -use dpp::state_transition::fee::calculate_state_transition_fee_factory::calculate_state_transition_fee; +use dpp::{ + state_transition::fee::calculate_state_transition_fee_factory::calculate_state_transition_fee, + ProtocolError, +}; use wasm_bindgen::prelude::*; use crate::{ conversion::create_state_transition_from_wasm_instance, fee::fee_result::FeeResultWasm, + utils::WithJsError, }; #[wasm_bindgen(js_name=calculateStateTransitionFee)] @@ -11,5 +15,8 @@ pub fn calculate_state_transition_fee_wasm( ) -> Result { let state_transition = create_state_transition_from_wasm_instance(state_transition_js)?; - Ok(calculate_state_transition_fee(&state_transition).into()) + Ok(calculate_state_transition_fee(&state_transition) + .map_err(ProtocolError::from) + .with_js_error()? + .into()) } diff --git a/packages/wasm-dpp/src/state_transition/fee/operations/pre_calculated_operation.rs b/packages/wasm-dpp/src/state_transition/fee/operations/pre_calculated_operation.rs index 0ba6380d95d..640c05606e2 100644 --- a/packages/wasm-dpp/src/state_transition/fee/operations/pre_calculated_operation.rs +++ b/packages/wasm-dpp/src/state_transition/fee/operations/pre_calculated_operation.rs @@ -1,7 +1,10 @@ use crate::{fee::dummy_fee_result::DummyFeesResultWasm, utils::Inner}; -use dpp::state_transition::fee::{ - operations::{OperationLike, PreCalculatedOperation}, - Refunds, +use dpp::{ + state_transition::fee::{ + operations::{OperationLike, PreCalculatedOperation}, + Refunds, + }, + ProtocolError, }; use js_sys::{Array, BigInt}; use wasm_bindgen::prelude::*; @@ -57,13 +60,23 @@ impl PreCalculatedOperationWasm { } #[wasm_bindgen(js_name = getProcessingCost)] - pub fn processing_cost(&self) -> BigInt { - BigInt::from(self.0.get_processing_cost()) + pub fn processing_cost(&self) -> Result { + Ok(BigInt::from( + self.0 + .get_processing_cost() + .map_err(ProtocolError::from) + .with_js_error()?, + )) } #[wasm_bindgen(js_name=getStorageCost)] - pub fn storage_cost(&self) -> BigInt { - BigInt::from(self.0.get_storage_cost()) + pub fn storage_cost(&self) -> Result { + Ok(BigInt::from( + self.0 + .get_storage_cost() + .map_err(ProtocolError::from) + .with_js_error()?, + )) } #[wasm_bindgen(getter)] diff --git a/packages/wasm-dpp/src/state_transition/fee/operations/read_operation.rs b/packages/wasm-dpp/src/state_transition/fee/operations/read_operation.rs index abb657d07cd..9d980bfbdb3 100644 --- a/packages/wasm-dpp/src/state_transition/fee/operations/read_operation.rs +++ b/packages/wasm-dpp/src/state_transition/fee/operations/read_operation.rs @@ -1,4 +1,7 @@ -use dpp::state_transition::fee::operations::{OperationLike, ReadOperation}; +use dpp::{ + state_transition::fee::operations::{OperationLike, ReadOperation}, + ProtocolError, +}; use js_sys::{Array, BigInt}; use wasm_bindgen::prelude::*; @@ -32,13 +35,23 @@ impl ReadOperationWasm { } #[wasm_bindgen(getter,js_name = processingCost)] - pub fn processing_cost(&self) -> BigInt { - BigInt::from(self.0.get_processing_cost()) + pub fn processing_cost(&self) -> Result { + Ok(BigInt::from( + self.0 + .get_processing_cost() + .map_err(ProtocolError::from) + .with_js_error()?, + )) } #[wasm_bindgen(getter, js_name=storageCost)] - pub fn storage_cost(&self) -> BigInt { - BigInt::from(self.0.get_storage_cost()) + pub fn storage_cost(&self) -> Result { + Ok(BigInt::from( + self.0 + .get_storage_cost() + .map_err(ProtocolError::from) + .with_js_error()?, + )) } #[wasm_bindgen(getter)] diff --git a/packages/wasm-dpp/src/state_transition/fee/operations/signature_verification_operation.rs b/packages/wasm-dpp/src/state_transition/fee/operations/signature_verification_operation.rs index ffa27dc4723..85fbc7cd360 100644 --- a/packages/wasm-dpp/src/state_transition/fee/operations/signature_verification_operation.rs +++ b/packages/wasm-dpp/src/state_transition/fee/operations/signature_verification_operation.rs @@ -4,6 +4,7 @@ use anyhow::anyhow; use dpp::{ identity::KeyType, state_transition::fee::operations::{OperationLike, SignatureVerificationOperation}, + ProtocolError, }; use js_sys::{Array, BigInt}; use wasm_bindgen::prelude::*; @@ -41,13 +42,23 @@ impl SignatureVerificationOperationWasm { } #[wasm_bindgen(js_name = getProcessingCost)] - pub fn get_processing_cost(&self) -> BigInt { - BigInt::from(self.0.get_processing_cost()) + pub fn get_processing_cost(&self) -> Result { + Ok(BigInt::from( + self.0 + .get_processing_cost() + .map_err(ProtocolError::from) + .with_js_error()?, + )) } #[wasm_bindgen(js_name=getStorageCost)] - pub fn get_storage_cost(&self) -> BigInt { - BigInt::from(self.0.get_storage_cost()) + pub fn get_storage_cost(&self) -> Result { + Ok(BigInt::from( + self.0 + .get_storage_cost() + .map_err(ProtocolError::from) + .with_js_error()?, + )) } #[wasm_bindgen(getter)] From 5feec4717557f7f175222770598079da7ee446c1 Mon Sep 17 00:00:00 2001 From: Pawel Iwan Date: Fri, 31 Mar 2023 12:38:32 +0200 Subject: [PATCH 2/4] cargo fmt --- .../fee/calculate_state_transition_fee_factory.rs | 15 ++++++++++----- .../fee/operations/read_operation.rs | 3 --- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs b/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs index 64cd798b1e4..2216fc4b0aa 100644 --- a/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs +++ b/packages/rs-dpp/src/state_transition/fee/calculate_state_transition_fee_factory.rs @@ -1,11 +1,16 @@ -use crate::{state_transition::{ - fee::calculate_state_transition_fee_from_operations_factory::calculate_state_transition_fee_from_operations, - StateTransition, StateTransitionLike, -}, NonConsensusError}; +use crate::{ + state_transition::{ + fee::calculate_state_transition_fee_from_operations_factory::calculate_state_transition_fee_from_operations, + StateTransition, StateTransitionLike, + }, + NonConsensusError, +}; use super::FeeResult; -pub fn calculate_state_transition_fee(state_transition: &StateTransition) -> Result { +pub fn calculate_state_transition_fee( + state_transition: &StateTransition, +) -> Result { let execution_context = state_transition.get_execution_context(); calculate_state_transition_fee_from_operations( diff --git a/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs b/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs index 4a74727bd3c..e50bd2389ad 100644 --- a/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs +++ b/packages/rs-dpp/src/state_transition/fee/operations/read_operation.rs @@ -10,9 +10,6 @@ use crate::{ NonConsensusError, }; -const ProcessingCostOverflowError: NonConsensusError = - NonConsensusError::Overflow("the processing cost is too big"); - #[derive(Default, Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] pub struct ReadOperation { From 147284eaeaed14db5b4a932a055bb715b0102ef3 Mon Sep 17 00:00:00 2001 From: Pawel Iwan Date: Fri, 31 Mar 2023 13:06:20 +0200 Subject: [PATCH 3/4] fix clippy warnings --- .../src/block_time_window/validate_time_in_block_time_window.rs | 2 +- .../validate_identity_update_transition_state.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs b/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs index 759c085648e..db94727cfb7 100644 --- a/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs +++ b/packages/rs-dpp/src/block_time_window/validate_time_in_block_time_window.rs @@ -1,4 +1,4 @@ -use crate::{prelude::TimestampMillis, NonConsensusError, ProtocolError}; +use crate::{prelude::TimestampMillis, NonConsensusError}; use super::validation_result::TimeWindowValidationResult; diff --git a/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs b/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs index 3a10e1118a5..3e457b65847 100644 --- a/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs +++ b/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs @@ -5,7 +5,6 @@ use std::sync::Arc; use crate::consensus::signature::{IdentityNotFoundError, SignatureError}; use crate::identity::state_transition::identity_update_transition::IdentityUpdateTransitionAction; -use crate::ProtocolError; use crate::{ block_time_window::validate_time_in_block_time_window::validate_time_in_block_time_window, identity::validation::{RequiredPurposeAndSecurityLevelValidator, TPublicKeysValidator}, From 834bd21ad10928aa9290e934a2ed3cfe6c55b4ed Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Tue, 4 Apr 2023 17:51:30 +0800 Subject: [PATCH 4/4] chore: fix merge errors --- .../validate_identity_update_transition_state.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs b/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs index de26da63b04..9610ebebf72 100644 --- a/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs +++ b/packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs @@ -11,12 +11,7 @@ use crate::consensus::state::identity::identity_public_key_is_read_only_error::I use crate::consensus::state::identity::invalid_identity_public_key_id_error::InvalidIdentityPublicKeyIdError; use crate::consensus::state::identity::invalid_identity_revision_error::InvalidIdentityRevisionError; use crate::consensus::state::state_error::StateError; -use crate::consensus::ConsensusError::StateError; use crate::identity::state_transition::identity_update_transition::IdentityUpdateTransitionAction; -use crate::StateError::{ - IdentityPublicKeyIsDisabledError, IdentityPublicKeyIsReadOnlyError, - InvalidIdentityPublicKeyIdError, InvalidIdentityRevisionError, -}; use crate::{ block_time_window::validate_time_in_block_time_window::validate_time_in_block_time_window, identity::validation::{RequiredPurposeAndSecurityLevelValidator, TPublicKeysValidator},