Skip to content

Conversation

@QuantumExplorer
Copy link
Member

@QuantumExplorer QuantumExplorer commented Sep 11, 2025

Summary by CodeRabbit

  • Breaking Changes

    • Unified managed-wallet API to a single wrapper type; balance now reports a locked amount.
    • Validation enum renamed to DashSpvValidationMode; client config exposed as an explicit opaque struct in the public API.
  • New Features

    • Added runtime version and test-mode entry points.
    • New mempool strategy option: Selective.
  • Improvements

    • Richer sync/filter diagnostics showing peer addresses.
    • Improved C/C++ ABI compatibility.
  • Bug Fixes

    • Fixed potential use-after-free in wallet account collection error path.
  • Documentation

    • Updated API docs and a parameter rename for consistency.
  • Chores

    • Updated build tooling/configuration and header generation behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 11, 2025

Warning

Rate limit exceeded

@QuantumExplorer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 38 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 13b817b and f41a6c0.

📒 Files selected for processing (1)
  • dash-spv-ffi/build.rs (1 hunks)

Walkthrough

Reworks FFI boundaries and generated headers (client and wallet wrappers become opaque struct-with-void*), renames/qualifies several public types, updates cbindgen/build script, adds NetworkManager method to expose last-message peer address, and enriches sync/network logging. Swift headers and docs mirror ABI changes.

Changes

Cohort / File(s) Summary
FFI tooling & cbindgen
dash-spv-ffi/Cargo.toml, dash-spv-ffi/build.rs, dash-spv-ffi/cbindgen.toml
Bump cbindgen version; build.rs now watches cbindgen.toml/src, loads config from cbindgen.toml with fallback, matches on generate() result and emits cargo warnings on success; cbindgen config removes export prefix, adds forward-decl for FFIClientConfig, renames symbols (FFIValidationModeDashSpvValidationMode, FFIErrorCodeDashSpvErrorCode) and adjusts macro_expansion settings.
Dash SPV public headers
dash-spv-ffi/include/dash_spv_ffi.h, swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
Public ABI now exposes struct FFIClientConfig { void *inner; } (repr C opaque wrapper); FFIValidationMode renamed to DashSpvValidationMode; many function signatures qualify pointer types with struct and parameter restrict renamed to restrict_peers; added extern "C"/namespace and new helpers (version, enable_test_mode), plus mempool strategy enum extended.
Dash SPV config impl
dash-spv-ffi/src/config.rs
FFIClientConfig changed to #[repr(C)] with inner: *mut c_void; inner ClientConfig boxed on heap; accessors and setters use unsafe casts to dereference inner pointer; destroy frees both inner and wrapper; exported parameter name restrictrestrict_peers.
Networking & diagnostics
dash-spv/src/network/connection.rs, dash-spv/src/network/mod.rs, dash-spv/src/network/multi_peer.rs, dash-spv/src/client/message_handler.rs, dash-spv/src/sync/filters.rs, dash-spv/src/sync/sequential/mod.rs
Add get_last_message_peer_addr async API to NetworkManager (default None) and implementations in TcpNetworkManager/MultiPeer; add TcpConnection::address() getter; enrich CFHeaders/CFilters and outgoing-request logs with peer SocketAddr when available; no functional/control-flow changes.
Key-wallet cbindgen & headers
key-wallet-ffi/cbindgen.toml, key-wallet-ffi/include/key_wallet_ffi.h, key-wallet-ffi/include/key_wallet_ffi_test.h, swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h
Unify public wrapper to FFIManagedWalletInfo (anonymous struct with void *inner); remove old forward declarations; update exported signatures to use FFIManagedWalletInfo*; add managed_wallet_info_free (compat ffi_managed_wallet_free updated); address-pool APIs extended with pool_type and info_out; balance output uses locked_out.
Key-wallet Rust impls
key-wallet-ffi/src/address_pool.rs, key-wallet-ffi/src/managed_account_collection.rs, key-wallet-ffi/src/managed_wallet.rs, key-wallet-ffi/src/transaction_checking.rs
Switch to FFIManagedWalletInfo wrapper with inner()/inner_mut() accessors; inner boxed as opaque *mut c_void; constructors/frees updated; fixed potential use-after-free in error path; propagate signature and doc changes.
API docs
dash-spv-ffi/FFI_API.md, key-wallet-ffi/FFI_API.md
Documentation updated to reflect parameter rename restrictrestrict_peers, the FFIManagedWalletFFIManagedWalletInfo type rename, new managed_wallet_info_free, and other signature updates across FFI docs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Peer as Peer
  participant Net as NetworkManager
  participant Sync as Sync/Handlers
  participant Log as Logger

  Note over Net,Sync: message origin enrichment flow
  Peer->>Net: Incoming CFHeaders / CFilter
  Net-->>Sync: deliver message
  Sync->>Net: get_last_message_peer_addr()
  alt address available
    Net-->>Sync: Some(SocketAddr)
    Sync->>Log: Log "received ... from <addr>"
  else none
    Net-->>Sync: None
    Sync->>Log: Log "received ... (peer unknown)"
  end
  Sync->>Sync: continue existing processing (unchanged)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

I nibbled the headers, boxed secrets neat,
Hid inner hearts where C callers meet.
I traced last pawprints across the net,
Logged tiny footprints where peers were met.
Hop, cbindgen, hop — ABI tidy and sweet! 🥕🐇

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the primary work in this changeset: an FFI refactor aimed at eliminating header-generation hacks for a unified Swift header. The diff shows coordinated changes to cbindgen/build.rs/generated headers and many FFI wrapper/type adjustments that directly implement that goal.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/cleanFFI

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.

@github-actions
Copy link

⚠️ FFI Documentation Update Required

The FFI API documentation is out of date. Please regenerate it by running:

For key-wallet-ffi:

cd key-wallet-ffi
make update-docs

For dash-spv-ffi:

cd dash-spv-ffi
make update-docs

Then commit the changes:

git add */FFI_API.md
git commit -m "docs: update FFI API documentation"

This ensures the documentation stays in sync with the actual FFI functions.

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: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (14)
dash-spv/src/sync/filters.rs (2)

1243-1248: Bug: using blockchain height where storage index is required (missing conversion)

storage.get_header() expects a storage index, but batch_end is a blockchain height. This will misaddress headers under checkpoint mode and can fail or fetch wrong headers.

Apply:

-            let stop_hash = storage
-                .get_header(batch_end)
+            let stop_hash = storage
+                .get_header(self.blockchain_to_storage_height(batch_end))
                 .await
                 .map_err(|e| SyncError::Storage(format!("Failed to get stop header: {}", e)))?
                 .ok_or_else(|| SyncError::Storage("Stop header not found".to_string()))?
                 .block_hash();

1812-1827: Bug: header tip height not converted from storage to blockchain height

get_tip_height() returns a storage height. You then pass it as an end_height in blockchain space to find_height_for_block_hash(), which will constrain the search incorrectly under checkpoints.

Apply:

-        let header_tip_height = storage
-            .get_tip_height()
-            .await
-            .map_err(|e| SyncError::Storage(format!("Failed to get header tip height: {}", e)))?
-            .unwrap_or(0);
+        let storage_tip_height = storage
+            .get_tip_height()
+            .await
+            .map_err(|e| SyncError::Storage(format!("Failed to get header tip height: {}", e)))?
+            .unwrap_or(0);
+        let header_tip_height = self.storage_to_blockchain_height(storage_tip_height);
dash-spv-ffi/cbindgen.toml (1)

21-23: Type renames: cbindgen renames require syncing docs/Swift or revert them

cbindgen.toml maps FFIValidationMode→DashSpvValidationMode and FFIErrorCode→DashSpvErrorCode (dash-spv-ffi/cbindgen.toml); generated header defines DashSpvValidationMode (dash-spv-ffi/include/dash_spv_ffi.h and swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h) while docs and Swift wrappers still reference FFIValidationMode/FFIErrorCode (dash-spv-ffi/FFI_API.md; swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/ValidationMode.swift and Core/FFITypes.swift). Update docs/Swift to the DashSpv* names or remove the renames in cbindgen.toml to keep FFI* names.

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

161-167: Guard u64→u32 timestamp narrowing

Casting u64 to u32 can truncate for large inputs. Reject or clamp explicitly.

Apply:

-                timestamp: if timestamp > 0 {
-                    Some(timestamp as u32)
-                } else {
-                    None
-                },
+                timestamp: if timestamp == 0 {
+                    None
+                } else if let Ok(ts) = u32::try_from(timestamp) {
+                    Some(ts)
+                } else {
+                    FFIError::set_error(
+                        error,
+                        FFIErrorCode::InvalidInput,
+                        format!("Timestamp out of range for u32: {}", timestamp),
+                    );
+                    return false;
+                },

Make the same change in the InChainLockedBlock arm.

Also applies to: 181-187

key-wallet-ffi/src/address_pool.rs (5)

339-353: Fix off-by-one: current_gap incorrect when no address has been used

If no address is used, gap should equal generated_count, but the current math yields generated_count - 1. Compute gap from generated_count and highest_used (+1).

-    // Fill the info structure
-    let generated_count = pool.addresses.len();
-    let used_count = pool.used_indices.len();
-    let highest_used = pool.highest_used.unwrap_or(0);
-    let highest_generated = pool.highest_generated.unwrap_or(0);
-    let current_gap = highest_generated.saturating_sub(highest_used);
+    // Fill the info structure
+    let generated_count = pool.addresses.len();
+    let used_count = pool.used_indices.len();
+    // gap = (#generated) - (highest_used + 1); if none used then highest_used = -1 => gap = #generated
+    let used_prefix_len = pool.highest_used.map(|u| (u as usize) + 1).unwrap_or(0);
+    let current_gap = generated_count.saturating_sub(used_prefix_len);
 
     *info_out = FFIAddressPoolInfo {
         pool_type,
         generated_count: generated_count as c_uint,
         used_count: used_count as c_uint,
         current_gap: current_gap as c_uint,
         gap_limit: pool.gap_limit as c_uint,
         highest_used_index: pool.highest_used.map(|i| i as i32).unwrap_or(-1),
     };

459-463: Validate gap_limit; reject zero or unreasonably large values

Prevent accidental unbounded pools. Suggest 1..=1000 (tweak as per crate defaults).

-    // Set the gap limit
-    pool.gap_limit = gap_limit;
+    // Validate and set the gap limit
+    if gap_limit == 0 || gap_limit > 1000 {
+        FFIError::set_error(
+            error,
+            FFIErrorCode::InvalidInput,
+            "gap_limit must be in 1..=1000".to_string(),
+        );
+        return false;
+    }
+    pool.gap_limit = gap_limit;

552-559: Fix off-by-one when no addresses are generated yet

If highest_generated is None, generating to index N must create N+1 addresses.

-                    let current = external_addresses.highest_generated.unwrap_or(0);
-                    if target_index > current {
-                        let needed = target_index - current;
-                        external_addresses.generate_addresses(needed, &key_source, true)
-                    } else {
-                        Ok(Vec::new())
-                    }
+                    let needed = match external_addresses.highest_generated {
+                        Some(cur) if target_index > cur => target_index - cur,
+                        None => target_index.saturating_add(1),
+                        _ => 0,
+                    };
+                    if needed > 0 {
+                        external_addresses.generate_addresses(needed, &key_source, true)
+                    } else {
+                        Ok(Vec::new())
+                    }

576-583: Apply the same off-by-one fix for internal pool generation

-                    let current = internal_addresses.highest_generated.unwrap_or(0);
-                    if target_index > current {
-                        let needed = target_index - current;
-                        internal_addresses.generate_addresses(needed, &key_source, true)
-                    } else {
-                        Ok(Vec::new())
-                    }
+                    let needed = match internal_addresses.highest_generated {
+                        Some(cur) if target_index > cur => target_index - cur,
+                        None => target_index.saturating_add(1),
+                        _ => 0,
+                    };
+                    if needed > 0 {
+                        internal_addresses.generate_addresses(needed, &key_source, true)
+                    } else {
+                        Ok(Vec::new())
+                    }

605-613: Apply the same off-by-one fix for single-pool accounts

-                let current = pool.highest_generated.unwrap_or(0);
-                if target_index > current {
-                    let needed = target_index - current;
-                    pool.generate_addresses(needed, &key_source, true)
-                } else {
-                    Ok(Vec::new())
-                }
+                let needed = match pool.highest_generated {
+                    Some(cur) if target_index > cur => target_index - cur,
+                    None => target_index.saturating_add(1),
+                    _ => 0,
+                };
+                if needed > 0 {
+                    pool.generate_addresses(needed, &key_source, true)
+                } else {
+                    Ok(Vec::new())
+                }
key-wallet-ffi/src/managed_wallet.rs (1)

1084-1095: Free address arrays with the provided FFI function, not libc::free

The array and inner strings were allocated via Rust; use the matching deallocator.

-            unsafe {
-                let addresses = std::slice::from_raw_parts(addresses_out, count_out);
-                for &addr_ptr in addresses {
-                    let addr_str = CStr::from_ptr(addr_ptr).to_string_lossy();
-                    assert!(!addr_str.is_empty());
-                    println!("External address: {}", addr_str);
-                    let _ = CString::from_raw(addr_ptr);
-                }
-                libc::free(addresses_out as *mut libc::c_void);
-            }
+            unsafe {
+                let addresses = std::slice::from_raw_parts(addresses_out, count_out);
+                for &addr_ptr in addresses {
+                    let addr_str = CStr::from_ptr(addr_ptr).to_string_lossy();
+                    assert!(!addr_str.is_empty());
+                    println!("External address: {}", addr_str);
+                }
+                // Free array and contained strings correctly
+                crate::address::address_array_free(addresses_out, count_out);
+            }
key-wallet-ffi/include/key_wallet_ffi.h (1)

3391-3403: ABI mismatch: FFITransactionCheckResult/FFIAccountMatch missing in header.

Rust fills fields total_received_for_credit_conversion and affected_accounts (array of FFIAccountMatch), but the header’s FFITransactionCheckResult lacks both fields and there’s no FFIAccountMatch typedef. This will corrupt memory for C/Swift callers. Add the struct and fields, or remove them from Rust. Suggested header fix:

+// Describes which account(s) were affected by a transaction
+typedef struct {
+    FFIAccountType account_type;
+    unsigned int   account_index;
+    unsigned int   registration_index;
+    uint64_t       received;
+    uint64_t       sent;
+    unsigned int   external_addresses_count;
+    unsigned int   internal_addresses_count;
+    bool           has_external_addresses;
+    bool           has_internal_addresses;
+} FFIAccountMatch;

-typedef struct {
-    bool     is_relevant;
-    uint64_t total_received;
-    uint64_t total_sent;
-    uint32_t affected_accounts_count;
-} FFITransactionCheckResult;
+typedef struct {
+    bool            is_relevant;
+    uint64_t        total_received;
+    uint64_t        total_sent;
+    uint64_t        total_received_for_credit_conversion;
+    FFIAccountMatch *affected_accounts;   // array, freed by transaction_check_result_free
+    uint32_t        affected_accounts_count;
+} FFITransactionCheckResult;

Also ensure transaction_check_result_free frees the affected_accounts buffer.

swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h (1)

3391-3403: Swift header also missing FFIAccountMatch and fields in FFITransactionCheckResult.

Same ABI break applies here; Swift bridging will be wrong without these fields/types.

+typedef struct {
+    FFIAccountType account_type;
+    unsigned int   account_index;
+    unsigned int   registration_index;
+    uint64_t       received;
+    uint64_t       sent;
+    unsigned int   external_addresses_count;
+    unsigned int   internal_addresses_count;
+    bool           has_external_addresses;
+    bool           has_internal_addresses;
+} FFIAccountMatch;

-typedef struct {
-    bool     is_relevant;
-    uint64_t total_received;
-    uint64_t total_sent;
-    uint32_t affected_accounts_count;
-} FFITransactionCheckResult;
+typedef struct {
+    bool            is_relevant;
+    uint64_t        total_received;
+    uint64_t        total_sent;
+    uint64_t        total_received_for_credit_conversion;
+    FFIAccountMatch *affected_accounts;
+    uint32_t        affected_accounts_count;
+} FFITransactionCheckResult;

Ensure FFIBridge.swift frees affected_accounts via transaction_check_result_free.

dash-spv-ffi/include/dash_spv_ffi.h (1)

184-199: Allocator mismatch in docs will cause UB.

Docs state raw_tx (and implied addresses) are allocated by the caller, but Rust frees them via Vec::from_raw_parts (Rust allocator). These buffers must be allocated by Rust and freed by the provided destroy functions. Fix docs in Rust sources so generated headers reflect correct ownership.

Apply in Rust (illustrative):

-/// - `raw_tx`: A pointer to the raw transaction bytes. The caller is responsible for:
-///   - Allocating this memory before passing it to Rust
-///   - Ensuring the pointer remains valid for the lifetime of this struct
-///   - Freeing the memory after use with `dash_spv_ffi_unconfirmed_transaction_destroy_raw_tx`
+/// - `raw_tx`: Pointer to bytes allocated by Rust. Ownership is transferred to the caller,
+///   who must free it with `dash_spv_ffi_unconfirmed_transaction_destroy_raw_tx`.
- * - `raw_tx` must be a valid pointer to memory allocated by the caller
+ * - `raw_tx` must be a pointer previously allocated by Rust and returned via FFI

Also applies to: 883-884, 897-899

swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (1)

184-199: Fix allocator ownership docs (raw_tx/addresses).

