num-bigint icon indicating copy to clipboard operation
num-bigint copied to clipboard

to_bytes_le() returns [0] for some values

Open cp289 opened this issue 4 months ago • 2 comments

I ran across this behavior in my randomized tests: Playground

use num_bigint::*; // 0.4.6
fn main() {
    let exp_bytes = [37, 134, 4, 249, 45, 233, 54, 83, 189, 215, 77, 35, 203, 154, 209, 135, 47, 69, 181, 250, 127, 181, 86, 202, 55, 29, 187, 208, 129, 163, 184, 146, 154, 187, 240, 247, 27, 94, 227, 219, 46, 50, 169, 174, 251, 47, 217, 173, 90, 61, 247, 165, 189, 184, 58, 120, 232, 190, 13, 213, 69, 80, 175, 19, 230, 172, 246, 56, 36, 180, 250, 177, 247, 135, 240, 201, 203, 5, 11, 190, 105, 180, 172, 242, 44, 41, 242, 169, 227, 93, 187, 41, 21, 241, 63, 224, 131, 192, 5, 138];
    let big_int = BigInt::from_bytes_le(Sign::NoSign, &exp_bytes);
    let (_, act_bytes) = big_int.to_bytes_le();
    assert_eq!(act_bytes, exp_bytes);
}

Result:

thread 'main' panicked at src/main.rs:6:5:
assertion `left == right` failed
  left: [0]
 right: [37, 134, 4, 249, 45, 233, 54, 83, 189, 215, 77, 35, 203, 154, 209, 135, 47, 69, 181, 250, 127, 181, 86, 202, 55, 29, 187, 208, 129, 163, 184, 146, 154, 187, 240, 247, 27, 94, 227, 219, 46, 50, 169, 174, 251, 47, 217, 173, 90, 61, 247, 165, 189, 184, 58, 120, 232, 190, 13, 213, 69, 80, 175, 19, 230, 172, 246, 56, 36, 180, 250, 177, 247, 135, 240, 201, 203, 5, 11, 190, 105, 180, 172, 242, 44, 41, 242, 169, 227, 93, 187, 41, 21, 241, 63, 224, 131, 192, 5, 138]

I thought this was a bug, but perhaps I'm misunderstanding the meaning of Sign::NoSign. Is this meant to signify a value with zero magnitude?

cp289 avatar Sep 10 '25 07:09 cp289

I thought this was a bug, but perhaps I'm misunderstanding the meaning of Sign::NoSign. Is this meant to signify a value with zero magnitude?

Yes, and so the input bytes are not used by from_bytes_le in that case. Maybe we should state this explicitly in all the from*(sign, ...) methods? (and they could do a better job of avoiding work in that case too, as a micro-optimization)

cuviper avatar Sep 10 '25 16:09 cuviper

Maybe we should state this explicitly in all the from*(sign, ...) methods?

That wouldn't hurt, but I think just adding some docs to Sign enum fields would be sufficient.

I can open a PR with these docs later tonight.

cp289 avatar Sep 10 '25 19:09 cp289