Skip to content

Conversation

@xdustinface
Copy link
Collaborator

@xdustinface xdustinface commented Dec 24, 2025

This adds synced_height to the metadata of the ManagedWalletInfo which allows to keep track of the synced height of each individual wallet and also allows to leverage the synced height within wallet info for things like confirmation calculations.

Few kind of related things included in this PR:

  • Drop wallet_manager_update_height from the wallet FFI since it's internal and i think we shouldn't be able to modify it from the FFI.
  • Drop a duplicated test in regard to the wallet height stuff

Summary by CodeRabbit

  • New Features

    • Added API to retrieve the current synced block height for individual managed wallets.
  • Improvements

    • Wallet metadata now tracks per-wallet synced heights.
    • Global height updates propagate to all managed wallets; wallets can also maintain individual synced heights.
  • Breaking Changes

    • Removed the direct external API to set wallet/network height; height updates are handled internally.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 24, 2025

📝 Walkthrough

Walkthrough

Removed the FFI height-update API, added per-wallet synced-height tracking and a new FFI getter. WalletManager.update_height now propagates heights to each ManagedWalletInfo; WalletMetadata gained a synced_height field and WalletInfoInterface exposes synced_height.

Changes

Cohort / File(s) Summary
FFI docs & header
key-wallet-ffi/FFI_API.md, key-wallet-ffi/include/key_wallet_ffi.h
Deleted wallet_manager_update_height declaration and docs; added managed_wallet_synced_height declaration and updated function counts.
FFI: managed wallet implementation
key-wallet-ffi/src/managed_wallet.rs
Added C-export managed_wallet_synced_height(...) -> c_uint, pointer validation, error handling; imported c_uint and WalletInfoInterface.
FFI: wallet manager implementation & tests
key-wallet-ffi/src/wallet_manager.rs, key-wallet-ffi/src/wallet_manager_tests.rs
Removed wallet_manager_update_height FFI function; test replaced to use internal runtime to update manager height and assert via current-height query.
Core wallet manager logic
key-wallet-manager/src/wallet_manager/mod.rs
update_height now updates manager current_height and calls per-wallet update_synced_height(height) for every wallet info entry.
Wallet metadata & interface
key-wallet/src/wallet/metadata.rs, key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
Added synced_height: CoreBlockHeight to WalletMetadata; added synced_height() to WalletInfoInterface; renamed/implemented update_synced_height(...) (was update_chain_height).
Integration & unit tests
key-wallet-manager/tests/integration_test.rs, key-wallet/src/transaction_checking/wallet_checker.rs
Expanded integration tests to verify propagation and per-wallet updates; updated tests to call update_synced_height where applicable.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant WalletManager
    participant WalletInfo
    participant Metadata

    rect rgb(245,245,255)
    Note over Client,WalletManager: Old public flow (removed FFI)
    Client->>WalletManager: wallet_manager_update_height(height)
    WalletManager->>WalletManager: set current_height
    Note over WalletManager: No per-wallet propagation
    end

    rect rgb(245,255,245)
    Note over Client,Metadata: New flow (propagated)
    Client->>WalletManager: update_height(height)
    WalletManager->>WalletManager: set current_height
    loop For each wallet
        WalletManager->>WalletInfo: update_synced_height(height)
        WalletInfo->>Metadata: metadata.synced_height = height
    end
    Note over Client,WalletInfo: Query per-wallet height via FFI
    Client->>WalletInfo: managed_wallet_synced_height()
    WalletInfo->>Metadata: read synced_height
    Metadata-->>WalletInfo: synced_height
    WalletInfo-->>Client: synced_height value
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I'm a rabbit in code, with a hop and a grin,
Per-wallet heights now live under the skin,
Manager tells all, each wallet keeps pace,
Queries return truth — no more global race! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the primary change: adding per-wallet synced_height tracking to ManagedWalletInfo, which is the core feature addition across all modified files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/wallet-sync-height

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7316f44 and 0505ed2.

