-
Notifications
You must be signed in to change notification settings - Fork 8
chore: drop unused utxo.rs in key-wallet crate
#304
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
base: v0.41-dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
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
1if confirmed or0if unconfirmed, losing the actual confirmation count. This is critical for transaction security assessment and double-spend protection.Since
utxo.heightis available, calculate confirmations properly by passing the current chain height to this function and computingcurrent_height - utxo.height + 1for 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
📒 Files selected for processing (3)
key-wallet-ffi/src/utxo.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-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 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
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 theWalletInfoInterface::utxos()trait method, and include necessary error handling types.
102-102: API migration correctly adapts to newutxos()interface.The changes properly implement the migration from
get_utxos()toinner().utxos(). The implementation returns all UTXOs viaBTreeSet<&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.
utxo.rs in key-wallet crateutxo.rs in key-wallet crate
Just some cleanup of unused/duplicated code.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.