feat: add functions to update client configuration and enable masternodes #130
Conversation
WalkthroughAdds new FFI functions for runtime client config updates and masternode-sync toggling, extends checkpoint retrieval APIs and an FFIArray type, exposes a DashSpvClient::update_config async method, and reorders module exports and imports. Documentation and C header updated to reflect new APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as C caller
participant FFI as FFI layer
participant RT as Tokio runtime
participant S as DashSpvClient
participant V as ValidationManager
participant M as Mempool Filter
C->>FFI: dash_spv_ffi_client_update_config(client*, config*)
FFI-->>FFI: validate pointers, clone config
FFI->>RT: spawn/block_on(async update_config)
RT->>S: update_config(new_config)
S-->>V: rebuild ValidationManager (validation_mode)
alt mempool-related fields changed
S->>M: update_mempool_filter()
end
S-->>RT: Result
RT-->>FFI: status
FFI-->>C: int32_t error code
sequenceDiagram
autonumber
participant C as C caller
participant H as FFI header
participant CP as checkpoints module
C->>H: checkpoint_latest / before_height / before_timestamp
H->>CP: lookup by network/height/timestamp
CP-->>H: out_height + out_hash
H-->>C: status + outputs
C->>H: checkpoints_between_heights(start,end)
H->>CP: collect checkpoints in range
CP-->>H: return FFIArray<Checkpoint>
H-->>C: FFIArray handle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dash-spv-ffi/include/dash_spv_ffi.h (1)
565-607: Annotate all exposed FFI functions with #[no_mangle] extern "C” and sync Swift SDK header
- dash_spv_ffi_config_set_masternode_sync_enabled and dash_spv_ffi_client_update_config are declared in dash-spv-ffi/include/dash_spv_ffi.h but lack
#[no_mangle] extern "C"in Rust—add the attribute to theirpub extern "C" fndefinitions (or remove their header declarations if they’ve been deprecated).- (Optional) For
dash_spv_ffi_checkpoint_*APIs, consider adding anout_hash_lenparameter to validate the buffer length and prevent caller-side OOB.- After regenerating the C header, run
./sync-headers.shand commit the updatedswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hso it matchesdash-spv-ffi/include/dash_spv_ffi.h.
🧹 Nitpick comments (7)
dash-spv/src/client/mod.rs (1)
618-631: Runtime updates: verify propagation beyond validation/mempool
- If max_peers/peer list changed, confirm NetworkManager reflects it at runtime (e.g., set_max_peers or equivalent), or document as “apply on next restart.”
- If enable_masternodes toggled true at runtime, ensure SequentialSyncManager begins MN sync (or explicitly reject runtime toggle). Consider invoking/update wiring to kick off MN sync and then calling update_chainlock_validation() once an engine exists.
dash-spv-ffi/src/config.rs (1)
211-227: Masternode toggle FFI looks good; add getter + testSetter is correct (null-checked, returns FFIErrorCode). Add a matching getter and a unit test under tests/unit to keep FFI symmetric and verified.
Example getter:
+#[no_mangle] +pub unsafe extern "C" fn dash_spv_ffi_config_get_masternode_sync_enabled( + config: *const FFIClientConfig, +) -> bool { + if config.is_null() { + return false; + } + (*config).inner.enable_masternodes +}dash-spv-ffi/FFI_API.md (1)
203-218: Update-config docs — good; keep safety note conciseContent is correct. If the generator allows, avoid duplicating “Description” vs “Safety” bullet text to reduce noise.
dash-spv-ffi/src/client.rs (3)
359-366: Avoid awaiting while holding a std::sync::Mutex guard (minor risk).You await update_config while holding the inner lock. If update_config ever tries to re-enter code that needs this lock, it could deadlock. Consider a follow-up that avoids holding the guard across .await, or refactor the client holder to a tokio::sync::Mutex to await the lock instead.
362-365: Unify “client not initialized” error variant with other APIs.start/stop return Storage::NotFound; here it’s Config. Align for consistent error codes seen by FFI callers.
Apply this diff:
- } else { - Err(dash_spv::SpvError::Config("Client not initialized".to_string())) - } + } else { + Err(dash_spv::SpvError::Storage( + dash_spv::StorageError::NotFound("Client not initialized".to_string()), + )) + }
359-367: Use the run_async helper for consistency.Purely stylistic; keeps this in line with other helpers.
- let result = client.runtime.block_on(async { + let result = client.run_async(|| async { let mut guard = client.inner.lock().unwrap(); if let Some(ref mut spv_client) = *guard { spv_client.update_config(new_config).await.map_err(|e| e) } else { Err(dash_spv::SpvError::Config("Client not initialized".to_string())) } - }); + });dash-spv-ffi/include/dash_spv_ffi.h (1)
428-437: Consider adding a getter for the masternode sync flag.A dash_spv_ffi_config_get_masternode_sync_enabled accessor would make the setter round-tripable from C/Swift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
dash-spv-ffi/FFI_API.md(5 hunks)dash-spv-ffi/include/dash_spv_ffi.h(3 hunks)dash-spv-ffi/src/checkpoints.rs(1 hunks)dash-spv-ffi/src/client.rs(1 hunks)dash-spv-ffi/src/config.rs(1 hunks)dash-spv-ffi/src/lib.rs(1 hunks)dash-spv/src/client/mod.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
dash-spv-ffi/src/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
dash-spv-ffi/src/**/*.rs: FFI functions must be declared with #[no_mangle] extern "C"
Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Accept input strings from C as *const c_char (borrowed; do not free inputs)
Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Add cbindgen annotations for complex types so headers are generated correctly
Files:
dash-spv-ffi/src/checkpoints.rsdash-spv-ffi/src/client.rsdash-spv-ffi/src/config.rsdash-spv-ffi/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv-ffi/src/checkpoints.rsdash-spv/src/client/mod.rsdash-spv-ffi/src/client.rsdash-spv-ffi/src/config.rsdash-spv-ffi/src/lib.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/src/checkpoints.rsdash-spv-ffi/src/client.rsdash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/config.rsdash-spv-ffi/src/lib.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv-ffi/src/checkpoints.rsdash-spv/src/client/mod.rsdash-spv-ffi/src/client.rsdash-spv-ffi/src/config.rsdash-spv-ffi/src/lib.rs
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/src/checkpoints.rsdash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/config.rsdash-spv-ffi/src/lib.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/client/mod.rs
dash-spv-ffi/include/dash_spv_ffi.h
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
The public C header is generated; ensure it reflects FFI changes after builds
Files:
dash-spv-ffi/include/dash_spv_ffi.h
🧠 Learnings (10)
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
dash-spv-ffi/src/checkpoints.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#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/src/client/mod.rsdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/include/dash_spv_ffi.hdash-spv-ffi/src/lib.rs
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : FFI functions must be declared with #[no_mangle] extern "C"
Applied to files:
dash-spv-ffi/FFI_API.mddash-spv-ffi/src/client.rsdash-spv-ffi/src/config.rsdash-spv-ffi/src/lib.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/src/lib.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/src/lib.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly
Applied to files:
dash-spv-ffi/src/client.rsdash-spv-ffi/src/lib.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Applied to files:
dash-spv-ffi/src/client.rs
🧬 Code graph analysis (3)
dash-spv/src/client/mod.rs (4)
dash-spv/src/client/config.rs (1)
new(237-243)dash-spv/src/mempool_filter.rs (2)
new(31-47)new(238-244)dash-spv/src/client/message_handler.rs (1)
new(35-59)dash-spv/src/sync/headers.rs (1)
new(32-42)
dash-spv-ffi/src/client.rs (1)
dash-spv-ffi/src/error.rs (2)
set_last_error(26-31)from(53-65)
dash-spv-ffi/include/dash_spv_ffi.h (3)
dash-spv-ffi/src/client.rs (1)
dash_spv_ffi_client_update_config(349-375)dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_set_masternode_sync_enabled(217-226)dash-spv-ffi/src/checkpoints.rs (3)
dash_spv_ffi_checkpoint_latest(29-56)dash_spv_ffi_checkpoint_before_height(64-92)dash_spv_ffi_checkpoint_before_timestamp(100-128)
⏰ 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_ripemd160)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Clippy (Non-strict)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
🔇 Additional comments (6)
dash-spv-ffi/src/checkpoints.rs (1)
4-4: Import reorder only — OKNo functional change. Safe and consistent with style.
dash-spv-ffi/src/lib.rs (1)
2-2: Re-export/order change — OK; ensure headers regenLooks fine. Please confirm cbindgen ran and the generated C header reflects the checkpoints exports in CI, and sync into downstream SDKs if applicable.
Also applies to: 11-11
dash-spv-ffi/FFI_API.md (1)
7-7: Confirm autogenerated counts are in sync“Total Functions: 63” should match the generated header after cbindgen. Re-run the doc/header generation step and sync headers into Swift if your pipeline doesn’t do this automatically.
dash-spv-ffi/src/client.rs (2)
342-375: FFI surface and error mapping look correct.Signature, null checks, runtime handoff, and FFIErrorCode mapping are consistent with the existing APIs.
342-375: Runtime network-change rejection enforced –update_confignow returnsSpvError::Config("Cannot change network at runtime")whennew_config.network != self.config.network(dash-spv/src/client/mod.rs:618–621).dash-spv-ffi/include/dash_spv_ffi.h (1)
201-211: New update-config API signature/docs: LGTM.Matches the Rust FFI and existing error semantics.
| /// Update the running client's configuration. | ||
| /// | ||
| /// # Safety | ||
| /// - `client` must be a valid pointer to an `FFIDashSpvClient`. | ||
| /// - `config` must be a valid pointer to an `FFIClientConfig`. | ||
| /// - The network in `config` must match the client's network; changing networks at runtime is not supported. | ||
| #[no_mangle] | ||
| pub unsafe extern "C" fn dash_spv_ffi_client_update_config( | ||
| client: *mut FFIDashSpvClient, | ||
| config: *const FFIClientConfig, | ||
| ) -> i32 { | ||
| null_check!(client); | ||
| null_check!(config); | ||
|
|
||
| let client = &(*client); | ||
| let new_config = (&*config).clone_inner(); | ||
|
|
||
| let result = client.runtime.block_on(async { | ||
| let mut guard = client.inner.lock().unwrap(); | ||
| if let Some(ref mut spv_client) = *guard { | ||
| spv_client.update_config(new_config).await.map_err(|e| e) | ||
| } else { | ||
| Err(dash_spv::SpvError::Config("Client not initialized".to_string())) | ||
| } | ||
| }); | ||
|
|
||
| match result { | ||
| Ok(()) => FFIErrorCode::Success as i32, | ||
| Err(e) => { | ||
| set_last_error(&e.to_string()); | ||
| FFIErrorCode::from(e) as i32 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Add a unit test for dash_spv_ffi_client_update_config.
Per prior guidance, each new FFI should have a unit test under dash-spv-ffi/tests. Validate success path and “client not initialized”.
Run to check for tests:
🏁 Script executed:
#!/bin/bash
rg -nP --glob 'dash-spv-ffi/tests/**' '(client_update_config|dash_spv_ffi_client_update_config)'Length of output: 98
Add Rust and C tests for dash_spv_ffi_client_update_config
Create a Rust unit test in dash-spv-ffi/tests/unit and a C test in dash-spv-ffi/tests/c_tests, each covering the success path and the “client not initialized” error.
🤖 Prompt for AI Agents
In dash-spv-ffi/src/client.rs around lines 342 to 375, add tests: create a Rust
unit test file at dash-spv-ffi/tests/unit/test_update_config.rs that covers (1)
success path — construct an FFIDashSpvClient with a running runtime and an inner
spv_client that will accept update_config, call
dash_spv_ffi_client_update_config and assert it returns FFIErrorCode::Success
and does not set last error, and (2) “client not initialized” — construct an
FFIDashSpvClient whose inner is None, call the function and assert it returns
the mapped error code and that last error contains "Client not initialized".
Also add a C test at dash-spv-ffi/tests/c_tests/test_update_config.c that links
against the built library, exercises the same two scenarios by calling
dash_spv_ffi_client_update_config (one with a properly initialized client
wrapper created via existing FFI helpers or setup helper you add, and one with a
null/empty inner), checks the returned int equals the expected FFIErrorCode
values, and verifies the last error string via the FFI getter; ensure to clean
up resources and make tests deterministic (use mocks or simple stub spv client
that returns Ok or error).
| // Rebuild mempool filter if needed | ||
| if mempool_changed { | ||
| self.update_mempool_filter().await; | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: disabling mempool tracking leaves the filter active
If enable_mempool_tracking is switched from true to false, update_mempool_filter() still recreates a filter, so mempool logic remains active despite the config. Apply conditional rebuild/teardown.
- // Rebuild mempool filter if needed
- if mempool_changed {
- self.update_mempool_filter().await;
- }
+ // Apply mempool tracking changes
+ if mempool_changed {
+ if self.config.enable_mempool_tracking {
+ self.update_mempool_filter().await;
+ } else {
+ // Disable and optionally clear transient state
+ self.mempool_filter = None;
+ if !self.config.persist_mempool {
+ *self.mempool_state.write().await = MempoolState::default();
+ }
+ }
+ }🤖 Prompt for AI Agents
In dash-spv/src/client/mod.rs around lines 636 to 640, the current logic always
calls self.update_mempool_filter().await when mempool_changed, which recreates
the filter even if enable_mempool_tracking was turned off; change the branch so
that when mempool_changed you check the current enable_mempool_tracking flag: if
enabled, call update_mempool_filter().await to (re)create the filter, otherwise
perform a teardown/disable step (clear the existing filter state/handle and stop
mempool processing) instead of recreating it. Ensure the teardown cleans any
held resources/state and prevents future mempool work, and only rebuilds when
the flag is true.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
dash-spv-ffi/include/dash_spv_ffi.h (3)
33-50: FFIArray: clarify destruction semantics and element-count fieldsTighten the docs so C callers don’t free the struct itself and don’t treat len/capacity as bytes.
Apply this diff within the FFIArray comment:
/** * FFI-safe array that transfers ownership of memory to the C caller. * * # Safety * * This struct represents memory that has been allocated by Rust but ownership * has been transferred to the C caller. The caller is responsible for: * - Not accessing the memory after it has been freed - * - Calling `dash_spv_ffi_array_destroy` to properly deallocate the memory - * - Ensuring the data, len, and capacity fields remain consistent + * - Calling `dash_spv_ffi_array_destroy` to properly deallocate the underlying buffer + * (this function DOES NOT free the FFIArray struct itself) + * - Ensuring the data, len, and capacity fields remain consistent + * - Interpreting `len` and `capacity` as element counts (not bytes) + * - Casting `data` to the appropriate element type; `elem_size` and `elem_align` + * describe the element layout and can be asserted by the caller */Also, per our learnings: remember to regenerate and sync the generated header to downstream SDKs after builds (e.g., run the header sync script for the Swift SDK).
199-241: Checkpoint APIs: add hash-size constant and note array element semanticsMake the 32-byte requirement self-documenting and clarify that FFICheckpoint elements need no per-element free.
Apply this doc tweak:
- * - `out_hash` must point to at least 32 writable bytes. + * - `out_hash` must point to at least DASH_HASH_SIZE writable bytes.And for the range API:
- * Returns an `FFIArray` of `FFICheckpoint` items. The caller owns the memory and - * must free the array buffer using `dash_spv_ffi_array_destroy` when done. + * Returns an `FFIArray` of `FFICheckpoint` items. The caller owns the memory and + * must free the array buffer using `dash_spv_ffi_array_destroy` when done. + * Elements are plain POD; no per-element destructor is required.Add this near the top of the header (outside this hunk):
#define DASH_HASH_SIZE 32uOptional: consider using uint64_t for the timestamp param to avoid Y2038 issues on 32-bit C callers.
471-480: Add a getter and default-value doc for masternode sync flagExpose a read API for symmetry and document the default to prevent ambiguity.
Proposed new API (add near other getters):
// Returns current setting; if config is null, returns the library default. bool dash_spv_ffi_config_get_masternode_sync_enabled(const FFIClientConfig *config);Doc tweak:
* - `config` must be a valid pointer to an FFIClientConfig created by dash_spv_ffi_config_new/mainnet/testnet * - The caller must ensure the config pointer remains valid for the duration of this call + * - Changing this at runtime via dash_spv_ffi_client_update_config may trigger masternode (re)sync behavior
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
dash-spv-ffi/include/dash_spv_ffi.h(3 hunks)dash-spv-ffi/src/checkpoints.rs(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- dash-spv-ffi/src/checkpoints.rs
🧰 Additional context used
📓 Path-based instructions (3)
dash-spv-ffi/include/dash_spv_ffi.h
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
The public C header is generated; ensure it reflects FFI changes after builds
Files:
dash-spv-ffi/include/dash_spv_ffi.h
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}
📄 CodeRabbit inference engine (CLAUDE.md)
Be careful with memory management at FFI boundaries (C/Swift interop)
Files:
dash-spv-ffi/include/dash_spv_ffi.h
*-ffi/**
📄 CodeRabbit inference engine (AGENTS.md)
FFI bindings live in *-ffi crates
Files:
dash-spv-ffi/include/dash_spv_ffi.h
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift} : Be careful with memory management at FFI boundaries (C/Swift interop)
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift} : Be careful with memory management at FFI boundaries (C/Swift interop)
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : FFI functions must be declared with #[no_mangle] extern "C"
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Accept input strings from C as *const c_char (borrowed; do not free inputs)
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : In C code, free all strings returned by the FFI using dash_string_free
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:47:37.438Z
Learning: Transaction IDs (txids) and block hashes in Dash are always 32 bytes and should be passed as fixed-size byte arrays in FFI interfaces, not as C strings, to avoid inefficiency and encoding issues.
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/callbacks.rs : Use C function pointers for async progress/completion callbacks; callbacks may be invoked from any thread
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/**/FFIBridge.swift : Manage FFI pointers manually using RAII-style wrappers and defer blocks to ensure cleanup
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:49:53.811Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:49:53.811Z
Learning: In Rust FFI code, using thread-local storage for error reporting is unsafe in multi-threaded contexts, as callbacks may execute on different threads than the caller, leading to lost or invisible errors. FFI error handling should use global, thread-safe storage or return errors directly via return values or out parameters.
Applied to files:
dash-spv-ffi/include/dash_spv_ffi.h
🧬 Code graph analysis (1)
dash-spv-ffi/include/dash_spv_ffi.h (3)
dash-spv-ffi/src/checkpoints.rs (4)
dash_spv_ffi_checkpoint_latest(29-54)dash_spv_ffi_checkpoint_before_height(62-88)dash_spv_ffi_checkpoint_before_timestamp(96-122)dash_spv_ffi_checkpoints_between_heights(129-162)dash-spv-ffi/src/client.rs (2)
dash_spv_ffi_client_new(126-181)dash_spv_ffi_client_update_config(349-375)dash-spv-ffi/src/config.rs (1)
dash_spv_ffi_config_set_masternode_sync_enabled(217-226)
⏰ 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). (4)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: SPV Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: RPC Tests (stable, true)
| /** | ||
| * Update the running client's configuration. | ||
| * | ||
| * # Safety | ||
| * - `client` must be a valid pointer to an `FFIDashSpvClient`. | ||
| * - `config` must be a valid pointer to an `FFIClientConfig`. | ||
| * - The network in `config` must match the client's network; changing networks at runtime is not supported. | ||
| */ | ||
| int32_t dash_spv_ffi_client_update_config(struct FFIDashSpvClient *client, | ||
| const FFIClientConfig *config); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Update-config API: document blocking behavior and enforce network invariants
Callers need to know this is synchronous (blocks on runtime). Also please verify a hard check rejects network changes at this boundary.
Doc tweak:
* - `client` must be a valid pointer to an `FFIDashSpvClient`.
* - `config` must be a valid pointer to an `FFIClientConfig`.
- * - The network in `config` must match the client's network; changing networks at runtime is not supported.
+ * - The network in `config` must match the client's network; changing networks at runtime is not supported.
+ * - This call is synchronous and blocks until the reconfiguration is applied; avoid calling on UI threads.To verify the invariant is enforced, run:
Also confirm unit tests exist for both new FFI functions under dash-spv-ffi/tests.
Finally, regenerate this header and sync to swift-dash-core-sdk (./sync-headers.sh).
🏁 Script executed:
#!/bin/bash
# Ensure update_config rejects network changes (search implementations)
rg -n -C3 'update_config\s*\(' dash-spv*/**/*.rs
rg -n -C3 'network' dash-spv*/**/client*.rsLength of output: 5280
🏁 Script executed:
rg -n 'dash_spv_ffi_client_update_config|dash_spv_ffi_config_set_masternode_sync_enabled' dash-spv-ffi/testsLength of output: 110
🏁 Script executed:
#!/bin/bash
# Locate DashSpvClient::update_config implementation in dash-spv crate
rg -n 'update_config(' -C5 dash-spv/srcLength of output: 120
🏁 Script executed:
#!/bin/bash
# Search for the DashSpvClient::update_config method definition in dash-spv crate
rg -n 'fn update_config' -C5 dash-spv/srcLength of output: 897
Update header docs and add missing FFI tests
- In dash-spv-ffi/include/dash_spv_ffi.h, extend the doc comment on dash_spv_ffi_client_update_config to note that it blocks synchronously on the runtime and should not be called on UI threads:
* - The network in `config` must match the client's network; changing networks at runtime is not supported.
+ * - This call is synchronous and blocks until the reconfiguration is applied; avoid calling on UI threads.- Add unit tests in dash-spv-ffi/tests to cover both dash_spv_ffi_client_update_config (and verify it rejects network changes) and dash_spv_ffi_config_set_masternode_sync_enabled.
- Regenerate the header and sync to swift-dash-core-sdk via ./sync-headers.sh.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Update the running client's configuration. | |
| * | |
| * # Safety | |
| * - `client` must be a valid pointer to an `FFIDashSpvClient`. | |
| * - `config` must be a valid pointer to an `FFIClientConfig`. | |
| * - The network in `config` must match the client's network; changing networks at runtime is not supported. | |
| */ | |
| int32_t dash_spv_ffi_client_update_config(struct FFIDashSpvClient *client, | |
| const FFIClientConfig *config); | |
| /** | |
| * Update the running client's configuration. | |
| * | |
| * # Safety | |
| * - `client` must be a valid pointer to an `FFIDashSpvClient`. | |
| * - `config` must be a valid pointer to an `FFIClientConfig`. | |
| * - The network in `config` must match the client's network; changing networks at runtime is not supported. | |
| * - This call is synchronous and blocks until the reconfiguration is applied; avoid calling on UI threads. | |
| */ | |
| int32_t dash_spv_ffi_client_update_config(struct FFIDashSpvClient *client, | |
| const FFIClientConfig *config); |
🤖 Prompt for AI Agents
In dash-spv-ffi/include/dash_spv_ffi.h around lines 244 to 254, the doc comment
for dash_spv_ffi_client_update_config needs to be extended to state that the
call blocks synchronously on the runtime and must not be invoked from UI
threads; update the comment accordingly to warn callers about blocking behavior
and the restriction on changing networks at runtime. Then add unit tests under
dash-spv-ffi/tests that (1) exercise dash_spv_ffi_client_update_config to ensure
it accepts valid config updates but rejects network changes, and (2) test
dash_spv_ffi_config_set_masternode_sync_enabled for correct behavior; ensure
tests cover error paths and return codes. Finally regenerate headers and
propagate them to swift-dash-core-sdk by running ./sync-headers.sh and commit
the updated header and generated artifacts.
dash_spv_ffi_client_update_configfunction to update the running client's configuration without restarting.dash_spv_ffi_config_set_masternode_sync_enabledfunction to enable or disable masternode synchronization.Summary by CodeRabbit
New Features
Documentation