-
Notifications
You must be signed in to change notification settings - Fork 9
refactor: consolidate UTXO helpers in test_utils module
#334
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.
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
Conversation
test_utils.rstest_utils.rs
📝 WalkthroughWalkthroughThe PR centralizes test helpers into a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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. Comment |
There was a problem hiding this 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 localtest_utxohelper intest_optimal_consolidation.rsto centralized utilities.The migration is incomplete.
key-wallet/tests/test_optimal_consolidation.rscontains a localtest_utxo(value: u64, confirmed: bool)helper that duplicates centralized functionality already available viatest_utxo_full(value, height, vout, confirmed)intest_utils.rs. Replace the local helper with calls totest_utxo_full(value, 100, 0, confirmed)or add a convenience wrapperpub 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:
Missing doc comments: Adding documentation for the public functions would help users understand the purpose and parameters (especially the
confirmedparameter's behavior).Empty ScriptBuf on Line 32: The
TxOutis created with an emptyScriptBuf, 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., usingaddress.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
📒 Files selected for processing (6)
key-wallet/src/tests/mod.rskey-wallet/src/tests/test_utils.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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
UseBTreeMapfor ordered data (accounts, transactions) andHashMapfor 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 returnResult<T>for all fallible operations
Files:
key-wallet/src/tests/mod.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-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 thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor 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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-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.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/utxo.rskey-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.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/tests/test_utils.rskey-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.rskey-wallet/src/tests/test_utils.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-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.rskey-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.rskey-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_utilsmodule 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_fulldirectly 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) andtest_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.
|
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? |
|
With a quick search I found two files where we are creating test instances of the Utxo struct:
I honestly think that the best approach is to have a
Not sure right now tbh but I use to do it that way |
There was a problem hiding this 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
Vecwith a single element vianew_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_coinbaseis hardcoded tofalse.The test helper always sets
is_coinbasetofalse, 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
📒 Files selected for processing (12)
key-wallet-ffi/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-wallet/Cargo.tomlkey-wallet/src/lib.rskey-wallet/src/test_utils/address.rskey-wallet/src/test_utils/mod.rskey-wallet/src/test_utils/utxo.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-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
UseBTreeMapfor ordered data (accounts, transactions) andHashMapfor 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 returnResult<T>for all fallible operations
Files:
key-wallet/src/lib.rskey-wallet/src/test_utils/mod.rskey-wallet/src/test_utils/address.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/src/test_utils/utxo.rskey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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 thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor 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.rskey-wallet/src/test_utils/mod.rskey-wallet/src/test_utils/address.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet-ffi/src/utxo_tests.rskey-wallet/src/test_utils/utxo.rskey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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.rskey-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.rskey-wallet/src/test_utils/mod.rskey-wallet/src/test_utils/address.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-wallet/src/test_utils/utxo.rskey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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.rskey-wallet/src/test_utils/mod.rskey-wallet/src/test_utils/address.rskey-wallet-ffi/Cargo.tomlkey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-wallet/src/test_utils/utxo.rskey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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.rskey-wallet-ffi/Cargo.tomlkey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-wallet/tests/test_optimal_consolidation.rskey-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.rskey-wallet-ffi/Cargo.tomlkey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet/src/test_utils/utxo.rskey-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.rskey-wallet/src/test_utils/address.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-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.rskey-wallet-ffi/Cargo.tomlkey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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.rskey-wallet/src/test_utils/address.rskey-wallet-ffi/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-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.rskey-wallet-ffi/Cargo.tomlkey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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.rskey-wallet/src/test_utils/address.rskey-wallet-ffi/Cargo.tomlkey-wallet-ffi/src/utxo_tests.rskey-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.rskey-wallet/src/test_utils/address.rskey-wallet-ffi/src/utxo_tests.rskey-wallet/tests/test_optimal_consolidation.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet-ffi/src/utxo_tests.rskey-wallet/src/test_utils/utxo.rskey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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.rskey-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.rskey-wallet/src/test_utils/address.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/src/utxo.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet/src/test_utils/utxo.rskey-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.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/Cargo.tomlkey-wallet/tests/test_optimal_consolidation.rskey-wallet/src/utxo.rskey-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.tomlkey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet-ffi/src/utxo_tests.rskey-wallet/src/utxo.rskey-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.tomlkey-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.tomlkey-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.tomlkey-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.tomlkey-wallet/Cargo.tomlkey-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.rskey-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.rskey-wallet-ffi/src/utxo_tests.rskey-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.tomlkey-wallet-ffi/src/utxo_tests.rskey-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-walletwith thetest-utilsfeature 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 viaUtxo::new_test(). The remainingTxidimport is still used intest_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 useid=0for 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-utilsfeature 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-utilsfeature 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 useid=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
utxomodule doesn't require re-export since it containsimplblocks on theUtxotype itself, making the methods directly accessible viaUtxo::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
Utxofromkey_walletenables 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
Utxotest constructor, aligning with the PR's objective to consolidate UTXO helpers.
11-16: The duplicateid=0values do not affect coin selection correctness.CoinSelectordoes 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 aVec. NoHashMaporHashSetstructures keyed by outpoint exist in the selection logic.The test is valid as written.
test_utils.rstest_utils module
There was a problem hiding this 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 usingnew_testfor single UTXO creation.For creating a single UTXO, you could use
Utxo::new_test(10, 20000, 200, false, false)directly instead ofnew_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
📒 Files selected for processing (7)
key-wallet-ffi/src/utxo_tests.rskey-wallet/src/test_utils/utxo.rskey-wallet/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-wallet/src/wallet/managed_wallet_info/transaction_building.rskey-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 thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor 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.rskey-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
UseBTreeMapfor ordered data (accounts, transactions) andHashMapfor 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 returnResult<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.rskey-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.rskey-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.rskey-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.rskey-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.rskey-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.rskey-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.rskey-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.rskey-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.rskey-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_testhelper, 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_testto 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-walletare correctly feature-gated with#[cfg(any(test, feature = "test-utils"))]inlib.rs, enabling both standard test builds and cross-crate reuse when the feature is explicitly enabled. Thetest-utilsfeature is defined inCargo.tomland properly configured, allowing dependent crates likekey-wallet-ffito access test helpers by enabling this feature. The implementation avoids thecfg(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 becauseUtxois a public struct defined inkey-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. Thetest-utilsfeature defined inkey-walletis unrelated to this import.Likely an incorrect or invalid review comment.
Consolidate all UTXO creation helpers within
key-walletcrate in a single filetest_utils.rs.Summary by CodeRabbit
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.