📒 Files selected for processing (10)
  • key-wallet-ffi/FFI_API.md
  • key-wallet-ffi/include/key_wallet_ffi.h
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-ffi/src/wallet_manager.rs
  • key-wallet-ffi/src/wallet_manager_tests.rs
  • key-wallet-manager/src/wallet_manager/mod.rs
  • key-wallet-manager/tests/integration_test.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/wallet/metadata.rs
💤 Files with no reviewable changes (1)
  • key-wallet-ffi/src/wallet_manager.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • key-wallet/src/wallet/metadata.rs
  • key-wallet-ffi/src/wallet_manager_tests.rs
  • key-wallet-ffi/FFI_API.md
  • key-wallet-manager/src/wallet_manager/mod.rs
  • key-wallet-ffi/src/managed_wallet.rs
🧰 Additional context used
📓 Path-based instructions (6)
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
Use BTreeMap for ordered data (accounts, transactions) and HashMap for 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 return Result<T> for all fallible operations

Files:

  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (CLAUDE.md)

Never hardcode network parameters, addresses, or keys

Files:

  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
  • key-wallet-manager/tests/integration_test.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

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

**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with #[cfg(test)] annotation; integration tests use the tests/ directory
Use snake_case for function and variable names
Use UpperCamelCase for types and traits
Use SCREAMING_SNAKE_CASE for constants
Format code with rustfmt before commits; ensure cargo fmt --all is run
Run cargo clippy --workspace --all-targets -- -D warnings for linting; avoid warnings in CI
Prefer async/await via tokio for asynchronous operations

Files:

  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
  • key-wallet-manager/tests/integration_test.rs
**/{dash-network,dash-spv,key-wallet}/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use async/await for async operations in network and wallet modules

Files:

  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
key-wallet/**/transaction_checking/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Implement transaction classification and routing through TransactionRouter to avoid checking all accounts for every transaction

Files:

  • key-wallet/src/transaction_checking/wallet_checker.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/tests/**/*.rs: Create integration tests for network operations
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate

**/tests/**/*.rs: Use descriptive test names (e.g., test_parse_address_mainnet)
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored

Files:

  • key-wallet-manager/tests/integration_test.rs
🧠 Learnings (14)
📓 Common learnings
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
📚 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.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/**/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/include/key_wallet_ffi.h
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
  • key-wallet-manager/tests/integration_test.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/include/key_wallet_ffi.h
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
  • key-wallet-manager/tests/integration_test.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/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks

Applied to files:

  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations

Applied to files:

  • key-wallet/src/transaction_checking/wallet_checker.rs
  • key-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data

Applied to files:

  • key-wallet/src/transaction_checking/wallet_checker.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-manager/tests/integration_test.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-manager/tests/integration_test.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-manager/tests/integration_test.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-manager/tests/integration_test.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-manager/tests/integration_test.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/tests/integration_test.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-manager/tests/integration_test.rs
🧬 Code graph analysis (3)
key-wallet-ffi/include/key_wallet_ffi.h (3)
key-wallet-ffi/src/managed_wallet.rs (1)
  • managed_wallet_synced_height (597-612)
key-wallet-ffi/src/types.rs (1)
  • error (121-132)
key-wallet-ffi/src/error.rs (1)
  • error (44-49)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (1)
  • current_height (929-931)
key-wallet-manager/tests/integration_test.rs (4)
dash-spv/src/client/core.rs (1)
  • wallet (158-160)
key-wallet-manager/src/wallet_manager/mod.rs (2)
  • current_height (929-931)
  • wallet_count (318-320)
key-wallet/src/mnemonic.rs (2)
  • generate (106-129)
  • generate (133-144)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
  • synced_height (65-65)
  • synced_height (162-164)
⏰ 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: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Core Components Tests
  • GitHub Check: Key Wallet Components Tests
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: SPV Components Tests
  • GitHub Check: Pre-commit (ubuntu-latest)
  • GitHub Check: Pre-commit (macos-latest)
  • GitHub Check: conflict-check
🔇 Additional comments (7)
key-wallet/src/transaction_checking/wallet_checker.rs (1)

830-830: LGTM: Method rename aligns with new API.

