perf(mining): eliminate polling delays when block templates change - #700
Conversation
|
I was just starting to work on this - thanks! |
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
APPROVE — no blocking findings.
The retained atomic version and watch notification close both relevant races: an update before subscription is detected by the version comparison, while an update during a blocked receive wakes the waiter. Build replacement, clearing, and consumption still converge on increment_version; the biased tokio::select! prioritizes cancellation/template invalidation over an old result; and dropping mpsc::Receiver::recv is cancellation-safe, so the next miner result remains available. Existing version and seal validation stays in place, and the 250 ms fallback correctly remains for proposal failures that publish no new build.
Validation at exact head 6c7018d719c098d169913b8246146aab505dbda2:
git diff --check f1176cea...HEAD— passed.taplo format --check --config taplo.toml— passed.scripts/fmt.sh --all -- --check— passed with pinnednightly-2026-08-30rustfmt.SKIP_WASM_BUILD=1 cargo test --locked -p sc-consensus-qpow— 13 passed.SKIP_WASM_BUILD=1 cargo test --locked -p quantus-node --bin quantus-node— 62 passed.SKIP_WASM_BUILD=1 cargo clippy --locked -p sc-consensus-qpow -p quantus-node --all-targets -- -D warnings— passed.SKIP_WASM_BUILD=1 RUSTDOCFLAGS='-D warnings' cargo doc --locked --no-deps -p sc-consensus-qpow -p quantus-node— passed.- Live format and Clippy/doc CI jobs passed; the Linux/macOS test matrix was still running at review time.
No blocking findings.
Problem
After a mining template changes, the external-miner loop can wait for the remainder of a 500 ms result timeout before broadcasting replacement work. When no candidate exists, both mining paths can also wait for the remainder of a 250 ms retry interval after a new proposal is ready. These waits leave miners on obsolete work or idle despite an available candidate.
Change
Add a version-change notification to the mining handle while retaining atomic version reads for the local mining hot path. Waiters subscribe before checking the version, so an update before subscription or during registration is observed. All existing build replacement, clearing and consumption paths notify through the existing version increment method.
External mining selects between results, template changes and shutdown, prioritizing invalidation when multiple branches are ready. Result reception is cancellation-safe, and the existing version checks and seal verification remain in place. The no-candidate path wakes immediately on a version change while retaining the 250 ms fallback for failed proposal retries.
This removes polling-related delays; it does not change consensus, difficulty adjustment, the 12-second target block interval, or the miner wire protocol. The 500/250 ms figures are existing timer intervals, not measured end-to-end speedups. No mainnet performance gain is claimed.
Validation
cargo +stable test --offline -p sc-consensus-qpow: 13 tests passed, including updates before subscription, waking multiple pending waiters without a timer, and cancellation followed by another wait.cargo +stable test -p quantus-node --bin quantus-node: 62 tests passed.Native tests use Rust 1.95 and
SKIP_WASM_BUILD=1; the pinned Rust 1.93 toolchain and Wasm build have not been validated locally.