Skip to content

Commit 2d10f57

Browse files
author
Eric Price
committed
fix(channels_sv2): widen hash_rate_from_target multiply to U512
The *100 -> *100_000 scaling fix in hash_rate_from_target lowered the safe target ceiling from ~2^246.4 to ~2^236.4, because the intermediate product `(t+1) * shares_occurrency_frequence` is computed in U256. Routine vardiff-driven targets at low realized share rates push above the new boundary, and broadcast SetTarget messages carrying a channel's `requested_max_target` (~2^253) trip it every time. In production, this surfaced on a slot running translator_sv2 with vardiff disabled: every upstream SetTarget logs WARN: Failed to derive hashrate from SetTarget target: ArithmeticOverflow (channel_id=4294967295) and the translator's SV1 hashrate gauge stops updating. The fix widens the multiply step to U512 so the intermediate product fits regardless of target magnitude. The numerator stays in U256 (it is 2^256 - t, which always fits), and the final result narrows back to u128 via low_u128() exactly as before. The precision improvement from the *100_000 scaling is preserved. Two regression tests: - target.rs: pin three real `maximum_target` values captured from the affected slot's translator log, including the channel's `requested_max_target` (0x1745d174_5d1745d1...). All previously errored ArithmeticOverflow; they now return finite hashrates. - vardiff/test/mod.rs: update the test_try_vardiff_with_less_spm_than_expected_classic expected values for the 240s and 300s checkpoints. Upstream's 74.2 / 62.327995 values came from the `try_vardiff` Err-fallback path (`hashrate * realized_spm / shares_per_minute`) which only ran because hash_rate_from_target overflowed at those high targets. With overflow eliminated, the main path returns the integer- truncated 74.0 / 62.0 from low_u128().
1 parent 85d6f8b commit 2d10f57

2 files changed

Lines changed: 89 additions & 6 deletions

File tree

sv2/channels-sv2/src/target.rs

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use alloc::string::String;
55
use binary_sv2::U256;
66
use bitcoin::{hash_types::BlockHash, hashes::Hash, Target};
77
use core::{cmp::max, fmt::Write, ops::Div};
8-
use primitive_types::U256 as U256Primitive;
8+
use primitive_types::{U256 as U256Primitive, U512};
99

