Repository navigation
feat(blockchain): add --disable-duty-sync-gate to ungate duties #452
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| use tracing::debug; | ||
|
|
||
| use crate::metrics::SyncStatus; | ||
|
|
||
| /// Local head lag beyond which the node is considered to be syncing. | ||
|
|
@@ -12,12 +14,35 @@ const NETWORK_STALL_THRESHOLD: u64 = 8; | |
| /// Recovery band that prevents the sync status from flapping near the threshold. | ||
| const SYNC_HYSTERESIS_BAND: u64 = 2; | ||
|
|
||
| #[derive(Default)] | ||
| pub(crate) struct SyncStatusTracker { | ||
| syncing: bool, | ||
| /// Whether the syncing state suppresses validator duties. | ||
| /// | ||
| /// When `false`, [`Self::update`] still tracks `syncing` and drives the | ||
| /// `lean_node_sync_status` metric, but [`Self::duties_allowed`] always | ||
| /// returns `true`: the gate is observe-only. Seeded from the CLI | ||
| /// `--disable-duty-sync-gate` flag (gating stays on by default). | ||
| gate_duties: bool, | ||
| } | ||
|
|
||
| impl Default for SyncStatusTracker { | ||
| fn default() -> Self { | ||
| Self { | ||
| syncing: false, | ||
| gate_duties: true, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl SyncStatusTracker { | ||
| /// Build a tracker, choosing whether the syncing state gates duties. | ||
| pub(crate) fn new(gate_duties: bool) -> Self { | ||
| Self { | ||
| gate_duties, | ||
| ..Self::default() | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn update( | ||
| &mut self, | ||
| current_slot: u64, | ||
|
|
@@ -26,6 +51,7 @@ impl SyncStatusTracker { | |
| ) -> SyncStatus { | ||
| let head_lag = current_slot.saturating_sub(head_slot); | ||
| let network_lag = current_slot.saturating_sub(max_seen_slot); | ||
| let was_syncing = self.syncing; | ||
|
|
||
| if network_lag > NETWORK_STALL_THRESHOLD { | ||
| self.syncing = false; | ||
|
|
@@ -35,6 +61,18 @@ impl SyncStatusTracker { | |
| self.syncing = head_lag > SYNC_LAG_THRESHOLD; | ||
| } | ||
|
|
||
| if self.syncing != was_syncing { | ||
| debug!( | ||
| current_slot, | ||
| head_slot, | ||
| max_seen_slot, | ||
| head_lag, | ||
| network_lag, | ||
| syncing = self.syncing, | ||
| "Sync status changed" | ||
| ); | ||
| } | ||
|
|
||
| if self.syncing { | ||
| SyncStatus::Syncing | ||
| } else { | ||
|
|
@@ -43,7 +81,8 @@ impl SyncStatusTracker { | |
| } | ||
|
|
||
| pub(crate) fn duties_allowed(&self) -> bool { | ||
| !self.syncing | ||
| // Gate disabled: the syncing state is observe-only, never suppresses duties. | ||
| !self.gate_duties || !self.syncing | ||
| } | ||
|
Comment on lines
83
to
86
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The PR description states "8/8 pass, incl. two new tests covering gate-on (default) and gate-disabled," but the test module in this file ends at 6 tests (all exercising Prompt To Fix With AIThis is a comment left during a code review.
Path: crates/blockchain/src/sync_status.rs
Line: 83-86
Comment:
**Claimed tests for `duties_allowed()` are absent from the diff**
The PR description states "8/8 pass, incl. two new tests covering gate-on (default) and gate-disabled," but the test module in this file ends at 6 tests (all exercising `update()` via `SyncStatusTracker::default()`) and no new test functions appear anywhere in the diff. `duties_allowed()` has zero test coverage in this changeset, so a future regression in the `!self.gate_duties || !self.syncing` expression — or an accidental revert of the `Default` impl's `gate_duties: true` — would go undetected.
How can I resolve this? If you propose a fix, please make it concise. |
||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.