Skip to content

Conversation

@xdustinface
Copy link
Collaborator

@xdustinface xdustinface commented Jan 3, 2026

Consolidate all UTXO creation helpers within key-wallet crate in a single file test_utils.rs.

Summary by CodeRabbit

  • Tests

    • Centralized and expanded test utilities for UTXO and address creation, including batch UTXO generation to simplify and standardize test fixtures.
    • Updated many test suites to use the new test helpers, reducing boilerplate and improving consistency.
  • Refactor

    • Moved and reorganized test helpers into a dedicated test utilities module and adjusted test setups to rely on it.

✏️ Tip: You can customize this high-level summary in your review settings.

@xdustinface xdustinface changed the title refactor: consolidate utxos helpers in test_utils.rs refactor: consolidate UTXO helpers in test_utils.rs Jan 3, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

The PR centralizes test helpers into a new test_utils module (providing test_address, Utxo::new_test, Utxo::new_test_batch), updates tests to use these helpers, and wires a test-utils feature/dev-dependency so other crates can use the test utilities.

Changes

Cohort / File(s) Summary
Test Utilities Infrastructure
key-wallet/src/lib.rs, key-wallet/src/test_utils/mod.rs, key-wallet/src/test_utils/address.rs, key-wallet/src/test_utils/utxo.rs
Adds a gated test_utils module with address and utxo submodules. Exposes test_address() and adds Utxo::new_test() and Utxo::new_test_batch() test constructors.
Package Configuration
key-wallet/Cargo.toml, key-wallet-ffi/Cargo.toml
Adds test-utils feature in key-wallet; updates dev-dependencies so key-wallet-ffi and dev builds can enable test-utils and test-utils-dependent testing.
UTXO Module Tests
key-wallet/src/utxo.rs
Adds pub fn new_test(...) to Utxo and replaces former in-file test helper usages in UTXO tests.
Wallet Coin Selection Tests
key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
Removes local test_utxo() helper and converts tests to use Utxo::new_test(...).
Transaction Builder Tests
key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
Removes in-file test helpers (UTXO/address) and imports external test_address; replaces UTXO constructions with Utxo::new_test(...) across many tests.
Transaction Building Tests
key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
Drops low-level test imports (ScriptBuf, OutPoint, TxOut) and migrates tests to Utxo::new_test(...).
Optimal Consolidation Tests
key-wallet/tests/test_optimal_consolidation.rs
Replaces verbose OutPoint/TxOut/Address setup with Utxo::new_test(...) calls in test setup.
FFI UTXO Tests
key-wallet-ffi/src/utxo_tests.rs
Replaces manual per-UTXO construction with Utxo::new_test_batch(...) and looped insertion into account UTXO maps.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I dug a path to tidy the lair,

little helpers gathered with care,
one new burrow for address and coin,
fewer hops for tests to join,
carrots of clarity—cheers in the air!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 79.31% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: consolidating UTXO test helpers into a dedicated test_utils module, which is the core refactoring across multiple files.
✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
key-wallet/src/wallet/managed_wallet_info/transaction_building.rs (1)

180-409: Consolidate local test_utxo helper in test_optimal_consolidation.rs to centralized utilities.

The migration is incomplete. key-wallet/tests/test_optimal_consolidation.rs contains a local test_utxo(value: u64, confirmed: bool) helper that duplicates centralized functionality already available via test_utxo_full(value, height, vout, confirmed) in test_utils.rs. Replace the local helper with calls to test_utxo_full(value, 100, 0, confirmed) or add a convenience wrapper pub fn test_utxo_with_confirmed(value: u64, confirmed: bool) to the centralized utilities.

key-wallet/src/tests/test_utils.rs (1)

25-37: Consider adding doc comments and validating ScriptBuf usage.

Two optional improvements to enhance test utility quality:

  1. Missing doc comments: Adding documentation for the public functions would help users understand the purpose and parameters (especially the confirmed parameter's behavior).

  2. Empty ScriptBuf on Line 32: The TxOut is created with an empty ScriptBuf, which may not be realistic for tests that validate script execution or UTXO spending logic. Consider whether tests might benefit from a proper script corresponding to the address (e.g., using address.script_pubkey()).

Example: Adding doc comment and proper script
+/// Creates a test UTXO with full control over parameters.
+///
+/// # Arguments
+/// * `value` - Amount in satoshis
+/// * `height` - Block height
+/// * `vout` - Transaction output index
+/// * `confirmed` - Whether the UTXO is confirmed
 pub fn test_utxo_full(value: u64, height: u32, vout: u32, confirmed: bool) -> Utxo {
     let outpoint = OutPoint {
         txid: Txid::from_raw_hash(sha256d::Hash::from_slice(&[1u8; 32]).unwrap()),
         vout,
     };
+    let address = test_address();
     let txout = TxOut {
         value,
-        script_pubkey: ScriptBuf::new(),
+        script_pubkey: address.script_pubkey(),
     };
-    let mut utxo = Utxo::new(outpoint, txout, test_address(), height, false);
+    let mut utxo = Utxo::new(outpoint, txout, address, height, false);
     utxo.is_confirmed = confirmed;
     utxo
 }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd12793 and 88f98b2.

📒 Files selected for processing (6)
  • key-wallet/src/tests/mod.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
🧰 Additional context used
📓 Path-based instructions (5)
key-wallet/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/**/*.rs: Separate immutable structures (Account, Wallet) containing only identity information from mutable wrappers (ManagedAccount, ManagedWalletInfo) with state management
Never serialize or log private keys in production; use public keys or key fingerprints for identification instead
Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
Use BTreeMap for ordered data (accounts, transactions) and HashMap for lookups (address mappings); apply memory management strategies for old transaction data
Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Use the ? operator for error propagation, provide context in error messages, never panic in library code, and return Result<T> for all fallible operations

Files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
key-wallet/**/tests/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/**/tests/**/*.rs: Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Use deterministic testing with known test vectors and fixed seeds for reproducible results
Use property-based testing for complex invariants such as gap limit constraints

Files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/tests/test_utils.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with #[cfg(test)] annotation; integration tests use the tests/ directory
Use snake_case for function and variable names
Use UpperCamelCase for types and traits
Use SCREAMING_SNAKE_CASE for constants
Format code with rustfmt before commits; ensure cargo fmt --all is run
Run cargo clippy --workspace --all-targets -- -D warnings for linting; avoid warnings in CI
Prefer async/await via tokio for asynchronous operations

**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types (thiserror) and propagate errors appropriately in Rust
Use tokio runtime for async operations in Rust
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format code using cargo fmt
Run clippy with all features and all targets, treating warnings as errors
Never log or expose private keys in any code
Always validate inputs from untrusted sources in Rust
Use secure random number generation for keys

Files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/tests/**/*.rs: Use descriptive test names (e.g., test_parse_address_mainnet)
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored

Files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/tests/test_utils.rs
**/*test*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*test*.rs: Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate

Files:

  • key-wallet/src/tests/test_utils.rs
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) to enable both full wallets and watch-only wallets with the same interface
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use deterministic testing with known test vectors and fixed seeds for reproducible results
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Pre-generate addresses in batches (typically 20-100) and store them in pools; only derive on-demand when the pool is exhausted
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use property-based testing for complex invariants such as gap limit constraints
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) to enable both full wallets and watch-only wallets with the same interface

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use deterministic testing with known test vectors and fixed seeds for reproducible results

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*.rs : Write unit tests for new functionality in Rust

Applied to files:

  • key-wallet/src/tests/mod.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*integration*.rs : Write integration tests for network operations

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/*.rs : Unit tests should live alongside code with `#[cfg(test)]` annotation; integration tests use the `tests/` directory

Applied to files:

  • key-wallet/src/tests/mod.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*test*.rs : Test both mainnet and testnet configurations

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Pre-generate addresses in batches (typically 20-100) and store them in pools; only derive on-demand when the pool is exhausted

Applied to files:

  • key-wallet/src/tests/mod.rs
  • key-wallet/src/tests/test_utils.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: FFI bindings are organized in `*-ffi` crates; shared helpers in `internals/` and `test-utils/`

Applied to files:

  • key-wallet/src/tests/mod.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together

Applied to files:

  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations

Applied to files:

  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`

Applied to files:

  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations

Applied to files:

  • key-wallet/src/utxo.rs
  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite

Applied to files:

  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.

Applied to files:

  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data

Applied to files:

  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : Implement transaction classification and routing through `TransactionRouter` to avoid checking all accounts for every transaction

Applied to files:

  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Generate addresses in batches using gap limit and staged generation instead of unbounded address generation to prevent memory and performance issues

Applied to files:

  • key-wallet/src/tests/test_utils.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use property-based testing for complex invariants such as gap limit constraints

Applied to files:

  • key-wallet/src/tests/test_utils.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 74
File: key-wallet/src/address.rs:30-40
Timestamp: 2025-06-15T15:31:44.136Z
Learning: In the key-wallet crate, QuantumExplorer prefers keeping wildcard arms that default unknown Network variants to testnet prefixes rather than using exhaustive matches or panics. This fallback behavior is intentional.

Applied to files:

  • key-wallet/src/tests/test_utils.rs
🧬 Code graph analysis (5)
key-wallet/src/utxo.rs (1)
key-wallet/src/tests/test_utils.rs (1)
  • test_utxo_full (25-37)
key-wallet/src/wallet/managed_wallet_info/transaction_building.rs (1)
key-wallet/tests/test_optimal_consolidation.rs (1)
  • test_utxo (11-35)
key-wallet/src/tests/test_utils.rs (1)
key-wallet/src/utxo.rs (3)
  • value (61-63)
  • new (41-58)
  • new (130-138)
key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs (2)
key-wallet/src/tests/test_utils.rs (2)
  • test_address (17-19)
  • test_utxo (21-23)
key-wallet/tests/test_optimal_consolidation.rs (1)
  • test_utxo (11-35)
key-wallet/src/wallet/managed_wallet_info/coin_selection.rs (2)
key-wallet/src/tests/test_utils.rs (1)
  • test_utxo (21-23)
key-wallet/tests/test_optimal_consolidation.rs (1)
  • test_utxo (11-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Thread Sanitizer
  • GitHub Check: Address Sanitizer
  • GitHub Check: Pre-commit (macos-latest)
  • GitHub Check: Pre-commit (windows-latest)
  • GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (6)
key-wallet/src/tests/mod.rs (1)

5-6: LGTM - Clean consolidation of test utilities.

The public exposure of the test_utils module successfully centralizes UTXO creation helpers, eliminating duplication across test files and improving maintainability.

key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs (1)

838-838: LGTM - Successful migration to centralized test utilities.

The import correctly references the new shared test utilities module, eliminating local test helper duplication and aligning with the PR's consolidation objective.

key-wallet/src/utxo.rs (1)

310-333: LGTM - Proper use of test_utxo_full for fine-grained control.

The tests correctly use test_utxo_full directly to control specific parameters:

  • Line 314: Creates an unconfirmed UTXO (confirmed=false) to test confirmation logic
  • Lines 332-333: Uses different vout values (0 and 1) to ensure unique OutPoints for distinct UTXOs

This demonstrates the value of having both test_utxo (simplified defaults) and test_utxo_full (explicit control) available.

key-wallet/src/wallet/managed_wallet_info/coin_selection.rs (1)

693-756: LGTM - Simplified test data construction.

The migration to test_utxo(value) from the centralized utilities successfully simplifies test setup by defaulting to confirmed UTXOs, which is appropriate for all coin selection tests in this file.

key-wallet/src/tests/test_utils.rs (2)

1-19: LGTM! Deterministic test fixtures with consistent network usage.

The use of a fixed test public key and consistent Testnet network aligns well with the learning to "Use deterministic testing with known test vectors and fixed seeds for reproducible results." This ensures test reproducibility across runs.

Based on learnings, deterministic testing with fixed seeds is the preferred approach for the key-wallet crate.


21-23: LGTM! Convenient wrapper with sensible defaults.

The convenience function provides sensible defaults for the common case of a confirmed UTXO, reducing test boilerplate.

@ZocoLini
Copy link
Collaborator

ZocoLini commented Jan 3, 2026

Before approving the PR, I would like to ask: I think it would be best to have a testutils/ in src, only available under cfg(test) and behind a feature. That way, we would have them available for integration tests, unit tests, and other crates (dash-spv). I am making this suggestion under the assumption that we would take advantage of those utils in more places.

@xdustinface
Copy link
Collaborator Author

Before approving the PR, I would like to ask: I think it would be best to have a testutils/ in src, only available under cfg(test) and behind a feature. That way, we would have them available for integration tests, unit tests, and other crates (dash-spv). I am making this suggestion under the assumption that we would take advantage of those utils in more places.

Can you see a place where this particular helpers could be reused other than where they are used now? Surely this would make sense if we have helpers reused in other places. Feel free to explore the repo in this regard and add an extra module to the wallet if there is a use for it. Just a sidethought, i think if you add it feature gated you dont need the cfg(test) restriction?

@xdustinface xdustinface requested a review from ZocoLini January 5, 2026 20:02
@ZocoLini
Copy link
Collaborator

ZocoLini commented Jan 5, 2026

With a quick search I found two files where we are creating test instances of the Utxo struct:

  • key-wallet/tests/test_optimal_consolidation.rs line 11
  • key-wallet-ffi/src/utxo_tests.rs in multiple lines

I honestly think that the best approach is to have a testutils/ modules, with different submodules addressing different structs, specially contrustors ment to be used in tests, exposed to unit, integration and other crates tests.

Just a sidethought, i think if you add it feature gated you dont need the cfg(test) restriction?

Not sure right now tbh but I use to do it that way

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI Agents
In @key-wallet/src/wallet/managed_wallet_info/transaction_building.rs:
- Around line 190-194: The utxos vector uses identical id=0 for every UTXO,
producing duplicate outpoints; update the Utxo::new_test calls in the utxos
variable so each UTXO has a unique id (e.g., 0, 1, 2) to ensure distinct
outpoints for transaction building; locate the utxos definition and change the
id argument in each Utxo::new_test invocation accordingly.
🧹 Nitpick comments (4)
key-wallet/src/test_utils/address.rs (1)

1-11: Test address helper implemented correctly.

The implementation creates a valid P2PKH address from a hardcoded test public key on Testnet. Using unwrap() is acceptable for test utilities.

Optional: Consider adding documentation

Adding a doc comment would help users understand the purpose and characteristics of this test address:

+/// Creates a deterministic test address for use in unit and integration tests.
+///
+/// Returns a P2PKH address on Testnet derived from a fixed public key.
+/// This ensures reproducible test results across runs.
 pub fn test_address() -> Address {
     Address::p2pkh(&dashcore::PublicKey::from_slice(&TEST_PUBKEY_BYTES).unwrap(), Network::Testnet)
 }
key-wallet/src/test_utils/utxo.rs (3)

8-10: Consider a direct implementation for single UTXO creation.

The current implementation creates a Vec with a single element via new_test_batch(id..id + 1, ...) and immediately removes it, which is inefficient. While acceptable for test code, a direct implementation would be cleaner and avoid the allocation overhead.

🔎 Proposed refactor
 pub fn new_test(id: u8, value: u64, height: u32, confirmed: bool) -> Self {
-    Self::new_test_batch(id..id + 1, value, height, confirmed).remove(0)
+    let outpoint = OutPoint::new(Txid::from([id; 32]), 0);
+    let txout = TxOut {
+        value,
+        script_pubkey: ScriptBuf::new(),
+    };
+    let mut utxo = Utxo::new(outpoint, txout, test_address(), height, false);
+    utxo.is_confirmed = confirmed;
+    utxo
 }

23-26: Empty ScriptBuf may cause issues in script-dependent tests.

The test helper creates UTXOs with an empty script_pubkey. While acceptable for basic tests, this may cause issues if any tests validate or parse the script content. Consider whether a realistic P2PKH or P2SH script would be more appropriate.


28-28: is_coinbase is hardcoded to false.

The test helper always sets is_coinbase to false, which prevents testing coinbase-specific maturity rules (100 confirmations). Consider adding an optional parameter if coinbase UTXO testing becomes necessary.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88f98b2 and 9fdf96e.

📒 Files selected for processing (12)
  • key-wallet-ffi/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/Cargo.toml
  • key-wallet/src/lib.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/tests/test_optimal_consolidation.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
🧰 Additional context used
📓 Path-based instructions (5)
key-wallet/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/**/*.rs: Separate immutable structures (Account, Wallet) containing only identity information from mutable wrappers (ManagedAccount, ManagedWalletInfo) with state management
Never serialize or log private keys in production; use public keys or key fingerprints for identification instead
Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
Use BTreeMap for ordered data (accounts, transactions) and HashMap for lookups (address mappings); apply memory management strategies for old transaction data
Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Use the ? operator for error propagation, provide context in error messages, never panic in library code, and return Result<T> for all fallible operations

Files:

  • key-wallet/src/lib.rs
  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with #[cfg(test)] annotation; integration tests use the tests/ directory
Use snake_case for function and variable names
Use UpperCamelCase for types and traits
Use SCREAMING_SNAKE_CASE for constants
Format code with rustfmt before commits; ensure cargo fmt --all is run
Run cargo clippy --workspace --all-targets -- -D warnings for linting; avoid warnings in CI
Prefer async/await via tokio for asynchronous operations

**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types (thiserror) and propagate errors appropriately in Rust
Use tokio runtime for async operations in Rust
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format code using cargo fmt
Run clippy with all features and all targets, treating warnings as errors
Never log or expose private keys in any code
Always validate inputs from untrusted sources in Rust
Use secure random number generation for keys

Files:

  • key-wallet/src/lib.rs
  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
**/*test*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*test*.rs: Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate

Files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/tests/test_optimal_consolidation.rs
key-wallet/**/tests/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/**/tests/**/*.rs: Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Use deterministic testing with known test vectors and fixed seeds for reproducible results
Use property-based testing for complex invariants such as gap limit constraints

Files:

  • key-wallet/tests/test_optimal_consolidation.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/tests/**/*.rs: Use descriptive test names (e.g., test_parse_address_mainnet)
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored

Files:

  • key-wallet/tests/test_optimal_consolidation.rs
🧠 Learnings (35)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations

Applied to files:

  • key-wallet/src/lib.rs
  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/*.rs : Unit tests should live alongside code with `#[cfg(test)]` annotation; integration tests use the `tests/` directory

Applied to files:

  • key-wallet/src/lib.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*.rs : Write unit tests for new functionality in Rust

Applied to files:

  • key-wallet/src/lib.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) to enable both full wallets and watch-only wallets with the same interface

Applied to files:

  • key-wallet/src/lib.rs
  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use deterministic testing with known test vectors and fixed seeds for reproducible results

Applied to files:

  • key-wallet/src/lib.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use property-based testing for complex invariants such as gap limit constraints

Applied to files:

  • key-wallet/src/lib.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: FFI bindings are organized in `*-ffi` crates; shared helpers in `internals/` and `test-utils/`

Applied to files:

  • key-wallet/src/lib.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*.rs : Use conditional compilation with feature flags for optional features

Applied to files:

  • key-wallet/src/lib.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Run 'cargo fmt --check' to verify formatting, 'cargo clippy --all-targets --all-features -- -D warnings' for linting, and 'cargo check --all-features' to verify all features compile

Applied to files:

  • key-wallet/src/lib.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*test*.rs : Test both mainnet and testnet configurations

Applied to files:

  • key-wallet/src/lib.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Pre-generate addresses in batches (typically 20-100) and store them in pools; only derive on-demand when the pool is exhausted

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/test_utils/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Generate addresses in batches using gap limit and staged generation instead of unbounded address generation to prevent memory and performance issues

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet-ffi/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/test_utils/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Use staged gap limit management with `GapLimitStage` tracking `last_used_index` and `used_indices` to enable efficient address discovery without loading entire chains into memory

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)

Applied to files:

  • key-wallet/src/test_utils/mod.rs
  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function

Applied to files:

  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks

Applied to files:

  • key-wallet/src/test_utils/address.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/Cargo.toml
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations

Applied to files:

  • key-wallet-ffi/Cargo.toml
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Never serialize or log private keys in production; use public keys or key fingerprints for identification instead

Applied to files:

  • key-wallet-ffi/Cargo.toml
  • key-wallet/Cargo.toml
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.

Applied to files:

  • key-wallet-ffi/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 74
File: key-wallet/src/address.rs:30-40
Timestamp: 2025-06-15T15:31:44.136Z
Learning: In the key-wallet crate, QuantumExplorer prefers keeping wildcard arms that default unknown Network variants to testnet prefixes rather than using exhaustive matches or panics. This fallback behavior is intentional.

Applied to files:

  • key-wallet-ffi/Cargo.toml
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/bip32/**/*.rs : Cache intermediate key derivation results and batch derive child keys when possible to optimize derivation performance

Applied to files:

  • key-wallet-ffi/Cargo.toml
  • key-wallet/Cargo.toml
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use the `?` operator for error propagation, provide context in error messages, never panic in library code, and return `Result<T>` for all fallible operations

Applied to files:

  • key-wallet-ffi/Cargo.toml
  • key-wallet/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions

Applied to files:

  • key-wallet-ffi/Cargo.toml
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite

Applied to files:

  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`

Applied to files:

  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Prepare support for future Dash upgrades including Schnorr/Taproot support, descriptor wallets, multi-signature account types, and Lightning Network payment channels

Applied to files:

  • key-wallet/Cargo.toml
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/account/**/*.rs : Use enum-based type system for `AccountType` with specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics

Applied to files:

  • key-wallet/Cargo.toml
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
📚 Learning: 2025-06-15T16:42:18.187Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 74
File: key-wallet-ffi/src/lib_tests.rs:41-48
Timestamp: 2025-06-15T16:42:18.187Z
Learning: In key-wallet-ffi, the HDWallet::derive_xpriv method returns Result<String, KeyWalletError>, not an ExtPrivKey wrapper. When unwrapped, it yields a String that can have .is_empty() called on it.

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*integration*.rs : Write integration tests for network operations

Applied to files:

  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : Implement transaction classification and routing through `TransactionRouter` to avoid checking all accounts for every transaction

Applied to files:

  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
🧬 Code graph analysis (6)
key-wallet/src/wallet/managed_wallet_info/coin_selection.rs (1)
key-wallet/src/test_utils/utxo.rs (1)
  • new_test (8-10)
key-wallet-ffi/src/utxo_tests.rs (4)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
  • utxos (71-71)
  • utxos (186-192)
key-wallet/src/managed_account/mod.rs (1)
  • utxos (867-869)
key-wallet/src/managed_account/managed_account_trait.rs (1)
  • utxos (48-48)
key-wallet/src/test_utils/utxo.rs (1)
  • new_test_batch (12-33)
key-wallet/src/test_utils/utxo.rs (2)
key-wallet/src/test_utils/address.rs (1)
  • test_address (9-11)
key-wallet/src/utxo.rs (2)
  • new (41-58)
  • new (130-138)
key-wallet/tests/test_optimal_consolidation.rs (2)
key-wallet/src/wallet/managed_wallet_info/coin_selection.rs (1)
  • test_optimal_consolidation_strategy (740-765)
key-wallet/src/test_utils/utxo.rs (1)
  • new_test (8-10)
key-wallet/src/utxo.rs (1)
key-wallet/src/test_utils/utxo.rs (1)
  • new_test (8-10)
key-wallet/src/wallet/managed_wallet_info/transaction_building.rs (1)
key-wallet/src/test_utils/utxo.rs (1)
  • new_test (8-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: Pre-commit (ubuntu-latest)
  • GitHub Check: Pre-commit (macos-latest)
  • GitHub Check: Pre-commit (windows-latest)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: Address Sanitizer
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Thread Sanitizer
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_script)
🔇 Additional comments (17)
key-wallet-ffi/Cargo.toml (1)

36-36: LGTM! Dev-dependency configured correctly.

The dev-dependency on key-wallet with the test-utils feature properly enables test utilities for FFI tests while maintaining appropriate feature flags.

key-wallet/src/wallet/managed_wallet_info/transaction_building.rs (2)

183-183: Import cleanup looks correct.

The removed imports (ScriptBuf, OutPoint, TxOut) are no longer needed since test UTXOs are now created via Utxo::new_test(). The remaining Txid import is still used in test_coinbase_transaction (line 282).


295-296: Test utility migration completed successfully.

The migration from manual UTXO construction to Utxo::new_test() is consistent across all test cases. However, all tests use id=0 for UTXO creation. Please verify that this doesn't create duplicate outpoints (see comment on lines 190-194).

Also applies to: 331-331, 365-365, 389-389

key-wallet/Cargo.toml (2)

19-19: LGTM! Test utilities feature flag added correctly.

The empty test-utils feature flag follows standard Rust patterns for conditional compilation. This enables gating test utilities behind #[cfg(feature = "test-utils")] without requiring additional dependencies.


51-51: Dev-dependencies correctly updated.

The test-utils feature is appropriately enabled in dev-dependencies, making test utilities available during testing while keeping them out of production builds.

key-wallet/src/wallet/managed_wallet_info/coin_selection.rs (1)

697-701: Test utility migration completed successfully.

All test cases consistently use Utxo::new_test() instead of manual construction. However, all UTXOs use id=0, which may create duplicate outpoints depending on the implementation. Please verify that UTXOs created with the same id have unique outpoints (same concern as in transaction_building.rs).

Also applies to: 715-719, 731-731, 743-749

key-wallet/src/lib.rs (1)

15-16: LGTM! Proper feature gating for test utilities.

The test utilities module is correctly gated with cfg(any(test, feature = "test-utils")), making the helpers available to unit tests, integration tests, and other crates (via the feature flag) as discussed in the PR comments.

key-wallet/src/test_utils/mod.rs (1)

1-4: LGTM! Proper module organization.

The module structure correctly declares both submodules and re-exports address utilities. The utxo module doesn't require re-export since it contains impl blocks on the Utxo type itself, making the methods directly accessible via Utxo::new_test().

key-wallet/src/utxo.rs (2)

313-313: LGTM! Tests now use centralized test utilities.

The tests have been successfully migrated to use Utxo::new_test() from the centralized test utilities, eliminating duplicate test helper code and aligning with the PR objectives.


331-332: LGTM! Consistent use of test helpers.

Both UTXO instances correctly use the consolidated Utxo::new_test() helper with different IDs to ensure unique outpoints.

key-wallet-ffi/src/utxo_tests.rs (5)

6-6: LGTM! Proper import of consolidated test utilities.

The import of Utxo from key_wallet enables access to the new test helper methods defined in the centralized test utilities.


314-318: LGTM! Clean batch UTXO creation.

The use of Utxo::new_test_batch(0..2, ...) for creating 2 UTXOs is concise and leverages the centralized test utilities effectively. The subsequent insertion by outpoint is correct.


342-346: LGTM! Consistent batch usage.

Correct use of new_test_batch(10..11, ...) to create a single UTXO with a distinct ID range, avoiding conflicts with UTXOs from other accounts.


363-367: LGTM! Proper multi-account UTXO setup.

The CoinJoin account correctly uses new_test_batch(20..22, ...) for 2 UTXOs with distinct IDs, supporting the test validation of 5 total UTXOs across accounts.


424-428: LGTM! Appropriate single-UTXO batch creation.

Using new_test_batch(1..2, ...) for a single UTXO is consistent with the batch-oriented test setup throughout this file.

key-wallet/tests/test_optimal_consolidation.rs (2)

1-2: LGTM! Clean consolidation of test utilities.

The import correctly brings in the centralized Utxo test constructor, aligning with the PR's objective to consolidate UTXO helpers.


11-16: The duplicate id=0 values do not affect coin selection correctness. CoinSelector does not use outpoints for deduplication or identity tracking—selection is purely value-based. The algorithm filters UTXOs by spendability and confirmation status, then selects them by value via sorting in a Vec. No HashMap or HashSet structures keyed by outpoint exist in the selection logic.

The test is valid as written.

@xdustinface xdustinface changed the title refactor: consolidate UTXO helpers in test_utils.rs refactor: consolidate UTXO helpers in test_utils module Jan 7, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
key-wallet-ffi/src/utxo_tests.rs (2)

340-343: Consider using new_test for single UTXO creation.

For creating a single UTXO, you could use Utxo::new_test(10, 20000, 200, false, false) directly instead of new_test_batch(10..11, ...), which would be slightly more explicit. However, the current batch-based approach is also correct and maintains consistency across the test.

Alternative approach for single UTXO
-        let utxos = Utxo::new_test_batch(10..11, 20000, 200, false, false);
-        for utxo in utxos {
-            bip32_account.utxos.insert(utxo.outpoint, utxo);
-        }
+        let utxo = Utxo::new_test(10, 20000, 200, false, false);
+        bip32_account.utxos.insert(utxo.outpoint, utxo);

418-421: LGTM with optional refactor suggestion.

The batch helper is used correctly for single UTXO creation. Same as the previous comment, consider using Utxo::new_test(1, 10000, 100, false, false) for clarity when creating a single UTXO, though the current approach is also valid.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9fdf96e and 4e23217.

📒 Files selected for processing (7)
  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/test_utils/utxo.rs
  • key-wallet/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/tests/test_optimal_consolidation.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
  • key-wallet/tests/test_optimal_consolidation.rs
  • key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
  • key-wallet/src/test_utils/utxo.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with #[cfg(test)] annotation; integration tests use the tests/ directory
Use snake_case for function and variable names
Use UpperCamelCase for types and traits
Use SCREAMING_SNAKE_CASE for constants
Format code with rustfmt before commits; ensure cargo fmt --all is run
Run cargo clippy --workspace --all-targets -- -D warnings for linting; avoid warnings in CI
Prefer async/await via tokio for asynchronous operations

**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types (thiserror) and propagate errors appropriately in Rust
Use tokio runtime for async operations in Rust
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format code using cargo fmt
Run clippy with all features and all targets, treating warnings as errors
Never log or expose private keys in any code
Always validate inputs from untrusted sources in Rust
Use secure random number generation for keys

Files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
**/*test*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*test*.rs: Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate

Files:

  • key-wallet-ffi/src/utxo_tests.rs
key-wallet/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/**/*.rs: Separate immutable structures (Account, Wallet) containing only identity information from mutable wrappers (ManagedAccount, ManagedWalletInfo) with state management
Never serialize or log private keys in production; use public keys or key fingerprints for identification instead
Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
Use BTreeMap for ordered data (accounts, transactions) and HashMap for lookups (address mappings); apply memory management strategies for old transaction data
Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Use the ? operator for error propagation, provide context in error messages, never panic in library code, and return Result<T> for all fallible operations

Files:

  • key-wallet/src/utxo.rs
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*test*.rs : Use proptest for property-based testing where appropriate
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) to enable both full wallets and watch-only wallets with the same interface

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use property-based testing for complex invariants such as gap limit constraints

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use deterministic testing with known test vectors and fixed seeds for reproducible results

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Generate addresses in batches using gap limit and staged generation instead of unbounded address generation to prevent memory and performance issues

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Pre-generate addresses in batches (typically 20-100) and store them in pools; only derive on-demand when the pool is exhausted

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/account/**/*.rs : Use enum-based type system for `AccountType` with specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use the `?` operator for error propagation, provide context in error messages, never panic in library code, and return `Result<T>` for all fallible operations

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-06-15T16:42:18.187Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 74
File: key-wallet-ffi/src/lib_tests.rs:41-48
Timestamp: 2025-06-15T16:42:18.187Z
Learning: In key-wallet-ffi, the HDWallet::derive_xpriv method returns Result<String, KeyWalletError>, not an ExtPrivKey wrapper. When unwrapped, it yields a String that can have .is_empty() called on it.

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations

Applied to files:

  • key-wallet-ffi/src/utxo_tests.rs
  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function

Applied to files:

  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks

Applied to files:

  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite

Applied to files:

  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*integration*.rs : Write integration tests for network operations

Applied to files:

  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)

Applied to files:

  • key-wallet/src/utxo.rs
📚 Learning: 2025-12-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*test*.rs : Test both mainnet and testnet configurations

Applied to files:

  • key-wallet/src/utxo.rs
🧬 Code graph analysis (2)
key-wallet-ffi/src/utxo_tests.rs (4)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
  • utxos (71-71)
  • utxos (186-192)
key-wallet/src/managed_account/mod.rs (1)
  • utxos (867-869)
key-wallet/src/managed_account/managed_account_trait.rs (1)
  • utxos (48-48)
key-wallet/src/test_utils/utxo.rs (1)
  • new_test_batch (12-34)
key-wallet/src/utxo.rs (1)
key-wallet/src/test_utils/utxo.rs (1)
  • new_test (8-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Pre-commit (windows-latest)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: Address Sanitizer
  • GitHub Check: Thread Sanitizer
🔇 Additional comments (6)
key-wallet/src/utxo.rs (3)

313-313: LGTM! Test helper consolidation successful.

The test correctly uses the new Utxo::new_test helper, which consolidates UTXO creation logic. The parameters are clear and appropriate for testing spendability scenarios.


331-332: LGTM! Consistent use of test helpers.

The test correctly uses Utxo::new_test to create multiple UTXOs with different parameters, demonstrating proper consolidation of test utilities.


307-352: Test helper accessibility and feature gating are properly configured.

Verification confirms that test utilities in key-wallet are correctly feature-gated with #[cfg(any(test, feature = "test-utils"))] in lib.rs, enabling both standard test builds and cross-crate reuse when the feature is explicitly enabled. The test-utils feature is defined in Cargo.toml and properly configured, allowing dependent crates like key-wallet-ffi to access test helpers by enabling this feature. The implementation avoids the cfg(test)-only pattern that would prevent cross-crate access.

key-wallet-ffi/src/utxo_tests.rs (3)

314-317: LGTM! Effective use of batch test helper.

The code correctly uses Utxo::new_test_batch(0..2, ...) to create 2 UTXOs for the BIP44 account. The range-based ID assignment ensures unique outpoints, and the iteration pattern is clean and consistent.


359-362: LGTM! Consistent batch helper usage.

The batch creation pattern is correctly applied for the CoinJoin account with distinct IDs (20-21) to avoid collisions with other accounts' UTXOs.


6-6: No action needed—this is standard public API usage.

The import use key_wallet::Utxo; works because Utxo is a public struct defined in key-wallet/src/utxo.rs, not a test utility. Cross-crate imports of public types are part of normal Rust module visibility and do not require special feature gating. The test-utils feature defined in key-wallet is unrelated to this import.

Likely an incorrect or invalid review comment.

@xdustinface xdustinface merged commit c8b1548 into v0.42-dev Jan 7, 2026
53 checks passed
@xdustinface xdustinface deleted the refactor/utxo-helpers branch January 7, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants