Skip to content

bug: SyncManager does not emit progress update on manager error path #469

@lklimek

Description

@lklimek

Summary

When a sync manager (e.g., MasternodesSyncManager) returns an error from handle_message(), the SyncManager::run() loop correctly emits a SyncEvent::ManagerError event but does not call try_emit_progress(). This means consumers watching the progress channel never learn that the manager's state transitioned to SyncState::Error.

Location

File: dash-spv/src/sync/sync_manager.rs
Commit: a05d256f59743c69df912462dd77dd487e1ff5b2

In the run() method's main select loop, there are several places where handle_message, handle_sync_event, handle_network_event, or tick return errors. In each of these Err branches, try_emit_progress() is not called, even though it is called in the corresponding Ok branches.

For example (message handling path):

// Ok branch — emits progress ✓
Ok(events) => {
    // ...
    self.try_emit_progress(progress_before, &context.progress_sender);
}
// Err branch — does NOT emit progress ✗
Err(e) => {
    tracing::error!("{} error handling message: {}", identifier, e);
    let error_event = SyncEvent::ManagerError { manager: identifier, error: e.to_string() };
    context.emit_sync_event(error_event);
    // ← missing: self.try_emit_progress(progress_before, &context.progress_sender);
}

The same pattern repeats for:

  • handle_sync_event error branch
  • handle_network_event error branch
  • tick error branch

Impact

The individual manager (e.g., MasternodesSyncManager) correctly calls self.set_state(SyncState::Error) before returning the error, so the aggregate SyncProgress::state() would return SyncState::Error. However, since try_emit_progress() is skipped on the error path, this state change is never sent through the watch channel.

Consumers that rely on the progress channel to detect errors (rather than the SyncEvent broadcast) will never see the transition and remain stuck believing sync is still in progress.

Observed Behavior

In Dash Evo Tool, SPV sync gets permanently stuck in "Syncing" status when masternode sync fails with:

ERROR dash_spv::sync::masternodes::sync_manager: QRInfo failed after 4 retries: Required rotated chain lock sig at h - 0 not present for masternode diff block hash: ...
ERROR dash_spv::sync::sync_manager: Masternode error handling message: Masternode sync failed: ...

The monitor_network() future continues running (the error is handled internally), so the caller never receives an Err result either.

Suggested Fix

Add try_emit_progress() calls to all error branches in the run() method:

Err(e) => {
    tracing::error!("{} error handling message: {}", identifier, e);
    let error_event = SyncEvent::ManagerError { manager: identifier, error: e.to_string() };
    context.emit_sync_event(error_event);
    self.try_emit_progress(progress_before, &context.progress_sender); // ← add this
}

🤖 Co-authored by Claudius the Magnificent AI Agent

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions