channels_sv2: clamp target to max_target instead of erroring#2118
Open
vnprc wants to merge 1 commit intostratum-mining:mainfrom
Open
channels_sv2: clamp target to max_target instead of erroring#2118vnprc wants to merge 1 commit intostratum-mining:mainfrom
vnprc wants to merge 1 commit intostratum-mining:mainfrom
Conversation
When a pool's vardiff computes a target easier than the client's declared max_target, update_channel() returned Err(RequestedMaxTargetOutOfRange). This silently broke vardiff: once triggered, no further difficulty adjustments succeeded for that channel. The pool logged a WARN every ~60s per affected channel while the miner stayed stuck at its last successfully-set difficulty. The max_target field in OpenExtendedMiningChannel means "the easiest difficulty I will accept." If the pool's calculation produces something easier, the correct response is to assign exactly max_target — not to error. The client explicitly declared this difficulty acceptable. Apply the same clamping logic to both ExtendedChannel and StandardChannel, in both new() (channel creation) and update_channel() (vardiff updates). Update the corresponding tests to assert clamping behavior rather than expecting an error. Also remove two stray println! debug statements from ExtendedChannel::new(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a pool's vardiff computes a target easier than the client's declared
max_target,update_channel()returnsErr(RequestedMaxTargetOutOfRange).This silently breaks vardiff for the affected channel: the error is swallowed by
the caller, no difficulty update is applied, and the miner is permanently stuck
at its last successfully-set difficulty. The pool logs a
WARNroughly every60 seconds per affected channel while mining continues at the wrong difficulty.
This is not a hypothetical edge case. It triggers reliably for low-hashrate
miners whose natural vardiff target overshoots their declared
max_target.Root Cause
The
max_targetfield inOpenExtendedMiningChannelmeans "the easiestdifficulty I will accept." When vardiff computes a target easier than
max_target, the correct response is to assign exactlymax_target— theclient has already declared that difficulty acceptable. The current code errors
instead, which is both incorrect per the spec and operationally harmful.
Fix
Replace the
RequestedMaxTargetOutOfRangeerror path with a clamp in fourplaces:
ExtendedChannel::new()— channel creationExtendedChannel::update_channel()— vardiff updatesStandardChannel::new()— channel creationStandardChannel::update_channel()— vardiff updatesAlso removes two stray
println!debug statements fromExtendedChannel::new().Tests
The existing
test_update_channeltests in bothextended.rsandstandard.rsalready exercise this path (
very_small_hashrate = 0.1). They previouslyasserted
is_err()/RequestedMaxTargetOutOfRange. They now assertis_ok()and verify the channel target was clamped to
not_so_permissive_max_target.All 50 unit tests pass.
Notes
RequestedMaxTargetOutOfRangeis preserved in both error enums — it remainsreachable from the
UpdateChannelmessage handler path where the client itselfsends a bad
max_target.Pre-existing CI failure
cargo clippycurrently fails on upstreammaindue toneedless_lifetimesinbinary_sv2— unrelated to this PR.