The update from update_chain_height to update_synced_height correctly reflects the API change in WalletInfoInterface. The test logic remains sound—advancing the synced height to trigger coinbase maturation processing.

key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (3)

64-65: LGTM: Clean getter API addition.

The new synced_height() accessor properly exposes the per-wallet synchronization state, enabling wallet-specific logic such as confirmation calculations.


117-117: Good API rename for clarity.

Renaming update_chain_height to update_synced_height better reflects that this updates the wallet's synchronization progress rather than the global chain state. This aligns with the per-wallet tracking introduced in this PR.


332-334: Correct ordering: metadata update before maturation processing.

Setting metadata.synced_height before calling process_matured_transactions ensures that the wallet's synced state is current when evaluating which immature transactions have reached maturity. This maintains consistency between the recorded height and the maturation logic.

key-wallet-manager/tests/integration_test.rs (2)

8-8: Good import for new test requirements.

Adding WalletInfoInterface provides access to the synced_height() and update_synced_height() methods needed for the expanded test scenarios.


159-230: Excellent test coverage for per-wallet synced height tracking.

This test thoroughly validates the new per-wallet height tracking:

  • Verifies wallets initialize with synced_height == 0
  • Confirms global update_height() propagates to all wallets (lines 194-201, 204-209)
  • Tests individual wallet updates via update_synced_height() (lines 212-222)
  • Validates that subsequent global updates override individual settings (lines 225-229)

Creating two distinct wallets ensures proper isolation of synced_height state. The test structure is clear and provides comprehensive coverage of the feature's intended behavior.

key-wallet-ffi/include/key_wallet_ffi.h (1)

3092-3104: LGTM: Clean FFI exposure of synced_height.

The new managed_wallet_synced_height function properly exposes the per-wallet synced height to FFI consumers. The function signature is appropriate:

  • const pointer enforces read-only access
  • Return type unsigned int correctly maps to u32/CoreBlockHeight
  • Error parameter follows FFI conventions
  • Safety documentation clearly specifies pointer validity requirements

Based on the related code snippet from managed_wallet.rs:596-611, the implementation correctly validates the pointer before dereferencing and properly handles the error case.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xdustinface xdustinface marked this pull request as ready for review December 24, 2025 09:18
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
key-wallet/src/wallet/metadata.rs (1)

19-20: Consider serde default for new synced_height to keep old data readable

If WalletMetadata is ever deserialized from older serialized payloads (missing this new field), serde will fail unless you either bump the format or provide a default. Adding a field-level serde(default) keeps deserialization backward compatible while still exposing the new field.

Proposed serde-default tweak
-    /// Synced to block height
-    pub synced_height: CoreBlockHeight,
+    /// Synced to block height
+    #[cfg_attr(feature = "serde", serde(default))]
+    pub synced_height: CoreBlockHeight,
key-wallet-ffi/src/wallet_manager_tests.rs (1)

441-471: Test correctly adapted to API changes.

The test properly verifies height retrieval via wallet_manager_current_height. The direct access to internal FFIWalletManager fields (runtime, manager) is acceptable here since the public wallet_manager_update_height FFI was intentionally removed—this simulates how height updates propagate internally.

Consider adding a complementary test for managed_wallet_synced_height to verify per-wallet synced height propagation (the new API exposed at FFI boundary). The integration tests in key-wallet-manager/tests/integration_test.rs cover this, but FFI-level coverage would strengthen confidence.

key-wallet-ffi/src/managed_wallet.rs (1)

589-612: Implementation follows established FFI patterns.

The function correctly:

  • Validates the input pointer for null and returns 0 with an error
  • Uses the WalletInfoInterface trait to access synced_height()
  • Sets success before returning the value

Minor observation: Line 609 uses an unsafe block inside an unsafe fn, which is explicit but redundant. This is stylistically acceptable and matches some other functions in the codebase.

Missing unit test coverage: The tests module (lines 647-1146) doesn't include a test for managed_wallet_synced_height. Consider adding a test similar to test_managed_wallet_get_balance that validates null pointer handling and successful height retrieval.

