refactor: removed watch only unused #120
Conversation
WalkthroughThe PR removes the wallet FFI surface from dash-spv-ffi, switches balance/UTXO FFI types to key-wallet-ffi, deletes watch-item APIs, updates headers/tests accordingly, adjusts balance fields, removes wallet module and tests, updates peer_reputation.json, revises Swift bridges to drop watch items, and renames derivation enums in key-wallet-ffi. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Swift as Swift SPVClient
participant CFFI as dash_spv_ffi (C ABI)
participant Rust as dash-spv-ffi (Rust)
participant KW as key-wallet-ffi (FFI Types)
App->>Swift: getTotalBalance()
Swift->>CFFI: dash_spv_ffi_client_get_total_balance()
CFFI->>Rust: client.get_total_balance()
Rust->>KW: Construct FFIBalance (confirmed, unconfirmed, immature, total)
KW-->>Rust: FFIBalance*
Rust-->>CFFI: FFIBalance*
CFFI-->>Swift: FFIBalance*
Swift-->>App: Read fields (no destroy required)
note over Rust,KW: UTXO queries return KW::FFIUTXO via conversion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
dash-spv-ffi/peer_reputation.json (1)
1-79: Sync peer_reputation.json between dash-spv and dash-spv-ffi
- FFI-only entries in dash-spv-ffi/peer_reputation.json:
• 34.220.134.30:19999
• 34.222.21.14:19999- Connection_attempts mismatches:
• 18.237.170.32:19999 (SPV=1 vs FFI=2)
• 34.210.84.163:19999 (SPV=3 vs FFI=1)
• 34.214.48.68:19999 (SPV=2 vs FFI=1)Update dash-spv-ffi/peer_reputation.json to exactly mirror dash-spv/peer_reputation.json.
dash-spv-ffi/tests/test_mempool_tracking.rs (1)
118-136: Use-after-free risk: user_data freed while client may still hold it.You free test_callbacks (Box::from_raw) immediately after setting callbacks; if the client triggers a callback later, user_data dangles. Free it after destroying the client.
Apply this diff:
- // Clean up - let _ = Box::from_raw(test_callbacks_ptr); - dash_spv_ffi_client_destroy(client); - dash_spv_ffi_config_destroy(config); + // Clean up: destroy client before releasing user_data to avoid UAF + dash_spv_ffi_client_destroy(client); + dash_spv_ffi_config_destroy(config); + let _ = Box::from_raw(test_callbacks_ptr);swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (1)
318-321: UTXO arrays contain nested heap pointers: provide a typed deep-free.Add dash_spv_ffi_utxos_destroy(FFIArray*) to free address/script buffers per element, then the array. Document to use it instead of dash_spv_ffi_array_destroy for UTXO arrays.
struct FFIArray dash_spv_ffi_client_get_utxos(struct FFIDashSpvClient *client); struct FFIArray dash_spv_ffi_client_get_utxos_for_address(struct FFIDashSpvClient *client, const char *address); +/* Deep-destroy a UTXO FFIArray (frees nested address/script buffers) */ +void dash_spv_ffi_utxos_destroy(struct FFIArray *arr);Also applies to: 366-371
♻️ Duplicate comments (2)
dash-spv-ffi/tests/test_client.rs (1)
159-161: Same FFIBalance ownership concern as noted elsewhere.Comment assumes no destruction needed; ensure it matches the FFI contract to avoid leaks/UB.
See the ownership comment in dash-spv-ffi/tests/test_event_callbacks.rs Lines 283-285.
dash-spv-ffi/tests/test_mempool_tracking.rs (1)
173-178: Same FFIBalance ownership concern as noted elsewhere.Pointer is borrowed without destruction; confirm lifetime/ownership and provide destroy or return-by-value accordingly.
See the ownership comment in dash-spv-ffi/tests/test_event_callbacks.rs Lines 283-285.
🧹 Nitpick comments (17)
dash-spv/peer_reputation.json (3)
25-34: New peer 34.210.84.163 has attempts=3 — mismatch with FFI (1).Either normalize to the same value across components or confirm separate heuristics per module.
46-56: Demotion of 18.237.170.32 to end with attempts=1.FFI has attempts=2 for the same peer. Please verify whether the difference is intentional. Consider consolidating a single reputation store or a sync step to avoid drift.
1-57: Set-size divergence vs. FFI (5 entries here vs. 7 in FFI).FFI includes 34.222.21.14 and 34.220.134.30 that are absent here. If both files seed peers for the same network (port 19999), consider harmonizing the peer sets or explaining the role-based split. Also consider sorting deterministically (e.g., by IP) to reduce churn in diffs.
dash-spv-ffi/peer_reputation.json (5)
3-12: 34.214.48.68 attempts=1 conflicts with dash-spv (2).Align the count or confirm independent reputations per component.
14-23: Peer 34.222.21.14 exists only in FFI.If intentional (FFI-only seed), add a brief note in docs/commit message; otherwise add to dash-spv list for parity.
25-34: Peer 34.220.134.30 exists only in FFI.Same parity concern as above.
47-56: 34.210.84.163 attempts=1 conflicts with dash-spv (3).Suggest normalizing or clarifying policy for initializing attempts across files.
1-79: Network parity and determinism.All peers use port 19999 (testnet). If both files target the same environment, consider a single source of truth or an automated sync step. Deterministic ordering (e.g., lexicographic by IP:port) would also minimize noisy diffs.
dash-spv-ffi/include/dash_spv_ffi.h (1)
372-372: Public function always returns nulldash_spv_ffi_client_get_balance_with_mempool() is declared here but the impl returns null with a “not available” error. Consider removing or marking deprecated to avoid confusion.
-FFIBalance *dash_spv_ffi_client_get_balance_with_mempool(struct FFIDashSpvClient *client); +// Deprecated: not available in current dash-spv version; will always return NULL and set last error. +FFIBalance *dash_spv_ffi_client_get_balance_with_mempool(struct FFIDashSpvClient *client);dash-spv-ffi/tests/test_mempool_tracking.rs (1)
175-178: Outdated comment references removed mempool fields.Update comment to reflect new fields (unconfirmed/immature).
- assert_eq!(balance_ref.confirmed, 0); // No confirmed balance in mempool - // mempool and mempool_instant fields contain the actual mempool balance + assert_eq!(balance_ref.confirmed, 0); // No confirmed balance in mempool + // Unconfirmed (and possibly immature) carry mempool exposure nowkey-wallet-ffi/src/derivation.rs (3)
512-515: Doc fix: free function name mismatch.Documentation mentions extended_public_key_free; the actual symbol is derivation_xpub_free.
-/// - The returned pointer must be freed with `extended_public_key_free` +/// - The returned pointer must be freed with `derivation_xpub_free`
550-551: Doc fix: string free function name mismatch.Use derivation_string_free in docs to match the exported symbol.
-/// - The returned string must be freed with `string_free` +/// - The returned string must be freed with `derivation_string_free`
690-697: Doc fix: name the correct string free function.Aligns the docs with derivation_string_free().
-/// - The pointer must not be used after calling this function -/// - This function must only be called once per allocation +/// - The pointer must not be used after calling this function +/// - This function must only be called once per allocation +/// - Use `derivation_string_free` for strings returned by this moduledash-spv-ffi/src/client.rs (2)
1864-1869: Mempool balance mapping ok; add destroyer note.Field mapping matches the new FFIBalance schema; just ensure the new dash_spv_ffi_balance_destroy exists.
20-21: Minor: remove redundant local import of CString.CString is already imported at file scope; keep imports consistent.
- use std::ffi::CString;key-wallet-ffi/include/key_wallet_ffi.h (1)
133-150: Enum rename is ABI-safe (same discriminants); call out breaking API for source code.Numeric values are preserved, but C/Swift code using old identifiers will fail to compile. Provide a short migration note or temporary macros mapping old names to PATH_* to ease the transition.
+/* Temporary compatibility aliases – consider removing in a future release */ +#define UNKNOWN PATH_UNKNOWN +#define BIP32 PATH_BIP32 +#define BIP44 PATH_BIP44 +/* ...continue as needed for removed identifiers... */swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (1)
372-380: Docs: balance_with_mempool currently returns NULL with error set.Consider adding a brief doc comment here to set expectations for callers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (23)
dash-spv-ffi/Cargo.toml(1 hunks)dash-spv-ffi/README.md(0 hunks)dash-spv-ffi/include/dash_spv_ffi.h(3 hunks)dash-spv-ffi/peer_reputation.json(5 hunks)dash-spv-ffi/src/client.rs(6 hunks)dash-spv-ffi/src/lib.rs(0 hunks)dash-spv-ffi/src/wallet.rs(0 hunks)dash-spv-ffi/tests/c_tests/test_advanced.c(0 hunks)dash-spv-ffi/tests/test_client.rs(1 hunks)dash-spv-ffi/tests/test_event_callbacks.rs(2 hunks)dash-spv-ffi/tests/test_mempool_tracking.rs(1 hunks)dash-spv-ffi/tests/test_wallet.rs(0 hunks)dash-spv-ffi/tests/unit/test_client_lifecycle.rs(1 hunks)dash-spv-ffi/tests/unit/test_wallet_operations.rs(0 hunks)dash-spv/peer_reputation.json(1 hunks)docs/implementation-notes/WALLET_SPV_INTEGRATION.md(0 hunks)key-wallet-ffi/include/key_wallet_ffi.h(1 hunks)key-wallet-ffi/src/derivation.rs(5 hunks)key-wallet-ffi/src/derivation_tests.rs(3 hunks)key-wallet-ffi/src/lib.rs(1 hunks)swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h(4 hunks)swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swift(1 hunks)swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift(0 hunks)
💤 Files with no reviewable changes (8)
- docs/implementation-notes/WALLET_SPV_INTEGRATION.md
- swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
- dash-spv-ffi/tests/unit/test_wallet_operations.rs
- dash-spv-ffi/README.md
- dash-spv-ffi/tests/c_tests/test_advanced.c
- dash-spv-ffi/tests/test_wallet.rs
- dash-spv-ffi/src/lib.rs
- dash-spv-ffi/src/wallet.rs
🧰 Additional context used
📓 Path-based instructions (11)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with the thiserror crate and propagate errors appropriately
Use the tokio runtime for async operations
Adhere to MSRV: code must compile on Rust 1.89 (avoid newer language/features)
Keep code formatted with rustfmt (cargo fmt)
Keep the codebase clippy-clean (clippy with -D warnings)
Files:
key-wallet-ffi/src/lib.rsdash-spv-ffi/tests/unit/test_client_lifecycle.rskey-wallet-ffi/src/derivation_tests.rsdash-spv-ffi/tests/test_mempool_tracking.rskey-wallet-ffi/src/derivation.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rsdash-spv-ffi/src/client.rs
{dash-network-ffi/**/*.rs,dash-spv-ffi/**/*.rs,key-wallet-ffi/**/*.rs,swift-dash-core-sdk/**/*.{swift,m,mm,c,h}}
📄 CodeRabbit inference engine (CLAUDE.md)
Be especially careful with memory safety at FFI boundaries
Files:
key-wallet-ffi/src/lib.rsdash-spv-ffi/tests/unit/test_client_lifecycle.rsswift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swiftkey-wallet-ffi/src/derivation_tests.rsdash-spv-ffi/tests/test_mempool_tracking.rskey-wallet-ffi/src/derivation.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rsdash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
{**/*.rs,**/Cargo.toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Use feature flags and conditional compilation for optional features
Files:
key-wallet-ffi/src/lib.rsdash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/Cargo.tomlkey-wallet-ffi/src/derivation_tests.rsdash-spv-ffi/tests/test_mempool_tracking.rskey-wallet-ffi/src/derivation.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rsdash-spv-ffi/src/client.rs
{key-wallet/**/*.rs,key-wallet-ffi/**/*.rs,swift-dash-core-sdk/**/*.swift}
📄 CodeRabbit inference engine (CLAUDE.md)
{key-wallet/**/*.rs,key-wallet-ffi/**/*.rs,swift-dash-core-sdk/**/*.swift}: Use secure random number generation for keys
Never log or expose private keys
Files:
key-wallet-ffi/src/lib.rsswift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swiftkey-wallet-ffi/src/derivation_tests.rskey-wallet-ffi/src/derivation.rs
dash-spv-ffi/tests/unit/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Add a corresponding Rust unit test for each new FFI function
Files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Write unit tests for new functionality
Use proptest for property-based testing where appropriate
Files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/tests/test_mempool_tracking.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rs
swift-dash-core-sdk/Sources/**/FFIBridge.swift
📄 CodeRabbit inference engine (swift-dash-core-sdk/CLAUDE.md)
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
At the FFI boundary, convert FFI errors to DashSDKError and use Result types; log FFI errors before converting
Manage FFI pointers manually using RAII-style wrappers and defer blocks to ensure cleanup
Files:
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swift
swift-dash-core-sdk/Sources/**/*.swift
📄 CodeRabbit inference engine (swift-dash-core-sdk/CLAUDE.md)
swift-dash-core-sdk/Sources/**/*.swift: Use actors for state management to maintain thread safety in SDK components
Use @available checks for features that require newer OS versions (e.g., SwiftData on iOS 17+)
Files:
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swift
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/client.rs
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
dash-spv-ffi/include/dash_spv_ffi.h
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
The public C header is generated; ensure it reflects FFI changes after builds
Files:
dash-spv-ffi/include/dash_spv_ffi.h
🧠 Learnings (28)
📓 Common learnings
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
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:15.391Z
Learning: Applies to {dash-network-ffi/**/*.rs,dash-spv-ffi/**/*.rs,key-wallet-ffi/**/*.rs,swift-dash-core-sdk/**/*.{swift,m,mm,c,h}} : Be especially careful with memory safety at FFI boundaries
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
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/**/SPVClient.swift : Do not place type definitions in SPVClient.swift; keep types in dedicated files instead
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h : After updating dash-spv-ffi, synchronize the generated header into the Swift SDK using ./sync-headers.sh so Sources/DashSPVFFI/include/dash_spv_ffi.h stays up to date
📚 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/tests/unit/test_client_lifecycle.rsdash-spv-ffi/Cargo.tomldash-spv-ffi/tests/test_mempool_tracking.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rsdash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add a corresponding Rust unit test for each new FFI function
Applied to files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/Cargo.tomldash-spv-ffi/tests/test_mempool_tracking.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-08-25T17:15:15.391Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:15.391Z
Learning: Applies to {dash-network,dash-spv}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Functions returning pointers transfer ownership to the caller; functions taking pointers only borrow
Applied to files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/tests/test_mempool_tracking.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-08-25T17:15:15.391Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:15.391Z
Learning: Applies to {dash-network-ffi/**/*.rs,dash-spv-ffi/**/*.rs,key-wallet-ffi/**/*.rs,swift-dash-core-sdk/**/*.{swift,m,mm,c,h}} : Be especially careful with memory safety at FFI boundaries
Applied to files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/Cargo.tomldash-spv-ffi/tests/test_mempool_tracking.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)
Applied to files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/Cargo.tomldash-spv-ffi/tests/test_mempool_tracking.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rsdash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Return Rust strings to C as *const c_char; allocate so the caller can free with dash_string_free
Applied to files:
dash-spv-ffi/tests/unit/test_client_lifecycle.rsdash-spv-ffi/src/client.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/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.tomldash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : FFI functions must be declared with #[no_mangle] extern "C"
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/client.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/Cargo.tomlswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly
Applied to files:
dash-spv-ffi/Cargo.tomldash-spv-ffi/src/client.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/Cargo.tomldash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rs
📚 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/Cargo.tomldash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-25T17:15:15.391Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:15.391Z
Learning: Applies to {key-wallet/**/*.rs,key-wallet-ffi/**/*.rs,swift-dash-core-sdk/**/*.swift} : Use secure random number generation for keys
Applied to files:
dash-spv-ffi/Cargo.toml
📚 Learning: 2025-08-25T17:15:15.391Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:15.391Z
Learning: Applies to {key-wallet/**/*.rs,key-wallet-ffi/**/*.rs,swift-dash-core-sdk/**/*.swift} : Never log or expose private keys
Applied to files:
dash-spv-ffi/Cargo.toml
📚 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/SwiftDashCoreSDK/Core/FFIBridge.swift
📚 Learning: 2025-08-16T04:15:29.335Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:15:29.335Z
Learning: Applies to swift-dash-core-sdk/Sources/**/FFIBridge.swift : Manage FFI pointers manually using RAII-style wrappers and defer blocks to ensure cleanup
Applied to files:
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swift
📚 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/SwiftDashCoreSDK/Core/FFIBridge.swift
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv-ffi/tests/test_mempool_tracking.rsdash-spv-ffi/tests/test_event_callbacks.rsdash-spv-ffi/tests/test_client.rsdash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_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/derivation.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/tests/**/*.rs : Place integration tests under the `tests/` directory
Applied to files:
dash-spv-ffi/tests/test_event_callbacks.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/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
Applied to files:
dash-spv-ffi/tests/test_client.rs
📚 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/**/SPVClient.swift : Do not place type definitions in SPVClient.swift; keep types in dedicated files instead
Applied to files:
dash-spv-ffi/src/client.rsswift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:47:37.438Z
Learning: Transaction IDs (txids) and block hashes in Dash are always 32 bytes and should be passed as fixed-size byte arrays in FFI interfaces, not as C strings, to avoid inefficiency and encoding issues.
Applied to files:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/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:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.hdash-spv-ffi/include/dash_spv_ffi.h
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/tests/c_tests/**/*.c : In C code, free all strings returned by the FFI using dash_string_free
Applied to files:
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/c_tests/**/*.c : Add a corresponding C test for each new FFI function
Applied to files:
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
🧬 Code graph analysis (4)
dash-spv-ffi/tests/test_event_callbacks.rs (1)
dash-spv/src/types.rs (1)
total(635-637)
dash-spv-ffi/src/client.rs (2)
dash-spv-ffi/src/error.rs (1)
set_last_error(26-31)dash-spv-ffi/src/types.rs (2)
new(15-22)new(301-313)
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (1)
dash-spv-ffi/src/client.rs (6)
txid(319-319)dash_spv_ffi_client_get_address_balance(965-1029)dash_spv_ffi_client_get_total_balance(1592-1626)dash_spv_ffi_client_get_balance_with_mempool(1732-1742)dash_spv_ffi_client_record_send(1774-1819)dash_spv_ffi_client_get_mempool_balance(1822-1877)
dash-spv-ffi/include/dash_spv_ffi.h (1)
dash-spv-ffi/src/client.rs (6)
dash_spv_ffi_client_get_address_balance(965-1029)dash_spv_ffi_client_get_total_balance(1592-1626)dash_spv_ffi_client_get_balance_with_mempool(1732-1742)dash_spv_ffi_client_record_send(1774-1819)txid(319-319)dash_spv_ffi_client_get_mempool_balance(1822-1877)
⏰ 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). (10)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Clippy (Non-strict)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: SPV Components Tests
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
🔇 Additional comments (18)
dash-spv/peer_reputation.json (2)
14-23: Confirm intent behind increased connection_attempts for 34.214.48.68.Attempts were bumped to 2 here but remain 1 in dash-spv-ffi. If these datasets are meant to stay in sync, align counts or document why histories diverge.
35-45: 34.217.58.158 attempts=2 matches FFI; LGTM.Consistent across files.
dash-spv-ffi/peer_reputation.json (3)
36-45: 18.237.170.32 attempts=2 conflicts with dash-spv (1).Please reconcile or justify the divergence.
58-67: 34.217.58.158 attempts=2 — consistent with dash-spv.No issues.
68-78: Addition of 34.210.26.195 with fresh counters — consistent with dash-spv.Looks good.
key-wallet-ffi/src/lib.rs (1)
31-34: Re-exports OK, but ensure dash-spv-ffi header can “see” these typesThese are useful, but cbindgen for dash-spv-ffi won’t emit FFIBalance/FFIUTXO unless they’re re-exported in that crate too. Without that, dash_spv_ffi.h will reference unknown types.
Suggest re-exporting in dash-spv-ffi/src/lib.rs:
+// Make wallet FFI types visible to cbindgen of this crate +pub use key_wallet_ffi::{FFIBalance, FFIUTXO};Then regenerate headers and sync to Swift with ./sync-headers.sh. Do you want me to wire this through and update headers?
key-wallet-ffi/src/derivation_tests.rs (2)
310-313: Enum rename usage is correctSwitch to PathBlockchainIdentities aligns with the new API; test remains valid.
773-774: Consistent enum variant across testsBoth call sites updated consistently; no further changes needed.
Also applies to: 797-798
dash-spv-ffi/tests/test_event_callbacks.rs (1)
279-281: Balance fields log update matches new layout.Using confirmed/unconfirmed/total reads correctly.
dash-spv-ffi/tests/test_client.rs (1)
156-158: Total balance assertion updated correctly.Sum now uses confirmed + unconfirmed + immature.
key-wallet-ffi/src/derivation.rs (3)
732-770: DIP9 base path match updated correctly to Path variants.*Mapping per network looks consistent.
Consider adding a brief comment noting that PathRoot (255) is intentionally reserved and not used here.
774-845: Additional-path construction aligns with new variants and keeps hardened indices.Error mapping and hardened index creation are correct.
15-31: FFIDerivationPathType mapping is correct – confirm external consumers updatedVerified that key_wallet_ffi.h defines PATH_* values matching each Rust Path* variant (0‒15 and 255). No internal C/ObjC/Swift references found in this repo; please ensure any downstream Swift/ObjC clients are updated to use the renamed variants.
dash-spv-ffi/src/client.rs (3)
9-16: LGTM: balance mapping helper.Mapping AddressBalance → FFIBalance with immature=0 is coherent with the new layout.
1848-1858: LGTM: async retrieval of MempoolBalance with proper error propagation.The await/guard pattern and error wiring look good.
1051-1053: Add a typed destructor for FFI UTXO arrays
ReturningFFIArray<FFIUtxo>(which owns heap-allocated fields) requires a deep-free function—define and exposedash_spv_ffi_utxos_destroy(to be called instead of the genericdash_spv_ffi_array_destroy), or delegateget_utxosto key-wallet-ffi’smanaged_wallet_get_utxosand reuse itsutxo_array_free.swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (2)
129-143: LGTM: new callback types for filter match and wallet transactions.Signatures align with client-side usage patterns.
1-20: Header sync reminder.After these additions, please rerun ./sync-headers.sh so this mirror matches the generated dash_spv_ffi.h.
| # Use key-wallet-ffi for all wallet-related FFI types | ||
| key-wallet-ffi = { path = "../key-wallet-ffi" } | ||
| # Still need these for SPV client internals (not for FFI types) | ||
| key-wallet = { path = "../key-wallet" } | ||
| key-wallet-manager = { path = "../key-wallet-manager" } | ||
| rand = "0.8" |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Dependency switch is correct; also re-export the types in this crate
Using key-wallet-ffi for FFI types is right. Please also re-export FFIBalance/FFIUTXO from dash-spv-ffi so cbindgen emits opaque declarations in dash_spv_ffi.h.
Proposed addition in dash-spv-ffi/src/lib.rs (new file change):
// Ensure wallet FFI types are visible to this crate's header
pub use key_wallet_ffi::{FFIBalance, FFIUTXO};Then regenerate headers and sync into Swift SDK (Sources/DashSPVFFI/include/dash_spv_ffi.h).
🏁 Script executed:
#!/bin/bash
# After re-export, verify header now includes typedef struct FFIBalance/FFIUTXO
rg -n '^typedef\s+struct\s+FFI(Balance|UTXO)\b' dash-spv-ffi/include/dash_spv_ffi.hLength of output: 85
🏁 Script executed:
#!/bin/bash
# Verify that lib.rs re-exports FFIBalance and FFIUTXO from key-wallet-ffi
rg -nP '^\s*pub\s+use\s+key_wallet_ffi::\{\s*FFIBalance\s*,\s*FFIUTXO\s*\}' dash-spv-ffi/src/lib.rsLength of output: 101
Add missing FFI type re-exports for cbindgen
In dash-spv-ffi/src/lib.rs, add:
pub use key_wallet_ffi::{FFIBalance, FFIUTXO};Then regenerate the C header (cbindgen) and sync it into the Swift SDK (e.g. via ./sync-headers.sh).
rg -n '^typedef\s\+struct\s\+FFI\(Balance\|UTXO\)\b' dash-spv-ffi/include/dash_spv_ffi.h🤖 Prompt for AI Agents
In dash-spv-ffi/Cargo.toml around lines 26-31, the review requests adding
missing FFI type re-exports: open dash-spv-ffi/src/lib.rs and add a public
re-export line for the FFI types (pub use key_wallet_ffi::{FFIBalance,
FFIUTXO};), then regenerate the C header with cbindgen and sync it into the
Swift SDK (e.g. run ./sync-headers.sh). After that run the provided verification
command (rg -n '^typedef\s\+struct\s\+FFI\(Balance\|UTXO\)\b'
dash-spv-ffi/include/dash_spv_ffi.h) to confirm the typedefs are present.
| FFIBalance *dash_spv_ffi_client_get_address_balance(struct FFIDashSpvClient *client, | ||
| const char *address); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
dash_spv_ffi.h references FFIBalance but never declares it (C compile break)
Functions return FFIBalance* but there’s no forward-declaration/definition of FFIBalance in this header. C consumers can’t compile unless they also include key_wallet_ffi.h (which they currently don’t).
Two options (pick one):
- Preferred: Re-export FFIBalance (and FFIUTXO) from dash-spv-ffi Rust crate so cbindgen emits opaque declarations in this header.
- Minimal: Manually add forward-decls to the generated header (less ideal since header is generated).
Header-side patch (if you go minimal):
@@
#include <stdlib.h>
+// Opaque wallet FFI types used by this API
+typedef struct FFIBalance FFIBalance;
+typedef struct FFIUTXO FFIUTXO;Rust-side (preferred) already provided in a sibling comment; then regenerate this header. Want me to push a cbindgen-friendly re-export and regenerate?
Also applies to: 353-353, 372-372, 378-379
🏁 Script executed:
#!/bin/bash
# Verify the header declares FFIBalance before first use
rg -n '^typedef\s+struct\s+FFIBalance\b' dash-spv-ffi/include/dash_spv_ffi.h
rg -n 'FFIBalance\s*\*dash_spv_ffi_client_get_' dash-spv-ffi/include/dash_spv_ffi.h -nLength of output: 528
Expose FFIBalance and FFIUTXO in dash-spv-ffi.h
C header declares functions returning FFIBalance* (lines 315, 353, 372, 378) but never forward-declares FFIBalance (or FFIUTXO), so C consumers will fail to compile.
Preferred: re-export FFIBalance and FFIUTXO from the Rust crate (cbindgen) and regenerate the header.
Minimal: insert after #include <stdlib.h>:
// Opaque wallet FFI types used by this API
typedef struct FFIBalance FFIBalance;
typedef struct FFIUTXO FFIUTXO;🤖 Prompt for AI Agents
In dash-spv-ffi/include/dash_spv_ffi.h around lines 315 to 316, the header
declares functions that return FFIBalance* (and other places reference FFIUTXO)
but never forward-declares those opaque types; add forward declarations for
FFIBalance and FFIUTXO immediately after the #include <stdlib.h> (or re-export
them from the Rust crate and regenerate the header via cbindgen as the preferred
solution) so C consumers can compile: add typedef struct FFIBalance FFIBalance;
and typedef struct FFIUTXO FFIUTXO; in the header.
💡 Verification agent
🧩 Analysis chain
Missing destroy for FFIBalance → leaks*
get_address_balance/total_balance/balance_with_mempool/mempool_balance return owning pointers but no dash_spv_ffi_balance_destroy() is exposed.
Add a destructor and call it in tests/examples:
@@
FFIBalance *dash_spv_ffi_client_get_mempool_balance(struct FFIDashSpvClient *client,
const char *address);
+
+// Destroys a balance returned by the above functions
+void dash_spv_ffi_balance_destroy(FFIBalance *bal);Implement in dash-spv-ffi/src/client.rs:
#[no_mangle]
pub unsafe extern "C" fn dash_spv_ffi_balance_destroy(bal: *mut FFIBalance) {
if !bal.is_null() {
drop(Box::from_raw(bal));
}
}I can add the impl + tests and wire the Swift bridge to call it.
Also applies to: 353-353, 372-372, 378-379
🏁 Script executed:
#!/bin/bash
# Find raw allocations of FFIBalance
rg -n 'Box::into_raw\(Box::new\(\s*FFIBalance' dash-spv-ffi/src
# Confirm a destroy symbol exists
rg -n 'dash_spv_ffi_balance_destroy' dash-spv-ffiLength of output: 1013
Add missing dash_spv_ffi_balance_destroy to avoid memory leaks
- Declare in dash-spv-ffi/include/dash_spv_ffi.h:
// Destroys a balance returned by the C-FFI void dash_spv_ffi_balance_destroy(FFIBalance *bal);
- Implement in dash-spv-ffi/src/client.rs with
#[no_mangle] extern "C"to drop the boxed pointer. - Un-comment and invoke
dash_spv_ffi_balance_destroyin tests/examples to free all returnedFFIBalance*.
🤖 Prompt for AI Agents
In dash-spv-ffi/include/dash_spv_ffi.h around lines 315-316, add a declaration
for a destructor to free balances returned by the FFI: declare void
dash_spv_ffi_balance_destroy(FFIBalance *bal); then implement this function in
dash-spv-ffi/src/client.rs as a #[no_mangle] extern "C" function that converts
the raw FFIBalance pointer back into the Box and drops it (ensuring null-checks
before unboxing). Finally, un-comment and call dash_spv_ffi_balance_destroy in
tests/examples wherever FFIBalance* values are returned so all allocated
balances are freed to avoid memory leaks.
| // Import wallet types from key-wallet-ffi | ||
| use key_wallet_ffi::{FFIBalance, FFIUTXO as FFIUtxo}; | ||
|
|
There was a problem hiding this comment.
ABI dependency surfaced: ensure headers and linkage include key-wallet-ffi types.
You now return FFIBalance* and build FFIUTXO (alias). The Swift/C header (dash_spv_ffi.h) must either include key_wallet_ffi.h or re-declare FFIBalance (and any used structs), otherwise it won’t compile. Also ensure Cargo exposes key-wallet-ffi to cbindgen so types resolve.
| fn wallet_utxo_to_ffi(utxo: key_wallet::Utxo) -> FFIUtxo { | ||
| use std::ffi::CString; | ||
|
|
||
| // Convert txid to byte array | ||
| let mut txid_bytes = [0u8; 32]; | ||
| txid_bytes.copy_from_slice(&utxo.outpoint.txid[..]); | ||
|
|
||
| // Create FFI UTXO | ||
| let address_str = utxo.address.to_string(); | ||
| let script_bytes = utxo.txout.script_pubkey.to_bytes(); | ||
|
|
||
| FFIUtxo { | ||
| txid: txid_bytes, | ||
| vout: utxo.outpoint.vout, | ||
| amount: utxo.txout.value, | ||
| address: CString::new(address_str).unwrap_or_default().into_raw(), | ||
| script_pubkey: if script_bytes.is_empty() { | ||
| std::ptr::null_mut() | ||
| } else { | ||
| let script_box = script_bytes.into_boxed_slice(); | ||
| Box::into_raw(script_box) as *mut u8 | ||
| }, | ||
| script_len: utxo.txout.script_pubkey.len(), | ||
| height: utxo.height, | ||
| confirmations: if utxo.is_confirmed { | ||
| 1 | ||
| } else { | ||
| 0 | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Deep-freeing required for nested allocations in FFIUtxo; current code leaks.
- address is allocated via CString::into_raw.
- script_pubkey is allocated via Box::into_raw for a slice but stored as u8* + len.
Returned via FFIArray<Vec> there’s no matching deep destructor; dash_spv_ffi_array_destroy will not free nested fields, resulting in leaks and allocator mismatches for consumers. Add a typed destroyer that frees address and script bytes for each element before dropping the vector. Also remove the redundant inner use CString.
Apply this patch (and add the declaration to the public header):
+#[no_mangle]
+pub unsafe extern "C" fn dash_spv_ffi_utxos_destroy(arr: *mut FFIArray) {
+ if arr.is_null() { return; }
+ let a = &mut *arr;
+ if a.data.is_null() { return; }
+ // Rebuild Vec<FFIUtxo>
+ let vec: Vec<FFIUtxo> = Vec::from_raw_parts(a.data as *mut FFIUtxo, a.len as usize, a.capacity as usize);
+ for mut u in vec.into_iter() {
+ if !u.address.is_null() {
+ let _ = CString::from_raw(u.address);
+ }
+ if !u.script_pubkey.is_null() && u.script_len > 0 {
+ // Reconstruct Box<[u8]> and drop
+ let slice = std::ptr::slice_from_raw_parts_mut(u.script_pubkey, u.script_len as usize);
+ let _ = Box::<[u8]>::from_raw(slice);
+ }
+ }
+ // Invalidate array
+ a.data = std::ptr::null_mut();
+ a.len = 0;
+ a.capacity = 0;
+}📝 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.
| fn wallet_utxo_to_ffi(utxo: key_wallet::Utxo) -> FFIUtxo { | |
| use std::ffi::CString; | |
| // Convert txid to byte array | |
| let mut txid_bytes = [0u8; 32]; | |
| txid_bytes.copy_from_slice(&utxo.outpoint.txid[..]); | |
| // Create FFI UTXO | |
| let address_str = utxo.address.to_string(); | |
| let script_bytes = utxo.txout.script_pubkey.to_bytes(); | |
| FFIUtxo { | |
| txid: txid_bytes, | |
| vout: utxo.outpoint.vout, | |
| amount: utxo.txout.value, | |
| address: CString::new(address_str).unwrap_or_default().into_raw(), | |
| script_pubkey: if script_bytes.is_empty() { | |
| std::ptr::null_mut() | |
| } else { | |
| let script_box = script_bytes.into_boxed_slice(); | |
| Box::into_raw(script_box) as *mut u8 | |
| }, | |
| script_len: utxo.txout.script_pubkey.len(), | |
| height: utxo.height, | |
| confirmations: if utxo.is_confirmed { | |
| 1 | |
| } else { | |
| 0 | |
| }, | |
| } | |
| } | |
| // At the bottom of dash-spv-ffi/src/client.rs, after existing functions: | |
| #[no_mangle] | |
| pub unsafe extern "C" fn dash_spv_ffi_utxos_destroy(arr: *mut FFIArray) { | |
| if arr.is_null() { | |
| return; | |
| } | |
| let a = &mut *arr; | |
| if a.data.is_null() { | |
| return; | |
| } | |
| // Rebuild Vec<FFIUtxo> so we take ownership of its elements and buffer | |
| let vec: Vec<FFIUtxo> = Vec::from_raw_parts( | |
| a.data as *mut FFIUtxo, | |
| a.len as usize, | |
| a.capacity as usize, | |
| ); | |
| // For each element, free its nested allocations | |
| for mut u in vec.into_iter() { | |
| if !u.address.is_null() { | |
| // Reclaim and drop the CString | |
| let _ = CString::from_raw(u.address); | |
| } | |
| if !u.script_pubkey.is_null() && u.script_len > 0 { | |
| // Reconstruct Box<[u8]> from the raw slice and drop it | |
| let slice = std::ptr::slice_from_raw_parts_mut( | |
| u.script_pubkey, | |
| u.script_len as usize, | |
| ); | |
| let _ = Box::<[u8]>::from_raw(slice); | |
| } | |
| } | |
| // Invalidate the FFIArray so that the caller doesn't double-free | |
| a.data = std::ptr::null_mut(); | |
| a.len = 0; | |
| a.capacity = 0; | |
| } |
🤖 Prompt for AI Agents
In dash-spv-ffi/src/client.rs around lines 19 to 49, the FFIUtxo construction
allocates nested memory (address via CString::into_raw and script_pubkey via
Box::into_raw) but there is no matching deep destructor so consumers leak memory
and may face allocator mismatches; add a public typed destroy function (e.g.,
dash_spv_ffi_array_ffitxo_destroy or similar) that iterates the returned FFI
array, for each FFIUtxo: if address is non-null reconstruct the CString from_raw
and let it drop, if script_pubkey is non-null reconstruct the boxed slice from
the raw pointer and the stored script_len and drop it, then free the
vector/array container itself; update the crate public header to declare this
new destroyer; also remove the redundant inner "use std::ffi::CString;" inside
the function and prefer a single top-level use if needed.
| Ok(balance) => Box::into_raw(Box::new(address_balance_to_ffi(balance))), | ||
| Err(e) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a destructor for FFIBalance pointers returned to C.
You allocate FFIBalance with Box::into_raw but provide no dash_spv_ffi_balance_destroy; C/Swift cannot free Rust-allocated memory. Provide a destroyer and expose it in the header.
+#[no_mangle]
+pub unsafe extern "C" fn dash_spv_ffi_balance_destroy(ptr: *mut FFIBalance) {
+ if !ptr.is_null() {
+ let _ = Box::from_raw(ptr);
+ }
+}📝 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.
| Ok(balance) => Box::into_raw(Box::new(address_balance_to_ffi(balance))), | |
| Err(e) => { | |
| // Add this somewhere alongside the other FFI destroyers in client.rs | |
| #[no_mangle] | |
| pub unsafe extern "C" fn dash_spv_ffi_balance_destroy(ptr: *mut FFIBalance) { | |
| if !ptr.is_null() { | |
| // Recreate the Box to have it dropped, freeing the memory | |
| let _ = Box::from_raw(ptr); | |
| } | |
| } |
🤖 Prompt for AI Agents
In dash-spv-ffi/src/client.rs around lines 1023-1024, you allocate FFIBalance
with Box::into_raw but never provide a destructor; add a pub extern "C"
no_mangle function named dash_spv_ffi_balance_destroy (or similar) that accepts
a *mut FFIBalance, returns void, checks for null, reconstructs the Box from the
raw pointer (Box::from_raw) and lets it drop to free memory, and expose a
matching declaration in the C/Swift header so callers can free Rust-allocated
FFIBalance instances.
| // Note: Wallet-specific tests have been moved to key-wallet-ffi | ||
| // The wallet functionality is no longer part of dash-spv-ffi | ||
| // dash-spv-ffi now focuses purely on SPV network operations | ||
| println!("⚠️ Wallet tests have been moved to key-wallet-ffi"); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify wallet-callback presence vs. comment.
Either drop on_wallet_transaction from callbacks here or adjust the comment if the field remains in FFIEventCallbacks for compatibility.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'struct\s+FFIEventCallbacks' dash-spv-ffi/include
rg -n 'on_wallet_transaction' dash-spv-ffi/includeLength of output: 419
Clarify wallet transaction callback presence in FFIEventCallbacks. FFIEventCallbacks in dash-spv-ffi/include/dash_spv_ffi.h still defines on_wallet_transaction (line 152); update the comment in dash-spv-ffi/tests/test_event_callbacks.rs (lines 340–343) to note that this callback remains for backward compatibility, or remove the field if it’s truly unused.
🤖 Prompt for AI Agents
In dash-spv-ffi/tests/test_event_callbacks.rs around lines 340 to 343, the
comment incorrectly states wallet tests and functionality were moved without
noting that FFIEventCallbacks still defines on_wallet_transaction for backward
compatibility; update the comment to explain that on_wallet_transaction remains
in dash-spv-ffi/include/dash_spv_ffi.h (line 152) for backward compatibility (or
remove that field from the header if you confirm it is unused across callers),
and ensure the test comment reflects the chosen action (either “wallet tests
moved; callback retained for backward compatibility” or remove the callback and
update both header and tests accordingly).
| // FFIBalance from key-wallet-ffi doesn't need explicit destruction | ||
| // dash_spv_ffi_balance_destroy(balance); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Reintroduced leak: balance handle not freed
With FFIBalance* returned, dropping the explicit destroy leaks memory.
Until we switch to out-params/by-value, restore destruction:
- if (!balance.is_null()) {
- // FFIBalance from key-wallet-ffi doesn't need explicit destruction
- // dash_spv_ffi_balance_destroy(balance);
- }
+ if !balance.is_null() {
+ dash_spv_ffi_balance_destroy(balance);
+ }🏁 Script executed:
#!/bin/bash
# Ensure all tests free FFIBalance*
rg -n 'get_address_balance\([^)]*\)' dash-spv-ffi/tests | rg -nv 'balance_destroy'Length of output: 837
Restore FFIBalance destruction in all tests
All calls to dash_spv_ffi_client_get_address_balance currently leak the returned FFIBalance*. After each call, add:
if (!balance.is_null()) {
dash_spv_ffi_balance_destroy(balance);
}Apply to:
- dash-spv-ffi/tests/test_client.rs:151
- dash-spv-ffi/tests/unit/test_client_lifecycle.rs:202
- dash-spv-ffi/tests/integration/test_full_workflow.rs:210, 341
- dash-spv-ffi/tests/c_tests/test_advanced.c:43
🤖 Prompt for AI Agents
In dash-spv-ffi/tests/unit/test_client_lifecycle.rs around lines 204 to 206, the
explicit destruction of the returned FFIBalance was commented out causing a
memory leak; restore proper teardown by adding a null-check and calling
dash_spv_ffi_balance_destroy on the returned pointer after each
dash_spv_ffi_client_get_address_balance call (i.e., if the pointer is not null,
call dash_spv_ffi_balance_destroy). Apply the same fix to the other locations
mentioned: dash-spv-ffi/tests/test_client.rs at line ~151,
dash-spv-ffi/tests/integration/test_full_workflow.rs at lines ~210 and ~341, and
dash-spv-ffi/tests/c_tests/test_advanced.c at line ~43, ensuring each call is
followed by a null-check then dash_spv_ffi_balance_destroy to avoid leaks.
| FFIBalance *dash_spv_ffi_client_get_address_balance(struct FFIDashSpvClient *client, | ||
| const char *address); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Undefined type FFIBalance in this header; include key_wallet_ffi.h.
This header references FFIBalance* but doesn’t declare it. Include the key-wallet FFI header to make these declarations valid.
#include <stdlib.h>
+
+/* Pull in shared wallet FFI types like FFIBalance and FFIUTXO */
+#include "key_wallet_ffi.h"📝 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.
| FFIBalance *dash_spv_ffi_client_get_address_balance(struct FFIDashSpvClient *client, | |
| const char *address); | |
| ++ b/Swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h | |
| @@ | |
| #include <stdlib.h> | |
| /* Pull in shared wallet FFI types like FFIBalance and FFIUTXO */ | |
| #include "key_wallet_ffi.h" | |
| FFIBalance *dash_spv_ffi_client_get_address_balance(struct FFIDashSpvClient *client, | |
| const char *address); |
🤖 Prompt for AI Agents
In swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h around lines
315-316 the type FFIBalance is used but not declared or included; add #include
"key_wallet_ffi.h" (or the correct relative path to that header) at the top of
dash_spv_ffi.h alongside the other includes so FFIBalance is defined before the
declaration of dash_spv_ffi_client_get_address_balance, and ensure include
order/guards remain correct.
| struct FFIArray dash_spv_ffi_client_get_watched_scripts(struct FFIDashSpvClient *client); | ||
|
|
||
| struct FFIBalance *dash_spv_ffi_client_get_total_balance(struct FFIDashSpvClient *client); | ||
| FFIBalance *dash_spv_ffi_client_get_total_balance(struct FFIDashSpvClient *client); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a destroy function for FFIBalance returned by the API.
Expose dash_spv_ffi_balance_destroy so Swift/C callers can free Rust allocations safely.
struct FFIArray dash_spv_ffi_client_get_watched_scripts(struct FFIDashSpvClient *client);
FFIBalance *dash_spv_ffi_client_get_total_balance(struct FFIDashSpvClient *client);
+
+/* Free a balance returned by any balance getter */
+void dash_spv_ffi_balance_destroy(FFIBalance *balance);📝 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.
| FFIBalance *dash_spv_ffi_client_get_total_balance(struct FFIDashSpvClient *client); | |
| struct FFIArray dash_spv_ffi_client_get_watched_scripts(struct FFIDashSpvClient *client); | |
| FFIBalance *dash_spv_ffi_client_get_total_balance(struct FFIDashSpvClient *client); | |
| /* Free a balance returned by any balance getter */ | |
| void dash_spv_ffi_balance_destroy(FFIBalance *balance); |
🤖 Prompt for AI Agents
In swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h around line
353, the API returns FFIBalance pointers but lacks a corresponding destroy
function; add a declaration for dash_spv_ffi_balance_destroy that accepts a
pointer to FFIBalance and frees Rust-allocated resources so Swift/C callers can
release memory safely, ensure the function is exposed with the correct C
linkage/visibility like the other dash_spv_ffi_* functions and document that
callers must invoke it when done with the returned FFIBalance.
| case script | ||
| case outpoint | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Watch-item bridge removal: LGTM; please sync headers and purge stale usages.
Confirm the generated C header is synced into the Swift SDK and that no leftover WatchItem symbols remain.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Ensure generated header is synced into Swift SDK
HDR_GEN="$(fd -a 'dash_spv_ffi.h' dash-spv-ffi/include | head -n1)"
HDR_SWIFT="$(fd -a 'dash_spv_ffi.h' swift-dash-core-sdk/Sources | head -n1)"
echo "Generated: $HDR_GEN"
echo "SwiftSDK : $HDR_SWIFT"
if [[ -n "${HDR_GEN:-}" && -n "${HDR_SWIFT:-}" ]]; then
diff -u "$HDR_GEN" "$HDR_SWIFT" || true
else
echo "Headers not found in expected locations."
fi
# Verify no stale watch-item bridging remains
rg -nS 'WatchItemType|FFIWatchItem|createFFIWatchItem|convertWatchItemType' swift-dash-core-sdk/SourcesLength of output: 1391
Purge leftover WatchItem bridging in SPVClient+Verification.swift
Generated C header is up-to-date between dash-spv-ffi and the Swift SDK, but SPVClient+Verification.swift still defines two stale methods (addWatchItemWithTracking and removeWatchItemWithTracking at lines 12 and 24) referencing WatchItemType. Remove these leftover WatchItem symbols.
🤖 Prompt for AI Agents
In swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/FFIBridge.swift around line
152, remove the leftover WatchItem bridging references that are no longer
present in the generated C header: delete the stale methods
addWatchItemWithTracking and removeWatchItemWithTracking (and any references to
WatchItemType) from SPVClient+Verification.swift, update imports/usages
accordingly so the file compiles against the current dash-spv-ffi header, and
run a build to confirm no remaining WatchItem symbols remain.
|
Definitely have to do some fixes based on code rabbit suggestions, but this does work, and is an improvement. So I'm merging it in. |
Summary by CodeRabbit
New Features
Refactor
Removals
Documentation
Tests
Chores