-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add network to FFIWalletManager struct
#325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThe changes introduce network property exposure across the manager and FFI layers. A public accessor method is added to the base Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (4)**/*.{rs,toml}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*-ffi/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/tests/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (17)📓 Common learnings📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-06-15T15:31:44.136ZApplied to files:
📚 Learning: 2025-12-22T17:59:37.849ZApplied to files:
📚 Learning: 2025-12-22T17:59:37.849ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-22T17:59:51.097ZApplied to files:
📚 Learning: 2025-08-21T05:01:58.949ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-22T17:59:37.849ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-01T07:59:58.608ZApplied to files:
🧬 Code graph analysis (4)key-wallet-ffi/src/wallet_manager_tests.rs (6)
dash-spv-ffi/tests/test_wallet_manager.rs (1)
key-wallet-ffi/src/wallet_manager.rs (3)
key-wallet-manager/src/wallet_manager/mod.rs (10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
key-wallet-ffi/src/wallet_manager_tests.rs (1)
237-276: Good coverage ofwallet_manager_networksuccess and error pathsThe test exercises the happy path (Testnet manager, correct network output) and both null‑pointer error paths with appropriate assertions on
FFIErrorCode. Memory handling (manager free) is also correct.If you want slightly stronger coverage, you could add an additional sub‑case using a different
FFINetwork(e.g.,Dash) to verify that all network variants round‑trip correctly, but that’s optional.key-wallet-ffi/include/key_wallet_ffi.h (1)
3987-4001: Consider reordering parameters for consistency.The parameter order
(manager, error, out_network)is inconsistent with the established convention in this codebase where output parameters come before the error parameter. Compare with:
wallet_manager_get_wallet_balance:(..., confirmed_out, unconfirmed_out, error)wallet_manager_get_wallet_ids:(..., wallet_ids_out, count_out, error)Suggested order:
(manager, out_network, error)This would improve consistency and maintainability, though it requires updating both the header and implementation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
key-wallet-ffi/FFI_API.mdkey-wallet-ffi/include/key_wallet_ffi.hkey-wallet-ffi/src/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/src/wallet_manager/mod.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
key-wallet-ffi/src/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/src/wallet_manager/mod.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings
**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with#[cfg(test)]annotation; integration tests use thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor asynchronous operations
Files:
key-wallet-ffi/src/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/src/wallet_manager/mod.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management
Files:
key-wallet-ffi/src/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rs
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
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.
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: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Create integration tests for network operations
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management
📚 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/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-ffi/FFI_API.mdkey-wallet-ffi/include/key_wallet_ffi.h
📚 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/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-ffi/FFI_API.mdkey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/include/key_wallet_ffi.h
📚 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/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Applied to files:
key-wallet-ffi/src/wallet_manager.rskey-wallet-manager/src/wallet_manager/mod.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Create integration tests for network operations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: When adding new features, define traits for abstractions, implement concrete types following existing patterns, add comprehensive unit tests, add integration tests for network interaction, and update error types in error.rs
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management
Applied to files:
key-wallet-manager/src/wallet_manager/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/**/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-manager/src/wallet_manager/mod.rs
🧬 Code graph analysis (4)
key-wallet-ffi/src/wallet_manager.rs (6)
key-wallet-ffi/src/types.rs (1)
error(121-132)key-wallet-ffi/src/error.rs (3)
error(44-49)set_error(53-59)set_success(63-69)key-wallet-manager/src/wallet_manager/mod.rs (1)
network(929-931)dash-spv/src/client/core.rs (1)
network(163-165)key-wallet/src/wallet/managed_wallet_info/mod.rs (1)
network(116-118)key-wallet/src/managed_account/mod.rs (1)
network(835-837)
key-wallet-ffi/src/wallet_manager_tests.rs (2)
key-wallet-ffi/src/error.rs (2)
error(44-49)success(36-41)key-wallet-ffi/src/wallet_manager.rs (3)
wallet_manager_create(105-126)wallet_manager_network(497-517)wallet_manager_free(905-911)
key-wallet-manager/src/wallet_manager/mod.rs (7)
key-wallet/src/wallet/managed_wallet_info/mod.rs (1)
network(116-118)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
network(32-32)network(127-129)key-wallet/src/managed_account/mod.rs (1)
network(835-837)key-wallet/src/account/bls_account.rs (1)
network(201-203)key-wallet/src/account/mod.rs (1)
network(153-155)key-wallet/src/account/eddsa_account.rs (1)
network(202-204)dash/src/address.rs (1)
network(944-946)
key-wallet-ffi/include/key_wallet_ffi.h (4)
key-wallet-ffi/src/wallet_manager.rs (1)
wallet_manager_network(497-517)key-wallet-ffi/src/managed_account.rs (1)
error(64-74)key-wallet-ffi/src/types.rs (1)
error(121-132)key-wallet-ffi/src/error.rs (1)
error(44-49)
⏰ 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: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (4)
key-wallet-manager/src/wallet_manager/mod.rs (1)
928-931:WalletManager::network()accessor is consistent and FFI‑friendlyThis getter cleanly exposes the manager’s
Network, matches the existing pattern onAccount/ManagedWalletInfo, and returningNetworkby value (Copy) is appropriate for use from FFI and avoids lifetime issues. No further changes needed.key-wallet-ffi/FFI_API.md (2)
7-7: Function count update looks consistentThe
Total Functions: 243metadata aligns with the addition of the new wallet‑manager API and keeps the index in sync. Assuming this file is auto‑generated, nothing to change here.
45-69: Wallet‑manager section correctly includeswallet_manager_networkThe Wallet Manager section’s function count and table have been updated to include
wallet_manager_network, which keeps the high‑level index aligned with the new FFI entry point.key-wallet-ffi/src/wallet_manager.rs (1)
488-517: Implementation looks correct.The function properly:
- Validates null pointers before dereferencing
- Uses async/await via
block_onconsistently with other FFI functions- Acquires a read lock (appropriate for read-only access)
- Handles errors correctly
- Returns the appropriate boolean status
The parameter order issue noted in the header review applies here as well, but the implementation itself is sound.
1d3b1b2 to
72379d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
key-wallet-ffi/src/wallet_manager_tests.rs (1)
237-276: Good test coverage; consider testing all network variants.The test correctly validates the FFI boundary for
wallet_manager_networkwith appropriate null-pointer checks. However, it only tests the Testnet network.Consider enhancing the test to verify all
FFINetworkvariants (Dash, Testnet, Regtest, Devnet) to ensure the function works correctly for each network type.Based on learnings, tests should cover both mainnet and testnet configurations.
💡 Optional enhancement to test all network variants
#[test] fn test_wallet_manager_network() { let mut error = FFIError::success(); let error = &mut error as *mut FFIError; - // Test with Testnet - let manager = wallet_manager::wallet_manager_create(FFINetwork::Testnet, error); - assert!(!manager.is_null()); - - let mut out_network = FFINetwork::Dash; - let success = - unsafe { wallet_manager::wallet_manager_network(manager, error, &mut out_network) }; - assert!(success); - assert_eq!(unsafe { (*error).code }, FFIErrorCode::Success); - assert_eq!(out_network, FFINetwork::Testnet); - - unsafe { - wallet_manager::wallet_manager_free(manager); - } + // Test all network variants + for network in [FFINetwork::Dash, FFINetwork::Testnet, FFINetwork::Regtest, FFINetwork::Devnet] { + let manager = wallet_manager::wallet_manager_create(network, error); + assert!(!manager.is_null()); + + let mut out_network = FFINetwork::Dash; + let success = + unsafe { wallet_manager::wallet_manager_network(manager, error, &mut out_network) }; + assert!(success); + assert_eq!(unsafe { (*error).code }, FFIErrorCode::Success); + assert_eq!(out_network, network, "Network mismatch for {:?}", network); + + unsafe { + wallet_manager::wallet_manager_free(manager); + } + } // Test with null manager let mut out_network = FFINetwork::Dash;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
key-wallet-ffi/FFI_API.mdkey-wallet-ffi/include/key_wallet_ffi.hkey-wallet-ffi/src/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/src/wallet_manager/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- key-wallet-ffi/src/wallet_manager.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/wallet_manager_tests.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings
**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with#[cfg(test)]annotation; integration tests use thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor asynchronous operations
Files:
key-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/wallet_manager_tests.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management
Files:
key-wallet-ffi/src/wallet_manager_tests.rs
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.{c,h} : Add corresponding C tests in `tests/c_tests/` for each new FFI function
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Create integration tests for network operations
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
📚 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-manager/src/wallet_manager/mod.rskey-wallet-ffi/include/key_wallet_ffi.hkey-wallet-ffi/FFI_API.mdkey-wallet-ffi/src/wallet_manager_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-manager/src/wallet_manager/mod.rskey-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Applied to files:
key-wallet-manager/src/wallet_manager/mod.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/include/key_wallet_ffi.hkey-wallet-ffi/FFI_API.mdkey-wallet-ffi/src/wallet_manager_tests.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-ffi/FFI_API.md
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.
Applied to files:
key-wallet-ffi/FFI_API.md
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Create integration tests for network operations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Cover critical parsing, networking, SPV, and wallet flows in tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/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/wallet_manager_tests.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-22T17:59:51.097Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T17:59:51.097Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., `test_parse_address_mainnet`)
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
🧬 Code graph analysis (3)
key-wallet-manager/src/wallet_manager/mod.rs (9)
key-wallet/src/wallet/managed_wallet_info/mod.rs (1)
network(116-118)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
network(32-32)network(127-129)key-wallet/src/managed_account/address_pool.rs (1)
network(1069-1072)key-wallet/src/managed_account/mod.rs (1)
network(835-837)key-wallet/src/bip38.rs (1)
network(504-507)key-wallet/src/account/bls_account.rs (1)
network(201-203)key-wallet/src/account/eddsa_account.rs (1)
network(202-204)key-wallet/src/account/mod.rs (1)
network(153-155)dash/src/address.rs (1)
network(944-946)
key-wallet-ffi/include/key_wallet_ffi.h (3)
key-wallet-ffi/src/wallet_manager.rs (1)
wallet_manager_network(497-517)key-wallet-ffi/src/types.rs (1)
error(121-132)key-wallet-ffi/src/error.rs (1)
error(44-49)
key-wallet-ffi/src/wallet_manager_tests.rs (2)
key-wallet-ffi/src/error.rs (2)
error(44-49)success(36-41)key-wallet-ffi/src/wallet_manager.rs (2)
wallet_manager_network(497-517)wallet_manager_free(905-911)
⏰ 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). (18)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: SPV Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (5)
key-wallet-manager/src/wallet_manager/mod.rs (1)
928-931: NewWalletManager::network()accessor is consistent and minimalSimple getter returning the
Networkenum by value matches the pattern used across the key-wallet types and cleanly exposes the manager’s configured network without adding mutability. Looks good.key-wallet-ffi/FFI_API.md (2)
7-7: Function counts and wallet-manager table updated consistentlyThe total function count (243) and the Wallet Manager section’s function count (20) correctly account for the new
wallet_manager_networkentry in the table. No issues from a docs/consistency standpoint.Also applies to: 45-46, 65-65
654-668:wallet_manager_networkdetailed docs now align with the FFI signatureThe documented signature and Safety/Description bullets use the
(manager, error, out_network)ordering, which matches the current FFI and header usage and resolves the earlier mismatch. Pointer contracts and lifetime notes are in line with the rest of the wallet-manager API docs.key-wallet-ffi/include/key_wallet_ffi.h (1)
3987-4001: LGTM!The function declaration is correct and follows FFI conventions. The signature matches the implementation, documentation is clear, and safety requirements are properly specified.
key-wallet-ffi/src/wallet_manager_tests.rs (1)
237-276: Note: AI summary mentions duplicate test insertion.The AI-generated summary states that "The patch inserts this test twice within the file, indicating duplicate test insertion." However, only one occurrence of
test_wallet_manager_networkis present in the provided code (lines 237-276).This discrepancy may indicate the duplicate was present in an earlier version but resolved before the final commit, or it could be an error in the summary. Either way, the final code contains no duplication.
72379d5 to
99b423f
Compare
wallet_manager_network to FFInetwork to FFIWalletManager struct
Adds a new field to the wallet manager FFI struct to get the network the manager is configured for.
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.