Skip to content

Conversation

@ZocoLini
Copy link
Collaborator

@ZocoLini ZocoLini commented Jan 10, 2026

Summary by CodeRabbit

  • Refactor

    • Removed FFI functions for mempool timeout and wallet creation time configuration
    • Removed log_level, mempool_timeout_secs, and wallet_creation_time configuration fields from ClientConfig
    • Simplified mempool validation logic
  • Tests

    • Removed related test cases for removed configuration options

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 10, 2026

📝 Walkthrough

Walkthrough

This pull request removes two deprecated FFI configuration functions (dash_spv_ffi_config_set_mempool_timeout and dash_spv_ffi_config_set_wallet_creation_time) along with their corresponding core implementation and associated configuration fields from the ClientConfig struct. Related test cases and documentation examples are updated accordingly.

Changes

Cohort / File(s) Summary
FFI API Documentation and Declarations
dash-spv-ffi/FFI_API.md, dash-spv-ffi/include/dash_spv_ffi.h
Removed two public FFI function declarations and their documentation: dash_spv_ffi_config_set_mempool_timeout (uint64_t timeout_secs) and dash_spv_ffi_config_set_wallet_creation_time (uint32_t timestamp). Total function counts decreased.
FFI Configuration Implementation
dash-spv-ffi/src/config.rs
Removed implementations of the two deprecated FFI functions that were wrapping underlying configuration setters.
Core Configuration
dash-spv/src/client/config.rs
Removed three public fields (log_level, mempool_timeout_secs, wallet_creation_time), their default initializations, and associated builder methods (with_log_level, with_mempool_timeout). Simplified mempool validation logic.
Configuration Tests
dash-spv/src/client/config_test.rs
Removed test assertions and validations referencing removed configuration fields (log_level, mempool_timeout_secs, wallet_creation_time) and builder methods.
Documentation / Examples
dash-spv/src/lib.rs
Removed .with_log_level("info") call from Quick Start example code.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Deprecated configs hop away,
Mempool timeouts have had their day,
Log levels fade to yesterday,
The codebase hops a cleaner way!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title accurately describes the main change: removal of ClientConfig fields (log_level, mempool_timeout_secs, wallet_creation_time) and related methods across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

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 (1)
dash-spv/src/lib.rs (1)

24-33: Quick Start: replace removed with_log_level with the intended logging initialization path

Since ClientConfig::with_log_level(...) is gone, consider adding a line (or short note) in the Quick Start showing how to configure logging now (e.g., dash_spv::init_console_logging(...), dash_spv::init_logging(...), or RUST_LOG), otherwise readers may look for the old config field. As per coding guidelines, keep configuration via ClientConfig but don’t imply config-driven log level if it no longer exists.

Also applies to: 58-59

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 742715d and d1cf997.

📒 Files selected for processing (6)
  • dash-spv-ffi/FFI_API.md
  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/src/config.rs
  • dash-spv/src/client/config.rs
  • dash-spv/src/client/config_test.rs
  • dash-spv/src/lib.rs
💤 Files with no reviewable changes (3)
  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/src/config.rs
  • dash-spv/src/client/config_test.rs
🧰 Additional context used
📓 Path-based instructions (3)
dash-spv/**/*.rs

📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)

dash-spv/**/*.rs: Use async/await throughout the codebase, built on tokio runtime
Use Arc for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Use Tokio channels for inter-component message passing between async tasks
Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features

Files:

  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.rs
**/*.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:

  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.rs
dash-spv/src/client/**/*.rs

📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)

Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization

Files:

  • dash-spv/src/client/config.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: 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/client/**/*.rs : Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
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
📚 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:

  • dash-spv-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 : All FFI types must have corresponding `_destroy()` functions for explicit memory management

Applied to files:

  • dash-spv-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:

  • dash-spv-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 : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust

Applied to files:

  • dash-spv-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 : Add cbindgen annotations for complex types in FFI functions

Applied to files:

  • dash-spv-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: When debugging FFI issues, check `dash_spv_ffi_get_last_error()` for error details

Applied to files:

  • dash-spv-ffi/FFI_API.md
📚 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/client/**/*.rs : Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization

Applied to files:

  • dash-spv-ffi/FFI_API.md
  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.rs
📚 Learning: 2025-06-26T16:01:37.609Z
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.

Applied to files:

  • dash-spv-ffi/FFI_API.md
  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.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/network/**/*.rs : Implement configurable timeouts with recovery mechanisms for network operations

Applied to files:

  • dash-spv-ffi/FFI_API.md
  • dash-spv/src/client/config.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/**/*.rs : Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features

Applied to files:

  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.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/**/*.rs : Use async/await throughout the codebase, built on tokio runtime

Applied to files:

  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.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/validation/**/*.rs : Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)

Applied to files:

  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.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/network/**/*.rs : Use DNS-first peer discovery with automatic DNS seeds (dnsseed.dash.org, testnet-seed.dashdot.io) when no explicit peers are configured; implement immediate startup with 10-second delay only for subsequent peer searches

Applied to files:

  • dash-spv/src/lib.rs
  • dash-spv/src/client/config.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 : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function

Applied to files:

  • dash-spv/src/lib.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/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory

Applied to files:

  • dash-spv/src/lib.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/**/*.rs : Use Tokio channels for inter-component message passing between async tasks

Applied to files:

  • dash-spv/src/lib.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/error.rs : Use domain-specific error types (NetworkError, StorageError, SyncError, ValidationError, SpvError) rather than generic error handling

Applied to files:

  • dash-spv/src/lib.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 **/*test*.rs : Test both mainnet and testnet configurations

Applied to files:

  • dash-spv/src/lib.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: Ubuntu / ffi
  • GitHub Check: Windows / rpc
  • GitHub Check: macOS / tools
  • GitHub Check: macOS / spv
  • GitHub Check: macOS / core
  • GitHub Check: Pre-commit (windows-latest)
  • GitHub Check: Pre-commit (macos-latest)
  • GitHub Check: Pre-commit (ubuntu-latest)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: Thread Sanitizer
  • GitHub Check: Address Sanitizer
🔇 Additional comments (2)
dash-spv-ffi/FFI_API.md (1)

5-8: Auto-generated doc counts: ensure they’re derived/validated, not hand-maintained

Given this file claims “Auto-generated”, it’d be good to ensure the “Total Functions: 68” and “Configuration Functions: 25” numbers come directly from the generator (or that CI checks regeneration), to avoid future drift as the FFI surface evolves.

Also applies to: 35-38

dash-spv/src/client/config.rs (1)

194-210: Mempool validation simplification: double-check downstream assumptions beyond max_mempool_transactions > 0

The new check is good as far as it goes, but please verify that mempool initialization/processing doesn’t rely on other invariants that used to be validated (now that the config surface was trimmed). In particular, ensure “mempool disabled” configurations (or edge values like max_mempool_transactions = 0 when disabled) can’t cause later panics or unintended behavior.

@xdustinface xdustinface changed the title chor: removed more ClientConfig fields chore: removed more ClientConfig fields Jan 11, 2026
@xdustinface xdustinface merged commit a01febe into v0.42-dev Jan 11, 2026
53 checks passed
@xdustinface xdustinface deleted the chore/config-cleanup branch January 11, 2026 01:58
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.

3 participants