1010
/// Converts a `u256` to a [`BlockHash`] type.
1111
pub fn u256_to_block_hash(v: U256<'static>) -> BlockHash {
@@ -188,15 +188,90 @@ pub fn hash_rate_from_target(target: U256<'static>, share_per_min: f64) -> Resul
188188
if shares_occurrency_frequence == 0_u128 {
189189
return Err(InputError::DivisionByZero);
190190
}
191-
let shares_occurrency_frequence = from_u128_to_u256(shares_occurrency_frequence);
192191
let target_plus_one = U256Primitive::from_big_endian(target_arr.as_ref())
193192
.checked_add(U256Primitive::one())
194193
.ok_or(InputError::ArithmeticOverflow)?;
194+
// Widen to U512 for the multiply: at large targets (close to 2^256, e.g.
195+
// a broadcast SetTarget carrying `requested_max_target`), the product
196+
// `(t+1) * shares_occurrency_frequence` exceeds U256 even though the
197+
// final ratio `numerator / denominator` is small. Doing the math in
198+
// U512 preserves the precision gained from the *100_000 scaling without
199+
// re-introducing the spurious overflow on legitimate high-target inputs.
200+
let target_plus_one = U512::from(target_plus_one);
201+
let shares_occurrency_frequence = U512::from(shares_occurrency_frequence);
195202
let denominator = target_plus_one
196203
.checked_mul(shares_occurrency_frequence)
197-
.and_then(|e| e.checked_div(U256Primitive::from(100_000)))
204+
.and_then(|e| e.checked_div(U512::from(100_000u64)))
198205
.ok_or(InputError::ArithmeticOverflow)?;
199-
let result = numerator.div(denominator).low_u128();
206+
let result = U512::from(numerator)
207+
.checked_div(denominator)
208+
.ok_or(InputError::DivisionByZero)?
209+
.low_u128();
200210
// we multiply back by 100 so that it cancels with the same factor at the denominator
201211
Ok(result as f64)
202212
}
213+
214+
#[cfg(test)]
215+
mod tests {
216+
use super::*;
217+
use binary_sv2::U256;
218+
219+
/// Builds a `binary_sv2::U256` from a 32-byte big-endian array.
220+
/// `hash_rate_from_target` reads its input as little-endian then reverses,
221+
/// so we must hand it the LE form here.
222+
fn target_from_be(be: [u8; 32]) -> U256<'static> {
223+
let mut le = be;
224+
le.reverse();
225+
U256::from(le)
226+
}
227+
228+
/// Regression for the U256 multiply-overflow in `hash_rate_from_target`
229+
/// that surfaced after the *100 → *100_000 scaling fix.
230+
///
231+
/// On the deployed slot-2 pool, `translator_sv2` (vardiff disabled) calls
232+
/// `hash_rate_from_target` on every upstream `SetTarget`. Vardiff drives
233+
/// the channel target up to its `requested_max_target` (≈ 2^253) when the
234+
/// channel is idle, and the resulting `(t+1) * shares_occurrency_frequence`
235+
/// product overflowed U256, returning `InputError::ArithmeticOverflow`.
236+
///
237+
/// Each case below is a real `maximum_target` captured from the slot-2
238+
/// translator log at `share_per_minute = 6.0`. The expected behavior is
239+
/// that the function returns `Ok(_)` (a small but well-defined hashrate)
240+
/// rather than erroring.
241+
#[test]
242+
fn hash_rate_from_target_does_not_overflow_on_high_targets() {
243+
// Real targets from translator_sv2_slot2 logs, 2026-05-17.
244+
let captures: &[[u8; 32]] = &[
245+
// 0x0000223d_a20843f8_b916c86e_4debea00_774ec094_ccd6a4eb_62605782_03599fb5
246+
[
247+
0x00, 0x00, 0x22, 0x3d, 0xa2, 0x08, 0x43, 0xf8, 0xb9, 0x16, 0xc8, 0x6e, 0x4d, 0xeb,
248+
0xea, 0x00, 0x77, 0x4e, 0xc0, 0x94, 0xcc, 0xd6, 0xa4, 0xeb, 0x62, 0x60, 0x57, 0x82,
249+
0x03, 0x59, 0x9f, 0xb5,
250+
],
251+
// 0x0a3d70a3_d70a3d70_a3d70a3d_70a3d70a_3d70a3d7_0a3d70a3_d70a3d70_a3d70a3c
252+
[
253+
0x0a, 0x3d, 0x70, 0xa3, 0xd7, 0x0a, 0x3d, 0x70, 0xa3, 0xd7, 0x0a, 0x3d, 0x70, 0xa3,
254+
0xd7, 0x0a, 0x3d, 0x70, 0xa3, 0xd7, 0x0a, 0x3d, 0x70, 0xa3, 0xd7, 0x0a, 0x3d, 0x70,
255+
0xa3, 0xd7, 0x0a, 0x3c,
256+
],
257+
// 0x1745d174_5d1745d1_745d1745_d1745d17_45d1745d_1745d174_5d1745d1_745d1744
258+
// (channel `requested_max_target` for the JDC extended channel)
259+
[
260+
0x17, 0x45, 0xd1, 0x74, 0x5d, 0x17, 0x45, 0xd1, 0x74, 0x5d, 0x17, 0x45, 0xd1, 0x74,
261+
0x5d, 0x17, 0x45, 0xd1, 0x74, 0x5d, 0x17, 0x45, 0xd1, 0x74, 0x5d, 0x17, 0x45, 0xd1,
262+
0x74, 0x5d, 0x17, 0x44,
263+
],
264+
];
265+
266+
for be in captures {
267+
let t = target_from_be(*be);
268+
let h =
269+
hash_rate_from_target(t, 6.0).expect("high targets must not overflow at spm=6.0");
270+
assert!(
271+
h.is_finite() && h >= 0.0,
272+
"expected finite non-negative hashrate, got {h} for target {:02x?}",
273+
be
274+
);
275+
}
276+
}
277+
}

sv2/channels-sv2/src/vardiff/test/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,13 @@ fn test_try_vardiff_with_less_spm_than_expected<V: Vardiff>(vardiff: &mut V) {
384384
.unwrap()
385385
.into();
386386

387-
assert_eq!(hashrate_after_240s, 74.2);
387+
// Was 74.2 on upstream, but that came from the `try_vardiff` fallback at
388+
// `classic.rs:250` (`hashrate * realized_spm / shares_per_minute = 106*7/10 = 74.2`)
389+
// which only triggers when `hash_rate_from_target` errors. Upstream's *100
390+
// scaling overflowed U256 at this high-target test point. With the U512
391+
// widening (target.rs) that overflow is gone, so the main path runs and
392+
// returns the integer-truncated 74 from `low_u128()`.
393+
assert_eq!(hashrate_after_240s, 74.0);
388394

389395
let simulation_duration = 300;
390396
// testing case when realized_shares_per_minute / shares_per_minute = 0.85
@@ -399,5 +405,7 @@ fn test_try_vardiff_with_less_spm_than_expected<V: Vardiff>(vardiff: &mut V) {
399405
.expect("try_vardiff failed")
400406
.unwrap();
401407

402-
assert_eq!(hashrate_after_300s, 62.327995);
408+
// Was 62.327995 on upstream via the same overflow-fallback path as the
409+
// 74.2 value above. Now exercises the main path; integer-truncated to 62.
410+
assert_eq!(hashrate_after_300s, 62.0);
403411
}

0 commit comments

Comments
 (0)