Skip to content

test(drive): improve identity module test coverage #3330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
QuantumExplorer merged 2 commits into from
Mar 16, 2026
Merged

test(drive): improve identity module test coverage #3330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9f4a75
test(drive): improve identity module test coverage
QuantumExplorer Mar 15, 2026
a9303fd
fix(test): address CodeRabbit review feedback
QuantumExplorer Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions packages/rs-drive/src/drive/identity/balance/update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,4 +932,74 @@ mod tests {
));
}
}

mod remove_from_identity_balance_errors {
use super::*;
use crate::error::identity::IdentityError;
use dpp::block::block_info::BlockInfo;
use dpp::version::PlatformVersion;

#[test]
fn should_fail_to_remove_more_than_balance() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let identity = create_test_identity(&drive, [0; 32], Some(15), None, platform_version)
.expect("expected to create an identity");

// Identity starts with balance from create_test_identity (which is 0 after creation
// since create_test_identity sets balance to 0). Let's add some balance first.
let added_balance = 100;

drive
.add_to_identity_balance(
identity.id().to_buffer(),
added_balance,
&BlockInfo::default(),
true,
None,
platform_version,
)
.expect("expected to add balance");

// Now try to remove more than available
let result = drive.remove_from_identity_balance(
identity.id().to_buffer(),
added_balance + 1,
&BlockInfo::default(),
true,
None,
platform_version,
None,
);

assert!(matches!(
result,
Err(Error::Identity(IdentityError::IdentityInsufficientBalance(
_
)))
));
}

#[test]
fn should_fail_to_remove_from_non_existent_identity() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let result = drive.remove_from_identity_balance(
[0; 32],
100,
&BlockInfo::default(),
true,
None,
platform_version,
None,
);

assert!(matches!(
result,
Err(Error::Drive(DriveError::CorruptedCodeExecution(_)))
));
}
}
}
234 changes: 234 additions & 0 deletions packages/rs-drive/src/drive/identity/fetch/balance/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,237 @@
mod fetch_identity_balance;
mod fetch_identity_balance_include_debt;
mod fetch_identity_negative_balance;

#[cfg(feature = "server")]
#[cfg(test)]
mod tests {
use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure;
use crate::util::test_helpers::test_utils::identities::create_test_identity;
use dpp::block::block_info::BlockInfo;
use dpp::identity::accessors::IdentityGettersV0;
use dpp::identity::Identity;
use dpp::version::PlatformVersion;

mod fetch_identity_balance {
use super::*;

#[test]
fn should_return_none_for_non_existent_identity() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let balance = drive
.fetch_identity_balance([0; 32], None, platform_version)
.expect("should not error");

assert!(balance.is_none());
}

#[test]
fn should_return_balance_for_existing_identity() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let identity = Identity::random_identity(3, Some(42), platform_version)
.expect("expected a random identity");

let expected_balance = identity.balance();

drive
.add_new_identity(
identity.clone(),
false,
&BlockInfo::default(),
true,
None,
platform_version,
)
.expect("expected to add identity");

let balance = drive
.fetch_identity_balance(identity.id().to_buffer(), None, platform_version)
.expect("should not error")
.expect("should have balance");

assert_eq!(balance, expected_balance);
}

#[test]
fn should_return_balance_with_costs_estimated() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let identity = Identity::random_identity(3, Some(42), platform_version)
.expect("expected a random identity");

drive
.add_new_identity(
identity.clone(),
false,
&BlockInfo::default(),
true,
None,
platform_version,
)
.expect("expected to add identity");

let block_info = BlockInfo::default();

// Estimated mode (apply=false)
let (balance, fee_result) = drive
.fetch_identity_balance_with_costs(
identity.id().to_buffer(),
&block_info,
false,
None,
platform_version,
)
.expect("should return balance with costs");

// In estimated (stateless) mode, the balance query does not read
// from the database. It returns Some(0) as a placeholder value
// while computing estimated costs.
assert!(fee_result.processing_fee > 0);
assert_eq!(balance, Some(0));
Copy link
Copy Markdown
Member Author

@QuantumExplorer QuantumExplorer Mar 16, 2026

Choose a reason for hiding this comment

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

Low: Estimated-mode test asserts a hardcoded placeholder