🔎 Suggested test skeleton
#[test]
fn test_managed_wallet_synced_height() {
    let mut error = FFIError::success();

    // Test with null managed wallet
    let height = unsafe {
        managed_wallet_synced_height(ptr::null(), &mut error)
    };
    assert_eq!(height, 0);
    assert_eq!(error.code, FFIErrorCode::InvalidInput);

    // Create a valid managed wallet and test synced_height retrieval
    // ... similar setup to test_managed_wallet_get_balance ...
}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3884314 and 7316f44.

📒 Files selected for processing (9)
  • key-wallet-ffi/FFI_API.md
  • key-wallet-ffi/include/key_wallet_ffi.h
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-ffi/src/wallet_manager.rs
  • key-wallet-ffi/src/wallet_manager_tests.rs
  • key-wallet-manager/src/wallet_manager/mod.rs
  • key-wallet-manager/tests/integration_test.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet/src/wallet/metadata.rs
💤 Files with no reviewable changes (1)
  • key-wallet-ffi/src/wallet_manager.rs
🧰 Additional context used
📓 Path-based instructions (6)
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
Use BTreeMap for ordered data (accounts, transactions) and HashMap for 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 return Result<T> for all fallible operations

Files:

  • key-wallet/src/wallet/metadata.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (CLAUDE.md)

Never hardcode network parameters, addresses, or keys

Files:

  • key-wallet/src/wallet/metadata.rs
  • key-wallet-ffi/src/wallet_manager_tests.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet-manager/tests/integration_test.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • key-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 the tests/ directory
Use snake_case for function and variable names
Use UpperCamelCase for types and traits
Use SCREAMING_SNAKE_CASE for constants
Format code with rustfmt before commits; ensure cargo fmt --all is run
Run cargo clippy --workspace --all-targets -- -D warnings for linting; avoid warnings in CI
Prefer async/await via tokio for asynchronous operations

Files:

  • key-wallet/src/wallet/metadata.rs
  • key-wallet-ffi/src/wallet_manager_tests.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet-manager/tests/integration_test.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-manager/src/wallet_manager/mod.rs
**/{dash-network,dash-spv,key-wallet}/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use async/await for async operations in network and wallet modules

Files:

  • key-wallet/src/wallet/metadata.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.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
  • key-wallet-ffi/src/managed_wallet.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/tests/**/*.rs: Create integration tests for network operations
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate

**/tests/**/*.rs: Use descriptive test names (e.g., test_parse_address_mainnet)
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored

Files:

  • key-wallet-manager/tests/integration_test.rs
🧠 Learnings (19)
📓 Common learnings
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
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/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together
📚 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
  • key-wallet-manager/tests/integration_test.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
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet-manager/tests/integration_test.rs
  • key-wallet-manager/src/wallet_manager/mod.rs
  • key-wallet-ffi/FFI_API.md
📚 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/wallet_manager_tests.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet-manager/tests/integration_test.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-ffi/FFI_API.md
📚 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 : 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/wallet_manager_tests.rs
  • key-wallet-manager/tests/integration_test.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/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks

Applied to files:

  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
  • key-wallet-manager/tests/integration_test.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-manager/tests/integration_test.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-manager/tests/integration_test.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-manager/tests/integration_test.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • 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 : Test both mainnet and testnet configurations

Applied to files:

  • key-wallet-manager/tests/integration_test.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/tests/integration_test.rs
  • key-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/**/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-manager/tests/integration_test.rs
  • key-wallet-ffi/src/managed_wallet.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.h
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)

Applied to files:

  • key-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 : Use the `?` operator for error propagation, provide context in error messages, never panic in library code, and return `Result<T>` for all fallible operations

Applied to files:

  • key-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2025-06-26T15:48:36.342Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:48:36.342Z
Learning: In Rust FFI code, unwrap() must not be used on CString::new, especially for error messages or untrusted input, as it can panic if the string contains null bytes. Instead, use unwrap_or_else with a fallback to a known-safe, hardcoded string to prevent panics across FFI boundaries.

