Skip to content

Conversation

@QuantumExplorer
Copy link
Member

@QuantumExplorer QuantumExplorer commented Sep 26, 2025

Summary by CodeRabbit

  • New Features

    • Configure client worker threads; non-blocking API to drain pending client events.
  • Improvements

    • Richer sync overview exposed: header & filter heights, peer count, filters downloaded, masternode info; updated percentage/ETA and stage descriptions across CLI/SDK.
  • Refactor

    • Detailed progress now embeds a unified overview snapshot; sync stages expanded to include filter-header, filter-download, and block-download phases.
  • Breaking Change

    • Per-filter progress callback/events removed; legacy boolean sync flags replaced by the new overview structure.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.94% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “fix: sync progress” indicates an intent to correct synchronization progress logic but fails to capture the extensive revisions introduced in this changeset. The PR implements a new FFISyncProgress struct, updates the FFISyncStage enum with intermediate DownloadingFilterHeaders, DownloadingFilters, and DownloadingBlocks stages, and removes the on_filter_headers_progress callback, among other API changes. These substantial refactorings and additions are not conveyed by the generic title. As a result, a reviewer cannot understand the scope or focus of the PR at a glance. A more descriptive title would improve clarity and discoverability in project history. Rename the pull request to succinctly reflect the primary changes by highlighting the addition of the FFISyncProgress overview, the introduction of new sync stages in FFISyncStage, and the removal of the deprecated filter headers callback. For example: “Refactor sync progress reporting: add FFISyncProgress overview, update sync stages, and remove on_filter_headers_progress callback.”
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/syncProgress

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e4b37d and f3256e3.

📒 Files selected for processing (9)
  • dash-spv-ffi/dash_spv_ffi.h (2 hunks)
  • dash-spv-ffi/include/dash_spv_ffi.h (2 hunks)
  • dash-spv-ffi/src/bin/ffi_cli.rs (7 hunks)
  • dash-spv-ffi/src/types.rs (5 hunks)
  • dash-spv/src/client/mod.rs (7 hunks)
  • dash-spv/src/types.rs (3 hunks)
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (6 hunks)
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift (8 hunks)
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (4 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/types.rs
  • dash-spv/src/client/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/types.rs
  • dash-spv/src/client/mod.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.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/types.rs
  • dash-spv/src/client/mod.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.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
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Be careful with memory management at FFI boundaries (C/Swift interop)

Files:

  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
  • dash-spv-ffi/src/types.rs
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
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/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Never log or expose private keys

Files:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
swift-dash-core-sdk/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
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/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.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
🧠 Learnings (13)
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds

Applied to files:

  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • 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/include/dash_spv_ffi.h
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • 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: Coordinate synchronization phases with `SequentialSyncManager`; each phase must complete before the next begins

Applied to files:

  • dash-spv/src/client/mod.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:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
📚 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:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
📚 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/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
📚 Learning: 2025-06-26T15:49:53.811Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:49:53.811Z
Learning: In Rust FFI code, using thread-local storage for error reporting is unsafe in multi-threaded contexts, as callbacks may execute on different threads than the caller, leading to lost or invisible errors. FFI error handling should use global, thread-safe storage or return errors directly via return values or out parameters.

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-06-26T15:48:36.342Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:48:36.342Z
Learning: In Rust FFI code, unwrap() must not be used on CString::new, especially for error messages or untrusted input, as it can panic if the string contains null bytes. Instead, use unwrap_or_else with a fallback to a known-safe, hardcoded string to prevent panics across FFI boundaries.

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.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:

  • dash-spv-ffi/src/bin/ffi_cli.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/src/bin/ffi_cli.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: When adding new FFI functions in the Rust layer, implement them with #[no_mangle] extern "C" and proper cbindgen annotations

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
🧬 Code graph analysis (3)
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (3)
dash-spv/src/sync/sequential/phases.rs (1)
  • progress (278-524)
dash-spv/src/client/mod.rs (1)
  • peer_count (1389-1391)
dash-spv/tests/block_download_test.rs (1)
  • peer_count (88-94)
dash-spv/src/client/mod.rs (5)
dash-spv/src/client/status_display.rs (1)
  • sync_progress (74-108)
dash-spv/src/network/multi_peer.rs (1)
  • peer_count (1092-1095)
dash-spv/src/network/mod.rs (2)
  • peer_count (56-56)
  • peer_count (254-260)
dash-spv/src/types.rs (3)
  • default (57-70)
  • default (423-425)
  • default (572-601)
dash-spv/src/sync/sequential/phases.rs (1)
  • progress (278-524)
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (2)
dash-spv-ffi/src/client.rs (1)
  • dash_spv_ffi_client_drain_events (360-365)
dash-spv-ffi/src/config.rs (1)
  • dash_spv_ffi_config_set_worker_threads (371-379)
⏰ 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: Strict Warnings and Clippy Checks
  • GitHub Check: Key Wallet Components Tests
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: SPV Components Tests
  • GitHub Check: Core Components Tests
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_outpoint_string)
🔇 Additional comments (45)
dash-spv-ffi/include/dash_spv_ffi.h (3)

31-36: FFI header matches Rust changes correctly.

The added DownloadingFilterHeaders, DownloadingFilters, and DownloadingBlocks enum variants and the renumbering of Complete and Failed properly reflect the sync stage additions. The new FFISyncProgress struct layout with fields like header_height, filter_header_height, masternode_height, peer_count, etc., aligns with the Rust types.


76-84: New progress structure matches backend changes.

The introduction of the standalone FFISyncProgress struct provides a clean way to expose the core sync metrics that were previously scattered across different structures. This matches the refactoring in the Rust layer where DetailedSyncProgress now embeds a SyncProgress instance.


93-93: Header correctly reflects the embedded progress overview.

The FFIDetailedSyncProgress now includes the overview field of type FFISyncProgress, which properly matches the Rust-side changes where DetailedSyncProgress contains an embedded sync_progress field.

dash-spv-ffi/src/bin/ffi_cli.rs (5)

4-4: Import atomic operations correctly.

The addition of AtomicBool and Ordering imports and the static SYNC_COMPLETED flag provide proper thread-safe synchronization for completion tracking.

Also applies to: 20-20


37-37: Progress display updated for new structure.

The changes to use overview.header_height and overview.peer_count correctly align with the new embedded progress structure. The completion callback properly sets the atomic flag.

Also applies to: 40-40, 50-50


174-186: Event callbacks simplified correctly.

Removing the on_filter_headers_progress callback and setting up minimal callbacks aligns with the broader changes to progress reporting via the detailed sync progress structure instead of individual callbacks.


195-196: Proper completion flag initialization.

Resetting SYNC_COMPLETED to false before starting sync ensures clean state for each sync operation.


216-226: Fix completion logic to address early exit issue.

The current logic still has the completion check that can cause early exit when filter sync is disabled. Based on the existing past review comment, this should check the actual sync stage instead of comparing heights.

