fix(windows-miner): back off transient header failures#8018
Conversation
|
Welcome to RustChain! Thanks for your first pull request. Before we review, please make sure:
Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150) A maintainer will review your PR soon. Thanks for contributing! |
elevasyncsolutions-jpg
left a comment
There was a problem hiding this comment.
Review of PR #8018 — fix(windows-miner): back off transient header failures
Overall this is a well-structured fix for #7368. Two substantive observations:
-
Missing jitter in exponential backoff — The
_schedule_header_retryuses pure exponential backoff (10, 20, 40, 80…). If multiple miners lose connectivity at the same time (e.g. a node restart), they will retry in lockstep. Adding a small random jitter (±25% of the delay) would spread retries and reduce node load on recovery. See AWS retry best practices. -
Redundant isinstance check — In
_is_retryable_header_status, the checkisinstance(status_code, int) and 500 <= status_code <= 599has an unnecessary isinstance guard —requestsalways returnsintforstatus_code. The real guard is against the case whereresponseisNone(network error), but that path already setslast_header_retryable = Truein the exception handler. The 5xx range check alone would be cleaner.
Positive: The separation of terminal vs retryable failures with the _reset_header_retry() mechanism is a significant improvement over the previous binary handled/not-handled logic. The bounded cap at 300s prevents runaway waits. Good defensive design.
|
Addressed the review feedback in 38b2890: retry jitter is now ±25% (while preserving the 300 s cap), and the redundant status-code type guard was removed. Bootstrap hashes were refreshed in abb066f. All 12 upstream checks, including the full 3,640-test CI job and Windows build, are green. Ready for maintainer review. |
|
Merged — bounded exponential backoff (10s base, 300s cap, retryable on 408/425/429) with the diagnostic surfaced, exactly what #16252 asked for. Awarded 12 RTC, sent to your wallet with the other 65. Thanks for the consistently clean work. — Sophia |
RTC RewardThis merged PR earned 5 RTC — sent to |
JHON12091986
left a comment
There was a problem hiding this comment.
Review: Looks good! Minor suggestion: ...
Closes #7368
Bounty: Scottcjn/rustchain-bounties#16252
BCOS:
BCOS-L1What this completes
#7565 stopped the original hot loop and surfaced the rejection diagnostic, but it marked every failed slot handled. A temporary connection failure or restarting node therefore never retried during the slot. This PR completes the missing retry-policy part of #7368:
class=terminal no_retryorclass=retryable retry_in=Nsand retains the node's diagnostic;Before / after reproduction
Before this PR on current
main, a network blip was treated as permanently handled:After this PR, temporary failures rebuild the header and back off:
Terminal rejection remains one-shot. This is the actual output from a one-time, identity-free invalid-header probe against the repository's public node endpoint on 2026-07-20:
The same run recorded
last_submitted_slot=-7368,retry_slot=null;_header_submission_due(-7368)stays false at later timestamps, so there is no second POST of that rejected header.Verification
Covered suites:
python miners/windows/check_bootstrap_hashes.py->Windows bootstrap hashes are in sync.python -m py_compile miners/windows/rustchain_windows_miner.pyThe live probe bypassed miner initialization and sent no wallet, key, signature, or user identity; it only exercised the public endpoint's terminal validation response.