Skip to content

Conversation

@xdustinface
Copy link
Collaborator

@xdustinface xdustinface commented Dec 24, 2025

Just some cleanup of unused/duplicated code.

Summary by CodeRabbit

  • Refactor
    • Restructured UTXO data access patterns in wallet infrastructure.
    • Removed UTXO retrieval methods from managed wallet functionality, including operations to query UTXOs by account type, identify spendable UTXOs, calculate total values, and retrieve counts.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 24, 2025

📝 Walkthrough

Walkthrough

The changes consolidate UTXO handling by removing the managed wallet's UTXO module and its associated public methods, while updating the FFI layer to iterate directly over UTXO items and access transaction identifiers and output indices through the nested outpoint structure.

Changes

Cohort / File(s) Summary
Removed UTXO Module from Managed Wallet
key-wallet/src/wallet/managed_wallet_info/mod.rs, key-wallet/src/wallet/managed_wallet_info/utxo.rs
Eliminated the entire UTXO submodule for managed wallets, removing public methods: get_utxos, get_utxos_by_account_type, get_spendable_utxos, get_total_utxo_value, get_utxo_count, and all associated tests.
Refactored FFI UTXO Iteration
key-wallet-ffi/src/utxo.rs
Updated iteration pattern from destructured pairs to direct UTXO items; changed txid and vout access to derive from utxo.outpoint instead of destructured outpoint parameter.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A warren once nested in branches of code,
Now gathered and streamlined along the main road.
The UTXO hops lighter, with simpler stride,
FFI and wallet now walk side by side! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title accurately describes the main change: removing the unused utxo.rs file from the key-wallet crate. It is specific, concise, and directly reflects the primary objective of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/unused-key-wallet-utxo

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.

@xdustinface xdustinface marked this pull request as ready for review December 24, 2025 08:36
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
key-wallet-ffi/src/utxo.rs (1)

123-127: Incorrect confirmations calculation loses critical security information.

The code sets confirmations to 1 if confirmed or 0 if unconfirmed, losing the actual confirmation count. This is critical for transaction security assessment and double-spend protection.

Since utxo.height is available, calculate confirmations properly by passing the current chain height to this function and computing current_height - utxo.height + 1 for confirmed UTXOs.

🔎 Recommended fix with current height parameter

Modify the function signature to accept current chain height:

 pub unsafe extern "C" fn managed_wallet_get_utxos(
     managed_info: *const FFIManagedWalletInfo,
+    current_height: u32,
     utxos_out: *mut *mut FFIUTXO,
     count_out: *mut usize,
     error: *mut FFIError,
 ) -> bool {

Then calculate confirmations properly:

-            // Calculate confirmations (0 if unconfirmed)
-            let confirmations = if utxo.is_confirmed {
-                1
-            } else {
-                0
-            };
+            // Calculate confirmations
+            let confirmations = if utxo.is_confirmed && utxo.height > 0 {
+                current_height.saturating_sub(utxo.height).saturating_add(1)
+            } else {
+                0
+            };
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3884314 and 3d8117d.

📒 Files selected for processing (3)
  • key-wallet-ffi/src/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/mod.rs
  • key-wallet/src/wallet/managed_wallet_info/utxo.rs
💤 Files with no reviewable changes (2)
  • key-wallet/src/wallet/managed_wallet_info/utxo.rs
  • key-wallet/src/wallet/managed_wallet_info/mod.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}

📄 CodeRabbit inference engine (CLAUDE.md)

Never hardcode network parameters, addresses, or keys

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings

**/*.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

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management

Files:

  • key-wallet-ffi/src/utxo.rs
🧠 Learnings (12)
📓 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/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together
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.
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/**/*.rs : Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing 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/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data
📚 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.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/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-ffi/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.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.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-ffi/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 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.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.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.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-ffi/src/utxo.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/*-ffi/**/*.rs : Be careful with FFI memory management

Applied to files:

  • key-wallet-ffi/src/utxo.rs
🧬 Code graph analysis (1)
key-wallet-ffi/src/utxo.rs (2)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
  • utxos (68-68)
  • utxos (179-185)
key-wallet/src/managed_account/mod.rs (1)
  • utxos (867-869)
🔇 Additional comments (2)
key-wallet-ffi/src/utxo.rs (2)

3-5: LGTM! Imports align with API migration.

The added imports support the transition from the removed get_utxos() method to the WalletInfoInterface::utxos() trait method, and include necessary error handling types.


102-102: API migration correctly adapts to new utxos() interface.

The changes properly implement the migration from get_utxos() to inner().utxos(). The implementation returns all UTXOs via BTreeSet<&Utxo>, and the updated iteration and field access patterns (utxo.outpoint.txid, utxo.outpoint.vout) correctly align with the new API. FFI boundary handling is appropriate with proper pointer validation and error handling.

@xdustinface xdustinface changed the title chore: Drop unused utxo.rs in key-wallet crate chore: drop unused utxo.rs in key-wallet crate Dec 24, 2025
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.

2 participants