feat: new managed platform account to track platform balances #368
Conversation
📝 WalkthroughWalkthroughRenamed ManagedAccount/WalletBalance to ManagedCoreAccount/WalletCoreBalance; added ManagedPlatformAccount and PlatformP2PKHAddress for DIP‑17; converted AccountType→AccountTypeToCheck conversions to TryFrom (PlatformPayment rejects); split transaction checks into core (check_core_transaction) and platform (WalletPlatformChecker) with expanded FFI surface. Changes
Sequence Diagram(s)mermaid style Client fill:rgba(200,200,255,0.5) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
key-wallet/src/managed_account/managed_account_collection.rs (1)
165-261: Avoid panic in library insert path.
insertcurrently panics on PlatformPayment; in library code this should be a recoverable error. ReturnResultso callers can handle the error case instead of panicking.🛠️ Proposed fix (return Result instead of panic)
- pub fn insert(&mut self, account: ManagedCoreAccount) { + pub fn insert(&mut self, account: ManagedCoreAccount) -> Result<()> { use crate::account::StandardAccountType; match &account.account_type { @@ -255,10 +255,11 @@ impl ManagedCoreAccountCollection { self.dashpay_external_accounts.insert(key, account); } ManagedAccountType::PlatformPayment { .. } => { // Platform Payment accounts should use insert_platform_account() instead // as they use ManagedPlatformAccount, not ManagedCoreAccount - panic!("Use insert_platform_account() for Platform Payment accounts") + return Err(Error::InvalidParameter( + "Use insert_platform_account() for Platform Payment accounts".to_string() + )); } } + Ok(()) }
🤖 Fix all issues with AI agents
In `@key-wallet/src/account/account_type.rs`:
- Around line 128-134: Replace the infallible conversion that panics for
AccountType::PlatformPayment with a fallible one: change the existing impl
From<AccountType> for AccountTypeToCheck into impl TryFrom<AccountType> for
AccountTypeToCheck that returns Result<AccountTypeToCheck, E>, add a new error
variant (e.g., AccountError::PlatformPaymentNotConvertible or
AccountConversionError::PlatformPaymentNotConvertible) to the crate's error
enum, and update all call sites to use TryFrom::try_from /
AccountTypeToCheck::try_from(...) and handle the Result accordingly instead of
allowing a panic from AccountType::PlatformPayment.
In `@key-wallet/src/managed_account/mod.rs`:
- Around line 34-36: The mod declarations for managed_platform_account and
platform_address in mod.rs are causing build failures because the corresponding
files are missing; either add the source files named managed_platform_account.rs
and platform_address.rs under the managed_account directory with the module
implementations, or remove/guard the pub mod lines in mod.rs (pub mod
managed_platform_account; pub mod platform_address;) behind cfg(feature = "...")
or delete them if not used; ensure the module file names and the pub mod
identifiers (managed_platform_account, platform_address) match exactly and
re-run cargo build.
In `@key-wallet/src/transaction_checking/mod.rs`:
- Around line 7-15: The build fails because the declared module pub mod
platform_checker; and its re-export pub use
platform_checker::WalletPlatformChecker reference a missing file; either add the
module file (create key-wallet/src/transaction_checking/platform_checker.rs or
platform_checker/mod.rs) implementing the WalletPlatformChecker type and any
expected items, or remove/gate the module and corresponding re-export (delete or
cfg-gate pub mod platform_checker; and pub use
platform_checker::WalletPlatformChecker) and update any callers of
WalletPlatformChecker to avoid unresolved symbols.
In `@key-wallet/src/transaction_checking/transaction_router/mod.rs`:
- Around line 291-298: The reference-based From implementation that converts
ManagedAccountType into AccountTypeToCheck currently panics for
ManagedAccountType::PlatformPayment; change the reference-based conversion to
implement TryFrom<&ManagedAccountType> for AccountTypeToCheck (returning
Result<AccountTypeToCheck, YourErrorType>) instead of From, mirror the same
conversion logic used in the owned TryFrom, and replace the panic branch with an
Err variant (e.g., Err(ConversionError::UnsupportedPlatformPayment)) so callers
can handle failures without unwinding.
- Around line 233-240: The current From implementation for converting
ManagedAccountType into AccountTypeToCheck panics on the
ManagedAccountType::PlatformPayment arm; replace this fallible conversion with a
TryFrom implementation that returns Result<AccountTypeToCheck, _> (e.g., a new
TransactionRouterError or a descriptive enum/error type) instead of panicking,
change the PlatformPayment branch to return Err(...) with a clear error
variant/message, and update any callsites that relied on From to handle the
Result (propagate or map the error) so callers can gracefully handle
PlatformPayment rather than crashing.
🧹 Nitpick comments (2)
key-wallet/src/account/account_type.rs (1)
97-137: Consider adding a compile-time safeguard or documentation.While switching to
TryFromaddresses the runtime behavior, you might also consider:
- Adding
#[doc(hidden)]or a doc comment warning that this conversion is partial- If
PlatformPaymentaccounts are now managed completely separately (viaManagedPlatformAccount), consider whetherAccountType::PlatformPaymentshould remain in this enum or be moved to a separate type hierarchyThis would provide clearer separation at the type level, aligning with the learning to "separate immutable structures containing only identity information from mutable wrappers with state management."
key-wallet-ffi/src/managed_account.rs (1)
4-5: Documentation still references old type name.The module doc comment mentions "ManagedAccount instances" but the code now uses
ManagedCoreAccount. Consider updating for consistency.📝 Suggested doc update
//! This module provides FFI-compatible managed account functionality that wraps -//! ManagedAccount instances from the key-wallet crate. FFIManagedAccount is a -//! simple wrapper around `Arc<ManagedAccount>` without additional fields. +//! ManagedCoreAccount instances from the key-wallet crate. FFIManagedAccount is a +//! simple wrapper around `Arc<ManagedCoreAccount>` without additional fields.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
key-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/managed_account.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/transaction.rskey-wallet-ffi/src/transaction_checking.rskey-wallet-ffi/src/types.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/account/mod.rskey-wallet/src/lib.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/managed_account/mod.rskey-wallet/src/test_utils/account.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/wallet/balance.rskey-wallet/src/wallet/helper.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/wallet/mod.rs
💤 Files with no reviewable changes (1)
- key-wallet/src/wallet/helper.rs
🧰 Additional context used
📓 Path-based instructions (9)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys
Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation for optional features
Use proptest for property-based testing where appropriate
Always validate inputs from untrusted sources
Never log or expose private keys
Format code using cargo fmt
Ensure clippy passes with all features enabled and -D warnings flag
**/*.rs: Keep unit tests in the source code alongside implementation using#[cfg(test)]modules
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtfollowing therustfmt.tomlconfiguration; runcargo fmt --allbefore commits
Avoidunwrap()andexpect()in library code; use explicit error types (e.g.,thiserror)
Usetokiofor async/await implementations
Run clippy with warnings-as-errors for linting:cargo clippy --workspace --all-targets -- -D warnings
Maintain Minimum Supported Rust Version (MSRV) of 1.89
Follow crate-specific idioms for mixed Rust editions (2021/2024)
Files:
key-wallet-ffi/src/transaction.rskey-wallet-ffi/src/types.rskey-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful memory safety handling at FFI boundaries
Be careful with FFI memory management
Files:
key-wallet-ffi/src/transaction.rskey-wallet-ffi/src/types.rskey-wallet-ffi/src/transaction_checking.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/managed_account.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/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.rs
**/key-wallet/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use secure random number generation for keys
Files:
key-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.rs
key-wallet/**/account/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Use enum-based type system for
AccountTypewith specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics
Files:
key-wallet/src/account/mod.rskey-wallet/src/account/account_type.rs
key-wallet/**/managed_account/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together
Files:
key-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_collection.rs
key-wallet/**/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Implement transaction classification and routing through
TransactionRouterto avoid checking all accounts for every transaction
Files:
key-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/transaction_checking/transaction_router/mod.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/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rs
**/*test*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Write descriptive test names (e.g.,
test_parse_address_mainnet)
Files:
key-wallet-ffi/src/utxo_tests.rskey-wallet/src/tests/balance_tests.rs
🧠 Learnings (27)
📓 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: 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/**/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
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
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
📚 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/transaction.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/account/account_type.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.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/transaction.rskey-wallet-ffi/src/types.rskey-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.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/transaction.rskey-wallet-ffi/src/types.rskey-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.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/transaction.rskey-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/transaction.rskey-wallet-ffi/src/types.rskey-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.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/types.rskey-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.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/types.rskey-wallet/src/wallet/mod.rskey-wallet/src/account/mod.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/lib.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/test_utils/account.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/wallet/managed_wallet_info/managed_accounts.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/managed_account/managed_account_collection.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/types.rskey-wallet-ffi/src/transaction_checking.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/src/wallet/mod.rskey-wallet/src/wallet/balance.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet-ffi/src/managed_wallet.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/wallet/mod.rskey-wallet/src/lib.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/managed_account/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet/src/managed_account/managed_account_collection.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/src/wallet/mod.rs
📚 Learning: 2026-01-16T14:53:56.692Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T14:53:56.692Z
Learning: Applies to **/key-wallet/**/*.rs : Use secure random number generation for keys
Applied to files:
key-wallet/src/wallet/mod.rskey-wallet/src/transaction_checking/mod.rs
📚 Learning: 2026-01-16T14:54:10.809Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2026-01-16T14:54:10.809Z
Learning: Follow a layered, trait-based architecture with clear separation of concerns across modules: client, network, storage, sync, validation, wallet, types, and error
Applied to files:
key-wallet/src/transaction_checking/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/**/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/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet-ffi/src/managed_wallet.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/transaction_checking/transaction_router/tests/routing.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/lib.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet-ffi/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/managed_account/mod.rskey-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/tests/balance_tests.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2026-01-16T14:54:24.093Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T14:54:24.093Z
Learning: This codebase is not intended for consensus-critical validation; do not rely on exact Dash Core consensus behavior
Applied to files:
key-wallet-ffi/src/transaction_checking.rs
📚 Learning: 2026-01-16T14:53:56.692Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T14:53:56.692Z
Learning: Applies to **/*-ffi/**/*.rs : Be careful with FFI memory management
Applied to files:
key-wallet-ffi/src/transaction_checking.rs
📚 Learning: 2026-01-16T14:53:56.692Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T14:53:56.692Z
Learning: Applies to **/*-ffi/**/*.rs : Exercise careful memory safety handling at FFI boundaries
Applied to files:
key-wallet-ffi/src/transaction_checking.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/lib.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/managed_account/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet/src/managed_account/managed_account_collection.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/lib.rskey-wallet-ffi/src/address_pool.rskey-wallet/src/managed_account/mod.rskey-wallet-ffi/src/managed_wallet.rskey-wallet/src/managed_account/managed_account_collection.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/transaction_checking/transaction_router/tests/provider.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/tests/balance_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/src/utxo_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/tests/balance_tests.rs
📚 Learning: 2026-01-16T14:54:10.809Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2026-01-16T14:54:10.809Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for swappable implementations (e.g., `NetworkManager`, `StorageManager`)
Applied to files:
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.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: Maintain backward compatibility for serialized wallets and monitor for new DIPs affecting wallet structure when updating derivation paths
Applied to files:
key-wallet/src/transaction_checking/transaction_router/mod.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/src/managed_account/managed_account_collection.rs
🧬 Code graph analysis (21)
key-wallet-ffi/src/transaction.rs (1)
key-wallet/src/tests/advanced_transaction_tests.rs (1)
tx(305-305)
key-wallet-ffi/src/types.rs (3)
key-wallet/src/managed_account/managed_account_trait.rs (1)
balance(36-36)key-wallet/src/managed_account/mod.rs (1)
balance(863-865)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
balance(77-77)balance(188-190)
key-wallet/src/wallet/mod.rs (2)
key-wallet/src/managed_account/mod.rs (1)
balance(863-865)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
balance(77-77)balance(188-190)
key-wallet/src/managed_account/managed_account_trait.rs (2)
key-wallet/src/managed_account/mod.rs (2)
balance(863-865)balance_mut(867-869)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
balance(77-77)balance(188-190)
key-wallet/src/wallet/balance.rs (3)
key-wallet/src/managed_account/mod.rs (2)
balance(863-865)new(65-75)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
balance(77-77)balance(188-190)key-wallet/src/wallet/managed_wallet_info/mod.rs (1)
new(52-62)
key-wallet/src/transaction_checking/transaction_router/tests/routing.rs (2)
key-wallet/src/tests/advanced_transaction_tests.rs (1)
tx(305-305)dash-spv/src/client/core.rs (1)
wallet(152-154)
key-wallet/src/account/account_type.rs (1)
dash/embedded/src/main.rs (1)
panic(72-76)
key-wallet/src/lib.rs (3)
key-wallet/src/managed_account/managed_account_trait.rs (1)
balance(36-36)key-wallet/src/managed_account/mod.rs (1)
balance(863-865)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
balance(77-77)balance(188-190)
key-wallet-ffi/src/utxo_tests.rs (1)
key-wallet-ffi/src/managed_account.rs (1)
new(30-34)
key-wallet-ffi/src/address_pool.rs (3)
key-wallet/src/account/mod.rs (1)
account_type(149-151)key-wallet/src/managed_account/managed_account_trait.rs (1)
account_type(18-18)key-wallet/src/managed_account/mod.rs (1)
account_type(839-841)
key-wallet/src/test_utils/account.rs (2)
key-wallet/src/managed_account/mod.rs (3)
new(65-75)account_type(839-841)network(847-849)key-wallet/src/managed_account/managed_account_trait.rs (2)
account_type(18-18)network(24-24)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (1)
key-wallet/src/managed_account/mod.rs (2)
balance(863-865)update_balance(270-289)
key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs (2)
key-wallet/src/tests/advanced_transaction_tests.rs (1)
tx(305-305)dash-spv/src/client/core.rs (1)
wallet(152-154)
key-wallet/src/wallet/managed_wallet_info/managed_accounts.rs (1)
key-wallet/src/managed_account/mod.rs (3)
from_account(78-98)from_bls_account(102-122)from_eddsa_account(126-137)
key-wallet/src/transaction_checking/account_checker.rs (2)
key-wallet/src/account/account_type.rs (1)
index(142-175)key-wallet/src/managed_account/managed_account_type.rs (1)
index(123-171)
key-wallet/src/transaction_checking/wallet_checker.rs (1)
key-wallet/src/transaction_checking/account_checker.rs (1)
account_index(172-200)
key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs (2)
key-wallet/src/tests/advanced_transaction_tests.rs (1)
tx(305-305)dash-spv/src/client/core.rs (1)
wallet(152-154)
key-wallet/src/wallet/managed_wallet_info/mod.rs (2)
key-wallet/src/managed_account/mod.rs (1)
balance(863-865)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
balance(77-77)balance(188-190)
key-wallet-ffi/src/managed_wallet.rs (4)
key-wallet-ffi/src/managed_account.rs (1)
new(30-34)key-wallet/src/wallet/balance.rs (1)
new(26-33)key-wallet/src/wallet/managed_wallet_info/mod.rs (1)
new(52-62)key-wallet/src/managed_account/managed_account_trait.rs (1)
balance(36-36)
key-wallet/src/wallet/managed_wallet_info/helpers.rs (7)
key-wallet/src/derivation.rs (1)
account(230-233)rpc-json/src/lib.rs (1)
vec(2580-2582)key-wallet/src/account/account_type.rs (2)
index(142-175)registration_index(178-186)key-wallet/src/account/mod.rs (1)
index(117-119)key-wallet/src/managed_account/mod.rs (1)
index(140-142)key-wallet/src/managed_account/managed_account_type.rs (2)
index(123-171)registration_index(179-187)key-wallet/src/bip32.rs (11)
index(134-136)index(142-144)index(150-152)index(158-160)index(166-168)index(282-284)index(290-292)index(298-300)index(306-308)index(314-316)index(1168-1170)
key-wallet/src/managed_account/managed_account_collection.rs (2)
key-wallet/src/managed_account/mod.rs (4)
new(65-75)network(847-849)is_watch_only(859-861)index(140-142)key-wallet/src/managed_account/address_pool.rs (6)
new(355-370)new(1029-1038)network(1069-1072)is_watch_only(202-204)key_source(1081-1084)base_path(1041-1044)
🪛 GitHub Actions: Sanitizer
key-wallet/src/transaction_checking/mod.rs
[error] 8-8: file not found for module platform_checker. to create the module, add 'key-wallet/src/transaction_checking/platform_checker.rs' or 'key-wallet/src/transaction_checking/platform_checker/mod.rs'
key-wallet/src/managed_account/mod.rs
[error] 34-34: file not found for module managed_platform_account. to create the module, add 'key-wallet/src/managed_account/managed_platform_account.rs' or 'key-wallet/src/managed_account/managed_platform_account/mod.rs'
[error] 36-36: file not found for module platform_address. to create the module, add 'key-wallet/src/managed_account/platform_address.rs' or 'key-wallet/src/managed_account/platform_address/mod.rs'
⏰ 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). (12)
- GitHub Check: Ubuntu / tools
- GitHub Check: Ubuntu / spv
- GitHub Check: Ubuntu / wallet
- GitHub Check: Ubuntu / rpc
- GitHub Check: Ubuntu / core
- GitHub Check: No-std Checks
- GitHub Check: MSRV Check (1.89)
- GitHub Check: Security Audit
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (windows-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@key-wallet/src/wallet/managed_wallet_info/helpers.rs`:
- Around line 205-210: The doc is wrong:
first_platform_payment_managed_account() currently always uses index 0; change
it to search for the smallest existing account index with key_class 0 instead of
hardcoding 0. Implement by iterating over whatever collection holds platform
payment accounts on the struct (or by trying increasing indices until
platform_payment_managed_account(index, 0) returns Some), return the first
Some(&ManagedPlatformAccount) found, and update the doc comment to reflect that
it returns the platform payment account with the lowest existing account index
for key_class 0; keep the method name first_platform_payment_managed_account and
use platform_payment_managed_account(index, 0) to locate accounts.
♻️ Duplicate comments (1)
key-wallet/src/transaction_checking/mod.rs (1)
7-15: Build break:platform_checkermodule missing.CI reports “file not found for module
platform_checker.” Addkey-wallet/src/transaction_checking/platform_checker.rs(orplatform_checker/mod.rs) or gate/remove the module + re-export to restore the build.
🧹 Nitpick comments (2)
key-wallet/src/wallet/managed_wallet_info/helpers.rs (1)
237-287: DRY up PlatformPaymentAccountKey construction.The key construction is repeated in multiple helpers. Consider a small private helper to reduce duplication and keep changes consistent.
♻️ Suggested refactor
impl ManagedWalletInfo { + fn platform_payment_key(account_index: u32, key_class: u32) -> PlatformPaymentAccountKey { + PlatformPaymentAccountKey { + account: account_index, + key_class, + } + } + pub fn platform_payment_managed_account( &self, account_index: u32, key_class: u32, ) -> Option<&ManagedPlatformAccount> { - let key = PlatformPaymentAccountKey { - account: account_index, - key_class, - }; + let key = Self::platform_payment_key(account_index, key_class); self.accounts.platform_payment_accounts.get(&key) } @@ pub fn platform_payment_managed_account_mut( &mut self, account_index: u32, key_class: u32, ) -> Option<&mut ManagedPlatformAccount> { - let key = PlatformPaymentAccountKey { - account: account_index, - key_class, - }; + let key = Self::platform_payment_key(account_index, key_class); self.accounts.platform_payment_accounts.get_mut(&key) } @@ pub fn has_platform_payment_account(&self, account_index: u32, key_class: u32) -> bool { - let key = PlatformPaymentAccountKey { - account: account_index, - key_class, - }; + let key = Self::platform_payment_key(account_index, key_class); self.accounts.platform_payment_accounts.contains_key(&key) }key-wallet/src/transaction_checking/transaction_router/mod.rs (1)
186-194: Consider making PlatformAccountConversionError a proper error type.It currently implements Display only. If thiserror is available, deriving it (or implementing std::error::Error manually) improves integration with error chaining and matches the crate’s error-handling conventions.
♻️ Suggested error type polish
-#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct PlatformAccountConversionError; +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[error("PlatformPayment accounts cannot be converted to AccountTypeToCheck")] +pub struct PlatformAccountConversionError;As per coding guidelines, prefer proper error types for fallible conversions.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
key-wallet-ffi/src/address_pool.rskey-wallet/src/account/account_type.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/wallet/managed_wallet_info/helpers.rs
🧰 Additional context used
📓 Path-based instructions (7)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys
Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation for optional features
Use proptest for property-based testing where appropriate
Always validate inputs from untrusted sources
Never log or expose private keys
Format code using cargo fmt
Ensure clippy passes with all features enabled and -D warnings flag
**/*.rs: Keep unit tests in the source code alongside implementation using#[cfg(test)]modules
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtfollowing therustfmt.tomlconfiguration; runcargo fmt --allbefore commits
Avoidunwrap()andexpect()in library code; use explicit error types (e.g.,thiserror)
Usetokiofor async/await implementations
Run clippy with warnings-as-errors for linting:cargo clippy --workspace --all-targets -- -D warnings
Maintain Minimum Supported Rust Version (MSRV) of 1.89
Follow crate-specific idioms for mixed Rust editions (2021/2024)
Files:
key-wallet-ffi/src/address_pool.rskey-wallet/src/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful memory safety handling at FFI boundaries
Be careful with FFI memory management
Files:
key-wallet-ffi/src/address_pool.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/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.rs
key-wallet/**/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Implement transaction classification and routing through
TransactionRouterto avoid checking all accounts for every transaction
Files:
key-wallet/src/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.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/transaction_checking/transaction_router/tests/conversions.rs
**/key-wallet/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use secure random number generation for keys
Files:
key-wallet/src/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.rs
key-wallet/**/account/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Use enum-based type system for
AccountTypewith specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics
Files:
key-wallet/src/account/account_type.rs
🧠 Learnings (23)
📓 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: 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/**/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
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
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
📚 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/address_pool.rskey-wallet/src/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/address_pool.rskey-wallet/src/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/address_pool.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/address_pool.rskey-wallet/src/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/address_pool.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/address_pool.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/address_pool.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.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-ffi/src/address_pool.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/address_pool.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/address_pool.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.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/address_pool.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/address_pool.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/transaction_checking/transaction_router/tests/conversions.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.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/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rs
📚 Learning: 2026-01-16T14:53:56.692Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T14:53:56.692Z
Learning: Applies to **/key-wallet/**/*.rs : Use secure random number generation for keys
Applied to files:
key-wallet/src/transaction_checking/mod.rs
📚 Learning: 2026-01-16T14:54:10.809Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2026-01-16T14:54:10.809Z
Learning: Follow a layered, trait-based architecture with clear separation of concerns across modules: client, network, storage, sync, validation, wallet, types, and error
Applied to files:
key-wallet/src/transaction_checking/mod.rs
📚 Learning: 2026-01-16T14:54:24.093Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T14:54:24.093Z
Learning: Applies to **/*.rs : Avoid `unwrap()` and `expect()` in library code; use explicit error types (e.g., `thiserror`)
Applied to files:
key-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rs
📚 Learning: 2026-01-16T14:53:56.692Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T14:53:56.692Z
Learning: Applies to **/*.rs : Use proper error types (thiserror) and propagate errors appropriately
Applied to files:
key-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/account/account_type.rs
📚 Learning: 2025-06-26T16:02:42.390Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:02:42.390Z
Learning: Passing exclusive mutable references to network and storage managers into the sync manager in async Rust can lead to borrow checker issues or runtime contention if concurrent access is needed elsewhere. It's advisable to document this architectural tradeoff and consider refactoring to interior mutability or message passing for shared access as the codebase evolves.
Applied to files:
key-wallet/src/transaction_checking/transaction_router/mod.rs
📚 Learning: 2026-01-16T14:54:10.809Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2026-01-16T14:54:10.809Z
Learning: Applies to dash-spv/**/*.rs : Run `cargo clippy --all-targets --all-features -- -D warnings` to catch potential bugs and style issues
Applied to files:
key-wallet/src/account/account_type.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/**/bip32/**/*.rs : Cache intermediate key derivation results and batch derive child keys when possible to optimize derivation performance
Applied to files:
key-wallet/src/account/account_type.rskey-wallet/src/wallet/managed_wallet_info/helpers.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: Maintain backward compatibility for serialized wallets and monitor for new DIPs affecting wallet structure when updating derivation paths
Applied to files:
key-wallet/src/wallet/managed_wallet_info/helpers.rs
🧬 Code graph analysis (4)
key-wallet-ffi/src/address_pool.rs (6)
key-wallet/src/managed_account/managed_account_trait.rs (1)
account_type(18-18)key-wallet/src/account/mod.rs (1)
account_type(149-151)key-wallet/src/managed_account/mod.rs (1)
account_type(839-841)key-wallet-ffi/src/error.rs (2)
set_error(54-65)error(44-49)key-wallet-ffi/src/types.rs (1)
error(124-135)key-wallet-ffi/src/managed_account.rs (1)
error(64-74)
key-wallet/src/transaction_checking/transaction_router/mod.rs (2)
key-wallet/src/account/account_type.rs (1)
try_from(102-139)key-wallet/src/managed_account/mod.rs (1)
account_type(839-841)
key-wallet/src/account/account_type.rs (1)
key-wallet/src/transaction_checking/transaction_router/mod.rs (2)
try_from(199-252)try_from(258-311)
key-wallet/src/wallet/managed_wallet_info/helpers.rs (6)
key-wallet/src/derivation.rs (1)
account(230-233)key-wallet/src/account/account_type.rs (2)
index(145-178)registration_index(181-189)key-wallet/src/managed_account/managed_account_trait.rs (1)
index(54-56)key-wallet/src/account/mod.rs (1)
index(117-119)key-wallet/src/managed_account/mod.rs (1)
index(140-142)key-wallet/src/managed_account/managed_account_type.rs (2)
index(123-171)registration_index(179-187)
🪛 GitHub Actions: Sanitizer
key-wallet/src/transaction_checking/mod.rs
[error] 8-8: file not found for module platform_checker
⏰ 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: Ubuntu ARM / ffi
- GitHub Check: Ubuntu ARM / core
- GitHub Check: macOS / rpc
- GitHub Check: Ubuntu ARM / wallet
- GitHub Check: Ubuntu / core
- GitHub Check: Ubuntu ARM / spv
- GitHub Check: Ubuntu ARM / rpc
- GitHub Check: Ubuntu / wallet
- GitHub Check: Ubuntu / rpc
- GitHub Check: macOS / tools
- GitHub Check: macOS / ffi
- GitHub Check: macOS / wallet
- GitHub Check: Ubuntu / tools
- GitHub Check: Documentation
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (windows-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (7)
key-wallet/src/account/account_type.rs (1)
99-137: Fallible conversion keeps PlatformPayment out of core checks.The TryFrom mapping cleanly rejects PlatformPayment and avoids panics in core routing. Based on coding guidelines, this is the right direction for fallible conversions in library code.
key-wallet-ffi/src/address_pool.rs (2)
21-116: ManagedCoreAccount lookup update is consistent.The helper lookups now return ManagedCoreAccount references, which matches the core-account rename and keeps the FFI surface consistent.
498-508: Good guardrail against PlatformPayment address-pool operations.The try_into gate with an explicit FFI error prevents platform-only accounts from entering core address pool flows.
key-wallet/src/transaction_checking/transaction_router/tests/conversions.rs (2)
25-141: Positive conversion tests updated cleanly to TryFrom.The TryInto-based tests cover all supported account types and keep expectations clear.
143-255: Nice to see explicit failure coverage for PlatformPayment.The negative cases validate the new error path for both owned and reference conversions.
key-wallet/src/transaction_checking/transaction_router/mod.rs (2)
165-184: Docs clarify core-only account types.Explicitly noting the PlatformPayment exclusion here makes the core routing contract clearer.
196-312: TryFrom conversions look solid.The fallible mappings remove the prior panic path and cleanly reject PlatformPayment while preserving all core mappings.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /// Get the first platform payment managed account | ||
| /// | ||
| /// Returns the platform payment account with the lowest account index and key_class 0. | ||
| pub fn first_platform_payment_managed_account(&self) -> Option<&ManagedPlatformAccount> { | ||
| self.platform_payment_managed_account(0, 0) | ||
| } |
There was a problem hiding this comment.
Doc comment doesn’t match behavior.
first_platform_payment_managed_account() hardcodes account index 0, but the doc says “lowest account index.” Either update the doc to match or implement a lookup for the lowest existing account index with key_class 0.
✏️ Doc fix (align with current behavior)
- /// Returns the platform payment account with the lowest account index and key_class 0.
+ /// Returns the platform payment account with account index 0 and key_class 0.🤖 Prompt for AI Agents
In `@key-wallet/src/wallet/managed_wallet_info/helpers.rs` around lines 205 - 210,
The doc is wrong: first_platform_payment_managed_account() currently always uses
index 0; change it to search for the smallest existing account index with
key_class 0 instead of hardcoding 0. Implement by iterating over whatever
collection holds platform payment accounts on the struct (or by trying
increasing indices until platform_payment_managed_account(index, 0) returns
Some), return the first Some(&ManagedPlatformAccount) found, and update the doc
comment to reflect that it returns the platform payment account with the lowest
existing account index for key_class 0; keep the method name
first_platform_payment_managed_account and use
platform_payment_managed_account(index, 0) to locate accounts.
There was a problem hiding this comment.
@QuantumExplorer, understood! I'll leave the implementation and documentation as-is.
🐰
🧠 Learnings used
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/**/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
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
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: Maintain backward compatibility for serialized wallets and monitor for new DIPs affecting wallet structure when updating derivation paths
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
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: Prepare support for future Dash upgrades including Schnorr/Taproot support, descriptor wallets, multi-signature account types, and Lightning Network payment channels
github-actions
bot
added
the
merge-conflict
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
github-actions
bot
removed
the
merge-conflict
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@key-wallet/src/managed_account/managed_platform_account.rs`:
- Around line 115-130: The negation of delta can overflow for i64::MIN; in
update_address_credit_balance replace the raw (-delta) as u64 with a safe
absolute/negation path: match on delta (or use delta.checked_abs()) to get an
Option<i64>, map the Some to u64 via as u64, and handle None (the i64::MIN case)
by using (i64::MAX as u64).saturating_add(1) (i.e. 2^63) as the negative amount,
then use that neg_amount with saturating_sub; keep the rest of the function
(address_balances.insert, recalculate_credit_balance, metadata.last_used)
unchanged.
In `@key-wallet/src/transaction_checking/platform_checker.rs`:
- Around line 136-148: The cast from u64 to i64 in
increase_platform_address_balance_for_account is unsafe; use a checked
conversion before calling account.update_address_credit_balance (which expects
an i64). Replace the direct cast with a safe conversion like
i64::try_from(amount).ok() and if it returns None handle the overflow (e.g.,
return None or an appropriate error) instead of performing amount as i64, so
update_address_credit_balance only receives a valid i64.
- Around line 121-134: The cast from u64 to i64 in
increase_platform_address_balance can overflow; before calling
account.update_address_credit_balance(*address, amount as i64) check that amount
<= i64::MAX and only perform the cast when safe, otherwise bail out (e.g.,
return None or propagate an error) so you don’t produce a wrapped negative
value; update the logic in increase_platform_address_balance to validate the
amount and use a safe conversion prior to calling update_address_credit_balance
on the matching account in accounts.platform_payment_accounts.
- Around line 150-163: The decrement currently casts amount to i64 then negates
(-(amount as i64)) which can overflow; in decrease_platform_address_balance
compute a signed delta safely by converting with i64::try_from(amount) and using
checked_neg (e.g., let delta = i64::try_from(amount).ok()?.checked_neg().ok()? )
and pass that delta to update_address_credit_balance; if conversion/negation
fails, return None (or otherwise handle the error) so you avoid overflow when
calling update_address_credit_balance from decrease_platform_address_balance.
🧹 Nitpick comments (4)
key-wallet/src/managed_account/platform_address.rs (2)
178-190: Consider adding a negative test case forfrom_address.The test covers the happy path (P2PKH roundtrip), but doesn't verify that non-P2PKH addresses are correctly rejected. This would ensure the error handling in
from_addressworks as documented.🧪 Suggested additional test
#[test] fn test_from_non_p2pkh_address_fails() { use dashcore::address::Payload; use dashcore::hashes::sha256d::Hash as Sha256dHash; use dashcore::hashes::Hash as HashTrait; // Create a P2SH address (non-P2PKH) let script_hash = Sha256dHash::from_slice(&[0x11; 32]).unwrap(); let payload = Payload::ScriptHash(script_hash.as_byte_array()[..20].try_into().unwrap().into()); let p2sh_addr = Address::new(Network::Testnet, payload); assert!(PlatformP2PKHAddress::from_address(&p2sh_addr).is_err()); }
67-71: Replacefrom_slice().expect()withfrom_byte_array()for infallible conversion.This type stores a validated
[u8; 20]array, so use the infalliblefrom_byte_arraymethod instead offrom_slice().expect()to align with the library code guideline of avoidingexpect().♻️ Suggested fix
pub fn to_address(&self, network: Network) -> Address { - let pubkey_hash = Hash160::from_slice(&self.0).expect("20 bytes is valid for Hash160"); + let pubkey_hash = Hash160::from_byte_array(self.0); let payload = Payload::PubkeyHash(pubkey_hash.into()); Address::new(network, payload) }key-wallet/src/managed_account/managed_platform_account.rs (2)
35-55: Consider field visibility for invariant protection.All fields are
pub, which allows external code to directly mutatecredit_balancewithout updatingaddress_balancesor vice versa, potentially breaking the invariant thatcredit_balance == address_balances.values().sum().If this is intentional for serialization/deserialization flexibility, consider documenting the invariant requirement. Otherwise, making fields private with accessor methods would provide better encapsulation.
Based on learnings, mutable wrappers like
ManagedPlatformAccountshould manage state carefully. Direct field access bypasses therecalculate_credit_balance()safeguard.
289-320: Consider recalculatingcredit_balanceon decode for data integrity.The decoder loads
credit_balancedirectly from the serialized data. If the stored data was corrupted or manually edited, this could lead to an inconsistent state wherecredit_balance != address_balances.values().sum().♻️ Suggested fix to ensure consistency on decode
Ok(Self { account, key_class, network, - credit_balance, + credit_balance: address_balances.values().sum(), // Recalculate for data integrity address_balances, addresses, metadata, is_watch_only, })Alternatively, you could add a validation step that compares the stored
credit_balancewith the sum and returns a decode error if they don't match.
| fn decrease_platform_address_balance( | ||
| &mut self, | ||
| address: &PlatformP2PKHAddress, | ||
| amount: u64, | ||
| ) -> Option<u64> { | ||
| // Find the account that contains this address | ||
| for account in self.accounts.platform_payment_accounts.values_mut() { | ||
| if account.contains_platform_address(address) { | ||
| let new_balance = account.update_address_credit_balance(*address, -(amount as i64)); | ||
| return Some(new_balance); | ||
| } | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
Same overflow concern with additional negation risk.
Line 158 has the same cast issue, and the negation -(amount as i64) could additionally overflow if the cast resulted in i64::MIN.
🔧 Suggested fix
fn decrease_platform_address_balance(
&mut self,
address: &PlatformP2PKHAddress,
amount: u64,
) -> Option<u64> {
+ let delta = i64::try_from(amount).ok().and_then(|v| v.checked_neg())?;
// Find the account that contains this address
for account in self.accounts.platform_payment_accounts.values_mut() {
if account.contains_platform_address(address) {
- let new_balance = account.update_address_credit_balance(*address, -(amount as i64));
+ let new_balance = account.update_address_credit_balance(*address, delta);
return Some(new_balance);
}
}
None
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn decrease_platform_address_balance( | |
| &mut self, | |
| address: &PlatformP2PKHAddress, | |
| amount: u64, | |
| ) -> Option<u64> { | |
| // Find the account that contains this address | |
| for account in self.accounts.platform_payment_accounts.values_mut() { | |
| if account.contains_platform_address(address) { | |
| let new_balance = account.update_address_credit_balance(*address, -(amount as i64)); | |
| return Some(new_balance); | |
| } | |
| } | |
| None | |
| } | |
| fn decrease_platform_address_balance( | |
| &mut self, | |
| address: &PlatformP2PKHAddress, | |
| amount: u64, | |
| ) -> Option<u64> { | |
| let delta = i64::try_from(amount).ok().and_then(|v| v.checked_neg())?; | |
| // Find the account that contains this address | |
| for account in self.accounts.platform_payment_accounts.values_mut() { | |
| if account.contains_platform_address(address) { | |
| let new_balance = account.update_address_credit_balance(*address, delta); | |
| return Some(new_balance); | |
| } | |
| } | |
| None | |
| } |
🤖 Prompt for AI Agents
In `@key-wallet/src/transaction_checking/platform_checker.rs` around lines 150 -
163, The decrement currently casts amount to i64 then negates (-(amount as i64))
which can overflow; in decrease_platform_address_balance compute a signed delta
safely by converting with i64::try_from(amount) and using checked_neg (e.g., let
delta = i64::try_from(amount).ok()?.checked_neg().ok()? ) and pass that delta to
update_address_credit_balance; if conversion/negation fails, return None (or
otherwise handle the error) so you avoid overflow when calling
update_address_credit_balance from decrease_platform_address_balance.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@key-wallet/src/managed_account/managed_platform_account.rs`:
- Around line 57-75: The constructor ManagedPlatformAccount::new must ensure the
provided network matches the AddressPool's network; update new to validate this
and make it fallible: change signature to return Result<Self, SomeErrorType>
(e.g., AccountError::NetworkMismatch), compare addresses.get_network() (or
addresses.network) against the network parameter, and return
Err(AccountError::NetworkMismatch) if they differ; alternatively, remove the
network parameter and derive network from AddressPool to keep new
infallible—apply the change in ManagedPlatformAccount::new and add/update the
error enum and call sites accordingly.
In `@key-wallet/src/transaction_checking/platform_checker.rs`:
- Around line 92-156: The helper methods set_platform_address_balance,
set_platform_address_balance_for_account, increase_platform_address_balance, and
increase_platform_address_balance_for_account must update address-usage state
when balances change instead of always passing None for key_source; modify their
signatures to accept an Option<KeySource> (or a KeySource where caller supplies
one) and propagate that into account.set_address_credit_balance /
account.add_address_credit_balance so the Account can mark addresses used and
run gap-limit maintenance, and additionally ensure increase_* checks if the
previous balance was 0 and the new balance >0 to mark the address used when no
explicit KeySource is provided.
QuantumExplorer
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@key-wallet/src/transaction_checking/platform_checker.rs`:
- Around line 121-134: The setter/increaser currently mutates balances if the
account exists without verifying the address belongs to that account; update
set_platform_address_balance_for_account (and the similar methods around lines
152-165) to first check account.contains_platform_address(address) and return
false (or None for Option-returning methods) if the address is not a member,
otherwise proceed to call account.set_address_credit_balance / increaser
functions; reference PlatformPaymentAccountKey, PlatformP2PKHAddress,
contains_platform_address, set_address_credit_balance and KeySource to locate
and implement the guard.
🧹 Nitpick comments (1)
key-wallet/src/transaction_checking/platform_checker.rs (1)
105-119: Consider indexing platform addresses to avoid repeated linear scans.
Each balance operation iterates all platform accounts; this is O(n) per call. If address operations are frequent or account counts grow, a lookup index (e.g.,HashMap<PlatformP2PKHAddress, PlatformPaymentAccountKey>) in the account collection would reduce lookup cost.Also applies to: 136-150, 167-190
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@key-wallet/src/managed_account/managed_account_collection.rs`:
- Around line 255-261: The insert() branch currently panics for
ManagedAccountType::PlatformPayment which is unacceptable in library code;
change insert() to return a Result (e.g., Result<(), InsertError>) instead of
panicking, detect the PlatformPayment case in the insert() implementation and
return an appropriate error variant (e.g., Err(InsertError::InvalidAccountType {
expected: "ManagedCoreAccount", found: "PlatformPayment" })) and update callers
to handle the Result; alternatively, if you prefer compile-time prevention,
refactor types so insert_platform_account() accepts ManagedPlatformAccount and
insert() only accepts ManagedCoreAccount, removing the PlatformPayment match arm
entirely—refer to ManagedAccountType::PlatformPayment, insert(),
insert_platform_account(), ManagedPlatformAccount, and ManagedCoreAccount when
making the change.
🧹 Nitpick comments (3)
key-wallet/src/managed_account/managed_platform_account.rs (3)
37-55: Public fields risk balance consistency if mutated directly.
credit_balanceandaddress_balancesare public, allowing direct mutation that bypassesrecalculate_credit_balance. If a caller modifiesaddress_balancesdirectly without updatingcredit_balance, the totals become inconsistent.Consider making these fields private or documenting that direct mutation requires calling
recalculate_credit_balance()afterward.
85-89:set_credit_balancedoesn't synchronize with per-address balances.This method sets
credit_balancedirectly without updatingaddress_balances. The docstring mentions "deserialization" use case, but callers may inadvertently use it and create inconsistency betweencredit_balanceandaddress_balances.values().sum().Consider adding a note to the docstring clarifying when this should (and shouldn't) be used, or adding an assertion/check.
275-286: Silently ignoring gap limit maintenance errors may hide issues.Line 285 discards the
Resultfrommaintain_gap_limit. While the comment says "best-effort," in a financial wallet context, failing to generate new addresses could affect address reuse patterns. Consider at least logging the error in debug builds or returning a status.♻️ Suggested improvement
fn mark_and_maintain_gap_limit( &mut self, address: &PlatformP2PKHAddress, key_source: &KeySource, ) { // Mark the address as used self.mark_platform_address_used(address); // Maintain gap limit - generate new addresses if needed - // We ignore errors here since this is a best-effort operation - let _ = self.addresses.maintain_gap_limit(key_source); + // Best-effort: log errors in debug builds but don't fail + if let Err(_e) = self.addresses.maintain_gap_limit(key_source) { + #[cfg(feature = "std")] + { + // Consider: eprintln!("Warning: gap limit maintenance failed: {}", _e); + } + } }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@key-wallet-ffi/include/key_wallet_ffi.h`:
- Around line 3823-3829: Update the three NOTE comment blocks that currently
reference Rust enum variant names and replace those names with the corresponding
C FFI enum identifiers (FFIAccountType constants) so FFI consumers aren’t
confused; specifically, change the Rust variant mentions in the block that
references wallet_add_platform_payment_account(),
wallet_add_dashpay_receiving_account(), and
wallet_add_dashpay_external_account_with_xpub_bytes() to use the appropriate
FFIAccountType constants (FFIAccountType::PlatformPayment,
FFIAccountType::DashpayReceivingFunds, FFIAccountType::DashpayExternalAccount or
the exact C-style names used in the header) and apply the same replacement to
the other two note blocks referenced in the comment.
- Around line 3918-3940: Update the documentation for
wallet_add_platform_payment_account to clarify that account_index and key_class
are raw (non-hardened) indices and that hardening is applied internally (via
ChildNumber::from_hardened_idx in the Rust implementation); specifically replace
or augment the “(hardened)” text for account_index and key_class with wording
like “pass raw index (e.g., 0, 1, 2); hardening is applied internally” so
callers do not double-harden the derivation path for FFIWallet ->
wallet_add_platform_payment_account returning FFIAccountResult.
🧹 Nitpick comments (1)
key-wallet-ffi/src/wallet.rs (1)
489-516: Consider centralizing the unsupported-account guard.The same match + error strings are repeated in three entry points; a small helper would reduce drift when messages or supported types change.
♻️ Suggested helper to de-duplicate the guard
+fn reject_unsupported_account_type( + account_type: FFIAccountType, +) -> Option<crate::types::FFIAccountResult> { + match account_type { + FFIAccountType::PlatformPayment => Some(crate::types::FFIAccountResult::error( + FFIErrorCode::InvalidInput, + "PlatformPayment accounts require account and key_class indices. \ + Use wallet_add_platform_payment_account() instead." + .to_string(), + )), + FFIAccountType::DashpayReceivingFunds => Some(crate::types::FFIAccountResult::error( + FFIErrorCode::InvalidInput, + "DashpayReceivingFunds accounts require identity IDs. \ + Use wallet_add_dashpay_receiving_account() instead." + .to_string(), + )), + FFIAccountType::DashpayExternalAccount => Some(crate::types::FFIAccountResult::error( + FFIErrorCode::InvalidInput, + "DashpayExternalAccount accounts require identity IDs. \ + Use wallet_add_dashpay_external_account_with_xpub_bytes() instead." + .to_string(), + )), + _ => None, + } +} ... - // Check for account types that require special handling - match account_type { - FFIAccountType::PlatformPayment => { - return crate::types::FFIAccountResult::error( - FFIErrorCode::InvalidInput, - "PlatformPayment accounts require account and key_class indices. \ - Use wallet_add_platform_payment_account() instead." - .to_string(), - ); - } - FFIAccountType::DashpayReceivingFunds => { - return crate::types::FFIAccountResult::error( - FFIErrorCode::InvalidInput, - "DashpayReceivingFunds accounts require identity IDs. \ - Use wallet_add_dashpay_receiving_account() instead." - .to_string(), - ); - } - FFIAccountType::DashpayExternalAccount => { - return crate::types::FFIAccountResult::error( - FFIErrorCode::InvalidInput, - "DashpayExternalAccount accounts require identity IDs. \ - Use wallet_add_dashpay_external_account_with_xpub_bytes() instead." - .to_string(), - ); - } - _ => {} // Other types are supported - } + if let Some(err) = reject_unsupported_account_type(account_type) { + return err; + }Also applies to: 716-743, 839-866
|
|
||
| # Note | ||
|
|
||
| This function does NOT support the following account types: | ||
| - `PlatformPayment`: Use `wallet_add_platform_payment_account()` instead | ||
| - `DashpayReceivingFunds`: Use `wallet_add_dashpay_receiving_account()` instead | ||
| - `DashpayExternalAccount`: Use `wallet_add_dashpay_external_account_with_xpub_bytes()` instead |
There was a problem hiding this comment.
Use the C enum identifiers in these notes to avoid FFI confusion.
The notes currently list Rust variant names, but C callers see FFIAccountType constants. Using the C identifiers makes the guidance unambiguous for FFI consumers.
📝 Suggested wording updates (apply to all three note blocks)
- * - `PlatformPayment`: Use `wallet_add_platform_payment_account()` instead
- * - `DashpayReceivingFunds`: Use `wallet_add_dashpay_receiving_account()` instead
- * - `DashpayExternalAccount`: Use `wallet_add_dashpay_external_account_with_xpub_bytes()` instead
+ * - `PLATFORM_PAYMENT`: Use `wallet_add_platform_payment_account()` instead
+ * - `DASHPAY_RECEIVING_FUNDS`: Use `wallet_add_dashpay_receiving_account()` instead
+ * - `DASHPAY_EXTERNAL_ACCOUNT`: Use `wallet_add_dashpay_external_account_with_xpub_bytes()` insteadAlso applies to: 3877-3883, 3903-3909
🤖 Prompt for AI Agents
In `@key-wallet-ffi/include/key_wallet_ffi.h` around lines 3823 - 3829, Update the
three NOTE comment blocks that currently reference Rust enum variant names and
replace those names with the corresponding C FFI enum identifiers
(FFIAccountType constants) so FFI consumers aren’t confused; specifically,
change the Rust variant mentions in the block that references
wallet_add_platform_payment_account(), wallet_add_dashpay_receiving_account(),
and wallet_add_dashpay_external_account_with_xpub_bytes() to use the appropriate
FFIAccountType constants (FFIAccountType::PlatformPayment,
FFIAccountType::DashpayReceivingFunds, FFIAccountType::DashpayExternalAccount or
the exact C-style names used in the header) and apply the same replacement to
the other two note blocks referenced in the comment.
| /* | ||
| Add a Platform Payment account (DIP-17) to the wallet | ||
|
|
||
| Platform Payment accounts use the derivation path: | ||
| `m/9'/coin_type'/17'/account'/key_class'/index` | ||
|
|
||
| # Arguments | ||
| * `wallet` - Pointer to the wallet | ||
| * `account_index` - The account index (hardened) in the derivation path | ||
| * `key_class` - The key class (hardened) - typically 0' for main addresses | ||
|
|
||
| # Safety | ||
|
|
||
| This function dereferences a raw pointer to FFIWallet. | ||
| The caller must ensure that: | ||
| - The wallet pointer is either null or points to a valid FFIWallet | ||
| - The FFIWallet remains valid for the duration of this call | ||
| */ | ||
|
|
||
| FFIAccountResult wallet_add_platform_payment_account(FFIWallet *wallet, | ||
| unsigned int account_index, | ||
| unsigned int key_class) | ||
| ; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the Rust implementation of wallet_add_platform_payment_account
rg -n "wallet_add_platform_payment_account" -g '*.rs' -A 20Repository: dashpay/rust-dashcore
Length of output: 10569
🏁 Script executed:
#!/bin/bash
# Find AccountType definition and PlatformPayment variant
rg -n "enum AccountType|impl.*AccountType" -g '*.rs' -A 30 | head -100Repository: dashpay/rust-dashcore
Length of output: 9121
🏁 Script executed:
#!/bin/bash
# Find AccountType enum definition with PlatformPayment
rg -n "pub enum AccountType" -g '*.rs' -A 50 | grep -A 50 "key-wallet/src/account"Repository: dashpay/rust-dashcore
Length of output: 8910
🏁 Script executed:
#!/bin/bash
# Search for PlatformPayment in account type file
rg -n "PlatformPayment" key-wallet/src/account/account_type.rs -B 2 -A 5Repository: dashpay/rust-dashcore
Length of output: 1858
🏁 Script executed:
#!/bin/bash
# Find the derivation path implementation for PlatformPayment
rg -n "DIP-17: m/9'" -B 5 -A 20 key-wallet/src/account/account_type.rsRepository: dashpay/rust-dashcore
Length of output: 1264
🏁 Script executed:
#!/bin/bash
# Check the bip32 library usage - from_hardened_idx converts raw to hardened
# Let's search for how ChildNumber is used in the codebase
rg -n "from_hardened_idx" -B 2 -A 2 key-wallet/src/ | head -40Repository: dashpay/rust-dashcore
Length of output: 3599
Clarify that account_index and key_class expect raw (non-hardened) values.
The Rust implementation applies hardening internally via ChildNumber::from_hardened_idx(), so callers must pass raw indices (e.g., 0, 1, 2). The current FFI header documentation stating "(hardened)" is misleading and could cause callers to incorrectly apply hardening twice, deriving wrong paths. Update the docs to say "pass raw index; hardening applied internally" or similar.
🤖 Prompt for AI Agents
In `@key-wallet-ffi/include/key_wallet_ffi.h` around lines 3918 - 3940, Update the
documentation for wallet_add_platform_payment_account to clarify that
account_index and key_class are raw (non-hardened) indices and that hardening is
applied internally (via ChildNumber::from_hardened_idx in the Rust
implementation); specifically replace or augment the “(hardened)” text for
account_index and key_class with wording like “pass raw index (e.g., 0, 1, 2);
hardening is applied internally” so callers do not double-harden the derivation
path for FFIWallet -> wallet_add_platform_payment_account returning
FFIAccountResult.
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
github-actions
bot
removed
the
merge-conflict
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
key-wallet/src/managed_account/managed_account_collection.rs (1)
435-437:⚠️ Potential issue | 🟠 MajorPropagate
derivation_patherrors instead of falling back to master.The
derivation_pathmethod can fail withInvalidNetworkorBip32errors—both critical failures that must not be silently swallowed. Falling back to the master path when derivation fails causes addresses to be derived on incorrect paths, silently corrupting account state. The containing function already returnsResult, so error propagation via the?operator is straightforward and consistent with how this method is handled elsewhere in the codebase.- let base_path = account_type - .derivation_path(network) - .unwrap_or_else(|_| crate::bip32::DerivationPath::master()); + let base_path = account_type.derivation_path(network)?;
🤖 Fix all issues with AI agents
In `@key-wallet/src/managed_account/managed_account_collection.rs`:
- Around line 640-668: The function create_managed_platform_account_from_account
does not verify that the provided PlatformPaymentAccountKey corresponds to the
Account's account_type; add an explicit validation at the start of
create_managed_platform_account_from_account that compares the key's identifying
fields (e.g., PlatformPaymentAccountKey.key_class or key.account/variant)
against account.account_type (or use a helper like
account.account_type.is_platform_payment()/matches!) and return an appropriate
crate::error::Error (e.g., an invalid input or mismatch error) if they differ,
before constructing KeySource/AddressPool and returning ManagedPlatformAccount.
🧹 Nitpick comments (1)
key-wallet/src/managed_account/managed_account_collection.rs (1)
368-374: Surface platform account conversion errors instead of silently skipping.If conversion fails, accounts are dropped without caller visibility. Consider returning
Result<Self, Error>and bubbling errors.♻️ One possible direction
-pub fn from_account_collection(account_collection: &AccountCollection) -> Self { +pub fn from_account_collection( + account_collection: &AccountCollection, +) -> Result<Self, crate::error::Error> { let mut managed_collection = Self::new(); // ... - for (key, account) in &account_collection.platform_payment_accounts { - if let Ok(managed_account) = - Self::create_managed_platform_account_from_account(account, key) - { - managed_collection.platform_payment_accounts.insert(*key, managed_account); - } - } + for (key, account) in &account_collection.platform_payment_accounts { + let managed_account = + Self::create_managed_platform_account_from_account(account, key)?; + managed_collection.platform_payment_accounts.insert(*key, managed_account); + } - managed_collection + Ok(managed_collection) }As per coding guidelines, return
Result<T>for fallible operations and avoid silently discarding errors.
| /// Create a ManagedPlatformAccount from an Account for Platform Payment accounts | ||
| fn create_managed_platform_account_from_account( | ||
| account: &Account, | ||
| key: &PlatformPaymentAccountKey, | ||
| ) -> Result<ManagedPlatformAccount, crate::error::Error> { | ||
| // Use the account's existing public key | ||
| let key_source = KeySource::Public(account.account_xpub); | ||
|
|
||
| // Get the derivation path for this account type | ||
| let base_path = account | ||
| .account_type | ||
| .derivation_path(account.network) | ||
| .unwrap_or_else(|_| crate::bip32::DerivationPath::master()); | ||
|
|
||
| // Create address pool for DIP-17 Platform Payment addresses | ||
| let addresses = AddressPool::new( | ||
| base_path, | ||
| AddressPoolType::Absent, | ||
| DIP17_GAP_LIMIT, | ||
| account.network, | ||
| &key_source, | ||
| )?; | ||
|
|
||
| Ok(ManagedPlatformAccount::new( | ||
| key.account, | ||
| key.key_class, | ||
| addresses, | ||
| account.is_watch_only, | ||
| )) |
There was a problem hiding this comment.
Validate that key matches account.account_type for platform accounts.
A mismatched key/account_type pair will store a platform account under the wrong map key, breaking lookups and balances.
🔒 Suggested validation
// Use the account's existing public key
let key_source = KeySource::Public(account.account_xpub);
+ let (acct, key_class) = match &account.account_type {
+ AccountType::PlatformPayment { account, key_class } => (*account, *key_class),
+ _ => {
+ return Err(crate::error::Error::InvalidParameter(
+ "Expected PlatformPayment account type".to_string(),
+ ))
+ }
+ };
+ if acct != key.account || key_class != key.key_class {
+ return Err(crate::error::Error::InvalidParameter(
+ "PlatformPayment key does not match account type".to_string(),
+ ));
+ }As per coding guidelines, always validate inputs from untrusted sources.
🤖 Prompt for AI Agents
In `@key-wallet/src/managed_account/managed_account_collection.rs` around lines
640 - 668, The function create_managed_platform_account_from_account does not
verify that the provided PlatformPaymentAccountKey corresponds to the Account's
account_type; add an explicit validation at the start of
create_managed_platform_account_from_account that compares the key's identifying
fields (e.g., PlatformPaymentAccountKey.key_class or key.account/variant)
against account.account_type (or use a helper like
account.account_type.is_platform_payment()/matches!) and return an appropriate
crate::error::Error (e.g., an invalid input or mismatch error) if they differ,
before constructing KeySource/AddressPool and returning ManagedPlatformAccount.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@key-wallet/src/managed_account/managed_platform_account.rs`:
- Around line 339-393: The Decode impl for ManagedPlatformAccount currently
trusts the decoded credit_balance and can desync from address_balances; after
decoding address_balances and before returning the struct in
ManagedPlatformAccount::decode, either (A) call the existing
recalculate_credit_balance() on the constructed Self and replace credit_balance
with the recomputed value, or (B) validate that decoded credit_balance ==
recomputed sum and if not return a bincode::error::DecodeError::OtherString with
a descriptive message; update the return path in bincode::Decode::decode to
perform this check (referencing ManagedPlatformAccount, credit_balance,
address_balances, and recalculate_credit_balance) and ensure the error uses
bincode::error::DecodeError::OtherString when choosing the strict validation
option.
In `@key-wallet/src/managed_account/platform_address.rs`:
- Around line 62-71: The to_address method uses
Hash160::from_slice(...).expect(...) which can panic; replace this with the
infallible Hash160::from_byte_array(...) path by converting self.0 into a [u8;
20] and calling Hash160::from_byte_array(array) to construct the pubkey_hash;
then build the Payload::PubkeyHash and Address as before. Update the code in
to_address to avoid expect() and use from_byte_array on the array derived from
self.0.
🧹 Nitpick comments (1)
key-wallet/src/managed_account/managed_platform_account.rs (1)
255-286: Surface gap‑limit maintenance failures to keep balance/usage updates atomic.Line 285 drops errors from
maintain_gap_limit, so balances can update while address‑usage/gap‑limit state does not. Consider returning aResultfrommark_and_maintain_gap_limit(and the balance mutators that call it) so callers can retry or rollback on failure.Based on learnings, implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together.
| #[cfg(feature = "bincode")] | ||
| impl bincode::Encode for ManagedPlatformAccount { | ||
| fn encode<E: bincode::enc::Encoder>( | ||
| &self, | ||
| encoder: &mut E, | ||
| ) -> core::result::Result<(), bincode::error::EncodeError> { | ||
| // Encode each field | ||
| bincode::Encode::encode(&self.account, encoder)?; | ||
| bincode::Encode::encode(&self.key_class, encoder)?; | ||
| bincode::Encode::encode(&self.network, encoder)?; | ||
| bincode::Encode::encode(&self.credit_balance, encoder)?; | ||
|
|
||
| // Encode address_balances as a vec of tuples | ||
| let address_balances_vec: Vec<(PlatformP2PKHAddress, u64)> = | ||
| self.address_balances.iter().map(|(k, v)| (*k, *v)).collect(); | ||
| bincode::Encode::encode(&address_balances_vec, encoder)?; | ||
|
|
||
| bincode::Encode::encode(&self.addresses, encoder)?; | ||
| bincode::Encode::encode(&self.metadata, encoder)?; | ||
| bincode::Encode::encode(&self.is_watch_only, encoder)?; | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "bincode")] | ||
| impl<Context> bincode::Decode<Context> for ManagedPlatformAccount { | ||
| fn decode<D: bincode::de::Decoder<Context = Context>>( | ||
| decoder: &mut D, | ||
| ) -> core::result::Result<Self, bincode::error::DecodeError> { | ||
| let account = bincode::Decode::decode(decoder)?; | ||
| let key_class = bincode::Decode::decode(decoder)?; | ||
| let network = bincode::Decode::decode(decoder)?; | ||
| let credit_balance = bincode::Decode::decode(decoder)?; | ||
|
|
||
| // Decode address_balances from vec of tuples | ||
| let address_balances_vec: Vec<(PlatformP2PKHAddress, u64)> = | ||
| bincode::Decode::decode(decoder)?; | ||
| let address_balances: BTreeMap<PlatformP2PKHAddress, u64> = | ||
| address_balances_vec.into_iter().collect(); | ||
|
|
||
| let addresses = bincode::Decode::decode(decoder)?; | ||
| let metadata = bincode::Decode::decode(decoder)?; | ||
| let is_watch_only = bincode::Decode::decode(decoder)?; | ||
|
|
||
| Ok(Self { | ||
| account, | ||
| key_class, | ||
| network, | ||
| credit_balance, | ||
| address_balances, | ||
| addresses, | ||
| metadata, | ||
| is_watch_only, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the structure definition of ManagedPlatformAccount
fd -e rs -path "*/managed_account/*" | head -20Repository: dashpay/rust-dashcore
Length of output: 233
🏁 Script executed:
# Find and examine the ManagedPlatformAccount struct definition
rg "struct ManagedPlatformAccount" -A 30 key-wallet/src/Repository: dashpay/rust-dashcore
Length of output: 3088
🏁 Script executed:
# Look for recalculate_credit_balance method
rg "recalculate_credit_balance" key-wallet/src/Repository: dashpay/rust-dashcore
Length of output: 174
🏁 Script executed:
# Check the bincode version in Cargo.toml
rg "bincode" Cargo.tomlRepository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Find recalculate_credit_balance implementation
rg "fn recalculate_credit_balance" -A 15 key-wallet/src/managed_account/managed_platform_account.rsRepository: dashpay/rust-dashcore
Length of output: 617
🏁 Script executed:
# Search for Cargo.toml files
fd -name "Cargo.toml" | head -10Repository: dashpay/rust-dashcore
Length of output: 295
🏁 Script executed:
# Extract bincode dependency version
cat key-wallet/Cargo.toml | grep -A 3 -B 1 "bincode"Repository: dashpay/rust-dashcore
Length of output: 842
🏁 Script executed:
# Look for other Decode implementations with validation patterns
rg "#\[cfg\(feature = \"bincode\"\)\]" -A 25 key-wallet/src/ | grep -A 20 "impl.*bincode::Decode"Repository: dashpay/rust-dashcore
Length of output: 16091
🏁 Script executed:
# Check if there are validation patterns or error returns in other Decode impls
rg "Decode::decode" -B 5 -A 10 key-wallet/src/ | head -100Repository: dashpay/rust-dashcore
Length of output: 7534
🏁 Script executed:
# Look at the full Decode implementation to see if there's any existing validation
sed -n '360,393p' key-wallet/src/managed_account/managed_platform_account.rsRepository: dashpay/rust-dashcore
Length of output: 1264
🌐 Web query:
bincode 2.x custom DecodeError validation patterns Rust
💡 Result:
What DecodeError can represent in bincode 2.x (and where “custom validation” fits)
In bincode 2.0.1, bincode::error::DecodeError is mainly about wire-format / decoding failures (unexpected end, limit exceeded, invalid enum variant, UTF-8, etc.), but it does give you two “escape hatches” intended for uncommon/custom failures:
DecodeError::Other(&'static str)DecodeError::OtherString(String)(requiresalloc)
[1]
So the standard pattern is: decode first, then validate, and if validation fails, convert that into DecodeError::Other*.
Pattern 1: Decode, then validate (most common)
Keep the derived Decode, and layer validation on top:
use bincode::{config, decode_from_slice};
use bincode::error::DecodeError;
#[derive(bincode::Decode)]
struct Msg {
n: u32,
}
impl Msg {
fn validate(self) -> Result<Self, DecodeError> {
if self.n == 0 {
return Err(DecodeError::Other("n must be non-zero"));
}
Ok(self)
}
}
fn decode_msg(bytes: &[u8]) -> Result<(Msg, usize), DecodeError> {
let (msg, read) = decode_from_slice(bytes, config::standard())?; // or config::legacy()
Ok((msg.validate()?, read))
}decode_from_slice returns (value, bytes_read). [2]
Pattern 2: Newtype wrapper that validates during Decode (keeps call sites clean)
This is useful when you want “validated by construction” types:
use bincode::{Decode};
use bincode::error::DecodeError;
#[derive(bincode::Decode)]
struct RawMsg {
n: u32,
}
struct Msg(RawMsg);
impl<Ctx> Decode<Ctx> for Msg {
fn decode<D: bincode::de::Decoder<Context = Ctx>>(decoder: &mut D) -> Result<Self, DecodeError> {
let raw = RawMsg::decode(decoder)?;
if raw.n == 0 {
return Err(DecodeError::Other("n must be non-zero"));
}
Ok(Msg(raw))
}
}When you do this, all decode failures—including validation—share the same Result<_, DecodeError> channel. [1]
Pattern 3: Convert domain errors into DecodeError::OtherString (better messages)
If you have a rich validation error type, convert it:
use bincode::error::DecodeError;
fn map_validation_err(e: impl core::fmt::Display) -> DecodeError {
DecodeError::OtherString(e.to_string()) // requires bincode `alloc` feature
}OtherString(String) is explicitly part of DecodeError in 2.0.1 (behind alloc). [1]
Notes
- If you’re migrating from bincode 1.x: decoding is now
bincode::decode_from_slice(&bytes, config)(and serde is optional). [3]
Sources: bincode 2.0.1 DecodeError variants and Other/OtherString [1], decode_from_slice signature/behavior [2], official migration guide [3].
Add validation to ensure decoded credit_balance matches the sum of address_balances.
The Decode implementation currently trusts the stored total without validation. If serialized data is corrupted or manually crafted, credit_balance could diverge from the sum of address_balances, leading to incorrect balance reporting after deserialization.
Call recalculate_credit_balance() after construction to restore consistency, or—following the validation pattern used elsewhere in the codebase (e.g., Mnemonic, ExtendedPubKey)—validate the invariant and return DecodeError::OtherString() on mismatch to catch data integrity issues:
Option 1: Recompute (silent correction)
- Ok(Self {
+ let mut account = Self {
account,
key_class,
network,
credit_balance,
address_balances,
addresses,
metadata,
is_watch_only,
- })
+ };
+ account.recalculate_credit_balance();
+ Ok(account)Option 2: Validate (strict, data-integrity check)
+ let expected_total: u64 = address_balances.values().sum();
+ if credit_balance != expected_total {
+ return Err(bincode::error::DecodeError::OtherString(
+ format!("credit_balance ({}) does not match address_balances sum ({})", credit_balance, expected_total)
+ ));
+ }
+
Ok(Self {
account,
key_class,
network,
credit_balance,
address_balances,
addresses,
metadata,
is_watch_only,
})🤖 Prompt for AI Agents
In `@key-wallet/src/managed_account/managed_platform_account.rs` around lines 339
- 393, The Decode impl for ManagedPlatformAccount currently trusts the decoded
credit_balance and can desync from address_balances; after decoding
address_balances and before returning the struct in
ManagedPlatformAccount::decode, either (A) call the existing
recalculate_credit_balance() on the constructed Self and replace credit_balance
with the recomputed value, or (B) validate that decoded credit_balance ==
recomputed sum and if not return a bincode::error::DecodeError::OtherString with
a descriptive message; update the return path in bincode::Decode::decode to
perform this check (referencing ManagedPlatformAccount, credit_balance,
address_balances, and recalculate_credit_balance) and ensure the error uses
bincode::error::DecodeError::OtherString when choosing the strict validation
option.
| /// Convert to a dashcore::Address (P2PKH) | ||
| /// | ||
| /// This creates a standard P2PKH address from the hash. | ||
| /// Note: This is for interoperability - Platform addresses typically | ||
| /// use bech32m encoding rather than base58 P2PKH format. | ||
| pub fn to_address(&self, network: Network) -> Address { | ||
| let pubkey_hash = Hash160::from_slice(&self.0).expect("20 bytes is valid for Hash160"); | ||
| let payload = Payload::PubkeyHash(pubkey_hash.into()); | ||
| Address::new(network, payload) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and understand the struct definition
cat -n key-wallet/src/managed_account/platform_address.rs | head -80Repository: dashpay/rust-dashcore
Length of output: 3376
🏁 Script executed:
# Let's check the dependencies to understand which dashcore_hashes version is used
cat Cargo.toml | grep -A 5 -B 5 dashcore_hashes || grep -r "dashcore_hashes" --include="Cargo.toml"Repository: dashpay/rust-dashcore
Length of output: 382
🏁 Script executed:
# Search for Hash160 usage and constructors in the codebase
rg "Hash160::" --type rust -A 2 -B 2 | head -100Repository: dashpay/rust-dashcore
Length of output: 639
🏁 Script executed:
# Let's look at the hashes directory structure
ls -la hashes/src/Repository: dashpay/rust-dashcore
Length of output: 1400
🏁 Script executed:
# Search for Hash160 definition and its constructors
find hashes -name "*.rs" | head -20Repository: dashpay/rust-dashcore
Length of output: 623
🏁 Script executed:
# Look for Hash160 implementation
rg "impl.*Hash160" hashes/ -A 30Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Read the Hash160 definition file
cat -n hashes/src/hash160.rsRepository: dashpay/rust-dashcore
Length of output: 5177
🏁 Script executed:
# Check the internal_macros.rs to see what constructors are generated by hash_type! macro
cat -n hashes/src/internal_macros.rs | head -150Repository: dashpay/rust-dashcore
Length of output: 7810
🏁 Script executed:
# Continue reading to find constructors
cat -n hashes/src/internal_macros.rs | sed -n '200,350p'Repository: dashpay/rust-dashcore
Length of output: 2326
🏁 Script executed:
# Search for hash_type macro definition
rg "macro_rules! hash_type" hashes/src/internal_macros.rs -A 100 | head -150Repository: dashpay/rust-dashcore
Length of output: 826
Avoid expect() in library code for Hash160 construction.
Line 68 introduces a panic path. Use the infallible from_byte_array constructor instead, which accepts [u8; 20] directly.
♻️ Proposed fix
- let pubkey_hash = Hash160::from_slice(&self.0).expect("20 bytes is valid for Hash160");
+ let pubkey_hash = Hash160::from_byte_array(self.0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Convert to a dashcore::Address (P2PKH) | |
| /// | |
| /// This creates a standard P2PKH address from the hash. | |
| /// Note: This is for interoperability - Platform addresses typically | |
| /// use bech32m encoding rather than base58 P2PKH format. | |
| pub fn to_address(&self, network: Network) -> Address { | |
| let pubkey_hash = Hash160::from_slice(&self.0).expect("20 bytes is valid for Hash160"); | |
| let payload = Payload::PubkeyHash(pubkey_hash.into()); | |
| Address::new(network, payload) | |
| } | |
| /// Convert to a dashcore::Address (P2PKH) | |
| /// | |
| /// This creates a standard P2PKH address from the hash. | |
| /// Note: This is for interoperability - Platform addresses typically | |
| /// use bech32m encoding rather than base58 P2PKH format. | |
| pub fn to_address(&self, network: Network) -> Address { | |
| let pubkey_hash = Hash160::from_byte_array(self.0); | |
| let payload = Payload::PubkeyHash(pubkey_hash.into()); | |
| Address::new(network, payload) | |
| } |
🤖 Prompt for AI Agents
In `@key-wallet/src/managed_account/platform_address.rs` around lines 62 - 71, The
to_address method uses Hash160::from_slice(...).expect(...) which can panic;
replace this with the infallible Hash160::from_byte_array(...) path by
converting self.0 into a [u8; 20] and calling Hash160::from_byte_array(array) to
construct the pubkey_hash; then build the Payload::PubkeyHash and Address as
before. Update the code in to_address to avoid expect() and use from_byte_array
on the array derived from self.0.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@key-wallet-ffi/FFI_API.md`:
- Around line 35-43: The table's separator row in FFI_API.md isn't using the
compact separator style required by markdownlint MD060; update the separator row
under the header so each column uses compact dashes with no extra spaces (e.g.
|---|---|---|) matching the three header columns, ensuring it aligns with the
header for the functions like account_result_free_error, error_message_free,
managed_account_result_free_error and
managed_platform_account_result_free_error.
- Around line 1270-1284: The args section for
wallet_add_platform_payment_account currently uses inline `*` bullets which
trigger MD037; change that inline sentence into a proper markdown list under
"Arguments" and list each parameter as its own bullet (e.g., `* wallet` -
Pointer to the wallet; `* account_index` - The account index (hardened) in the
derivation path; `* key_class` - The key class (hardened) - typically 0' for
main addresses) and remove the inline asterisks inside sentences; ensure
references to FFIWallet, account_index, and key_class remain unchanged and that
spacing/line breaks conform to the project's markdown linter rules.
🧹 Nitpick comments (1)
key-wallet-ffi/src/managed_account_collection.rs (1)
770-811: Sort platform payment keys for deterministic order.
Other index lists are sorted; here the iteration order depends on the map implementation. Sorting by(account, key_class)keeps FFI outputs stable. Please confirm downstream consumers don’t rely on current insertion order before changing.♻️ Proposed adjustment
- let keys: Vec<crate::managed_account::FFIPlatformPaymentAccountKey> = collection + let mut keys: Vec<crate::managed_account::FFIPlatformPaymentAccountKey> = collection .collection .platform_payment_accounts .keys() .map(crate::managed_account::FFIPlatformPaymentAccountKey::from) .collect(); + keys.sort_by_key(|k| (k.account, k.key_class)); @@ - let keys: Vec<String> = collection - .collection - .platform_payment_accounts - .keys() - .map(|k| format!("({},{})", k.account, k.key_class)) - .collect(); + let mut keys: Vec<(u32, u32)> = collection + .collection + .platform_payment_accounts + .keys() + .map(|k| (k.account, k.key_class)) + .collect(); + keys.sort(); + let keys: Vec<String> = keys + .into_iter() + .map(|(account, key_class)| format!("({},{})", account, key_class)) + .collect(); @@ - let platform_payment_keys: Vec<crate::managed_account::FFIPlatformPaymentAccountKey> = + let mut platform_payment_keys: Vec<crate::managed_account::FFIPlatformPaymentAccountKey> = collection .collection .platform_payment_accounts .keys() .map(crate::managed_account::FFIPlatformPaymentAccountKey::from) .collect(); + platform_payment_keys.sort_by_key(|k| (k.account, k.key_class));Also applies to: 1049-1068, 1160-1177
| Functions: 4 | ||
|
|
||
| | Function | Description | Module | | ||
| |----------|-------------|--------| | ||
| | `account_result_free_error` | Free an account result's error message (if any) Note: This does NOT free the... | account | | ||
| | `error_message_free` | Free an error message # Safety - `message` must be a valid pointer to a C... | error | | ||
| | `managed_account_result_free_error` | Free a managed account result's error message (if any) Note: This does NOT... | managed_account | | ||
| | `managed_platform_account_result_free_error` | Free a managed platform account result's error message (if any) Note: This... | managed_account | | ||
|
|
There was a problem hiding this comment.
Fix table separator spacing to satisfy markdownlint MD060.
The separator row lacks the compact spacing expected by the linter.
✏️ Proposed fix
-|----------|-------------|--------|
+| ---------- | ------------- | -------- |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Functions: 4 | |
| | Function | Description | Module | | |
| |----------|-------------|--------| | |
| | `account_result_free_error` | Free an account result's error message (if any) Note: This does NOT free the... | account | | |
| | `error_message_free` | Free an error message # Safety - `message` must be a valid pointer to a C... | error | | |
| | `managed_account_result_free_error` | Free a managed account result's error message (if any) Note: This does NOT... | managed_account | | |
| | `managed_platform_account_result_free_error` | Free a managed platform account result's error message (if any) Note: This... | managed_account | | |
| | Function | Description | Module | | |
| | ---------- | ------------- | -------- | | |
| | `account_result_free_error` | Free an account result's error message (if any) Note: This does NOT free the... | account | | |
| | `error_message_free` | Free an error message # Safety - `message` must be a valid pointer to a C... | error | | |
| | `managed_account_result_free_error` | Free a managed account result's error message (if any) Note: This does NOT... | managed_account | | |
| | `managed_platform_account_result_free_error` | Free a managed platform account result's error message (if any) Note: This... | managed_account | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 38-38: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 38-38: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 38-38: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 38-38: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 38-38: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 38-38: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@key-wallet-ffi/FFI_API.md` around lines 35 - 43, The table's separator row in
FFI_API.md isn't using the compact separator style required by markdownlint
MD060; update the separator row under the header so each column uses compact
dashes with no extra spaces (e.g. |---|---|---|) matching the three header
columns, ensuring it aligns with the header for the functions like
account_result_free_error, error_message_free, managed_account_result_free_error
and managed_platform_account_result_free_error.
| #### `wallet_add_platform_payment_account` | ||
|
|
||
| ```c | ||
| wallet_add_platform_payment_account(wallet: *mut FFIWallet, account_index: c_uint, key_class: c_uint,) -> crate::types::FFIAccountResult | ||
| ``` | ||
|
|
||
| **Description:** | ||
| Add a Platform Payment account (DIP-17) to the wallet Platform Payment accounts use the derivation path: `m/9'/coin_type'/17'/account'/key_class'/index` # Arguments * `wallet` - Pointer to the wallet * `account_index` - The account index (hardened) in the derivation path * `key_class` - The key class (hardened) - typically 0' for main addresses # Safety This function dereferences a raw pointer to FFIWallet. The caller must ensure that: - The wallet pointer is either null or points to a valid FFIWallet - The FFIWallet remains valid for the duration of this call | ||
|
|
||
| **Safety:** | ||
| This function dereferences a raw pointer to FFIWallet. The caller must ensure that: - The wallet pointer is either null or points to a valid FFIWallet - The FFIWallet remains valid for the duration of this call | ||
|
|
||
| **Module:** `wallet` | ||
|
|
||
| --- |
There was a problem hiding this comment.
Format the arguments list to avoid MD037 emphasis spacing.
Inline * bullets inside the sentence trigger the linter; render them as a proper list.
✏️ Proposed fix
-Add a Platform Payment account (DIP-17) to the wallet Platform Payment accounts use the derivation path: `m/9'/coin_type'/17'/account'/key_class'/index` # Arguments * `wallet` - Pointer to the wallet * `account_index` - The account index (hardened) in the derivation path * `key_class` - The key class (hardened) - typically 0' for main addresses # Safety This function dereferences a raw pointer to FFIWallet. The caller must ensure that: - The wallet pointer is either null or points to a valid FFIWallet - The FFIWallet remains valid for the duration of this call
+Add a Platform Payment account (DIP-17) to the wallet. Platform Payment accounts use the derivation path: `m/9'/coin_type'/17'/account'/key_class'/index`
+
+# Arguments
+- `wallet` - Pointer to the wallet
+- `account_index` - The account index (hardened) in the derivation path
+- `key_class` - The key class (hardened) - typically 0' for main addresses
+
+# Safety
+This function dereferences a raw pointer to FFIWallet. The caller must ensure that: - The wallet pointer is either null or points to a valid FFIWallet - The FFIWallet remains valid for the duration of this call📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### `wallet_add_platform_payment_account` | |
| ```c | |
| wallet_add_platform_payment_account(wallet: *mut FFIWallet, account_index: c_uint, key_class: c_uint,) -> crate::types::FFIAccountResult | |
| ``` | |
| **Description:** | |
| Add a Platform Payment account (DIP-17) to the wallet Platform Payment accounts use the derivation path: `m/9'/coin_type'/17'/account'/key_class'/index` # Arguments * `wallet` - Pointer to the wallet * `account_index` - The account index (hardened) in the derivation path * `key_class` - The key class (hardened) - typically 0' for main addresses # Safety This function dereferences a raw pointer to FFIWallet. The caller must ensure that: - The wallet pointer is either null or points to a valid FFIWallet - The FFIWallet remains valid for the duration of this call | |
| **Safety:** | |
| This function dereferences a raw pointer to FFIWallet. The caller must ensure that: - The wallet pointer is either null or points to a valid FFIWallet - The FFIWallet remains valid for the duration of this call | |
| **Module:** `wallet` | |
| --- | |
| #### `wallet_add_platform_payment_account` | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 1277-1277: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
[warning] 1277-1277: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🤖 Prompt for AI Agents
In `@key-wallet-ffi/FFI_API.md` around lines 1270 - 1284, The args section for
wallet_add_platform_payment_account currently uses inline `*` bullets which
trigger MD037; change that inline sentence into a proper markdown list under
"Arguments" and list each parameter as its own bullet (e.g., `* wallet` -
Pointer to the wallet; `* account_index` - The account index (hardened) in the
derivation path; `* key_class` - The key class (hardened) - typically 0' for
main addresses) and remove the inline asterisks inside sentences; ensure
references to FFIWallet, account_index, and key_class remain unchanged and that
spacing/line breaks conform to the project's markdown linter rules.
There was a problem hiding this comment.
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/managed_account.rs (1)
199-215:⚠️ Potential issue | 🟠 MajorFree
FFIError.messageafter reading at all error paths to prevent leaks.The code at lines 199-215, 311-327, 384-397, 445-458, and 1097-1113 copies
error.messageinto a RustStringbut never deallocates the original FFI-allocated pointer. Useerror.free_message()after extracting the message to prevent repeated memory leaks on error conditions.Suggested fix pattern (apply to all five locations)
- return FFIManagedCoreAccountResult::error( - error.code, - if error.message.is_null() { - "Failed to get managed wallet info".to_string() - } else { - let c_str = std::ffi::CStr::from_ptr(error.message); - c_str.to_string_lossy().to_string() - }, - ); + let message = if error.message.is_null() { + "Failed to get managed wallet info".to_string() + } else { + let msg = std::ffi::CStr::from_ptr(error.message).to_string_lossy().to_string(); + unsafe { error.free_message() }; + msg + }; + return FFIManagedCoreAccountResult::error(error.code, message);
🧹 Nitpick comments (7)
key-wallet/src/wallet/managed_wallet_info/managed_accounts.rs (2)
17-18: Update doc comment to reflect new type name.The doc comment still references
ManagedAccountbut the implementation now usesManagedCoreAccount.📝 Suggested doc update
- /// This creates a ManagedAccount wrapper around an existing Account in the wallet. + /// This creates a ManagedCoreAccount wrapper around an existing Account in the wallet.
44-45: Update inline comments throughout the file.Multiple inline comments still reference
ManagedAccountinstead ofManagedCoreAccount. Consider updating these for consistency:
- Line 44: "Create the ManagedAccount from the Account"
- Line 120: Same
- Line 165: "Create the ManagedAccount from the BLS Account"
- Line 238: Same
- Line 284: "Create the ManagedAccount from the EdDSA Account"
- Line 361: Same
📝 Example fix for line 44
- // Create the ManagedAccount from the Account + // Create the ManagedCoreAccount from the Account let managed_account = ManagedCoreAccount::from_account(account);key-wallet/src/managed_account/managed_account_collection.rs (2)
655-658: Silent fallback to master path may mask configuration errors.When
derivation_path()fails, the code silently falls back to the master derivation path. This could hide misconfigurations. Consider propagating the error or logging a warning.🔧 Option: Propagate error instead of silent fallback
- let base_path = account - .account_type - .derivation_path(account.network) - .unwrap_or_else(|_| crate::bip32::DerivationPath::master()); + let base_path = account + .account_type + .derivation_path(account.network) + .map_err(|e| crate::error::Error::DerivationError( + format!("Failed to get derivation path for platform account: {}", e) + ))?;
865-912: Note:all_accounts()excludes platform payment accounts by design.The method returns only
ManagedCoreAccountreferences and explicitly excludesplatform_payment_accounts. This appears intentional given the separateall_platform_accounts()method at line 1027, but consider adding a doc comment to clarify this behavior for API consumers.📝 Suggested documentation
/// Get all accounts + /// + /// Note: This returns only core accounts. For platform payment accounts, + /// use [`all_platform_accounts()`](Self::all_platform_accounts). pub fn all_accounts(&self) -> Vec<&ManagedCoreAccount> {key-wallet-ffi/src/managed_account.rs (1)
3-6: Update module docs to reference ManagedCoreAccount.
The header comment still mentions ManagedAccount/Arc<ManagedAccount>, which no longer matches the types.📝 Proposed doc update
-//! ManagedAccount instances from the key-wallet crate. FFIManagedCoreAccount is a -//! simple wrapper around `Arc<ManagedAccount>` without additional fields. +//! ManagedCoreAccount instances from the key-wallet crate. FFIManagedCoreAccount is a +//! simple wrapper around `Arc<ManagedCoreAccount>` without additional fields.key-wallet-ffi/include/key_wallet_ffi.h (2)
601-613: Clarify thataccountandkey_classare raw index values.The current documentation stating "(hardened)" is misleading. Based on the Rust implementation which applies hardening internally via
ChildNumber::from_hardened_idx(), callers should pass raw indices (e.g., 0, 1, 2). Consider updating the comments to:typedef struct { /* - Account index (hardened) + Account index (raw value; hardening applied internally) */ unsigned int account; /* - Key class (hardened) + Key class (raw value; hardening applied internally) */ unsigned int key_class; } FFIPlatformPaymentAccountKey;
782-796: Same raw index clarification needed here.The
FFIPlatformPaymentAccountSpecstruct has the same documentation issue regarding hardened indices. The Rust implementation applies hardening internally, so update the comments:typedef struct { /* - Account index (hardened) - the account' level in the derivation path + Account index (pass raw value, e.g., 0, 1, 2; hardening applied internally) */ uint32_t account; /* - Key class (hardened) - defaults to 0', 1' is reserved for change-like segregation + Key class (pass raw value; 0 is default, 1 is reserved for change-like segregation; hardening applied internally) */ uint32_t key_class; } FFIPlatformPaymentAccountSpec;
ManagedAccounttoManagedCoreAccountthroughout the codebase to align with the new account structure.WalletBalancetoWalletCoreBalanceto reflect the updated balance management.This update enhances the overall architecture by standardizing account management and balance tracking across the application.
Summary by CodeRabbit
Refactor
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.