-            let headers_done = SYNC_COMPLETED.load(Ordering::SeqCst);
-            let filters_complete = if disable_filter_sync || !prog.filter_sync_available {
-                false
-            } else {
-                prog.filter_header_height >= prog.header_height
-                    && prog.last_synced_filter_height >= prog.filter_header_height
-            };
-            if headers_done && (filters_complete || disable_filter_sync) {
+            // Check if sync stage indicates completion or failure
+            if SYNC_COMPLETED.load(Ordering::SeqCst) {
dash-spv/src/client/mod.rs (6)

22-26: Import and export new types correctly.

The addition of SyncPhase import and SyncStage to the public exports properly exposes the sync stage functionality while maintaining the necessary internal imports.


152-205: Stage mapping helper provides clean abstraction.

The map_phase_to_stage function properly translates between internal sync phases and the public sync stage API. The stage mapping correctly handles different phase types including the new DownloadingBlocks variant.


802-802: Track filters downloaded for progress emission.

Adding last_emitted_filters_downloaded ensures that progress updates are emitted when filter download counts change, completing the progress tracking for all sync metrics.


959-978: Progress composition with embedded sync_progress.

The changes to create sync_progress snapshots and embed them in DetailedSyncProgress provide a clean separation between core progress metrics and detailed performance data. The peer count and header height updates ensure current network state is reflected.

Also applies to: 1005-1005


1021-1081: Enhanced progress emission logic.

The expanded progress emission logic properly handles changes in header heights and filter download counts. The stage mapping using the progress snapshot ensures consistent progress reporting across the sync process.


2030-2034: Update resume logging with new structure.

The log message changes to use header_height and filter_header_height instead of boolean flags provide more informative progress state when resuming from saved state.

swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (5)

14-18: New properties align with FFI structure.

The addition of filterHeaderHeight, masternodeHeight, peerCount, filtersDownloaded, and lastSyncedFilterHeight properties matches the fields in the updated FFISyncProgress C structure.


27-32: Public initializer updated with new parameters.

The initializer signature correctly includes all the new fields with appropriate default values. This maintains API compatibility while supporting the expanded progress information.


41-45: Property assignments match new fields.

The assignments in the public initializer correctly map the new parameters to their corresponding properties.


48-61: FFI initializer properly maps new fields.

The internal initializer correctly extracts all the new fields from the FFISyncProgress structure, properly mapping C field names to Swift property names (e.g., filter_header_heightfilterHeaderHeight).


51-52: Review initialization logic for FFI path.

The FFI initializer sets progress = 0.0 and status = .downloadingHeaders as fixed values. Consider whether these should be calculated from the FFI progress data or if the current approach is intentional for this initialization path.

Could you clarify if the hardcoded progress = 0.0 and status = .downloadingHeaders in the FFI initializer are correct, or should these be derived from the ffiProgress data? The public initializer allows these to vary, but the FFI path forces specific values.

dash-spv/src/types.rs (3)

76-77: Embed sync progress in detailed progress.

The replacement of individual fields with an embedded sync_progress: SyncProgress provides a cleaner structure and aligns with the FFI layer's approach of having an overview section.


111-123: New sync stage variants added correctly.

The addition of DownloadingFilterHeaders, DownloadingFilters, and DownloadingBlocks variants with appropriate fields properly represents the granular sync phases that were previously missing from the public API.


131-133: Progress calculations updated for embedded structure.

The changes to use self.sync_progress.header_height in both calculate_percentage and calculate_eta correctly adapt to the new embedded progress structure.

Also applies to: 140-142

dash-spv-ffi/src/types.rs (5)

68-73: LGTM! New sync stages properly defined.

The addition of the new sync stages (DownloadingFilterHeaders, DownloadingFilters, DownloadingBlocks) with updated enum values and the shifting of Complete and Failed maintains proper ordering for FFI compatibility.


89-97: LGTM! New stage mappings correctly implemented.

The From<SyncStage> implementation properly maps the new sync stage variants to their corresponding FFI enum values, maintaining consistency with the enum definition changes.


112-112: LGTM! Overview field properly integrated.

The addition of the overview: FFISyncProgress field to FFIDetailedSyncProgress correctly consolidates sync progress information into a single, structured overview.


134-144: LGTM! New stage message formatting correctly implemented.

The stage message formatting for the new sync stages (DownloadingFilterHeaders, DownloadingFilters, DownloadingBlocks) provides appropriate user-friendly descriptions with proper parameter formatting.


149-161: LGTM! Overview construction properly integrated.

The construction of the overview field from progress.sync_progress and its use in the FFIDetailedSyncProgress struct correctly consolidates the sync progress data.

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

31-36: LGTM! FFI sync stage enum properly updated.

The FFI enum values correctly reflect the new sync stages with proper ordinal progression. The shifting of Complete (8) and Failed (9) maintains backward compatibility while accommodating the new intermediate stages.


76-84: LGTM! FFI sync progress structure properly defined.

The new FFISyncProgress struct correctly exposes all necessary sync progress fields for FFI consumption, providing comprehensive progress information.


93-93: LGTM! Overview field properly integrated in detailed progress.

The overview field of type FFISyncProgress correctly consolidates sync progress information within the detailed sync progress structure.


278-284: LGTM! Drain events API properly documented.

The new dash_spv_ffi_client_drain_events function is properly documented and provides non-blocking event processing capability.


707-713: LGTM! Worker threads configuration API properly added.

The dash_spv_ffi_config_set_worker_threads function is properly documented and allows configuration of Tokio worker threads with auto-detection (0 value).

Based on retrieved learnings and coding guidelines, ensure the header is synchronized from dash-spv-ffi to the Swift SDK using ./sync-headers.sh to maintain consistency between the generated header and the Swift bridge.

dash-spv-ffi/dash_spv_ffi.h (3)

31-36: LGTM! Sync stage enumeration properly updated.

The FFI sync stage enum correctly mirrors the changes in the Rust types, with new intermediate stages and proper ordinal progression.


76-84: LGTM! New progress structure properly defined.

The FFISyncProgress struct consolidates all sync progress fields and matches the implementation in the types.rs file.


93-93: LGTM! Overview integration properly implemented.

The embedding of FFISyncProgress as the overview field in FFIDetailedSyncProgress follows the new consolidated approach.

swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift (10)

11-11: LGTM! Overview field properly integrated.

The addition of the overview: SyncProgress field consolidates sync progress information into a structured format, replacing individual fields.


21-22: LGTM! Computed properties correctly implemented.

The computed properties currentHeight and connectedPeers properly delegate to the overview field, maintaining backward compatibility while transitioning to the new structure.


70-80: LGTM! Public initializer correctly updated.

The public initializer properly accepts the new overview: SyncProgress parameter and assigns it correctly, maintaining the API contract.


93-93: LGTM! FFI initialization properly updated.

The internal FFI initializer correctly creates the overview from ffiProgress.overview, adapting to the new FFI structure.


110-116: LGTM! New sync stages properly defined.

The addition of the new sync stage cases (downloadingFilterHeaders, downloadingFilters, downloadingBlocks) aligns with the updated FFI enum structure.


131-140: LGTM! Stage mapping correctly implemented.

The FFI stage initialization properly maps the new enum values (5, 6, 7, 8, 9) to their corresponding Swift cases, maintaining consistency with the updated FFI definitions.


154-159: LGTM! Stage descriptions appropriately defined.

The descriptions for the new sync stages are clear and informative, providing good user feedback during the sync process.


188-193: LGTM! Stage icons properly assigned.

The emoji icons for the new sync stages are appropriate and consistent with the existing icon style.


331-333: LGTM! Statistics properly extended with overview fields.

The statistics dictionary correctly includes the new overview-derived fields (Filter Header Height, Filters Downloaded, Peer Count) while maintaining existing keys for backward compatibility.


1266-1266: Remove trailing character after closing brace.

There appears to be a stray character after the closing brace on line 1266.

Based on previous review comments, this appears to be a false positive. The line ending is correct according to the maintainer's feedback.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

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

64-64: Consider documenting the worker_threads field.

The new worker_threads field in FFIClientConfig lacks documentation. Consider adding a comment explaining that it controls the number of Tokio runtime worker threads (with 0 meaning auto-detect).

 typedef struct FFIClientConfig {
   void *inner;
+  /// Number of Tokio worker threads (0 = auto)
   uint32_t worker_threads;
-
 } FFIClientConfig;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef7374d and e8b54f4.

📒 Files selected for processing (9)
  • dash-spv-ffi/dash_spv_ffi.h (1 hunks)
  • dash-spv-ffi/include/dash_spv_ffi.h (1 hunks)
  • dash-spv-ffi/src/bin/ffi_cli.rs (1 hunks)
  • dash-spv-ffi/src/types.rs (3 hunks)
  • dash-spv/src/client/mod.rs (1 hunks)
  • dash-spv/src/types.rs (2 hunks)
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (5 hunks)
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift (4 hunks)
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (5 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
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/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)

**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv/src/client/mod.rs
  • dash-spv-ffi/src/types.rs
  • dash-spv/src/types.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Be careful with memory management at FFI boundaries (C/Swift interop)

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
**/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv/src/client/mod.rs
  • dash-spv-ffi/src/types.rs
  • dash-spv/src/types.rs
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/dash_spv_ffi.h
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/client/mod.rs
  • dash-spv/src/types.rs
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/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Never log or expose private keys

Files:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
swift-dash-core-sdk/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
  • 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
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 (6)
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.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/src/bin/ffi_cli.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/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/include/dash_spv_ffi.h
  • dash-spv-ffi/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:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
📚 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:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
📚 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:

  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
🧬 Code graph analysis (6)
dash-spv-ffi/src/bin/ffi_cli.rs (1)
dash-spv/src/client/mod.rs (1)
  • peer_count (1299-1301)
dash-spv/src/client/mod.rs (2)
dash-spv/src/client/status_display.rs (1)
  • sync_progress (74-111)
dash-spv/src/types.rs (3)
  • default (66-82)
  • default (424-426)
  • default (573-602)
dash-spv-ffi/src/types.rs (1)
dash-spv/src/sync/sequential/phases.rs (1)
  • progress (278-524)
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (4)
dash-spv/src/client/mod.rs (1)
  • peer_count (1299-1301)
dash-spv/tests/block_download_test.rs (1)
  • peer_count (88-94)
dash-spv/tests/edge_case_filter_sync_test.rs (1)
  • peer_count (88-90)
dash-spv/tests/filter_header_verification_test.rs (1)
  • peer_count (80-82)
dash-spv/src/types.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (1)
  • current_height (1035-1037)
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (2)
dash-spv-ffi/src/client.rs (1)
  • dash_spv_ffi_client_drain_events (371-376)
dash-spv-ffi/src/config.rs (1)
  • dash_spv_ffi_config_set_worker_threads (371-379)
⏰ 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). (11)
  • GitHub Check: Strict Warnings and Clippy Checks
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: SPV Components Tests
  • GitHub Check: Core Components Tests
🔇 Additional comments (17)
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (3)

86-96: LGTM! The structural change to embed FFISyncProgress is correct.

The refactoring to embed FFISyncProgress as overview within FFIDetailedSyncProgress improves the API by providing a cleaner separation between core sync metrics and detailed progress information.


1-994: Ensure header synchronization with dash-spv-ffi.

This header file is auto-generated from the dash-spv-ffi crate. Please ensure that after merging these changes, you run ./sync-headers.sh to keep the Swift SDK's copy synchronized with the FFI crate's generated header.

Based on learnings.


719-719: No duplicate declaration found
The function dash_spv_ffi_config_set_worker_threads is declared only once in dash_spv_ffi.h (line 719).

Likely an incorrect or invalid review comment.

dash-spv-ffi/src/types.rs (1)

106-109: LGTM! Consistent transformation to use nested overview.

The implementation correctly populates the overview field using FFISyncProgress::from(progress.sync_progress.clone()), maintaining consistency with the header file changes.

dash-spv-ffi/src/bin/ffi_cli.rs (1)

30-45: LGTM! CLI correctly uses the new nested structure.

The display logic has been properly updated to access header_height and peer_count through the overview field, maintaining the same user experience while using the refactored structure.

dash-spv/src/types.rs (2)

88-89: LGTM! Core type refactoring is well-structured.

The addition of sync_progress: SyncProgress as a nested field provides a clean separation of concerns. The core metrics are now encapsulated in SyncProgress, while DetailedSyncProgress extends it with performance and timing information.


131-134: LGTM! Consistent usage of nested sync_progress throughout.

The percentage and ETA calculations correctly reference self.sync_progress.header_height instead of the removed current_height field, maintaining functional consistency.

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

86-96: LGTM! Header changes match the implementation.

The FFIDetailedSyncProgress structure correctly embeds FFISyncProgress as overview, matching the changes in the implementation files.

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

921-933: LGTM! Clean refactoring to use embedded overview pattern.

The change to use StatusDisplay for creating the sync progress snapshot and updating it with the latest network state is well-structured. The embedded overview approach centralizes progress information and maintains consistency across the codebase.

swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift (4)

11-22: LGTM! Backward compatibility maintained with computed properties.

The refactoring to use an overview: SyncProgress field while maintaining currentHeight and connectedPeers as computed properties is excellent. This preserves the existing API while adopting the new nested structure.


69-80: LGTM! Clean initializer update.

The public initializer correctly accepts the new overview parameter while maintaining the rest of the initialization logic unchanged.


92-93: LGTM! FFI integration properly updated.

The internal initializer correctly creates the overview from the FFI structure.


310-313: LGTM! Enhanced statistics display.

Good addition of filter-related statistics to the dictionary, providing more comprehensive sync progress information.

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

86-96: LGTM! Well-structured FFI type refactoring.

The FFIDetailedSyncProgress struct now properly embeds FFISyncProgress as an overview field. This maintains separation of concerns while providing a comprehensive progress snapshot.

Based on learnings

swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (3)

14-21: LGTM! Comprehensive sync progress fields added.

The new fields provide detailed visibility into the sync state across different components (filter headers, masternodes, filters). This aligns well with the overview pattern adopted throughout the PR.


31-38: LGTM! Sensible default values for optional parameters.

Good use of default parameter values maintaining backward compatibility while allowing detailed progress tracking when needed.


65-72: LGTM! FFI mapping properly handles new fields.

The internal initializer correctly maps all new FFI fields to the Swift properties.

return SyncProgressStream(client: self)
}
}
}
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 | 🟡 Minor

Remove trailing tilde character.

There's a stray ~ character at the end of line 1245 that should be removed.

-}
+}

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

🤖 Prompt for AI Agents
In swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift around line
1245, there's a stray trailing tilde character (~) at the end of the line;
remove that character so the line ends cleanly (ensure no extra whitespace or
other stray characters remain and run quick build/lint to confirm no syntax
errors).

Copy link
Member

Choose a reason for hiding this comment

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

This is not accurate

}
}
}
}
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 | 🟡 Minor

Remove trailing tilde character.

There's a stray ~ character at the end of line 155 that should be removed.

-}
+}

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

🤖 Prompt for AI Agents
In swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift at
line 155, remove the stray trailing tilde character (~) at the end of the line;
edit that line to delete the tilde so the file compiles and contains no
unintended character.

Copy link
Member

Choose a reason for hiding this comment

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

This is not accurate

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (9)
dash-spv/src/client/mod.rs (2)

921-933: Avoid overriding header_height after computing SyncProgress

You compute SyncProgress via StatusDisplay and then overwrite header_height with current_height. This risks subtle inconsistencies if StatusDisplay’s calculate_header_height changes. Prefer a single source of truth: either trust the StatusDisplay value or have StatusDisplay accept/compute the absolute height consistently.

- let mut sync_progress = match status_display.sync_progress().await {
+ let mut sync_progress = match status_display.sync_progress().await {
     Ok(p) => p,
     Err(e) => {
         tracing::warn!("Failed to compute sync progress snapshot: {}", e);
         SyncProgress::default()
     }
 };
 // Update peer count with the latest network information.
 sync_progress.peer_count = self.network.peer_count() as u32;
- sync_progress.header_height = current_height;
+ // If StatusDisplay::sync_progress() returns checkpoint-aware height, keep it.
+ // Otherwise ensure calculate_header_height uses the same definition as current_height.

965-973: Deduplicate progress snapshot construction

You build DetailedSyncProgress twice with near-identical logic. Extract a small helper to construct progress from (sync_progress, peer_best, current/abs height) to reduce drift and make maintenance easier.

+ fn build_progress(
+   sync_progress: SyncProgress,
+   peer_best: u32,
+   current_height: u32,
+   headers_per_second: f64,
+   total_bytes_downloaded: u64,
+   sync_start_time: SystemTime,
+ ) -> DetailedSyncProgress {
+   DetailedSyncProgress {
+     sync_progress,
+     peer_best_height: peer_best,
+     percentage: if peer_best > 0 {
+       (current_height as f64 / peer_best as f64 * 100.0).min(100.0)
+     } else { 0.0 },
+     headers_per_second,
+     bytes_per_second: 0,
+     estimated_time_remaining: if headers_per_second > 0.0 && peer_best > current_height {
+       let remaining = peer_best - current_height;
+       Some(Duration::from_secs_f64(remaining as f64 / headers_per_second))
+     } else { None },
+     sync_stage: if current_height < peer_best {
+       crate::types::SyncStage::DownloadingHeaders { start: current_height, end: peer_best }
+     } else { crate::types::SyncStage::Complete },
+     total_headers_processed: current_height as u64,
+     total_bytes_downloaded,
+     sync_start_time,
+     last_update_time: SystemTime::now(),
+   }
+ }

Then call build_progress in both places.

Also applies to: 974-1026

dash-spv-ffi/src/client.rs (2)

789-821: Progress callback passes ephemeral pointer; document lifetime explicitly

FFIDetailedSyncProgress is boxed per-callback and dropped after the call. Add an explicit safety comment near the callback invocation stating the pointer is only valid during the call to prevent consumers from storing it.

- callback(ffi_progress.as_ref(), *user_data);
+ // SAFETY: Pointer is valid only for the duration of this call. Do not store.
+ callback(ffi_progress.as_ref(), *user_data);

1271-1271: Drop misleading log for removed callback

Logging “Filter headers progress callback: false” is confusing now that the field no longer exists. Remove the line or replace with a generic note that per-filter progress callbacks were removed.

- tracing::debug!("   Filter headers progress callback: {}", false);
+ tracing::debug!("   Per-filter progress callback removed; using detailed progress instead");
dash-spv/src/types.rs (1)

782-791: Consider embedding SyncProgress in SpvEvent::SyncProgress

For consistency with the new model, you might evolve this event to carry SyncProgress (plus any extras) rather than raw fields. Optional for now.

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

86-97: Clarify lifetime/units of transient fields in progress callbacks

Document that stage_message.ptr (and any nested pointers) are only valid for the duration of the progress callback, and specify timestamp/seconds units for sync_start_timestamp and estimated_seconds_remaining in the Rust docs so cbindgen carries it here.

Can you confirm these docs exist in the Rust source that generates this header? As per coding guidelines

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

64-64: worker_threads added: avoid direct struct mutation from Swift

Ensure Swift uses the setter dash_spv_ffi_config_set_worker_threads and not field writes, and add a wrapper in FFIBridge.swift.

If missing, add a thin wrapper:

public func setWorkerThreads(_ threads: UInt32) throws {
    let rc = dash_spv_ffi_config_set_worker_threads(self.ptr, threads)
    try FFI.throwIfError(rc)
}

Based on learnings


278-285: New non-blocking drain_events: expose in Swift API

Add a Swift wrapper and decide call sites (e.g., timer/runloop, or after enqueueing work) to drive callback delivery.

Example wrapper:

@discardableResult
public func drainEvents() throws -> Int32 {
    let rc = dash_spv_ffi_client_drain_events(self.ptr)
    try FFI.throwIfError(rc)
    return rc
}

Based on learnings

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

86-97: Add CI check to ensure dash-spv-ffi headers remain synchronized

Headers dash-spv-ffi/include/dash_spv_ffi.h and dash-spv-ffi/dash_spv_ffi.h are byte-identical (verified), but automate a diff in CI (or consolidate into one canonical file) to prevent future drift.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e8b54f4 and 2973561.

📒 Files selected for processing (11)
  • dash-spv-ffi/dash_spv_ffi.h (1 hunks)
  • dash-spv-ffi/include/dash_spv_ffi.h (1 hunks)
  • dash-spv-ffi/src/bin/ffi_cli.rs (2 hunks)
  • dash-spv-ffi/src/callbacks.rs (2 hunks)
  • dash-spv-ffi/src/client.rs (3 hunks)
  • dash-spv-ffi/tests/integration/test_full_workflow.rs (0 hunks)
  • dash-spv-ffi/tests/test_event_callbacks.rs (0 hunks)
  • dash-spv-ffi/tests/unit/test_async_operations.rs (0 hunks)
  • dash-spv/src/client/mod.rs (2 hunks)
  • dash-spv/src/types.rs (2 hunks)
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (5 hunks)
💤 Files with no reviewable changes (3)
  • dash-spv-ffi/tests/test_event_callbacks.rs
  • dash-spv-ffi/tests/unit/test_async_operations.rs
  • dash-spv-ffi/tests/integration/test_full_workflow.rs
🧰 Additional context used
📓 Path-based instructions (10)
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/callbacks.rs
  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
dash-spv-ffi/src/callbacks.rs

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

Use C function pointers for async progress/completion callbacks; callbacks may be invoked from any thread

Files:

  • dash-spv-ffi/src/callbacks.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)

**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable

Files:

  • dash-spv-ffi/src/callbacks.rs
  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv/src/client/mod.rs
  • dash-spv/src/types.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Be careful with memory management at FFI boundaries (C/Swift interop)

Files:

  • dash-spv-ffi/src/callbacks.rs
  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
**/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]

Files:

  • dash-spv-ffi/src/callbacks.rs
  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv/src/client/mod.rs
  • dash-spv/src/types.rs
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • dash-spv-ffi/src/callbacks.rs
  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/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
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
swift-dash-core-sdk/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
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/client/mod.rs
  • dash-spv/src/types.rs
🧠 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
📚 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/src/callbacks.rs
  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`

Applied to files:

  • dash-spv-ffi/src/client.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.

Applied to files:

  • dash-spv-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/dash_spv_ffi.h
  • dash-spv-ffi/src/bin/ffi_cli.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/dash_spv_ffi.h
  • 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/src/bin/ffi_cli.rs
🧬 Code graph analysis (4)
dash-spv-ffi/src/bin/ffi_cli.rs (1)
dash-spv/src/client/mod.rs (1)
  • peer_count (1337-1339)
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (2)
dash-spv-ffi/src/client.rs (1)
  • dash_spv_ffi_client_drain_events (361-366)
dash-spv-ffi/src/config.rs (1)
  • dash_spv_ffi_config_set_worker_threads (371-379)
dash-spv/src/client/mod.rs (2)
dash-spv/src/client/status_display.rs (1)
  • sync_progress (74-111)
dash-spv/src/types.rs (3)
  • default (66-82)
  • default (424-426)
  • default (573-602)
dash-spv/src/types.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (1)
  • current_height (1035-1037)
⏰ 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). (16)
  • GitHub Check: Key Wallet Components Tests
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: SPV Components Tests
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: Strict Warnings and Clippy Checks
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: Core Components Tests
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: fuzz (dash_deser_net_msg)
🔇 Additional comments (16)
dash-spv-ffi/src/client.rs (4)

252-253: Removal of FilterHeadersProgress event handling is correct

Event forwarding aligns with the updated model; detailed sync progress now covers this data.


938-939: Good: clarify progress routing

The comment makes it clear that detailed progress is the single channel now.


970-1003: All references to removed callbacks/variants cleaned up: grep confirms only comments remain noting removal in callbacks.rs and client.rs; no code, header, or test references to on_filter_headers_progress or FilterHeadersProgress.


145-158: Expose worker_threads in FFI headers
No occurrences of worker_threads found in committed .h/.hpp files; verify cbindgen-generated headers include this field so callers can set it.

dash-spv/src/types.rs (3)

88-90: Embedding SyncProgress in DetailedSyncProgress is a solid consolidation

This reduces duplication and keeps the overview consistent across layers.


132-134: Percentage/ETA now use sync_progress.header_height

This is consistent with the new model. LGTM.

Also applies to: 141-143


25-63: Confirm SystemTime serde support in SyncProgress (dash-spv/src/types.rs): verify your serde setup supports serializing/deserializing the sync_start and last_update SystemTime fields or switch to explicit epoch‐based types.

dash-spv-ffi/src/bin/ffi_cli.rs (2)

170-181: Minimal event callbacks setup aligns with removed per-filter callback

No dangling fields; clean initialization.


34-38: FFIDetailedSyncProgress ‘overview’ field confirmed FFI struct defines overview (no sync_progress), matching the CLI’s p.overview.* usage.

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

148-150: Clean removal of on_filter_headers_progress from FFIEventCallbacks

Matches the new progress model; fewer moving parts at the FFI boundary.


166-180: Default impl updated accordingly

No references left; good.


388-389: Comment clarifies public API change

Clear guidance for integrators. LGTM.

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

86-97: FFIDetailedSyncProgress redesign looks correct

Nested overview consolidates duplicated fields; types/ordering are FFI-safe.

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

397-398: Doc tweak LGTM

Clarifies cancel semantics; no API change.


707-714: Setter for worker_threads present: confirm default semantics in docs

0 = auto is implied in the comment; ensure Swift docs mirror this and wrappers don’t clamp 0.

Based on learnings


86-97: FFIDetailedSyncProgress headers are in sync. FFISyncProgress presence and fields match upstream; verify Swift models map via overview and leverage computed properties (e.g., currentHeight, peerCount).

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

2882-2894: Avoid double-counting filters_received updates

Line 2883 calls Self::update_filter_received before we know whether height was already present. On retries or duplicate deliveries we’ll hit this branch again, incrementing filters_received without adding a new height, so the reported progress can exceed the total requested filters. Please insert the height first, check if it was new, and only then bump the counter.

-        if let Ok(Some(height)) = storage.get_header_height_by_hash(block_hash).await.map_err(|e| {
-            SyncError::Storage(format!("Failed to get header height by hash: {}", e))
-        })? {
-            // Increment the received counter so high-level progress reflects the update
-            Self::update_filter_received(stats).await;
-
-            // Get the shared filter heights arc from stats
-            let stats_lock = stats.read().await;
-            let received_filter_heights = stats_lock.received_filter_heights.clone();
-            drop(stats_lock); // Release the stats lock before acquiring the mutex
-
-            // Now lock the heights and insert
-            let mut heights = received_filter_heights.lock().await;
-            heights.insert(height);
+        if let Ok(Some(height)) = storage.get_header_height_by_hash(block_hash).await.map_err(|e| {
+            SyncError::Storage(format!("Failed to get header height by hash: {}", e))
+        })? {
+            // Get the shared filter heights arc from stats
+            let stats_lock = stats.read().await;
+            let received_filter_heights = stats_lock.received_filter_heights.clone();
+            drop(stats_lock); // Release the stats lock before acquiring the mutex
+
+            // Now lock the heights and insert
+            let mut heights = received_filter_heights.lock().await;
+            let is_new = heights.insert(height);
+            drop(heights);
+
+            if is_new {
+                // Increment the received counter so high-level progress reflects the update
+                Self::update_filter_received(stats).await;
+            }
dash-spv-ffi/src/client.rs (1)

808-824: Release callback locks before invoking user FFI callback

Lines 808-824 invoke the progress callback while holding both sync_callbacks and CALLBACK_REGISTRY mutex guards. If the callback re-enters the FFI (e.g., calling dash_spv_ffi_client_cancel_sync once enough data is downloaded), it tries to take those same locks inside stop_client_internal, leading to a guaranteed deadlock. Pull the callback pointer and user data out of the locks first, drop the guards, and only then call back into user code.

@@
-                                    {
-                                        let cb_guard = sync_callbacks_clone.lock().unwrap();
-
-                                        if let Some(ref callback_data) = *cb_guard {
-                                            let registry = CALLBACK_REGISTRY.lock().unwrap();
-                                            if let Some(CallbackInfo::Detailed {
-                                                progress_callback: Some(callback),
-                                                user_data,
-                                                ..
-                                            }) = registry.get(callback_data.callback_id)
-                                            {
-                                                // SAFETY: The callback and user_data are safely stored in the registry
-                                                // and accessed through thread-safe mechanisms. The registry ensures
-                                                // proper lifetime management without raw pointer passing across threads.
-                                                callback(ffi_progress.as_ref(), *user_data);
-                                            }
-                                        }
-                                    }
+                                    let callback_id = {
+                                        let cb_guard = sync_callbacks_clone.lock().unwrap();
+                                        cb_guard.as_ref().map(|data| data.callback_id)
+                                    };
+
+                                    let (progress_callback, user_data_ptr) = if let Some(callback_id) =
+                                        callback_id
+                                    {
+                                        let registry = CALLBACK_REGISTRY.lock().unwrap();
+                                        if let Some(CallbackInfo::Detailed {
+                                            progress_callback: Some(callback),
+                                            user_data,
+                                            ..
+                                        }) = registry.get(callback_id)
+                                        {
+                                            (Some(*callback), *user_data)
+                                        } else {
+                                            (None, std::ptr::null_mut())
+                                        }
+                                    } else {
+                                        (None, std::ptr::null_mut())
+                                    };
+
+                                    if let Some(callback) = progress_callback {
+                                        // SAFETY: The callback and user_data are retrieved under the registry lock,
+                                        // and we only pass borrowed data whose lifetime is bound to this call.
+                                        callback(ffi_progress.as_ref(), user_data_ptr);
+                                    }
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (1)

98-117: Update needed for new FFISyncStage enum values.

The ffiStatus mapping in SyncStatus.init?(ffiStatus:) needs to be updated to handle the new DownloadingFilterHeaders (5) and DownloadingFilters (6) enum values, and adjust the mapping for Complete (now 7) and Failed (now 8).

Apply this diff to fix the enum mapping:

    internal init?(ffiStatus: UInt32) {
        switch ffiStatus {
        case 0:
            self = .idle
        case 1:
            self = .connecting
        case 2:
            self = .downloadingHeaders
-        case 3:
-            self = .downloadingFilters
-        case 4:
-            self = .scanning
-        case 5:
-            self = .synced
-        case 6:
-            self = .error
+        case 3:
+            self = .scanning  // Maps to Validating
+        case 4:
+            self = .scanning  // Maps to Storing  
+        case 5:
+            self = .downloadingFilters  // Maps to DownloadingFilterHeaders
+        case 6:
+            self = .downloadingFilters  // Maps to DownloadingFilters
+        case 7:
+            self = .synced   // Maps to Complete
+        case 8:
+            self = .error    // Maps to Failed
        default:
            return nil
        }
    }
🧹 Nitpick comments (1)
dash-spv-ffi/src/types.rs (1)

46-58: Handle the unwrap_or(0) pattern more explicitly for Optional values.

The conversion from Option<u32> to u32 using unwrap_or(0) for last_synced_filter_height might hide meaningful state. Consider documenting that 0 indicates "not yet synced" vs actual height 0.

Also, the cast from usize to u32 on line 54 could potentially overflow on 32-bit systems if filters_downloaded exceeds u32::MAX. Consider using saturating conversion:

-            filters_downloaded: progress.filters_downloaded as u32,
+            filters_downloaded: progress.filters_downloaded.min(u32::MAX as usize) as u32,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 389f0a3 and 85c5df4.

📒 Files selected for processing (17)
  • dash-spv-ffi/dash_spv_ffi.h (2 hunks)
  • dash-spv-ffi/include/dash_spv_ffi.h (2 hunks)
  • dash-spv-ffi/src/bin/ffi_cli.rs (3 hunks)
  • dash-spv-ffi/src/client.rs (2 hunks)
  • dash-spv-ffi/src/types.rs (5 hunks)
  • dash-spv-ffi/tests/performance/test_benchmarks.rs (2 hunks)
  • dash-spv-ffi/tests/test_types.rs (1 hunks)
  • dash-spv-ffi/tests/unit/test_type_conversions.rs (0 hunks)
  • dash-spv/src/client/mod.rs (6 hunks)
  • dash-spv/src/client/status_display.rs (0 hunks)
  • dash-spv/src/storage/sync_state.rs (1 hunks)
  • dash-spv/src/sync/filters.rs (1 hunks)
  • dash-spv/src/sync/sequential/mod.rs (1 hunks)
  • dash-spv/src/types.rs (3 hunks)
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (6 hunks)
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift (8 hunks)
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (4 hunks)
💤 Files with no reviewable changes (2)
  • dash-spv-ffi/tests/unit/test_type_conversions.rs
  • dash-spv/src/client/status_display.rs
🧰 Additional context used
📓 Path-based instructions (13)
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
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)

**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable

Files:

  • dash-spv-ffi/src/client.rs
  • dash-spv/src/storage/sync_state.rs
  • dash-spv/src/types.rs
  • dash-spv/src/sync/filters.rs
  • dash-spv-ffi/tests/performance/test_benchmarks.rs
  • dash-spv-ffi/tests/test_types.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
  • dash-spv/src/client/mod.rs
  • dash-spv/src/sync/sequential/mod.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Be careful with memory management at FFI boundaries (C/Swift interop)

Files:

  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/tests/performance/test_benchmarks.rs
  • dash-spv-ffi/tests/test_types.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/types.rs
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
**/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]

Files:

  • dash-spv-ffi/src/client.rs
  • dash-spv/src/storage/sync_state.rs
  • dash-spv/src/types.rs
  • dash-spv/src/sync/filters.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.rs
  • dash-spv/src/client/mod.rs
  • dash-spv/src/sync/sequential/mod.rs
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • dash-spv-ffi/src/client.rs
  • dash-spv-ffi/tests/performance/test_benchmarks.rs
  • dash-spv-ffi/tests/test_types.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/dash_spv_ffi.h
  • dash-spv-ffi/src/types.rs
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/storage/sync_state.rs
  • dash-spv/src/types.rs
  • dash-spv/src/sync/filters.rs
  • dash-spv/src/client/mod.rs
  • dash-spv/src/sync/sequential/mod.rs
dash-spv/src/storage/**/*.rs

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

Provide both MemoryStorageManager and DiskStorageManager behind the StorageManager trait

Files:

  • dash-spv/src/storage/sync_state.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use proptest for property-based testing where appropriate

**/tests/**/*.rs: Integration tests must live under tests/ (e.g., rpc-integration-test)
Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored
Add regression tests for fixes; consider property tests (e.g., proptest) where valuable
Keep test vectors deterministic

Files:

  • dash-spv-ffi/tests/performance/test_benchmarks.rs
  • dash-spv-ffi/tests/test_types.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/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/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Never log or expose private keys

Files:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
swift-dash-core-sdk/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift
  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
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 (11)
📚 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/src/client.rs
  • dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.

Applied to files:

  • dash-spv-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/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/src/bin/ffi_cli.rs
  • dash-spv-ffi/include/dash_spv_ffi.h
  • dash-spv-ffi/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/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv-ffi/src/types.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 : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.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/include/dash_spv_ffi.h
  • dash-spv-ffi/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: Coordinate synchronization phases with `SequentialSyncManager`; each phase must complete before the next begins

Applied to files:

  • dash-spv/src/client/mod.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:

  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
📚 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:

  • swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h
  • swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift
🧬 Code graph analysis (6)
dash-spv/src/types.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (1)
  • current_height (1035-1037)
dash-spv-ffi/src/bin/ffi_cli.rs (1)
dash-spv/src/client/mod.rs (1)
  • peer_count (1396-1398)
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (2)
dash-spv/src/sync/sequential/phases.rs (1)
  • progress (278-524)
dash-spv/src/client/mod.rs (1)
  • peer_count (1396-1398)
dash-spv-ffi/src/types.rs (1)
dash-spv/src/sync/sequential/phases.rs (1)
  • progress (278-524)
dash-spv/src/client/mod.rs (4)
dash-spv/src/client/status_display.rs (1)
  • sync_progress (74-108)
dash-spv/src/network/mod.rs (2)
  • peer_count (56-56)
  • peer_count (254-260)
dash-spv/src/types.rs (3)
  • default (57-70)
  • default (420-422)
  • default (569-598)
dash-spv/src/sync/sequential/phases.rs (1)
  • progress (278-524)
swift-dash-core-sdk/Sources/DashSPVFFI/include/dash_spv_ffi.h (2)
dash-spv-ffi/src/client.rs (1)
  • dash_spv_ffi_client_drain_events (360-365)
dash-spv-ffi/src/config.rs (1)
  • dash_spv_ffi_config_set_worker_threads (371-379)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: Core Components Tests
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Strict Warnings and Clippy Checks
🔇 Additional comments (35)
dash-spv/src/types.rs (3)

76-78: LGTM! Embedded sync progress overview structure.

The updated DetailedSyncProgress structure now properly embeds SyncProgress as sync_progress, replacing the previous individual fields. This aligns with the broader changes that centralize progress information into a single overview structure.


111-118: Well-designed filter sync stage additions.

The new DownloadingFilterHeaders and DownloadingFilters variants provide proper structure for tracking different aspects of filter synchronization with meaningful progress fields (current/target for headers, completed/total for filters).


128-129: Verify calculation consistency across stages.

The calculation methods now correctly use self.sync_progress.header_height instead of a direct field, maintaining consistency with the new embedded structure. Ensure all sync stage calculations use the embedded progress data consistently.

Also applies to: 137-138

dash-spv-ffi/src/bin/ffi_cli.rs (3)

34-37: Correctly updated to use embedded overview structure.

The progress callback now accesses p.overview.header_height and p.overview.peer_count from the embedded FFISyncProgress overview, maintaining consistency with the restructured FFIDetailedSyncProgress.


170-182: Properly cleaned up event callbacks.

The removal of on_filter_headers_progress from the FFIEventCallbacks initialization aligns with the broader changes that eliminate per-header progress callbacks in favor of the overview-based progress reporting.


209-213: Improved completion logic with embedded progress.

The completion detection now uses prog.header_height >= prog.filter_header_height along with other conditions, properly utilizing the embedded overview structure for determining sync completion status.

dash-spv/src/client/mod.rs (4)

22-26: Proper import and export of sync phase components.

The addition of SyncPhase import and SyncStage export provides the necessary type mapping infrastructure for the phase-to-stage conversion functionality.


152-205: Well-designed phase-to-stage mapping function.

The map_phase_to_stage function provides comprehensive mapping from internal sync phases to public sync stages, handling all sync phases appropriately and providing meaningful progress context for each stage.


961-980: Enhanced progress reporting with embedded overview.

The progress reporting now properly constructs SyncProgress snapshots and embeds them in DetailedSyncProgress, providing comprehensive progress information including peer count, header height, and filter sync status. The emission logic correctly tracks changes to avoid duplicate progress events.

Also applies to: 1015-1087


2032-2036: Clear logging message for resume state.

The logging message correctly references header_height and filter_header_height from the saved state, providing clear visibility into the resume operation.

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

75-83: New FFISyncProgress structure properly defined.

The FFISyncProgress struct provides comprehensive sync progress information including header heights, masternode height, peer count, and filter sync status. This replaces individual boolean flags with a more informative overview structure.


92-92: Correctly updated to embed overview structure.

The FFIDetailedSyncProgress now contains an overview field of type FFISyncProgress instead of separate current_height and connected_peers fields, providing a cleaner and more comprehensive progress representation.


151-161: Properly removed filter headers progress callback.

The removal of the on_filter_headers_progress field from FFIEventCallbacks aligns with the elimination of per-header progress callbacks in favor of the overview-based progress reporting system.


31-34: No action needed: Swift SDK mapping covers new FFISyncStage values. SPVClient.swift’s init(ffiStage:) already handles raw values 5–8, mapping them to .downloadingFilterHeaders, .downloadingFilters, .complete, and .failed.

swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift (2)

14-18: New fields properly added to sync progress model.

The addition of filterHeaderHeight, masternodeHeight, peerCount, filtersDownloaded, and lastSyncedFilterHeight provides comprehensive sync status information matching the FFISyncProgress structure.


59-63: Correctly mapped FFI progress to new properties.

The internal FFI-based initializer properly maps all fields from FFISyncProgress to the corresponding Swift properties, providing accurate progress information from the underlying Rust implementation.

dash-spv-ffi/src/types.rs (6)

37-44: LGTM! Clean FFISyncProgress struct implementation.

The new FFISyncProgress struct correctly encapsulates all sync-related progress fields with appropriate types. The structure aligns well with the FFI boundaries and follows Rust naming conventions.


68-72: LGTM! Updated FFISyncStage enum values are correct.

The addition of DownloadingFilterHeaders = 5 and DownloadingFilters = 6 with subsequent renumbering of Complete = 7 and Failed = 8 is properly implemented.


88-93: LGTM! Proper handling of new SyncStage variants.

The FFI conversion correctly maps the new DownloadingFilterHeaders and DownloadingFilters variants from the Rust enum to the FFI enum.


108-108: LGTM! Clean integration of overview field.

The overview: FFISyncProgress field properly replaces the individual sync progress fields, providing a cleaner and more maintainable structure.


130-137: LGTM! Informative stage messages for new sync stages.

The stage messages correctly describe the filter header and filter downloading progress with appropriate current/target information.


142-154: LGTM! Proper construction of FFIDetailedSyncProgress with embedded overview.

The conversion properly creates the overview from the sync progress and correctly integrates it into the FFIDetailedSyncProgress struct.

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

31-33: LGTM! FFISyncStage enum values properly updated.

The addition of DownloadingFilterHeaders = 5 and DownloadingFilters = 6 with renumbered Complete = 7 and Failed = 8 correctly aligns with the Rust FFI definitions.

Based on learnings.


75-83: LGTM! Well-structured FFISyncProgress type.

The new FFISyncProgress struct properly defines all sync progress fields with appropriate C types and follows FFI naming conventions.


92-92: LGTM! Clean integration of overview field in FFIDetailedSyncProgress.

Replacing individual sync fields with the embedded struct FFISyncProgress overview provides better encapsulation and maintainability.


277-283: LGTM! Proper documentation for new drain_events function.

The function declaration and safety documentation are well-written and follow FFI conventions.


396-397: Updated cancellation documentation accurately reflects new behavior.

The documentation correctly states that cancellation now "stops the SPV client, clears callbacks, and joins active threads so the sync operation halts immediately."


706-712: Remove duplicate declaration of dash_spv_ffi_config_set_worker_threads.

This function is declared twice - once here at lines 706-712 and again in the actual implementation section. The duplicate declaration should be removed to avoid confusion.

Based on learnings.

⛔ Skipped due to learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/include/dash_spv_ffi.h : The public C header is generated; ensure it reflects FFI changes after builds
swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Core/SPVClient.swift (7)

11-23: LGTM! Clean refactor to use overview field.

The restructuring to embed overview: SyncProgress and expose currentHeight and connectedPeers as computed properties is a clean architectural improvement that maintains backwards compatibility.


110-116: LGTM! Properly updated SyncStage enum.

The addition of downloadingFilterHeaders and downloadingFilters cases correctly aligns with the FFI layer changes.


130-133: LGTM! Correct FFI enum mapping.

The switch cases for values 5 and 6 correctly map to the new filter-related sync stages.


151-154: LGTM! Clear descriptions for new sync stages.

The descriptions "Downloading filter headers" and "Downloading filters" clearly communicate the sync stage purpose to users.


183-186: LGTM! Appropriate icons for new sync stages.

The icons 🧾 for filter headers and 🪄 for filters provide good visual differentiation.


324-326: LGTM! Enhanced statistics with new sync progress fields.

The addition of Filter Header Height, Filters Downloaded, and Peer Count to the statistics dictionary properly exposes the new overview fields for debugging and monitoring.


1259-1259: No trailing tilde character found at line 1259.

The line ends cleanly with a closing brace }. The previously reported issue appears to have been resolved.

Comment on lines 31 to 35
headersSynced: Bool = false,
filterHeadersSynced: Bool = false,
masternodesSynced: Bool = false,
filtersDownloaded: UInt32 = 0,
lastSyncedFilterHeight: UInt32 = 0
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 | 🟠 Major

Inconsistent parameter names in public initializer.

The public initializer includes deprecated boolean parameters (headersSynced, filterHeadersSynced, masternodesSynced) that are not used in the property assignments, while missing some of the new fields from the parameter list. This creates confusion and potential API inconsistency.

Apply this diff to fix the parameter consistency:

    public init(
        currentHeight: UInt32,
        totalHeight: UInt32,
        progress: Double,
        status: SyncStatus,
        estimatedTimeRemaining: TimeInterval? = nil,
        message: String? = nil,
        filterSyncAvailable: Bool = false,
        filterHeaderHeight: UInt32 = 0,
        masternodeHeight: UInt32 = 0,
        peerCount: UInt32 = 0,
-        headersSynced: Bool = false,
-        filterHeadersSynced: Bool = false,
-        masternodesSynced: Bool = false,
        filtersDownloaded: UInt32 = 0,
        lastSyncedFilterHeight: UInt32 = 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.

Suggested change
headersSynced: Bool = false,
filterHeadersSynced: Bool = false,
masternodesSynced: Bool = false,
filtersDownloaded: UInt32 = 0,
lastSyncedFilterHeight: UInt32 = 0
public init(
currentHeight: UInt32,
totalHeight: UInt32,
progress: Double,
status: SyncStatus,
estimatedTimeRemaining: TimeInterval? = nil,
message: String? = nil,
filterSyncAvailable: Bool = false,
filterHeaderHeight: UInt32 = 0,
masternodeHeight: UInt32 = 0,
peerCount: UInt32 = 0,
filtersDownloaded: UInt32 = 0,
lastSyncedFilterHeight: UInt32 = 0
) {
// existing initializer body…
}
🤖 Prompt for AI Agents
In swift-dash-core-sdk/Sources/SwiftDashCoreSDK/Models/SyncProgress.swift around
lines 31-35, the public initializer parameters are inconsistent with the
struct's properties: deprecated booleans (headersSynced, filterHeadersSynced,
masternodesSynced) are present but not assigned, and the newer fields
(filtersDownloaded, lastSyncedFilterHeight) are missing from the parameter list;
update the initializer signature to accept parameters that match the actual
properties (include filtersDownloaded: UInt32 = 0 and lastSyncedFilterHeight:
UInt32 = 0, remove the unused deprecated boolean parameters or mark them
deprecated if maintaining source-compatibility), and inside the initializer
assign each incoming parameter to the corresponding property so names/types and
default values align with the struct fields.

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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e7ea3e and 8e4b37d.

📒 Files selected for processing (5)
  • dash-spv-ffi/src/bin/ffi_cli.rs (3 hunks)
  • dash-spv/src/client/mod.rs (7 hunks)
  • dash-spv/src/client/status_display.rs (0 hunks)
  • dash-spv/src/sync/filters.rs (1 hunks)
  • dash-spv/src/sync/sequential/mod.rs (1 hunks)
💤 Files with no reviewable changes (1)
  • dash-spv/src/client/status_display.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • dash-spv/src/sync/filters.rs
🧰 Additional context used
📓 Path-based instructions (6)
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/bin/ffi_cli.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)

**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv/src/client/mod.rs
  • dash-spv/src/sync/sequential/mod.rs
{dash-network-ffi,dash-spv-ffi,key-wallet-ffi,swift-dash-core-sdk}/**/*.{rs,c,h,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Be careful with memory management at FFI boundaries (C/Swift interop)

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
**/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
  • dash-spv/src/client/mod.rs
  • dash-spv/src/sync/sequential/mod.rs
*-ffi/**

📄 CodeRabbit inference engine (AGENTS.md)

FFI bindings live in *-ffi crates

Files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
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/client/mod.rs
  • dash-spv/src/sync/sequential/mod.rs
🧠 Learnings (8)
📚 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/src/bin/ffi_cli.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/src/bin/ffi_cli.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/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Expose complex Rust types to C as opaque pointers (e.g., FFIDashSpvClient*)

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.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 : Provide corresponding _destroy() functions for all FFI-owned types to enforce explicit memory management

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-16T04:14:07.718Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-08-16T04:14:07.718Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types so headers are generated correctly

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`

