Skip to content

Conversation

@fjarri
Copy link
Contributor

@fjarri fjarri commented Nov 30, 2023

I understand that this change may be controversial, but let me just put it out there. The reason being, why panic when we don't have to, and also to prevent bugs when something works on a 32 bit but doesn't on a 64 bit target.

  • BoxedUint::zero_with_precision(), one_with_precision(), max() round up bits_precision to a multiple of Limb::BITS. It's a little weird in the case of max(), but I am not sure this method should be public in the first place (and perhaps should be renamed).
  • Support arbitrary bits_precision in BoxedUint::random() (generating a random number in range [0, 2^bits_precision)).
  • Fix the tests in boxed/rand.rs which didn't actually test BoxedUint.
  • Round up bits_precision when decoding BoxedUint from bytes, to match the constructors' behavior.
  • Round up nlimbs() in the same way. Fixes nlimbs!() macro doesn't support rounding up to the limb size #258.

An option here is to keep the given bits_precision as a field in the BoxedUint without rounding it up. Then we will have to zeroize MSBs in arithmetic operations.

@fjarri fjarri force-pushed the round-up-precision branch from d85d5e4 to fce073b Compare November 30, 2023 04:14
@tarcieri
Copy link
Member

I'm generally OK with this. We had similar requests in the past: #258

@fjarri
Copy link
Contributor Author

fjarri commented Dec 1, 2023

Should I add rounding up there too? I think it is self-evident that nlimbs!(127) does not produce an U127, but instead gives the number of limbs enough to fit 127 bits.

@fjarri fjarri force-pushed the round-up-precision branch from fce073b to a6a937b Compare December 1, 2023 00:58
@tarcieri
Copy link
Member

tarcieri commented Dec 1, 2023

Should I add rounding up there too?

@fjarri sure, sounds good

@fjarri fjarri force-pushed the round-up-precision branch from a6a937b to bc9709c Compare December 2, 2023 18:22
@fjarri
Copy link
Contributor Author

fjarri commented Dec 2, 2023

Done

@tarcieri
Copy link
Member

tarcieri commented Dec 2, 2023

@fjarri this needs a rebase now

@fjarri fjarri force-pushed the round-up-precision branch from bc9709c to 41462bd Compare December 2, 2023 18:35
@fjarri
Copy link
Contributor Author

fjarri commented Dec 2, 2023

Rebased

@tarcieri tarcieri merged commit 7d174f0 into RustCrypto:master Dec 2, 2023
@fjarri fjarri deleted the round-up-precision branch December 2, 2023 19:05
@tarcieri tarcieri mentioned this pull request Jan 22, 2025
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.

nlimbs!() macro doesn't support rounding up to the limb size

2 participants