Skip to content

Comments

fix(stratum_v1): support float difficulty in mining.set_difficulty#24

Open
average-gary wants to merge 7 commits into256foundation:mainfrom
average-gary:fix/stratum-difficulty-float-parsing
Open

fix(stratum_v1): support float difficulty in mining.set_difficulty#24
average-gary wants to merge 7 commits into256foundation:mainfrom
average-gary:fix/stratum-difficulty-float-parsing

Conversation

@average-gary
Copy link

Summary

Stratum v1 pools may send difficulty as either integer or float values (e.g., 0.001 for low-difficulty vardiff). The previous implementation used as_u64() which rejected float values with the error difficulty not a number.

Problem

When connecting to pools or translators that use variable difficulty with sub-integer values, mujina would fail with:

WARN stratum_v1::client: Error handling notification
     error=Invalid message format: difficulty not a number

This was encountered when testing with the SRI (Stratum Reference Implementation) translator which sends float difficulties for vardiff.

Changes

  • Change DifficultyChanged event payload from u64 to f64
  • Update handle_set_difficulty to parse with as_f64(), falling back to as_u64() for compatibility
  • Update ProtocolState.difficulty field to Option<f64>
  • Use Difficulty::from_f64() instead of Difficulty::from() when converting to internal Difficulty type
  • Add test case for float difficulty parsing

Testing

  • All existing tests pass
  • Added new test test_handle_set_difficulty_float that verifies float parsing
  • Manually tested against SRI translator with vardiff enabled

@rkuester
Copy link
Collaborator

rkuester commented Feb 3, 2026

This issue was also reported, with a log of the Stratum v1 conversation, in #27.

@rkuester rkuester force-pushed the fix/stratum-difficulty-float-parsing branch from d92a7d4 to dc89858 Compare February 3, 2026 22:09
@rkuester
Copy link
Collaborator

rkuester commented Feb 3, 2026

Branch main had some style inconsistencies, see d76cc42 and 49aa16b. I rebased this PR atop the fixes in main to get rid of the formatting noise in the PR.

Copy link
Collaborator

@rkuester rkuester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Glad you submitted this. @pool2win and I considered implementing fractional difficulties in Mujina and Hydrapool, but weren't sure it was valid Stratum v1 (whatever that is!). Good to know some servers do implement it.

A few comments...

@average-gary average-gary force-pushed the fix/stratum-difficulty-float-parsing branch from dc89858 to 1d16cb2 Compare February 17, 2026 19:28
rkuester and others added 7 commits February 20, 2026 12:00
Add cases for whole and fractional f64 difficulties in the existing
display test, with comments grouping them by behavior.
from_f64 truncates to integer arithmetic before dividing.
For values >= 1.0, the fractional part is dropped (200.5
becomes 200). For sub-1.0 values, the reciprocal is
truncated (0.003 becomes 1/333, round-tripping as
0.003003).

This test asserts the correct behavior and is marked
#[should_panic] to document the known bug while keeping
CI green.
Implement Div<f64> for U256 by decomposing the float into
its exact rational form (mantissa * 2^exponent) via
num-traits' Float::integer_decode() and performing the
division entirely in integer arithmetic, preserving all
53 bits of mantissa precision.

For negative exponents (fractional divisors), a 512-bit
intermediate avoids overflow when left-shifting the
dividend.

Bring in num-traits 0.2 for the float decomposition.
The previous implementation truncated the f64 to integer
arithmetic before dividing. For values >= 1.0, the
fractional part was dropped (200.5 became 200). For
sub-1.0 values, the reciprocal was truncated (0.003
became 1/333, round-tripping as 0.003003). Delegate to
U256's Div<f64>, which preserves all 53 bits of mantissa
precision.

Remove the #[should_panic] annotation from the preceding
round-trip test, since it now passes.
Stratum v1 pools may send difficulty as either integer or
float values (e.g., 0.001 for low-difficulty vardiff).
The previous implementation used as_u64() which rejected
float values with "difficulty not a number".

Switch the DifficultyChanged event payload and
ProtocolState.difficulty field from u64 to f64, and parse
incoming difficulty with as_f64(). Construct the internal
Difficulty type via Difficulty::from_f64(). Add a guard
for non-finite values as defense-in-depth (JSON has no
NaN/Infinity representation, so serde_json never produces
them).

Add tests for float difficulty parsing.
The SourceState.difficulty field was u64, which truncated
fractional difficulties (e.g. 0.001 became 0). Change to
f64 to preserve the value received from the pool.
The 256-bit target-to-difficulty conversion can introduce
tiny arithmetic residuals in the low-order digits. Round
to 12 significant digits, derived from the conversion's
error bound with four orders of magnitude of margin.
@rkuester rkuester force-pushed the fix/stratum-difficulty-float-parsing branch 2 times, most recently from 7a66f83 to c7f2495 Compare February 20, 2026 21:47
Copy link
Collaborator

@rkuester rkuester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for making those changes! I reviewed them, and squashed them into your commit.

Then I realized Difficulty wasn't quite ready to handle from_f64() and as_f64() accurately, and added several commits shore that up.

Would you please take a quick look at my additions, and give this branch one more test, with a pool that actually sends fractional values, before I merge it?

@rkuester
Copy link
Collaborator

Would you please take a quick look at my additions, and give this branch one more test, with a pool that actually sends fractional values, before I merge it?

In particular, cargo run --bin mujina-cli -- api miner should show the fractional difficulty correctly, once things are up and running.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants