Skip to content

Commit

Permalink
refactor(drive): remove duplicated withdrawal amount validation (#2191)
Browse files Browse the repository at this point in the history
  • Loading branch information
@shumkov
shumkov authored Oct 3, 2024
1 parent 75296d5 commit 3404f59
Show file tree
Hide file tree
Showing 4 changed files with 357 additions and 32 deletions.
1 change: 1 addition & 0 deletions ...ensus/basic/identity/invalid_identity_credit_withdrawal_transition_output_script_error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ impl InvalidIdentityCreditWithdrawalTransitionOutputScriptError {
}

pub fn output_script(&self) -> CoreScript {
// TODO: We shouldn't clone in getter 🤦
self.output_script.clone()
}
}
Expand Down
103 changes: 98 additions & 5 deletions ...alidation/state_transition/state_transitions/identity_credit_withdrawal/balance/v0/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ use dpp::state_transition::identity_credit_withdrawal_transition::accessors::Ide
use dpp::state_transition::identity_credit_withdrawal_transition::IdentityCreditWithdrawalTransition;

use crate::error::execution::ExecutionError;
use crate::execution::types::execution_operation::ValidationOperation;
use crate::execution::types::state_transition_execution_context::{
StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0,
};
use dpp::validation::SimpleConsensusValidationResult;
use dpp::version::PlatformVersion;

Expand All @@ -34,7 +30,11 @@ impl IdentityCreditTransferTransitionBalanceValidationV0 for IdentityCreditWithd
"expected to have a balance on identity for credit withdrawal transition",
)))?;

if balance < self.amount().checked_add(platform_version.fee_version.state_transition_min_fees.credit_withdrawal).ok_or(Error::Execution(ExecutionError::Overflow("overflow when adding amount and min_leftover_credits_before_processing in identity credit withdrawal")))? {
let amount_and_fees = self.amount()
.checked_add(platform_version.fee_version.state_transition_min_fees.credit_withdrawal)
.ok_or_else(|| Error::Execution(ExecutionError::Overflow("overflow when adding amount and min_leftover_credits_before_processing in identity credit withdrawal")))?;

if balance < amount_and_fees {
return Ok(SimpleConsensusValidationResult::new_with_error(
IdentityInsufficientBalanceError::new(self.identity_id(), balance, self.amount())
.into(),
Expand All @@ -44,3 +44,96 @@ impl IdentityCreditTransferTransitionBalanceValidationV0 for IdentityCreditWithd
Ok(SimpleConsensusValidationResult::new())
}
}

#[cfg(test)]
mod tests {
use super::*;

use assert_matches::assert_matches;
use dpp::consensus::state::state_error::StateError;
use dpp::consensus::ConsensusError;
use dpp::prelude::Identifier;
use dpp::state_transition::identity_credit_withdrawal_transition::v0::IdentityCreditWithdrawalTransitionV0;
use platform_version::version::v1::PLATFORM_V1;

mod validate_advanced_minimum_balance_pre_check_v0 {
use super::*;

#[test]
fn should_return_invalid_result_if_balance_is_less_than_amount_and_fees() {
let balance = 100;

let amount = 200;

let identity = PartialIdentity {
id: Identifier::random(),
loaded_public_keys: Default::default(),
balance: Some(balance),
revision: None,
not_found_public_keys: Default::default(),
};

let transaction =
IdentityCreditWithdrawalTransition::V0(IdentityCreditWithdrawalTransitionV0 {
identity_id: identity.id,
amount,
core_fee_per_byte: 0,
pooling: Default::default(),
output_script: Default::default(),
nonce: 0,
user_fee_increase: 0,
signature_public_key_id: 0,
signature: Default::default(),
});

let platform_version = &PLATFORM_V1;

let result = transaction
.validate_advanced_minimum_balance_pre_check_v0(&identity, platform_version)
.expect("failed to validate minimum balance");

assert_matches!(
result.errors.as_slice(),
[ConsensusError::StateError(
StateError::IdentityInsufficientBalanceError(err)
)] if err.balance() == balance && err.required_balance() == amount && err.identity_id() == &identity.id
);
}

#[test]
fn should_return_valid_result() {
let balance = 200000000000;

let amount = 100;

let identity = PartialIdentity {
id: Identifier::random(),
loaded_public_keys: Default::default(),
balance: Some(balance),
revision: None,
not_found_public_keys: Default::default(),
};

let transaction =
IdentityCreditWithdrawalTransition::V0(IdentityCreditWithdrawalTransitionV0 {
identity_id: Default::default(),
amount,
core_fee_per_byte: 0,
pooling: Default::default(),
output_script: Default::default(),
nonce: 0,
user_fee_increase: 0,
signature_public_key_id: 0,
signature: Default::default(),
});

let platform_version = &PLATFORM_V1;

let result = transaction
.validate_advanced_minimum_balance_pre_check_v0(&identity, platform_version)
.expect("failed to validate minimum balance");

assert_matches!(result.errors.as_slice(), []);
}
}
}
28 changes: 1 addition & 27 deletions .../validation/state_transition/state_transitions/identity_credit_withdrawal/state/v0/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ use crate::platform_types::platform::PlatformRef;
use crate::rpc::core::CoreRPCLike;
use dpp::block::block_info::BlockInfo;

use dpp::consensus::signature::IdentityNotFoundError;
use dpp::consensus::state::identity::IdentityInsufficientBalanceError;
use dpp::prelude::ConsensusValidationResult;
use dpp::state_transition::identity_credit_withdrawal_transition::accessors::IdentityCreditWithdrawalTransitionAccessorsV0;
use dpp::state_transition::identity_credit_withdrawal_transition::IdentityCreditWithdrawalTransition;

use crate::execution::types::execution_operation::ValidationOperation;
Expand Down Expand Up @@ -49,29 +46,6 @@ impl IdentityCreditWithdrawalStateTransitionStateValidationV0
tx: TransactionArg,
platform_version: &PlatformVersion,
) -> Result<ConsensusValidationResult<StateTransitionAction>, Error> {
let maybe_existing_identity_balance = platform.drive.fetch_identity_balance(
self.identity_id().to_buffer(),
tx,
platform_version,
)?;

let Some(existing_identity_balance) = maybe_existing_identity_balance else {
return Ok(ConsensusValidationResult::new_with_error(
IdentityNotFoundError::new(self.identity_id()).into(),
));
};

if existing_identity_balance < self.amount() {
return Ok(ConsensusValidationResult::new_with_error(
IdentityInsufficientBalanceError::new(
self.identity_id(),
existing_identity_balance,
self.amount(),
)
.into(),
));
}

self.transform_into_action_v0(
platform,
block_info,
Expand All @@ -91,7 +65,7 @@ impl IdentityCreditWithdrawalStateTransitionStateValidationV0
) -> Result<ConsensusValidationResult<StateTransitionAction>, Error> {
let consensus_validation_result =
IdentityCreditWithdrawalTransitionAction::try_from_identity_credit_withdrawal(
&platform.drive,
platform.drive,
tx,
self,
block_info,
Expand Down
Loading

0 comments on commit 3404f59

Please sign in to comment.