In apply=false mode, the production code returns Some(0) without querying the database (it's a StatelessDirectQuery). This test goes through the overhead of creating and inserting an identity that is never read. The only meaningful assertion here is processing_fee > 0 (fee calculation wiring).

Consider adding a comment that this test validates fee calculation in estimation mode, not actual balance fetching. The inserted identity is irrelevant to the test outcome.

}
}

mod fetch_identity_balance_include_debt {
use super::*;
use crate::fees::op::LowLevelDriveOperation;

#[test]
fn should_return_none_for_non_existent_identity() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let balance = drive
.fetch_identity_balance_include_debt([0; 32], None, platform_version)
.expect("should not error");

assert!(balance.is_none());
}

#[test]
fn should_return_positive_balance_for_identity_with_funds() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let identity = create_test_identity(&drive, [0; 32], Some(1), None, platform_version)
.expect("expected an identity");

let added_balance = 1000;

drive
.add_to_identity_balance(
identity.id().to_buffer(),
added_balance,
&BlockInfo::default(),
true,
None,
platform_version,
)
.expect("should add balance");

let balance = drive
.fetch_identity_balance_include_debt(
identity.id().to_buffer(),
None,
platform_version,
)
.expect("should not error")
.expect("should have balance");

assert_eq!(balance, added_balance as i64);
}

#[test]
fn should_return_negative_balance_for_identity_with_debt() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let identity = create_test_identity(&drive, [0; 32], Some(1), None, platform_version)
.expect("expected an identity");

let negative_amount: u64 = 500;

// Set negative balance (debt)
let batch = vec![drive
.update_identity_negative_credit_operation(
identity.id().to_buffer(),
negative_amount,
platform_version,
)
.expect("expected operation")];

let mut drive_operations: Vec<LowLevelDriveOperation> = vec![];
drive
.apply_batch_low_level_drive_operations(
None,
None,
batch,
&mut drive_operations,
&platform_version.drive,
)
.expect("should apply batch");

let balance = drive
.fetch_identity_balance_include_debt(
identity.id().to_buffer(),
None,
platform_version,
)
.expect("should not error")
.expect("should have balance");

assert_eq!(balance, -(negative_amount as i64));
Copy link
Copy Markdown
Member Author

@QuantumExplorer QuantumExplorer Mar 16, 2026

Choose a reason for hiding this comment

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

Medium: Missing edge case — combined positive balance + negative credit

This test covers the credits == 0 + negative credit > 0 case. But the production code in fetch_identity_balance_include_debt_operations_v0 has a branch: if credits > 0, it returns the positive balance and NEVER checks the negative credit field. Missing edge cases:

  1. Both balance > 0 AND negative credit > 0 — which takes precedence? (data corruption scenario)
  2. Both balance == 0 AND negative credit == 0 — should return Some(0)

For a financial system, testing what happens when both fields are set is important even if it "shouldn't happen" — it documents the behavior under corruption.

}
}

mod fetch_identity_negative_balance {
use super::*;

#[test]
fn should_error_for_non_existent_identity() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let mut drive_operations = vec![];
// For a non-existent identity, the path doesn't exist, so grove_get_raw
// returns an error (PathParentLayerNotFound) since the identity subtree
// doesn't exist at all.
let result = drive.fetch_identity_negative_balance_operations(
[0; 32],
true,
None,
&mut drive_operations,
platform_version,
);

assert!(result.is_err());
Copy link
Copy Markdown
Member Author

@QuantumExplorer QuantumExplorer Mar 16, 2026

Choose a reason for hiding this comment

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

Low: assert!(result.is_err()) without checking error type

The comment explains the error is PathParentLayerNotFound, but the assertion accepts any error. Consider matching on the specific error variant:

assert!(matches!(
    result,
    Err(Error::GroveDB(_))
), "expected GroveDB error for non-existent identity path");

}

#[test]
fn should_return_zero_negative_balance_for_new_identity() {
let drive = setup_drive_with_initial_state_structure(None);
let platform_version = PlatformVersion::latest();

let identity = create_test_identity(&drive, [0; 32], Some(1), None, platform_version)
.expect("expected an identity");

let mut drive_operations = vec![];
let negative_balance = drive
.fetch_identity_negative_balance_operations(
identity.id().to_buffer(),
true,
None,
&mut drive_operations,
platform_version,
)
.expect("should not error")
.expect("should have negative balance");

assert_eq!(negative_balance, 0);
}
}
}
Loading
Loading