Applied to files:

  • key-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 : 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/managed_wallet.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/*-ffi/**/*.rs : Be careful with FFI memory management

Applied to files:

  • key-wallet-ffi/src/managed_wallet.rs
🧬 Code graph analysis (5)
key-wallet-ffi/src/wallet_manager_tests.rs (1)
key-wallet-ffi/src/wallet_manager.rs (3)
  • wallet_manager_create (105-126)
  • wallet_manager_current_height (786-805)
  • manager_guard (514-514)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (1)
  • current_height (929-931)
key-wallet-manager/tests/integration_test.rs (4)
dash-spv/src/client/core.rs (1)
  • wallet (158-160)
key-wallet-manager/src/wallet_manager/mod.rs (2)
  • current_height (929-931)
  • wallet_count (318-320)
key-wallet/src/mnemonic.rs (2)
  • generate (106-129)
  • generate (133-144)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
  • synced_height (65-65)
  • synced_height (162-164)
key-wallet-ffi/include/key_wallet_ffi.h (4)
key-wallet-ffi/src/managed_wallet.rs (1)
  • managed_wallet_synced_height (597-612)
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)
key-wallet-ffi/src/managed_wallet.rs (2)
key-wallet-ffi/src/types.rs (1)
  • error (121-132)
key-wallet-ffi/src/error.rs (2)
  • error (44-49)
  • set_error (53-59)
🔇 Additional comments (8)
key-wallet-manager/src/wallet_manager/mod.rs (1)

933-938: Height propagation through update_height looks correct

Updating current_height and then calling update_chain_height on each wallet info keeps manager and per-wallet state consistent with the new synced_height semantics.

key-wallet-ffi/FFI_API.md (1)

45-46: FFI docs for synced height are consistent with the new API shape

The updated function counts and the new managed_wallet_synced_height entry/section look consistent with the removal of wallet_manager_update_height and the move to per‑wallet synced height queries. Safety text and signature formatting match the existing conventions.

Please double‑check that:

  • managed_wallet_synced_height is declared identically in key-wallet-ffi/include/key_wallet_ffi.h, and
  • Its Rust implementation in key-wallet-ffi/src/managed_wallet.rs uses the same signature and safety contract as documented here.

Also applies to: 70-71, 101-101, 1107-1121

key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)

64-66: Trait and implementation of synced_height are wired correctly

Exposing synced_height() on WalletInfoInterface and backing it with metadata.synced_height in ManagedWalletInfo cleanly surfaces the per‑wallet synced height needed by the manager, tests, and FFI.

Also applies to: 162-164


332-334: Updating metadata.synced_height inside update_chain_height is appropriate

Tying metadata.synced_height directly to update_chain_height ensures all callers (manager, tests, FFI) see a coherent synced height whenever chain height advances.

key-wallet-manager/tests/integration_test.rs (2)

8-8: Importing WalletInfoInterface is necessary and minimal

The new import is exactly what’s needed to call synced_height() on wallet infos in the test, with no extra surface added.


159-229: Expanded test_block_height_tracking gives good coverage of the new semantics

This test now validates:

  • Manager height changes without wallets.
  • Initial per‑wallet synced_height state for newly created wallets.
  • Propagation of update_height to all wallets.
  • Independent per‑wallet height overrides via update_chain_height.
  • That a later manager update_height call re‑synchronizes all wallets.

The flow accurately captures the intended behavior of the new synced_height field and update logic.

key-wallet-ffi/include/key_wallet_ffi.h (1)

3092-3104: LGTM!

The new managed_wallet_synced_height function declaration is correctly structured with proper safety documentation. The signature correctly uses const for the managed wallet pointer, aligning with the FFI design philosophy of returning const pointers for read-only access. Based on learnings, this matches the pattern where wallet manager controls lifecycle and mutations through specific APIs.

key-wallet-ffi/src/managed_wallet.rs (1)

7-8: LGTM on import changes.

The new imports are correctly organized—c_uint is grouped with c_char in the raw types import, and WalletInfoInterface is appropriately imported to enable the synced_height() method call.

Also applies to: 14-14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants