BREAKING: Write random_mod in terms of new random_bits
#1026
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1026 +/- ##
==========================================
- Coverage 79.87% 79.82% -0.05%
==========================================
Files 163 163
Lines 17735 17750 +15
==========================================
+ Hits 14165 14169 +4
- Misses 3570 3581 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
That's probably a good change if we want to be able to impl |
|
@tarcieri PTAL - I added a mitigation for the compiler bug that allows us to keep using NB. this mitigation could also be applied to any of the other attempts, including the original reverted PR, at your preference; it may also make sense to merge it separately, in which case feel free to cherry-pick the commit onto a new branch or I can do the same. |
|
#1029 has just the cherry-picked mitigation commit. |
## 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
mrdomino
force-pushed
the
random-mod-bytewise
branch
from
f98c931 to
a25867b
Compare
|
Done. Two clean commits, one that adds the test (passing on 64-bit, failing on 32-bit), one that implements the change (and shows the diffs against the test.) |
2 participants
Breaking changes
Encoding::Repris no longer required to implementCopy, so consumers ofEncoding::Reprwill need to explicitly callclone.The numbers produced by both
random_bitsandrandom_modwill generally be different, and calling these functions will leave the RNG in a different state, than before.Fixes
ct_ltloop conditions over bigints on linux-aarch64 rust-lang/rust#149522, modifyingWord::borrowing_subto useoverflowing_subinstead ofWideWordcasts.Summary
This essentially applies #285 to
random_bitsas well asrandom_mod. Both functions behave the same way now, with the only difference being thatrandom_modadds 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
Uints 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
UintimplementEncoding. It additionally needsEncodingonBoxedUintto makeRandomModandRandomBitswork there; this is implemented, but requires dropping theCopyconstraint onEncoding::Repr.Fixes #1009