Make Uint::random_mod() work identically on 32- and 64-bit targets
#285
Conversation
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
Contributor
Author
|
Not sure why the |
Member
|
Looks like a random GitHub Actions runner failure. I restarted it. |
tarcieri
reviewed
Nov 8, 2023
tarcieri
approved these changes
Nov 9, 2023
Merged
mrdomino
added a commit
to mrdomino/crypto-bigint
that referenced
this pull request
Dec 2, 2025
## Breaking changes - `Encoding::Repr` can no longer be assumed to implement `Copy`, so consumers of `Encoding::Repr` will need to explicitly call `clone`. - The numbers produced by both `random_bits` and `random_mod` will generally be different, and calling these functions will leave the RNG in a different state, than before. ## Summary This essentially applies RustCrypto#285 to `random_bits` as well as `random_mod`. Both functions behave the same way now, with the only difference being that `random_mod` adds rejection sampling; otherwise both will produce the same numbers over the same entropy stream. Questions of platform dependence are now easy; we do not define these algorithms in terms of machine words but in terms of bytes. Randomly sampled `Uint`s are constructed little-endian over the entropy stream. This leverages the existing work making `Uint` implement `Encoding`. It additionally needs `Encoding` on `BoxedUint` to make `RandomMod` and `RandomBits` work there; this is implemented, but requires dropping the `Copy` constraint on `Encoding::Repr`. This change also uses a variable- rather than constant-time comparison for the rejection loop. The only real reason to do this is the linked [compiler bug][0], but the only downside is that this makes `random_mod` leak somewhat more timing data than it otherwise would. However, `random_mod` inherently must leak some timing data (in the form of rejections), and so should arguably just be named `random_mod_vartime` in the first place. [0]: rust-lang/rust#149522
mrdomino
added a commit
to mrdomino/crypto-bigint
that referenced
this pull request
Dec 10, 2025
## Breaking changes - `Encoding::Repr` can no longer be assumed to implement `Copy`, so consumers of `Encoding::Repr` will need to explicitly call `clone`. - The numbers produced by both `random_bits` and `random_mod` will generally be different, and calling these functions will leave the RNG in a different state, than before. ## Summary This essentially applies RustCrypto#285 to `random_bits` as well as `random_mod`. Both functions behave the same way now, with the only difference being that `random_mod` adds rejection sampling; otherwise both will produce the same numbers over the same entropy stream. Questions of platform dependence are now easy; we do not define these algorithms in terms of machine words but in terms of bytes. Randomly sampled `Uint`s are constructed little-endian over the entropy stream. This leverages the existing work making `Uint` implement `Encoding`. It additionally needs `Encoding` on `BoxedUint` to make `RandomMod` and `RandomBits` work there; this is implemented, but requires dropping the `Copy` constraint on `Encoding::Repr`. In order to use `random_bits_core` in `random_mod_core`, we need to make it produce `R::Error`; then we call `map_err` in `RandomBits`, rather than in `random_bits_core`, to get a `RandomBitsError`. [0]: rust-lang/rust#149522
tarcieri
pushed a commit
that referenced
this pull request
Dec 10, 2025
## Breaking changes - `Encoding::Repr` is no longer required to implement `Copy`, so consumers of `Encoding::Repr` will need to explicitly call `clone`. - The numbers produced by both `random_bits` and `random_mod` will generally be different, and calling these functions will leave the RNG in a different state, than before. ## Fixes - Adds a mitigation for rust-lang/rust#149522, modifying `Word::borrowing_sub` to use `overflowing_sub` instead of `WideWord` casts. ## Summary This essentially applies #285 to `random_bits` as well as `random_mod`. Both functions behave the same way now, with the only difference being that `random_mod` adds rejection sampling; otherwise both will produce the same numbers over the same entropy stream. Questions of platform dependence are now easy; we do not define these algorithms in terms of sequences of machine words but of bytes. Randomly sampled `Uint`s are now always constructed little-endian over the entropy stream. This does not preclude future machine-specific optimizations, but given how perilous the landscape has been (e.g. #1018), I’ve elected to err in the direction of parsimony rather than performance for this change. This leverages the existing work making `Uint` implement `Encoding`. It additionally needs `Encoding` on `BoxedUint` to make `RandomMod` and `RandomBits` work there; this is implemented, but requires dropping the `Copy` constraint on `Encoding::Repr`. Fixes #1009 [0]: rust-lang/rust#149522
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The current version of
Uint::random_mod()produces different results on 32- and 64-bit targets because it exhausts the byte stream differently if the number of bytes inmodulusis not a multiple of 8 (e.g. if it is 20 bytes, the 32-bit version will read 20 bytes - 5 limbs, but the 64-bit version will read 24 bytes - 3 limbs). This PR makes the behavior identical, so that an RNG with the same seed will produce the same stream ofUints (as long as the RNG itself produces the bytestream consistently, of course).I am not sure if that is actually a desired behavior, but I did bump into it when I generated a ZK proof challenge on a 64-bit and 32-bit clients, and expected them to be the same. If it is not a guarantee we want to provide, perhaps an explicit note in the docs will be helpful.
The code in this PR would be much simpler if I could use
Uint::from_le_bytes(), but then I would have to add anEncodingbound.