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 records per-wallet synced heights.
    • Global height updates now propagate to all managed wallets; wallets can still maintain individual synced heights.
  • Breaking Changes

    • Removed the external API to directly set the wallet/network height; height updates are now managed 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 and added a per-wallet synced-height field, accessor, and FFI getter. WalletManager.update_height now propagates heights to each ManagedWalletInfo; WalletInfoInterface exposes synced_height and update_synced_height; tests updated to reflect propagation and per-wallet updates.

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/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 with pointer validation and error handling; imported c_uint and WalletInfoInterface.
FFI: wallet manager impl & tests
key-wallet-ffi/src/wallet_manager.rs, key-wallet-ffi/src/wallet_manager_tests.rs
Removed FFI wallet_manager_update_height; tests updated to use internal runtime to call manager.update_height and assert via current-height query.
Core wallet manager logic
key-wallet-manager/src/wallet_manager/mod.rs
update_height now sets manager current_height and iterates all wallet infos to call update_synced_height(height) for each.
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 update_chain_heightupdate_synced_height and implemented synced-height assignment.
Integration & unit tests
key-wallet-manager/tests/integration_test.rs, key-wallet/src/transaction_checking/wallet_checker.rs
Expanded integration tests to verify global propagation and per-wallet overrides; updated unit tests to call update_synced_height where applicable.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant WalletManager
    participant WalletInfo
    participant Metadata

    rect rgba(220,235,255,0.6)
    Note over Client,WalletManager: Previous (removed) FFI flow
    Client->>WalletManager: wallet_manager_update_height(height)
    WalletManager->>WalletManager: set current_height
    Note over WalletManager: No per-wallet propagation
    end

    rect rgba(220,255,230,0.6)
    Note over Client,WalletManager: New propagated flow + FFI getter
    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
    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, I hop and I write,
Heights now per-wallet, each one in sight,
Manager whispers heights and wallets comply,
Queries return truth as blocks tick by — 🐇✨

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 title accurately and concisely describes the main change: tracking synced_height per ManagedWalletInfo, which is the core feature addition across the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0505ed2 and 5c38c9d.

📒 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-manager/src/wallet_manager/mod.rs
  • key-wallet-manager/tests/integration_test.rs
  • key-wallet-ffi/FFI_API.md
  • key-wallet-ffi/src/wallet_manager_tests.rs
  • key-wallet/src/wallet/metadata.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

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

**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types (thiserror) and propagate errors appropriately in Rust
Use tokio runtime for async operations in Rust
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format code using cargo fmt
Run clippy with all features and all targets, treating warnings as errors
Never log or expose private keys in any code
Always validate inputs from untrusted sources in Rust
Use secure random number generation for keys

Files:

  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet/src/transaction_checking/wallet_checker.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.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
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/transaction_checking/wallet_checker.rs
  • key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.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
🧠 Learnings (16)
📓 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
📚 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
📚 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/transaction_checking/wallet_checker.rs
  • 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: 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/managed_wallet.rs
  • 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: 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/managed_wallet.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/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-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*ffi/*.rs : Be careful with FFI memory management in C and Swift bindings

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-30T22:00:57.000Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T22:00:57.000Z
Learning: Applies to **/*ffi/*.rs : Careful handling of memory safety at 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/**/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/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-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/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: 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/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/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations

Applied to files:

  • key-wallet/src/transaction_checking/wallet_checker.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
🧬 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/error.rs (1)
  • error (44-49)
key-wallet-ffi/src/managed_account.rs (1)
  • error (64-74)
key-wallet-ffi/src/managed_wallet.rs (2)
key-wallet-ffi/src/error.rs (3)
  • error (44-49)
  • set_error (54-65)
  • set_success (70-81)
key-wallet-ffi/src/types.rs (1)
  • error (121-132)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (1)
  • current_height (934-936)
⏰ 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_sha512)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Core 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 (8)
key-wallet/src/transaction_checking/wallet_checker.rs (1)

830-830: LGTM - Test updated to use new API.

The test correctly uses the renamed update_synced_height method, which aligns with the trait changes in WalletInfoInterface.

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

3092-3104: LGTM - New FFI function properly exposes synced height.

The new managed_wallet_synced_height function follows the established FFI patterns with proper safety documentation and const pointer usage for read-only access.

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

64-65: LGTM - Clear API for accessing synced height.

The new synced_height() method provides a clean way to query the current synced block height from wallet info.


117-117: LGTM - Method rename improves clarity.

Renaming update_chain_height to update_synced_height better reflects that this updates the per-wallet synced height rather than a global chain height.


332-345: LGTM - Correct ordering of state updates.

The implementation properly updates metadata.synced_height before processing matured transactions, ensuring the synced height is current when maturation logic runs.

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

8-8: LGTM - Import added for new FFI function.

The c_uint import is needed for the return type of managed_wallet_synced_height.


14-14: LGTM - Import added for trait method access.

The WalletInfoInterface import enables calling synced_height() on the inner ManagedWalletInfo.


589-612: LGTM - FFI function follows established patterns.

The implementation correctly:

  • Validates the input pointer with null check
  • Sets appropriate error codes
  • Calls the trait method to retrieve synced height
  • Returns c_uint matching the C header

Note: The function returns 0 on error, which could theoretically be a valid synced height. However, callers should check the error parameter to distinguish errors from valid values, and this pattern is consistent with other height-related FFI functions in the codebase.


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

@github-actions
Copy link

github-actions bot commented Jan 1, 2026

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

1 similar comment
@github-actions
Copy link

github-actions bot commented Jan 1, 2026

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

@github-actions github-actions bot added the merge-conflict The PR conflicts with the target branch. label Jan 1, 2026
@xdustinface xdustinface force-pushed the feat/wallet-sync-height branch from 0505ed2 to 5c38c9d Compare January 1, 2026 20:20
@github-actions github-actions bot removed the merge-conflict The PR conflicts with the target branch. label Jan 1, 2026
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