Skip to content

feat: add cycle to SyncEvent::SyncComplete #459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xdustinface merged 1 commit into from
Feb 25, 2026
Merged

feat: add cycle to SyncEvent::SyncComplete #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dash-spv-ffi/include/dash_spv_ffi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ typedef void (*OnManagerErrorCallback)(enum FFIManagerId manager_id,
/**
* Callback for SyncEvent::SyncComplete
*/
typedef void (*OnSyncCompleteCallback)(uint32_t header_tip, void *user_data);
typedef void (*OnSyncCompleteCallback)(uint32_t header_tip, uint32_t cycle, void *user_data);

/**
* Sync event callbacks - one callback per SyncEvent variant.
Expand Down
4 changes: 2 additions & 2 deletions dash-spv-ffi/src/bin/ffi_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ extern "C" fn on_manager_error(
println!("[Sync] Manager error: {:?} - {}", manager_id, error_str);
}

extern "C" fn on_sync_complete(header_tip: u32, _user_data: *mut c_void) {
println!("[Sync] Sync complete at height: {}", header_tip);
extern "C" fn on_sync_complete(header_tip: u32, cycle: u32, _user_data: *mut c_void) {
println!("[Sync] Sync complete at height: {} (cycle {})", header_tip, cycle);
}

// ============================================================================
Expand Down
6 changes: 4 additions & 2 deletions dash-spv-ffi/src/callbacks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ pub type OnManagerErrorCallback =
Option<extern "C" fn(manager_id: FFIManagerId, error: *const c_char, user_data: *mut c_void)>;

/// Callback for SyncEvent::SyncComplete
pub type OnSyncCompleteCallback = Option<extern "C" fn(header_tip: u32, user_data: *mut c_void)>;
pub type OnSyncCompleteCallback =
Option<extern "C" fn(header_tip: u32, cycle: u32, user_data: *mut c_void)>;

/// Sync event callbacks - one callback per SyncEvent variant.
///
Expand Down Expand Up @@ -409,9 +410,10 @@ impl FFISyncEventCallbacks {
}
SyncEvent::SyncComplete {
header_tip,
cycle,
} => {
if let Some(cb) = self.on_sync_complete {
cb(*header_tip, self.user_data);
cb(*header_tip, *cycle, self.user_data);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion dash-spv-ffi/tests/unit/test_async_operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ mod tests {
data.headers_stored.store(true, Ordering::SeqCst);
}

extern "C" fn on_sync_complete(_header_tip: u32, user_data: *mut c_void) {
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);
}
Comment on lines +272 to 275
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ 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.

Copy link
Collaborator

@ZocoLini ZocoLini Feb 24, 2026

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

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

@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)

Expand Down
9 changes: 8 additions & 1 deletion dash-spv/src/sync/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,17 @@ pub enum SyncEvent {

/// Sync has reached the chain tip (all managers idle).
///
/// Emitted on every not-synced to synced transition. Cycle 0 is the
/// initial sync while subsequent cycles are incremental syncs triggered by
/// new blocks arriving from the network.
///
/// Emitted by: Coordinator
/// Consumed by: External listeners
SyncComplete {
/// Final header tip height
header_tip: u32,
/// Sync cycle (0 = initial, 1+ = incremental)
cycle: u32,
},
}

Expand Down Expand Up @@ -235,8 +241,9 @@ impl SyncEvent {
}
SyncEvent::SyncComplete {
header_tip,
cycle,
} => {
format!("SyncComplete(tip={})", header_tip)
format!("SyncComplete(tip={}, cycle={})", header_tip, cycle)
}
}
}
Expand Down
18 changes: 12 additions & 6 deletions dash-spv/src/sync/sync_coordinator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,9 @@ async fn run_progress_task(

let mut merged = select_all(streams);
let mut progress = SyncProgress::default();
let mut sync_complete_emitted = false;
let mut was_synced = false;
let mut sync_cycle: u32 = 0;
let mut cycle_start = sync_start_time;

loop {
tokio::select! {
Expand All @@ -349,14 +351,18 @@ async fn run_progress_task(

let _ = progress_sender.send(progress.clone());

if progress.is_synced() && !sync_complete_emitted {
let duration = sync_start_time.elapsed();
tracing::info!("Initial sync complete in {:.2}s", duration.as_secs_f64());
let is_synced = progress.is_synced();
if is_synced && !was_synced {
let duration = cycle_start.elapsed();
tracing::info!("Sync complete in {:.2}s (cycle {})", duration.as_secs_f64(), sync_cycle);

let header_tip = progress.headers().ok().map(|h| h.current_height()).unwrap_or(0);
let _ = sync_event_sender.send(SyncEvent::SyncComplete { header_tip });
sync_complete_emitted = true;
let _ = sync_event_sender.send(SyncEvent::SyncComplete { header_tip, cycle: sync_cycle });
sync_cycle += 1;
} else if !is_synced && was_synced {
cycle_start = Instant::now();
}
was_synced = is_synced;
}
}
}
Expand Down
Loading