Replicate the doc corrections here via regenerated headers to avoid UB for consumers.

Also applies to: 883-884, 897-899

🧹 Nitpick comments (22)
dash-spv/src/network/multi_peer.rs (1)

902-907: Style nit: prefer the imported alias for brevity.
Since SocketAddr is already imported, consider returning Option here for consistency.

dash-spv/src/sync/filters.rs (2)

311-312: Avoid unnecessary clone of CFHeaders

You can move cf_headers after verification; it isn’t used later.

-                    self.store_filter_headers(cf_headers.clone(), storage).await?;
+                    self.store_filter_headers(cf_headers, storage).await?;

1679-1683: Remove unused network param and trait-object indirection

handle_request_timeout doesn’t use _network; dropping it simplifies the signature and avoids trait-object coercion at the callsite.

-    async fn handle_request_timeout(
-        &mut self,
-        range: (u32, u32),
-        _network: &mut dyn NetworkManager,
-        storage: &S,
-    ) -> SyncResult<()> {
+    async fn handle_request_timeout(
+        &mut self,
+        range: (u32, u32),
+        storage: &S,
+    ) -> SyncResult<()> {

And update the call:

-            self.handle_request_timeout(range, network, storage).await?;
+            self.handle_request_timeout(range, storage).await?;

Also applies to: 1668-1669

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

70-90: API wrapper swap to FFIManagedWalletInfo: good

Returning the canonical wrapper and pointing frees to managed_wallet_info_free removes ambiguity. Doc note about compatibility free is helpful.

Consider recommending only managed_wallet_info_free in the doc to keep the free path singular going forward.

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

478-487: Bound large generation requests to batches respecting gap_limit

To avoid unbounded generation, iterate in chunks up to gap_limit until target_index is reached.

I can draft a batched generator if you want it integrated here.

Also applies to: 615-631

dash-spv-ffi/src/config.rs (3)

72-72: DRY the repeated inner pointer casts

Reduce unsafe duplication with small helpers.

// Place near the struct
#[inline(always)]
unsafe fn cfg_mut<'a>(p: *mut FFIClientConfig) -> &'a mut ClientConfig {
    &mut *((*p).inner as *mut ClientConfig)
}
#[inline(always)]
unsafe fn cfg_ref<'a>(p: *const FFIClientConfig) -> &'a ClientConfig {
    &*((*p).inner as *const ClientConfig)
}

Then replace usages like &mut *((*config).inner as *mut ClientConfig) with cfg_mut(config).

Also applies to: 97-99, 114-116, 139-147, 211-213, 251-253, 267-269, 284-286, 302-304, 323-331, 375-377, 392-395, 409-411, 426-429, 443-446, 478-480, 495-497, 513-516, 530-533


351-359: Limit internal helpers' visibility

Make get_inner/clone_inner pub(crate) to avoid accidental external reliance on layout.

-impl FFIClientConfig {
-    pub fn get_inner(&self) -> &ClientConfig {
+impl FFIClientConfig {
+    pub(crate) fn get_inner(&self) -> &ClientConfig {
         unsafe { &*(self.inner as *const ClientConfig) }
     }
 
-    pub fn clone_inner(&self) -> ClientConfig {
+    pub(crate) fn clone_inner(&self) -> ClientConfig {
         unsafe { (*(self.inner as *const ClientConfig)).clone() }
     }
 }

139-147: Centralize default P2P ports (avoid hardcoding)

  • dash-spv-ffi/src/config.rs (≈lines 140–145) still hardcodes: Dash=9999, Testnet=19999, Regtest=19899, Devnet=29999.
  • No repo helper like dashcore::params::params(network).default_port found; repo contains many hardcoded uses (examples: dash-spv/src/network/discovery.rs:34–36, test-utils/src/fixtures.rs:64–66, numerous tests).
  • Action: introduce/use a single source of truth for default P2P ports (add helper in dashcore or a shared constants module) and replace the scattered hardcoded values (including dash-spv-ffi/src/config.rs and discovery/tests).
key-wallet-ffi/src/managed_wallet.rs (2)

79-95: Prefer early range validation (start < end)

Add a fast check to avoid surprising calls and extra work.

     let network = network.into();
 
     // Get the account collection for the network
+    if end_index <= start_index {
+        FFIError::set_error(
+            error,
+            FFIErrorCode::InvalidInput,
+            "end_index must be greater than start_index".to_string(),
+        );
+        *count_out = 0;
+        *addresses_out = ptr::null_mut();
+        return false;
+    }

Apply analogously to the internal-range function.

Also applies to: 203-219, 346-363, 532-549


726-734: Both free functions duplicate logic

Consider keeping one canonical free (e.g., managed_wallet_free) and deprecate the other to avoid divergence.

Also applies to: 743-750

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

1698-1711: Clarify target_index semantics (inclusive).

Docs say “up to and including”; the implementation uses (target > current) then generates target-current. That is inclusive. Consider stating “inclusive” explicitly to prevent off-by-one misuse.


3368-3373: Freeing guidance mentions managed_wallet_info_free; reconcile with managed_wallet_free.

This section instructs callers to use managed_wallet_info_free (or ffi_managed_wallet_free), but the header also exports managed_wallet_free earlier. Either:

  • Deprecate managed_wallet_free in comments and forward it to managed_wallet_info_free in Rust, or
  • Update this note to mention both canonical and compatibility frees consistently.
- // The returned pointer must be freed with `managed_wallet_info_free` (or `ffi_managed_wallet_free` for compatibility)
+ // The returned pointer must be freed with `managed_wallet_info_free` (preferred).
+ // For backward compatibility, `ffi_managed_wallet_free` (and `managed_wallet_free`, if present) forward to the same deallocator.
swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h (2)

1698-1711: Document that target_index is inclusive.

Small doc tweak helps Swift callers avoid off-by-one mistakes.


3368-3373: Unify deallocation guidance for ManagedWalletInfo.

Mirror the core header suggestion: prefer managed_wallet_info_free; keep ffi_managed_wallet_free for compat; clarify any remaining usage of managed_wallet_free.

- // The returned pointer must be freed with `managed_wallet_info_free` (or `ffi_managed_wallet_free` for compatibility)
+ // The returned pointer must be freed with `managed_wallet_info_free` (preferred).
+ // `ffi_managed_wallet_free` remains for compatibility and forwards to the same deallocator.
dash-spv-ffi/include/dash_spv_ffi.h (6)

15-18: C++ namespace wrapper may break downstream C++ call sites.

Placing C-linkage functions inside namespace dash_spv_ffi changes qualified names for C++ callers. Verify all C++ (and Swift via Clang importer) integrations still build; consider an opt-in namespace via a macro or temporary global aliases for smoother migration.

Also applies to: 212-215, 931-938


489-489: Prefer 32-byte txid/raw variants at the FFI boundary.

Hex strings add parsing overhead and encoding risk. Keep current APIs for compatibility, but add byte-based alternatives:

  • int32_t dash_spv_ffi_client_record_send_raw(struct FFIDashSpvClient*, const uint8_t txid[32]);
  • int32_t dash_spv_ffi_client_broadcast_transaction_raw(struct FFIDashSpvClient*, const uint8_t* tx, uintptr_t len);

Also applies to: 927-929


779-781: Timestamp width (uint32_t) risks Y2038-like issues.

Use uint64_t for wallet creation time (seconds since epoch) to be future-proof across platforms.

-int32_t dash_spv_ffi_config_set_wallet_creation_time(struct FFIClientConfig *config,
-                                                     uint32_t timestamp);
+int32_t dash_spv_ffi_config_set_wallet_creation_time(struct FFIClientConfig *config,
+                                                     uint64_t timestamp);

171-177: Clarify FFIResult.error_message lifetime/ownership.

Document whether error_message is static/owned by callee, and when it becomes invalid (e.g., after next call or dash_spv_ffi_clear_error). Prevents accidental frees or use-after-free.


260-267: Comment references FFICheckpoint but type is absent.

Either export the FFICheckpoint struct layout in this header or adjust the comment to describe the element layout of FFIArray precisely.


920-923: Version string ownership.

State explicitly that the returned pointer is to a static NUL-terminated string and must not be freed by the caller.

swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (2)

489-489: Add byte-based txid/tx broadcast variants to Swift FFI too.

Expose raw txid/tx APIs in the header so Swift can avoid hex conversions.

Also applies to: 927-929


779-781: Use uint64_t for wallet_creation_time.

Prevent future overflow; regenerate after changing Rust signature.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be06e94 and ffbd8a5.

📒 Files selected for processing (21)
  • dash-spv-ffi/Cargo.toml (1 hunks)
  • dash-spv-ffi/FFI_API.md (1 hunks)
  • dash-spv-ffi/build.rs (1 hunks)
  • dash-spv-ffi/cbindgen.toml (2 hunks)
  • dash-spv-ffi/include/dash_spv_ffi.h (43 hunks)
  • dash-spv-ffi/src/config.rs (23 hunks)
  • dash-spv/src/client/message_handler.rs (2 hunks)
  • dash-spv/src/network/connection.rs (1 hunks)
  • dash-spv/src/network/mod.rs (2 hunks)
  • dash-spv/src/network/multi_peer.rs (1 hunks)
  • dash-spv/src/sync/filters.rs (10 hunks)
  • dash-spv/src/sync/sequential/mod.rs (2 hunks)
  • key-wallet-ffi/cbindgen.toml (1 hunks)
  • key-wallet-ffi/include/key_wallet_ffi.h (8 hunks)
  • key-wallet-ffi/include/key_wallet_ffi_test.h (1 hunks)
  • key-wallet-ffi/src/address_pool.rs (9 hunks)
  • key-wallet-ffi/src/managed_account_collection.rs (2 hunks)
  • key-wallet-ffi/src/managed_wallet.rs (8 hunks)
  • key-wallet-ffi/src/transaction_checking.rs (6 hunks)
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (43 hunks)
  • swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h (8 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
dash-spv/**/*.rs

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

dash-spv/**/*.rs: Enforce Rust formatting via cargo fmt --check on all Rust source files
All code must be clippy-clean: run cargo clippy --all-targets --all-features -- -D warnings

Files:

  • dash-spv/src/network/connection.rs
  • dash-spv/src/network/multi_peer.rs
  • dash-spv/src/sync/sequential/mod.rs
  • dash-spv/src/sync/filters.rs
  • dash-spv/src/client/message_handler.rs
  • dash-spv/src/network/mod.rs
dash-spv/src/network/**/*.rs

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

Implement DNS-first peer discovery with the specified seeds, immediate startup (no initial discovery delay), and exclusive peer mode when explicit peers are provided

Files:

  • dash-spv/src/network/connection.rs
  • dash-spv/src/network/multi_peer.rs
  • dash-spv/src/network/mod.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/src/network/connection.rs
  • dash-spv/src/network/multi_peer.rs
  • key-wallet-ffi/src/managed_account_collection.rs
  • dash-spv/src/sync/sequential/mod.rs
  • dash-spv/src/sync/filters.rs
  • dash-spv/src/client/message_handler.rs
  • key-wallet-ffi/src/address_pool.rs
  • dash-spv/src/network/mod.rs
  • dash-spv-ffi/build.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • dash-spv-ffi/src/config.rs
  • key-wallet-ffi/src/transaction_checking.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/src/network/connection.rs
  • dash-spv/src/network/multi_peer.rs
  • key-wallet-ffi/src/managed_account_collection.rs
  • dash-spv/src/sync/sequential/mod.rs
  • dash-spv/src/sync/filters.rs
  • dash-spv/src/client/message_handler.rs
  • key-wallet-ffi/src/address_pool.rs
  • dash-spv/src/network/mod.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • dash-spv-ffi/src/config.rs
  • key-wallet-ffi/src/transaction_checking.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:

  • key-wallet-ffi/src/managed_account_collection.rs
  • key-wallet-ffi/include/key_wallet_ffi_test.h
  • key-wallet-ffi/src/address_pool.rs
  • dash-spv-ffi/build.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • dash-spv-ffi/src/config.rs
  • key-wallet-ffi/src/transaction_checking.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h
  • key-wallet-ffi/include/key_wallet_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • key-wallet-ffi/src/managed_account_collection.rs
  • dash-spv-ffi/Cargo.toml
  • key-wallet-ffi/include/key_wallet_ffi_test.h
  • dash-spv-ffi/FFI_API.md
  • key-wallet-ffi/src/address_pool.rs
  • dash-spv-ffi/build.rs
  • key-wallet-ffi/cbindgen.toml
  • key-wallet-ffi/src/managed_wallet.rs
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/cbindgen.toml
  • key-wallet-ffi/src/transaction_checking.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • key-wallet-ffi/include/key_wallet_ffi.h
**/Cargo.toml

📄 CodeRabbit inference engine (AGENTS.md)

MSRV is 1.89; set and respect rust-version = "1.89" in Cargo.toml

Files:

  • dash-spv-ffi/Cargo.toml
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/config.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
swift-dash-core-sdk/**

📄 CodeRabbit inference engine (AGENTS.md)

Mobile SDK code resides in swift-dash-core-sdk/

Files:

  • swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h

📄 CodeRabbit inference engine (swift-dash-core-sdk/CLAUDE.md)

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

Files:

  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
🧠 Learnings (32)
📚 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/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation

Applied to files:

  • key-wallet-ffi/src/managed_account_collection.rs
  • key-wallet-ffi/src/address_pool.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-ffi/src/transaction_checking.rs
  • key-wallet-ffi/include/key_wallet_ffi.h
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#74
File: key-wallet/src/address.rs:30-40
Timestamp: 2025-06-15T15:31:44.136Z
Learning: In the key-wallet crate, QuantumExplorer prefers keeping wildcard arms that default unknown Network variants to testnet prefixes rather than using exhaustive matches or panics. This fallback behavior is intentional.

Applied to files:

  • key-wallet-ffi/src/managed_account_collection.rs
  • key-wallet-ffi/src/address_pool.rs
📚 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/**/*.rs : Always validate network consistency; never mix mainnet and testnet operations

Applied to files:

  • key-wallet-ffi/src/managed_account_collection.rs
  • key-wallet-ffi/src/address_pool.rs
  • key-wallet-ffi/src/transaction_checking.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/Cargo.toml
  • dash-spv-ffi/build.rs
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/cbindgen.toml
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/Cargo.toml
  • dash-spv-ffi/FFI_API.md
  • dash-spv-ffi/build.rs
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/cbindgen.toml
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/Cargo.toml
  • dash-spv-ffi/FFI_API.md
  • dash-spv-ffi/build.rs
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/cbindgen.toml
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/Cargo.toml
  • dash-spv-ffi/build.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/cbindgen.toml
  • key-wallet-ffi/src/transaction_checking.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
📚 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/**/Cargo.toml : Set MSRV to 1.89 using the `rust-version` field and keep the code compatible with Rust 1.89

Applied to files:

  • dash-spv-ffi/Cargo.toml
📚 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/Cargo.toml
  • dash-spv-ffi/build.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/Cargo.toml
  • dash-spv-ffi/cbindgen.toml
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
📚 Learning: 2025-08-21T04:45:50.436Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#108
File: key-wallet/src/gap_limit.rs:291-295
Timestamp: 2025-08-21T04:45:50.436Z
Learning: The rust-dashcore project uses Rust 1.89 as evidenced by rust-version = "1.89" in dash-spv/Cargo.toml. Modern Rust features like Option::is_none_or (stabilized in 1.82) can be used safely.

Applied to files:

  • dash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#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_test.h
  • key-wallet-ffi/src/address_pool.rs
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-ffi/src/transaction_checking.rs
  • swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h
  • key-wallet-ffi/include/key_wallet_ffi.h
📚 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/address_pool/**/*.rs : Do not generate unbounded addresses; enforce gap limit and staged address generation in address pools

Applied to files:

  • key-wallet-ffi/src/address_pool.rs
📚 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/address_pool/**/*.rs : Pre-generate addresses in bounded batches (typically 20–100) and store them in pools

Applied to files:

  • key-wallet-ffi/src/address_pool.rs
📚 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/ffi/**/*.rs : Expose safe C FFI with #[no_mangle] extern "C" functions only for vetted APIs

Applied to files:

  • key-wallet-ffi/src/address_pool.rs
  • key-wallet-ffi/cbindgen.toml
  • key-wallet-ffi/src/managed_wallet.rs
  • key-wallet-ffi/src/transaction_checking.rs
📚 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/managed_account/**/*.rs : Never attempt signing operations for watch-only accounts

Applied to files:

  • key-wallet-ffi/src/address_pool.rs
📚 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/tests/address_tests.rs : Place address generation and validation tests in tests/address_tests.rs

Applied to files:

  • key-wallet-ffi/src/address_pool.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 : Enforce Rust formatting via `cargo fmt --check` on all Rust source files

Applied to files:

  • dash-spv-ffi/build.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 : Accept input strings from C as *const c_char (borrowed; do not free inputs)

Applied to files:

  • dash-spv-ffi/build.rs
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/build.rs
  • dash-spv-ffi/src/config.rs
  • dash-spv-ffi/cbindgen.toml
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/src/config.rs
  • dash-spv-ffi/cbindgen.toml
  • key-wallet-ffi/src/transaction_checking.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/src/config.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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/src/config.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
📚 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/transaction_checking/**/*.rs : Use transaction type routing to check only relevant accounts

Applied to files:

  • key-wallet-ffi/src/transaction_checking.rs
  • swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h
  • key-wallet-ffi/include/key_wallet_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
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:15:57.225Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:57.225Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : When accessing SPV functionality, use DashSDK public APIs instead of direct client access; add public wrapper methods to DashSDK if needed

Applied to files:

  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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
  • swift-dash-core-sdk/Sources/DashSPVFFI/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
  • swift-dash-core-sdk/Sources/DashSPVFFI/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
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:46:56.854Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:46:56.854Z
Learning: Transaction IDs (txids) 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
  • swift-dash-core-sdk/Sources/DashSPVFFI/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 : Add a Swift wrapper in FFIBridge.swift for each new FFI function, handling type conversions and cross-language error mapping

Applied to files:

  • swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/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 : At the FFI boundary, convert FFI errors to DashSDKError and use Result types; log FFI errors before converting

Applied to files:

  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
🧬 Code graph analysis (10)
dash-spv/src/network/multi_peer.rs (1)
dash-spv/src/network/mod.rs (2)
  • get_last_message_peer_addr (107-109)
  • get_last_message_peer_addr (373-379)
dash-spv/src/sync/filters.rs (1)
dash-spv/src/client/mod.rs (2)
  • network (418-420)
  • network (1249-1252)
dash-spv/src/network/mod.rs (1)
dash-spv/src/network/multi_peer.rs (1)
  • get_last_message_peer_addr (903-906)
key-wallet-ffi/src/managed_wallet.rs (4)
key-wallet-ffi/src/account.rs (6)
  • new (20-24)
  • new (41-45)
  • new (62-66)
  • inner (27-29)
  • inner (48-50)
  • inner (69-71)
key-wallet-ffi/src/utxo.rs (1)
  • new (26-54)
key-wallet-ffi/src/managed_account.rs (2)
  • new (26-30)
  • inner (33-35)
key-wallet-ffi/src/types.rs (3)
  • new (165-169)
  • inner (172-174)
  • inner_mut (177-179)
dash-spv-ffi/src/config.rs (2)
key-wallet-ffi/src/managed_wallet.rs (2)
  • new (27-31)
  • inner (33-35)
dash-spv/src/client/config.rs (3)
  • new (250-257)
  • mainnet (260-262)
  • testnet (265-267)
key-wallet-ffi/src/transaction_checking.rs (1)
key-wallet-ffi/src/managed_wallet.rs (2)
  • managed_wallet_info_free (743-750)
  • new (27-31)
dash-spv-ffi/include/dash_spv_ffi.h (5)
dash-spv-ffi/src/checkpoints.rs (1)
  • dash_spv_ffi_checkpoint_latest (29-54)
dash-spv-ffi/src/config.rs (9)
  • dash_spv_ffi_config_new (32-38)
  • dash_spv_ffi_config_mainnet (41-47)
  • dash_spv_ffi_config_testnet (50-56)
  • dash_spv_ffi_config_set_data_dir (65-83)
  • dash_spv_ffi_config_set_validation_mode (91-100)
  • dash_spv_ffi_config_get_network (295-304)
  • dash_spv_ffi_config_get_data_dir (313-331)
  • dash_spv_ffi_config_destroy (340-349)
  • dash_spv_ffi_config_set_mempool_tracking (369-378)
dash-spv-ffi/src/platform_integration.rs (4)
  • ffi_dash_spv_get_core_handle (39-50)
  • ffi_dash_spv_release_core_handle (60-64)
  • ffi_dash_spv_get_quorum_public_key (76-196)
  • ffi_dash_spv_get_platform_activation_height (206-255)
dash-spv-ffi/src/types.rs (6)
  • dash_spv_ffi_string_destroy (307-311)
  • dash_spv_ffi_array_destroy (317-332)
  • dash_spv_ffi_string_array_destroy (345-364)
  • dash_spv_ffi_unconfirmed_transaction_destroy_raw_tx (461-469)
  • dash_spv_ffi_unconfirmed_transaction_destroy_addresses (481-489)
  • dash_spv_ffi_unconfirmed_transaction_destroy (500-528)
dash-spv-ffi/src/utils.rs (3)
  • dash_spv_ffi_init_logging (11-31)
  • dash_spv_ffi_version (34-36)
  • dash_spv_ffi_enable_test_mode (39-41)
swift-dash-core-sdk/Sources/KeyWalletFFI/include/key_wallet_ffi.h (2)
key-wallet-ffi/src/address_pool.rs (4)
  • managed_wallet_get_address_pool_info (248-357)
  • managed_wallet_set_gap_limit (369-464)
  • managed_wallet_generate_addresses_to_index (478-631)
  • managed_wallet_mark_address_used (644-800)
key-wallet-ffi/src/transaction_checking.rs (3)
  • wallet_create_managed_wallet (74-93)
  • managed_wallet_check_transaction (109-427)
  • ffi_managed_wallet_free (459-462)
key-wallet-ffi/include/key_wallet_ffi.h (2)
key-wallet-ffi/src/address_pool.rs (4)
  • managed_wallet_get_address_pool_info (248-357)
  • managed_wallet_set_gap_limit (369-464)
  • managed_wallet_generate_addresses_to_index (478-631)
  • managed_wallet_mark_address_used (644-800)
key-wallet-ffi/src/transaction_checking.rs (3)
  • wallet_create_managed_wallet (74-93)
  • managed_wallet_check_transaction (109-427)
  • ffi_managed_wallet_free (459-462)
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (6)
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 (1)
  • dash_spv_ffi_client_new (128-183)
dash-spv-ffi/src/config.rs (7)
  • dash_spv_ffi_config_new (32-38)
  • dash_spv_ffi_config_mainnet (41-47)
  • dash_spv_ffi_config_set_data_dir (65-83)
  • dash_spv_ffi_config_set_validation_mode (91-100)
  • dash_spv_ffi_config_get_network (295-304)
  • dash_spv_ffi_config_get_data_dir (313-331)
  • dash_spv_ffi_config_destroy (340-349)
dash-spv-ffi/src/platform_integration.rs (4)
  • ffi_dash_spv_get_core_handle (39-50)
  • ffi_dash_spv_release_core_handle (60-64)
  • ffi_dash_spv_get_quorum_public_key (76-196)
  • ffi_dash_spv_get_platform_activation_height (206-255)
dash-spv-ffi/src/types.rs (6)
  • dash_spv_ffi_string_destroy (307-311)
  • dash_spv_ffi_array_destroy (317-332)
  • dash_spv_ffi_string_array_destroy (345-364)
  • dash_spv_ffi_unconfirmed_transaction_destroy_raw_tx (461-469)
  • dash_spv_ffi_unconfirmed_transaction_destroy_addresses (481-489)
  • dash_spv_ffi_unconfirmed_transaction_destroy (500-528)
dash-spv-ffi/src/utils.rs (3)
  • dash_spv_ffi_init_logging (11-31)
  • dash_spv_ffi_version (34-36)
  • dash_spv_ffi_enable_test_mode (39-41)
⏰ 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 (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: Core Components Tests
  • GitHub Check: Strict Warnings and Clippy Checks
  • GitHub Check: Key Wallet Components Tests
  • GitHub Check: SPV Components Tests

Comment on lines +902 to +907
/// Get the socket address of the last peer that sent a message
pub async fn get_last_message_peer_addr(&self) -> Option<std::net::SocketAddr> {
let last_peer = self.last_message_peer.lock().await;
*last_peer
}

Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Bug: Trait method not overridden — callers via NetworkManager will always see None.
You added an inherent method get_last_message_peer_addr(), but NetworkManager isn’t overridden for MultiPeerNetworkManager. Calls through a NetworkManager dyn/impl will hit the trait default (None), defeating the new diagnostics.

Add the override in the trait impl and delegate to the inherent method:

@@
 impl NetworkManager for MultiPeerNetworkManager {
@@
     async fn get_last_message_peer_id(&self) -> crate::types::PeerId {
         // Call the instance method to avoid code duplication
         self.get_last_message_peer_id().await
     }
+
+    async fn get_last_message_peer_addr(&self) -> Option<std::net::SocketAddr> {
+        // Delegate to the inherent method that tracks the sender
+        Self::get_last_message_peer_addr(self).await
+    }

Run to confirm no override currently exists:


🏁 Script executed:

#!/bin/bash
rg -nP 'impl\s+NetworkManager\s+for\s+MultiPeerNetworkManager\b(?s).*?fn\s+get_last_message_peer_addr' -C2

Length of output: 0


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "---- Searching for occurrences of MultiPeerNetworkManager ----"
rg -n --hidden -S "MultiPeerNetworkManager" -C3 || true
echo "---- Searching for impl NetworkManager for MultiPeerNetworkManager ----"
rg -n --hidden -S -U "impl\s+NetworkManager\s+for\s+MultiPeerNetworkManager" -C5 || true
echo "---- Searching for get_last_message_peer_addr usages ----"
rg -n --hidden -S "get_last_message_peer_addr" -C3 || true
echo "---- Searching for all impl NetworkManager blocks ----"
rg -n --hidden -S "impl\s+NetworkManager\b" -C5 || true

Length of output: 39224


Bug: implement NetworkManager::get_last_message_peer_addr for MultiPeerNetworkManager

MultiPeerNetworkManager defines an inherent pub async fn get_last_message_peer_addr(...) but the impl NetworkManager does not override the trait (the trait default returns None), so calls via NetworkManager always receive None. Add the override in the impl block.

@@
 impl NetworkManager for MultiPeerNetworkManager {
@@
     async fn get_last_message_peer_id(&self) -> crate::types::PeerId {
         // Call the instance method to avoid code duplication
         self.get_last_message_peer_id().await
     }
+
+    async fn get_last_message_peer_addr(&self) -> Option<std::net::SocketAddr> {
+        // Delegate to the inherent method that tracks the sender
+        Self::get_last_message_peer_addr(self).await
+    }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In dash-spv/src/network/multi_peer.rs around lines 902 to 907, the inherent
async method get_last_message_peer_addr is implemented but the NetworkManager
trait impl does not override it (the trait default returns None), so
trait-dispatched calls always get None; fix by adding an override in impl
NetworkManager for MultiPeerNetworkManager with the exact async signature from
the trait that awaits the last_message_peer mutex and returns its value (or
simply call the existing inherent method), ensuring the function is async,
returns Option<std::net::SocketAddr>, and uses the same visibility and
error-free borrowing as the inherent implementation.

Comment on lines +258 to +276
// Try to include the peer address for diagnostics
let peer_addr = network.get_last_message_peer_addr().await;
match peer_addr {
Some(addr) => {
tracing::warn!(
"📋 Received overlapping filter headers from {}: expected start={}, received start={} (likely from recovery/retry)",
addr,
self.current_sync_height,
batch_start_height
);
}
None => {
tracing::warn!(
"📋 Received overlapping filter headers: expected start={}, received start={} (likely from recovery/retry)",
self.current_sync_height,
batch_start_height
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Peer-aware overlap logging is useful; verify trait surface provides addr accessor

Looks good. Please confirm NetworkManager exposes get_last_message_peer_addr() across all impls.

Run to verify trait + impls:


🏁 Script executed:

#!/bin/bash
rg -nP --type=rust -C3 $'trait\\s+NetworkManager\\b[\\s\\S]*?\\bget_last_message_peer_addr\\s*\\('
rg -nP --type=rust -C2 '\\bimpl\\b[\\s\\S]*?NetworkManager[\\s\\S]*?get_last_message_peer_addr\\s*\\('

Length of output: 72


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Searching for usages of get_last_message_peer_addr..."
rg -n --type=rust 'get_last_message_peer_addr' -S -C3 || true

echo
echo "Searching for trait definition `NetworkManager`..."
rg -n --type=rust 'trait\s+NetworkManager' -S -C3 || true

echo
echo "Searching for impl blocks for `NetworkManager`..."
rg -n --type=rust 'impl\s+.*NetworkManager' -S -C3 || true

Length of output: 15918


Override/forward get_last_message_peer_addr in MultiPeerNetworkManager

NetworkManager declares async fn get_last_message_peer_addr (dash-spv/src/network/mod.rs:36,107); TcpNetworkManager overrides it (dash-spv/src/network/mod.rs:373). MultiPeerNetworkManager has an inherent pub async fn get_last_message_peer_addr (dash-spv/src/network/multi_peer.rs:903) but its NetworkManager impl does not override/forward it (dash-spv/src/network/multi_peer.rs:996) — trait-callers will get the default None; implement/forward the trait method.

🤖 Prompt for AI Agents
In dash-spv/src/network/multi_peer.rs around lines 903 to 996, the
NetworkManager trait's async fn get_last_message_peer_addr is not
implemented/forwarded for MultiPeerNetworkManager, so trait callers get the
default None; implement the trait method in the impl NetworkManager for
MultiPeerNetworkManager and have it simply await and return the result of the
existing inherent pub async fn get_last_message_peer_addr (i.e., forward the
call to self.get_last_message_peer_addr().await or to the appropriate inner
manager/peer logic), ensuring the signature and returned Option type match the
trait.

Comment on lines +224 to +227
int32_t dash_spv_ffi_checkpoint_latest(FFINetwork network,
uint32_t *out_height,
uint8_t *out_hash)
;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Mirror header also missing FFINetwork (and FFICheckpoint).

Same blocking issue as the primary header—this copy won’t compile standalone. Regenerate and resync after exporting these types.

Also applies to: 237-241, 251-255, 264-267, 516-521

🤖 Prompt for AI Agents
In swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h around lines
224-227 (and similarly at 237-241, 251-255, 264-267, 516-521), the mirror header
is missing the FFINetwork and FFICheckpoint type definitions referenced by the
function prototypes, causing the header to be uncompilable standalone; update
this header to include or forward-declare the FFINetwork and FFICheckpoint types
(or include the generated header that defines them), then regenerate and resync
the mirror header after exporting those types so these function declarations
compile correctly.

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: 3

🧹 Nitpick comments (4)
key-wallet-ffi/FFI_API.md (4)

76-76: Deprecate alias and point callers to a single canonical free.

The table still advertises ffi_managed_wallet_free without indicating it’s an alias. Please mark it deprecated in favor of managed_wallet_info_free to avoid API ambiguity.

-| `ffi_managed_wallet_free` | Free a managed wallet (FFIManagedWalletInfo type)  # Safety  - `managed_walle... | transaction_checking |
+| `ffi_managed_wallet_free` (deprecated) | Alias for freeing ManagedWalletInfo. Prefer `managed_wallet_info_free`. | managed_wallet |

700-708: Clarify deprecation + fix module classification.

This function duplicates managed_wallet_info_free and lives under the wrong module header (“transaction_checking”). Recommend documenting it as deprecated and classifying under managed_wallet.

-ffi_managed_wallet_free(managed_wallet: *mut FFIManagedWalletInfo) -> ()
+ffi_managed_wallet_free(managed_wallet: *mut FFIManagedWalletInfo) -> () // deprecated: use managed_wallet_info_free

-**Description:**
-Free a managed wallet (FFIManagedWalletInfo type)  # Safety  - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - This function must only be called once per managed wallet
+**Description:**
+DEPRECATED: Use `managed_wallet_info_free`. Kept for backward compatibility. Frees an `FFIManagedWalletInfo`.

-**Safety:**
-- `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - This function must only be called once per managed wallet
+**Safety:**
+- `managed_wallet` must be a valid pointer to an `FFIManagedWalletInfo`.
+- Call at most once. Prefer `managed_wallet_info_free`.

Also change the “Module” line below to:

-**Module:** `transaction_checking`
+**Module:** `managed_wallet`

780-788: Type/semantics consistency: block_height and timestamp need explicit units; align with wallet_check_transaction.

  • Here block_height: c_uint while wallet_check_transaction uses u32. Please standardize or document why they differ.
  • Specify whether timestamp is seconds or milliseconds since UNIX epoch to prevent cross-language mistakes.
  • Call out that managed_wallet may be mutated when update_state=true and is not thread-safe to share concurrently without external synchronization.

Proposed doc tweak:

- # Safety  - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `wallet` must be a valid pointer to an FFIWallet (needed for address generation) - `tx_bytes` must be a valid pointer to transaction bytes with at least `tx_len` bytes - `result_out` must be a valid pointer to store the result - `error` must be a valid pointer to an FFIError - The affected_accounts array in the result must be freed with `transaction_check_result_free`
+ # Safety
+ - `managed_wallet` must be a valid pointer to an `FFIManagedWalletInfo` (may be mutated when `update_state=true`; do not share across threads without synchronization).
+ - `wallet` must be a valid pointer to an `FFIWallet` (used for address derivation; not mutated).
+ - `tx_bytes` must point to at least `tx_len` bytes.
+ - `block_height` is a 32‑bit height; ensure consistency with `wallet_check_transaction` (u32).
+ - `timestamp` is <UNITS HERE: seconds|milliseconds> since UNIX epoch.
+ - `result_out` must be valid; free with `transaction_check_result_free`.
+ - `error` may be null or a valid pointer.

812-820: Confirm inclusive semantics and invalid-index behavior.

Doc says “up to and including the specified index.” Please state behavior when target_index is less than the highest generated index (no-op vs. error) to help callers choose idempotent strategies.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffbd8a5 and e22c570.

📒 Files selected for processing (2)
  • dash-spv/src/network/mod.rs (2 hunks)
  • key-wallet-ffi/FFI_API.md (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • dash-spv/src/network/mod.rs
🧰 Additional context used
📓 Path-based instructions (1)
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • key-wallet-ffi/FFI_API.md
🧠 Learnings (7)
📓 Common learnings
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 : Add a Swift wrapper in FFIBridge.swift for each new FFI function, handling type conversions and cross-language error mapping
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-21T05:01:58.949Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#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/FFI_API.md
📚 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/ffi/**/*.rs : Expose safe C FFI with #[no_mangle] extern "C" functions only for vetted APIs

Applied to files:

  • key-wallet-ffi/FFI_API.md
📚 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/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation

Applied to files:

  • key-wallet-ffi/FFI_API.md
📚 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/transaction_checking/**/*.rs : Use transaction type routing to check only relevant accounts

Applied to files:

  • key-wallet-ffi/FFI_API.md
📚 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/address_pool/**/*.rs : Pre-generate addresses in bounded batches (typically 20–100) and store them in pools

Applied to files:

  • key-wallet-ffi/FFI_API.md
📚 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/address_pool/**/*.rs : Do not generate unbounded addresses; enforce gap limit and staged address generation in address pools

Applied to files:

  • key-wallet-ffi/FFI_API.md
⏰ 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). (1)
  • GitHub Check: Strict Warnings and Clippy Checks
🔇 Additional comments (1)
key-wallet-ffi/FFI_API.md (1)

1196-1204: LGTM: creation now returns FFIManagedWalletInfo and clarifies freeing path.

Return type and freeing guidance align with the new opaque wrapper model.

Comment on lines +876 to 884
managed_wallet_get_address_pool_info(managed_wallet: *const FFIManagedWalletInfo, network: FFINetwork, account_type: FFIAccountType, account_index: c_uint, pool_type: FFIAddressPoolType, info_out: *mut FFIAddressPoolInfo, error: *mut FFIError,) -> bool
```

**Description:**
Get address pool information for an account # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWallet - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null
Get address pool information for an account # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null

**Safety:**
- `managed_wallet` must be a valid pointer to an FFIManagedWallet - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null
- `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Describe all params (network/account/pool) and add missing safety notes.

The description/safety omit network, account_type, account_index, and pool_type. These are critical for selecting the pool; please document them.

-Get address pool information for an account  # Safety  - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null
+Get address pool information for an account.
+
+# Parameters
+- `network`: target network of the pool.
+- `account_type`/`account_index`: select the account (index ignored for types without indices).
+- `pool_type`: External/Internal/Single as applicable.
+
+# Safety
+- `managed_wallet` must be a valid `FFIManagedWalletInfo`.
+- `info_out` must be valid.
+- `error` may be null or valid.
📝 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.

Suggested change
managed_wallet_get_address_pool_info(managed_wallet: *const FFIManagedWalletInfo, network: FFINetwork, account_type: FFIAccountType, account_index: c_uint, pool_type: FFIAddressPoolType, info_out: *mut FFIAddressPoolInfo, error: *mut FFIError,) -> bool
```
**Description:**
Get address pool information for an account # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWallet - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null
Get address pool information for an account # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null
**Safety:**
- `managed_wallet` must be a valid pointer to an FFIManagedWallet - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null
- `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `info_out` must be a valid pointer to store the pool info - `error` must be a valid pointer to an FFIError or null
managed_wallet_get_address_pool_info(managed_wallet: *const FFIManagedWalletInfo, network: FFINetwork, account_type: FFIAccountType, account_index: c_uint, pool_type: FFIAddressPoolType, info_out: *mut FFIAddressPoolInfo, error: *mut FFIError,) -> bool
Get address pool information for an account.
# Parameters
- `network`: target network of the pool.
- `account_type`/`account_index`: select the account (index ignored for types without indices).
- `pool_type`: External/Internal/Single as applicable.
# Safety
- `managed_wallet` must be a valid `FFIManagedWalletInfo`.
- `info_out` must be valid.
- `error` may be null or valid.

Comment on lines +1020 to 1028
managed_wallet_mark_address_used(managed_wallet: *mut FFIManagedWalletInfo, network: FFINetwork, address: *const c_char, error: *mut FFIError,) -> bool
```

**Description:**
Mark an address as used in the pool This updates the pool's tracking of which addresses have been used, which is important for gap limit management and wallet recovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWallet - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null
Mark an address as used in the pool This updates the pool's tracking of which addresses have been used, which is important for gap limit management and wallet recovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null

**Safety:**
- `managed_wallet` must be a valid pointer to an FFIManagedWallet - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null
- `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Mention network and clarify address requirements.

network is in the signature but not documented. Also state expected address format (P2PKH string) and behavior when the address isn’t in the pool.

-Mark an address as used in the pool  This updates the pool's tracking of which addresses have been used, which is important for gap limit management and wallet recovery.  # Safety  - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null
+Mark an address as used in the pool for the given `network`. The address should be a valid P2PKH address string for that network. If the address is not found, the function returns false and sets `error`.
+
+# Safety
+- `managed_wallet` must be a valid `FFIManagedWalletInfo`.
+- `network` must match the address’ network.
+- `address` must be a valid null‑terminated C string.
+- `error` may be null or valid.
📝 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.

Suggested change
managed_wallet_mark_address_used(managed_wallet: *mut FFIManagedWalletInfo, network: FFINetwork, address: *const c_char, error: *mut FFIError,) -> bool
```
**Description:**
Mark an address as used in the pool This updates the pool's tracking of which addresses have been used, which is important for gap limit management and wallet recovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWallet - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null
Mark an address as used in the pool This updates the pool's tracking of which addresses have been used, which is important for gap limit management and wallet recovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null
**Safety:**
- `managed_wallet` must be a valid pointer to an FFIManagedWallet - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null
- `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `address` must be a valid C string - `error` must be a valid pointer to an FFIError or null
managed_wallet_mark_address_used(managed_wallet: *mut FFIManagedWalletInfo, network: FFINetwork, address: *const c_char, error: *mut FFIError,) -> bool
Mark an address as used in the pool for the given `network`. The address should be a valid P2PKH address string for that network. If the address is not found, the function returns false and sets `error`.
# Safety
- `managed_wallet` must be a valid `FFIManagedWalletInfo`.
- `network` must match the address's network.
- `address` must be a valid null-terminated C string.
- `error` may be null or valid.
🤖 Prompt for AI Agents
In key-wallet-ffi/FFI_API.md around lines 1020 to 1028, the docs omit the
meaning of the network parameter and the required address format and behavior
when the address is not present in the pool; update the signature docs to
describe that `network` selects which network’s address pool (e.g.,
mainnet/testnet) to operate on, state that `address` must be a P2PKH address
string (UTF-8 C string) in the network’s expected format, and clarify behavior:
if the address is not found in the selected pool the function should return
false and populate `error` with an "address not in pool" message (or clear
`error` if null) rather than silently succeed; also add the same `network` and
address-format safety notes to the Safety section.

Comment on lines +1036 to 1044
managed_wallet_set_gap_limit(managed_wallet: *mut FFIManagedWalletInfo, network: FFINetwork, account_type: FFIAccountType, account_index: c_uint, pool_type: FFIAddressPoolType, gap_limit: c_uint, error: *mut FFIError,) -> bool
```

**Description:**
Set the gap limit for an address pool The gap limit determines how many unused addresses to maintain at the end of the pool. This is important for wallet recovery and address discovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWallet - `error` must be a valid pointer to an FFIError or null
Set the gap limit for an address pool The gap limit determines how many unused addresses to maintain at the end of the pool. This is important for wallet recovery and address discovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `error` must be a valid pointer to an FFIError or null

**Safety:**
- `managed_wallet` must be a valid pointer to an FFIManagedWallet - `error` must be a valid pointer to an FFIError or null
- `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `error` must be a valid pointer to an FFIError or null

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Document all inputs and edge cases for gap limit.

Please describe how gap_limit interacts with existing pools and whether it triggers generation/trimming. Also document account/pool selectors and valid ranges.

-Set the gap limit for an address pool  The gap limit determines how many unused addresses to maintain at the end of the pool. This is important for wallet recovery and address discovery.  # Safety  - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `error` must be a valid pointer to an FFIError or null
+Set the gap limit for an address pool.
+The gap limit is the number of trailing unused addresses to maintain. Changing it may cause address generation up to the new target; it never trims existing addresses.
+
+# Parameters
+- `network`: target network.
+- `account_type`/`account_index`: select account (index ignored for non-indexed types).
+- `pool_type`: External/Internal/Single.
+- `gap_limit`: new limit (must be > 0).
+
+# Safety
+- `managed_wallet` must be a valid `FFIManagedWalletInfo`.
+- `error` may be null or valid.
📝 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.

Suggested change
managed_wallet_set_gap_limit(managed_wallet: *mut FFIManagedWalletInfo, network: FFINetwork, account_type: FFIAccountType, account_index: c_uint, pool_type: FFIAddressPoolType, gap_limit: c_uint, error: *mut FFIError,) -> bool
```
**Description:**
Set the gap limit for an address pool The gap limit determines how many unused addresses to maintain at the end of the pool. This is important for wallet recovery and address discovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWallet - `error` must be a valid pointer to an FFIError or null
Set the gap limit for an address pool The gap limit determines how many unused addresses to maintain at the end of the pool. This is important for wallet recovery and address discovery. # Safety - `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `error` must be a valid pointer to an FFIError or null
**Safety:**
- `managed_wallet` must be a valid pointer to an FFIManagedWallet - `error` must be a valid pointer to an FFIError or null
- `managed_wallet` must be a valid pointer to an FFIManagedWalletInfo - `error` must be a valid pointer to an FFIError or null
managed_wallet_set_gap_limit(managed_wallet: *mut FFIManagedWalletInfo, network: FFINetwork, account_type: FFIAccountType, account_index: c_uint, pool_type: FFIAddressPoolType, gap_limit: c_uint, error: *mut FFIError,) -> bool
Set the gap limit for an address pool.
The gap limit is the number of trailing unused addresses to maintain. Changing it may cause address generation up to the new target; it never trims existing addresses.
# Parameters
- `network`: target network.
- `account_type` / `account_index`: select account (index ignored for non-indexed types).
- `pool_type`: External / Internal / Single.
- `gap_limit`: new limit (must be > 0).
# Safety
- `managed_wallet` must be a valid `FFIManagedWalletInfo`.
- `error` may be null or valid.
🤖 Prompt for AI Agents
In key-wallet-ffi/FFI_API.md around lines 1036 to 1044, the gap_limit parameter
is under-documented: add precise documentation covering (1) valid numeric range
(min/max and behaviour on out-of-range values), (2) how gap_limit applies to a
specific account_index/network/account_type/pool_type selector and whether
providing global vs per-pool semantics are supported, (3) the exact behavior
when changing gap_limit — whether it triggers address generation to extend the
pool, trimming of unused addresses, or neither, and whether this runs
synchronously or schedules background work, (4) edge cases such as negative/zero
values, extremely large values, and concurrent calls, and (5) what error codes
are returned via FFIError for invalid selectors or failures; update the Safety
section to mention side effects (address generation/trimming) and
threading/ownership expectations and include example acceptable values and
expected post-call state for callers to rely on.

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
key-wallet-ffi/include/key_wallet_ffi_test.h (1)

1461-1468: Fix ABI mismatch: use FFINetwork (not FFINetworks) in test header prototypes

Rust extern "C" functions in key-wallet-ffi/src use FFINetwork; key-wallet-ffi/include/key_wallet_ffi_test.h declares FFINetworks for those prototypes and does not declare FFINetwork — this misdeclares the ABI and can corrupt arguments. Change the prototypes to FFINetwork and ensure FFINetwork is emitted into the test header by re-running cbindgen with the same config as the public header. Update nearby “# Safety” notes that reference FFIManagedWallet (use FFIManagedWalletInfo).

Edit these test-header locations (replace FFINetworks -> FFINetwork):

  • key-wallet-ffi/include/key_wallet_ffi_test.h: lines 1461-1468 (managed_wallet_get_address_pool_info)
  • key-wallet-ffi/include/key_wallet_ffi_test.h: lines 1482-1489 (managed_wallet_set_gap_limit)
  • key-wallet-ffi/include/key_wallet_ffi_test.h: lines 1505-1513 (managed_wallet_generate_addresses_to_index)
  • key-wallet-ffi/include/key_wallet_ffi_test.h: lines 1528-1532 (managed_wallet_mark_address_used)
  • key-wallet-ffi/include/key_wallet_ffi_test.h: lines 3069-3081 (managed_wallet_check_transaction)

Reference Rust declarations to verify parity:

  • key-wallet-ffi/src/address_pool.rs (managed_wallet_get_address_pool_info, managed_wallet_set_gap_limit, managed_wallet_generate_addresses_to_index, managed_wallet_mark_address_used)
  • key-wallet-ffi/src/transaction_checking.rs (managed_wallet_check_transaction)
♻️ Duplicate comments (1)
key-wallet-ffi/include/key_wallet_ffi_test.h (1)

427-431: FFIManagedWallet alias to FFIManagedWalletInfo looks good for compatibility.

The typedef cleanly preserves source/ABI compatibility while consolidating on the canonical wrapper.

🧹 Nitpick comments (1)
dash-spv-ffi/build.rs (1)

18-20: Log the actual header file path, not just the directory.

Current message points to the include dir; print the concrete file path.

Apply:

-            bindings.write_to_file(output_path.join("dash_spv_ffi.h"));
-            println!("cargo:warning=Generated C header at {:?}", output_path);
+            let out = output_path.join("dash_spv_ffi.h");
+            bindings.write_to_file(&out);
+            println!("cargo:warning=Generated C header at {:?}", out);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e22c570 and 13b817b.

📒 Files selected for processing (2)
  • dash-spv-ffi/build.rs (1 hunks)
  • key-wallet-ffi/include/key_wallet_ffi_test.h (8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/build.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/build.rs
  • key-wallet-ffi/include/key_wallet_ffi_test.h
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • dash-spv-ffi/build.rs
  • key-wallet-ffi/include/key_wallet_ffi_test.h
🧠 Learnings (17)
📓 Common learnings
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 : Add a Swift wrapper in FFIBridge.swift for each new FFI function, handling type conversions and cross-language error mapping
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/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly

Applied to files:

  • dash-spv-ffi/build.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/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds

Applied to files:

  • dash-spv-ffi/build.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/build.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/build.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/build.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 : Enforce Rust formatting via `cargo fmt --check` on all Rust source files

Applied to files:

  • dash-spv-ffi/build.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/build.rs
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Pre-PR checks: cargo fmt, cargo clippy, cargo test (workspace); update docs/CHANGELOG for user-facing changes

Applied to files:

  • dash-spv-ffi/build.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/**/Cargo.toml : Set MSRV to 1.89 using the `rust-version` field and keep the code compatible with Rust 1.89

Applied to files:

  • dash-spv-ffi/build.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/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function

Applied to files:

  • dash-spv-ffi/build.rs
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/*.rs : Lint with clippy; treat warnings as errors in CI

Applied to files:

  • dash-spv-ffi/build.rs
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Mirror strict CI locally if helpful: export RUSTFLAGS="-D warnings"

Applied to files:

  • dash-spv-ffi/build.rs
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#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_test.h
📚 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/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation

Applied to files:

  • key-wallet-ffi/include/key_wallet_ffi_test.h
📚 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/transaction_checking/**/*.rs : Use transaction type routing to check only relevant accounts

Applied to files:

  • key-wallet-ffi/include/key_wallet_ffi_test.h
📚 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/ffi/**/*.rs : Expose safe C FFI with #[no_mangle] extern "C" functions only for vetted APIs

Applied to files:

  • key-wallet-ffi/include/key_wallet_ffi_test.h
🧬 Code graph analysis (1)
key-wallet-ffi/include/key_wallet_ffi_test.h (2)
key-wallet-ffi/src/address_pool.rs (4)
  • managed_wallet_get_address_pool_info (248-357)
  • managed_wallet_set_gap_limit (369-464)
  • managed_wallet_generate_addresses_to_index (478-631)
  • managed_wallet_mark_address_used (644-800)
key-wallet-ffi/src/transaction_checking.rs (3)
  • wallet_create_managed_wallet (74-93)
  • managed_wallet_check_transaction (109-427)
  • ffi_managed_wallet_free (459-462)
⏰ 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). (19)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: Strict Warnings and Clippy Checks
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Core Components Tests
  • GitHub Check: SPV Components Tests
🔇 Additional comments (5)
dash-spv-ffi/build.rs (3)

10-12: Rebuild triggers added — good.

Adding explicit rerun-if-changed hooks for cbindgen config and sources is correct.


22-25: Fail-fast on header generation errors — good.

Panic here prevents stale headers from slipping through.


16-16: No change required — cbindgen language already pinned to C

Found language = "C" in dash-spv-ffi/cbindgen.toml and key-wallet-ffi/cbindgen.toml, so adding .with_language(cbindgen::Language::C) is redundant.

key-wallet-ffi/include/key_wallet_ffi_test.h (2)

3051-3051: wallet_create_managed_wallet now returns FFIManagedWalletInfo: aligned with Rust.*

Return type matches the extern "C" function; good.


3101-3101: ffi_managed_wallet_free accepts FFIManagedWalletInfo: correct aliasing to canonical free.*

Matches the Rust shim that forwards to managed_wallet_info_free.

Copy link
Member Author

@QuantumExplorer QuantumExplorer left a comment

Choose a reason for hiding this comment

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

Self Reviewed

@QuantumExplorer QuantumExplorer changed the title refactor: clean ffi refactor: cleaned ffi to remove need for hacks when building unified header for swift library Sep 11, 2025
@QuantumExplorer QuantumExplorer merged commit befd035 into v0.40-dev Sep 11, 2025
27 of 28 checks passed
@QuantumExplorer QuantumExplorer deleted the refactor/cleanFFI branch September 11, 2025 05:09
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