Applied to files:

  • dash-spv-ffi/src/bin/ffi_cli.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: Coordinate synchronization phases with `SequentialSyncManager`; each phase must complete before the next begins

Applied to files:

  • dash-spv/src/client/mod.rs
🧬 Code graph analysis (2)
dash-spv-ffi/src/bin/ffi_cli.rs (1)
dash-spv/src/client/mod.rs (1)
  • peer_count (1389-1391)
dash-spv/src/client/mod.rs (6)
dash-spv/src/client/status_display.rs (1)
  • sync_progress (74-108)
dash-spv/src/network/multi_peer.rs (1)
  • peer_count (1092-1095)
dash-spv/src/network/mod.rs (2)
  • peer_count (56-56)
  • peer_count (254-260)
dash-spv/src/network/mock.rs (1)
  • peer_count (155-161)
dash-spv/src/types.rs (3)
  • default (57-70)
  • default (420-422)
  • default (569-598)
dash-spv/src/sync/sequential/phases.rs (1)
  • progress (278-524)
⏰ 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). (15)
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: Strict Warnings and Clippy Checks
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_deserialize_address)

@QuantumExplorer QuantumExplorer merged commit c44c91b into v0.40-dev Sep 28, 2025
32 checks passed
@QuantumExplorer QuantumExplorer deleted the fix/syncProgress branch September 28, 2025 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants