feat: add `cycle` to `SyncEvent::SyncComplete` by xdustinface · Pull Request #459 · dashpay/rust-dashcore

xdustinface · 2026-02-20T20:24:55Z

Emit SyncComplete on every not-synced to synced transition instead of only once, with a cycle field (0 = initial, 1+ = incremental) so consumers can distinguish sync phases. Duration logging measures per-cycle time instead of time since client start.

Summary by CodeRabbit

Release Notes

New Features
- Enhanced sync completion tracking with cycle information now included in sync events for better progress monitoring and cycle management.

Emit `SyncComplete` on every not-synced to synced transition instead of only once, with a cycle field (0 = initial, 1+ = incremental) so consumers can distinguish sync phases. Duration logging measures per-cycle time instead of time since client start.

coderabbitai · 2026-02-20T20:25:06Z

📝 Walkthrough

Walkthrough

A cycle parameter is added to the OnSyncCompleteCallback function signature and SyncComplete event variant across FFI interfaces, callbacks, and synchronization logic to track synchronization cycles. The cycle value propagates from the sync coordinator through the event and callback systems, with cycle counting and timing logic implemented in the progress aggregation task.

Changes

Cohort / File(s)	Summary
FFI Header & Type Definitions `dash-spv-ffi/include/dash_spv_ffi.h`, `dash-spv-ffi/src/callbacks.rs`	OnSyncCompleteCallback function pointer type updated to include `uint32_t cycle` parameter between `header_tip` and `user_data`.
Callback Implementations `dash-spv-ffi/src/bin/ffi_cli.rs`, `dash-spv-ffi/tests/unit/test_async_operations.rs`	Callback function signatures updated to accept the new cycle parameter; ffi_cli now logs cycle value alongside header tip.
Sync Event & Coordination Logic `dash-spv/src/sync/events.rs`, `dash-spv/src/sync/sync_coordinator.rs`	SyncComplete event variant expanded with `cycle: u32` field; sync coordinator updated with cycle tracking, timing per cycle, and cycle counter incrementation on transition to synced state.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A cycle spins, a new dawn breaks,
Each sync complete, the counter wakes,
From tip to cycle, data flows,
Through callbacks dancing, onward goes! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: adding a `cycle` field to `SyncEvent::SyncComplete`, which is the primary alteration across all modified files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/sync-cycle

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.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dash-spv-ffi/tests/unit/test_async_operations.rs`:
- Around line 272-275: The integration test defines unused callbacks
on_sync_progress and on_sync_complete whose signatures don't match the official
OnSyncProgressCallback/OnSyncCompleteCallback (see callbacks.rs) and are never
registered via dash_spv_ffi_client_set_sync_event_callbacks; either remove these
unused functions from dash-spv-ffi/tests/integration/test_full_workflow.rs or
update their signatures to match the official callback types (and then pass them
into dash_spv_ffi_client_set_sync_event_callbacks so they are actually
registered), ensuring the function names (on_sync_progress, on_sync_complete)
and registration call match the FFI types used in callbacks.rs.

coderabbitai · 2026-02-20T21:05:03Z

dash-spv-ffi/tests/unit/test_async_operations.rs

+            extern "C" fn on_sync_complete(_header_tip: u32, _cycle: u32, user_data: *mut c_void) {
                let data = unsafe { &*(user_data as *const EventData) };
                data.sync_complete.store(true, Ordering::SeqCst);
            }


⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n 'on_sync_complete' --type=rs -C 8 dash-spv-ffi/tests/

Repository: dashpay/rust-dashcore

Length of output: 91

🏁 Script executed:

rg -n 'on_sync_complete' --type rust -C 8 dash-spv-ffi/tests/

Repository: dashpay/rust-dashcore

Length of output: 5172

🏁 Script executed:

rg -n 'OnSyncCompleteCallback|on_sync_complete.*=' --type rust -C 3 dash-spv-ffi/

Repository: dashpay/rust-dashcore

Length of output: 1121

🏁 Script executed:

rg -n 'FFISyncEventCallbacks' --type rust -C 5 dash-spv-ffi/tests/integration/test_full_workflow.rs

Repository: dashpay/rust-dashcore

Length of output: 47

🏁 Script executed:

rg -n 'extern "C" fn' --type rust -C 2 dash-spv-ffi/tests/integration/test_full_workflow.rs | head -60

Repository: dashpay/rust-dashcore

Length of output: 643

🏁 Script executed:

rg -n 'on_sync_progress\|on_sync_complete' --type rust -B 30 dash-spv-ffi/tests/integration/test_full_workflow.rs | grep -A 30 "struct\|FFI" | head -80

Repository: dashpay/rust-dashcore

Length of output: 47

🏁 Script executed:

sed -n '60,150p' dash-spv-ffi/tests/integration/test_full_workflow.rs

Repository: dashpay/rust-dashcore

Length of output: 3536

🏁 Script executed:

rg -n 'dash_spv_ffi_client_set_sync_event_callbacks\|on_sync' --type rust dash-spv-ffi/tests/integration/test_full_workflow.rs

Repository: dashpay/rust-dashcore

Length of output: 47

🏁 Script executed:

sed -n '1,200p' dash-spv-ffi/tests/integration/test_full_workflow.rs | grep -n 'on_sync_progress\|on_sync_complete\|set_sync_event'

Repository: dashpay/rust-dashcore

Length of output: 280

🏁 Script executed:

rg -n 'dash_spv_ffi' --type rust dash-spv-ffi/tests/integration/test_full_workflow.rs | grep -i 'callback\|event'

Repository: dashpay/rust-dashcore

Length of output: 47

🏁 Script executed:

wc -l dash-spv-ffi/tests/integration/test_full_workflow.rs && sed -n '90,200p' dash-spv-ffi/tests/integration/test_full_workflow.rs

Repository: dashpay/rust-dashcore

Length of output: 4179

Unit test callback signature is correct; integration test has unused callbacks with incompatible signature.

The unit test correctly implements on_sync_complete(_header_tip: u32, _cycle: u32, user_data: *mut c_void) matching the official OnSyncCompleteCallback type defined in callbacks.rs:212.

However, the integration test in dash-spv-ffi/tests/integration/test_full_workflow.rs (lines 71, 83) defines callbacks with an incompatible signature:

on_sync_progress(progress: f64, msg: *const c_char, user_data: *mut c_void)

on_sync_complete(success: bool, error: *const c_char, user_data: *mut c_void)

These callbacks are never used—they're defined but never passed to dash_spv_ffi_client_set_sync_event_callbacks or any other FFI function. Remove these unused callbacks or update them to match the official callback types and properly register them.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@dash-spv-ffi/tests/unit/test_async_operations.rs` around lines 272 - 275, The integration test defines unused callbacks on_sync_progress and on_sync_complete whose signatures don't match the official OnSyncProgressCallback/OnSyncCompleteCallback (see callbacks.rs) and are never registered via dash_spv_ffi_client_set_sync_event_callbacks; either remove these unused functions from dash-spv-ffi/tests/integration/test_full_workflow.rs or update their signatures to match the official callback types (and then pass them into dash_spv_ffi_client_set_sync_event_callbacks so they are actually registered), ensuring the function names (on_sync_progress, on_sync_complete) and registration call match the FFI types used in callbacks.rs.

those tests are out of scope and are not being executed, dont worry about it

@ZocoLini, understood! I'll skip those tests since they're out of scope. Thanks for clarifying.

🧠 Learnings used

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronize blockchain headers in Phase 1, masternode list in Phase 2, filter headers in Phase 3, filters in Phase 4, and blocks in Phase 5

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/tests/**/*.rs : Add integration tests when network interaction is involved

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/src/sync/**/*.rs : Implement independent state management for each sync phase tracked with clear state transitions

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/tests/**/*.rs : Add comprehensive unit tests in-module and integration tests in the tests/ directory

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv-ffi/CLAUDE.md:0-0 Timestamp: 2025-12-01T07:59:58.608Z Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/src/**/*.rs : Use async/await throughout the codebase, built on tokio runtime

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/src/network/**/*.rs : Implement configurable timeouts with recovery mechanisms for network operations

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/src/error.rs : Update error types in `error.rs` for new failure modes

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/src/sync/**/*.rs : Use `SyncState` enum to drive synchronization state machines

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv/CLAUDE.md:0-0 Timestamp: 2026-02-04T20:09:58.668Z Learning: Applies to dash-spv/src/sync/**/*.rs : Use `SyncManager` for organized phase-based synchronization with clear state transitions

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-16T14:53:56.708Z Learning: Applies to **/rpc-integration-test/**/*.rs : Write integration tests for network operations

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv-ffi/CLAUDE.md:0-0 Timestamp: 2025-12-01T07:59:58.608Z Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv-ffi/CLAUDE.md:0-0 Timestamp: 2025-12-01T07:59:58.608Z Learning: Applies to dash-spv-ffi/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust

Learnt from: CR Repo: dashpay/rust-dashcore PR: 0 File: dash-spv-ffi/CLAUDE.md:0-0 Timestamp: 2025-12-01T07:59:58.608Z Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)

ZocoLini

Do we really need to add this?? iOS doesn't need it and I think we should add features following iOS needs for the moment

ZocoLini · 2026-02-24T16:37:10Z

dash-spv-ffi/tests/unit/test_async_operations.rs

+            extern "C" fn on_sync_complete(_header_tip: u32, _cycle: u32, user_data: *mut c_void) {
                let data = unsafe { &*(user_data as *const EventData) };
                data.sync_complete.store(true, Ordering::SeqCst);
            }


those tests are out of scope and are not being executed, dont worry about it

xdustinface · 2026-02-25T01:46:30Z

Its mostly for the integration tests for now but i think it will also be helpful for iOS to know wether we finished the initial sync (without keeping state in iOS) or not since we move between synced and not synced with the recent/coming changes to the state machine. Alternative would be a initial_sync: bool flag but i thought the actual cycle counter might he helpful too for debugging or so.

Emit `SyncComplete` on every not-synced to synced transition instead of only once, with a cycle field (0 = initial, 1+ = incremental) so consumers can distinguish sync phases. Duration logging measures per-cycle time instead of time since client start.

xdustinface marked this pull request as ready for review February 20, 2026 21:00

coderabbitai bot reviewed Feb 20, 2026

View reviewed changes

xdustinface mentioned this pull request Feb 21, 2026

feat: add dashd integration tests for SPV sync #464

Merged

ZocoLini reviewed Feb 24, 2026

View reviewed changes

ZocoLini approved these changes Feb 25, 2026

View reviewed changes

xdustinface merged commit 9c41375 into v0.42-dev Feb 25, 2026
53 checks passed

xdustinface deleted the feat/sync-cycle branch February 25, 2026 22:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `cycle` to `SyncEvent::SyncComplete`#459

feat: add `cycle` to `SyncEvent::SyncComplete`#459
xdustinface merged 1 commit intov0.42-devfrom
feat/sync-cycle

xdustinface commented Feb 20, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Feb 20, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Feb 20, 2026 •

edited

Loading

Uh oh!

ZocoLini Feb 24, 2026

Uh oh!

coderabbitai bot Feb 24, 2026

Uh oh!

ZocoLini left a comment

Uh oh!

ZocoLini Feb 24, 2026

Uh oh!

xdustinface commented Feb 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

xdustinface commented Feb 20, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZocoLini Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

ZocoLini left a comment

Choose a reason for hiding this comment

Uh oh!

ZocoLini Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

xdustinface commented Feb 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xdustinface commented Feb 20, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 20, 2026 •

edited

Loading

coderabbitai bot Feb 20, 2026 •

edited

Loading