Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UniScalarRng #123

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add random scalar generator `UniScalarRng` for uniformly distributed scalar generation [#121]
moCello marked this conversation as resolved.
Show resolved Hide resolved

### Remove
- Remove dusk's `random` scalar implementation in favor of the `random` implementation that is part of the `Field` trait [#121]

## [0.13.1] - 2023-10-11

### Changed
Expand Down Expand Up @@ -198,6 +205,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Initial fork from [`zkcrypto/jubjub`]

<!-- Issues -->
[#121]: https://github.com/dusk-network/jubjub/issues/121
[#115]: https://github.com/dusk-network/jubjub/issues/115
[#109]: https://github.com/dusk-network/jubjub/issues/109
[#104]: https://github.com/dusk-network/jubjub/issues/104
Expand All @@ -215,6 +224,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#31]: https://github.com/dusk-network/jubjub/issues/31
[#25]: https://github.com/dusk-network/jubjub/issues/25

<!-- Versions -->
[Unreleased]: https://github.com/dusk-network/jubjub/compare/v0.13.1...HEAD
[0.13.1]: https://github.com/dusk-network/jubjub/compare/v0.13.0...v0.13.1
[0.13.0]: https://github.com/dusk-network/jubjub/compare/v0.12.1...v0.13.0
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ default-features = false
[dev-dependencies.blake2]
version = "0.9"

[dev-dependencies.rand]
version = "0.8"

[features]
default = ["alloc", "bits"]
alloc = ["ff/alloc", "group/alloc"]
Expand Down
2 changes: 1 addition & 1 deletion src/fr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//! $\mathbb{F}_r$ where `r =
//! 0x0e7db4ea6533afa906673b0101343b00a6682093ccc81082d0970e5ed6f72cb7`

mod dusk;
pub(crate) mod dusk;

use core::convert::TryInto;
use core::fmt;
Expand Down
155 changes: 136 additions & 19 deletions src/fr/dusk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,125 @@
//
// Copyright (c) DUSK NETWORK. All rights reserved.

use core::cmp::{Ord, Ordering, PartialOrd};
use core::convert::TryInto;
use core::ops::{Index, IndexMut};

use rand_core::RngCore;
use dusk_bls12_381::BlsScalar;
use dusk_bytes::{Error as BytesError, Serializable};
use rand_core::{CryptoRng, RngCore, SeedableRng};

use super::{Fr, MODULUS, R2};
use crate::util::sbb;

use core::cmp::{Ord, Ordering, PartialOrd};
use core::ops::{Index, IndexMut};
use dusk_bls12_381::BlsScalar;
/// Random number generator for generating scalars that are uniformly
/// distributed over the entire field of scalars.
///
/// Because scalars take 251 bits for encoding it is difficult to generate
/// random bit-pattern that ensures to encode a valid scalar.
/// Wrapping the values that are higher than [`MODULUS`], as done in
/// [`Self::random`], results in hitting some values more than others, whereas
/// zeroing out the highest two bits will eliminate some values from the
/// possible results.
///
/// This function achieves a uniform distribution of scalars by using rejection
/// sampling: random bit-patterns are generated until a valid scalar is found.
/// The scalar creation is not constant time but that shouldn't be a concern
/// since no information about the scalar can be gained by knowing the time of
/// its generation.
///
/// ## Example
///
/// ```
/// use rand::rngs::{StdRng, OsRng};
/// use rand::SeedableRng;
/// use dusk_jubjub::{JubJubScalar, UniScalarRng};
/// use ff::Field;
///
/// // using a seedable random number generator
/// let mut rng: UniScalarRng<StdRng> = UniScalarRng::seed_from_u64(0x42);
/// let _scalar = JubJubScalar::random(rng);
///
/// // using randomness derived from the os
/// let mut rng = UniScalarRng::<OsRng>::default();
/// let _ = JubJubScalar::random(rng);
/// ```
#[derive(Clone, Copy, Debug, Default)]
pub struct UniScalarRng<R>(R);

impl<R> CryptoRng for UniScalarRng<R> where R: CryptoRng {}

impl<R> RngCore for UniScalarRng<R>
where
R: RngCore,
{
fn next_u32(&mut self) -> u32 {
self.0.next_u32()
}

use dusk_bytes::{Error as BytesError, Serializable};
fn next_u64(&mut self) -> u64 {
self.0.next_u64()
}

use super::{Fr, MODULUS, R2};
// We use rejection sampling to generate a valid scalar.
fn fill_bytes(&mut self, dest: &mut [u8]) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we create a new RNG when we just want to create new random scalars from a given uniform RNG.

What we are attempting to achieve is to have a uniform implementation for JubJubScalar that takes a uniform sampler, not to create our own samples (that in the end will increase the type complexity of the stack).

Why not just implement this as a function JubJubScalar::random or JubJubScalar::uniform_random?

I favor replacing the previous implementation since the uniform bits is a desirable property in any context.

if dest.len() < 32 {
panic!("buffer too small to generate uniformly distributed random scalar");
}

impl Fr {
/// Generate a valid Scalar choosen uniformly using user-
/// provided rng.
///
/// By `rng` we mean any Rng that implements: `Rng` + `CryptoRng`.
pub fn random<T>(rand: &mut T) -> Fr
where
T: RngCore,
{
let mut bytes = [0u8; 64];
rand.fill_bytes(&mut bytes);

Fr::from_bytes_wide(&bytes)
// Loop until we find a canonical scalar.
// As long as the random number generator is implemented properly, this
// loop will terminate.
let mut scalar = [0u64; 4];
loop {
for integer in scalar.iter_mut() {
*integer = self.0.next_u64();
}
Comment on lines +78 to +80

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we need an iterator here. This will make it harder for the compiler to optimize. Being a small, fixed size array, why not just take the next_u64?


// Check that the generated potential scalar is smaller than MODULUS
let bx = scalar[3] <= MODULUS.0[3];
let b1 = bx && MODULUS.0[0] > scalar[0];
let b2 = bx && (MODULUS.0[1] + b1 as u64) > scalar[1];
let b3 = bx && (MODULUS.0[2] + b2 as u64) > scalar[2];
let b4 = bx && (MODULUS.0[3] + b3 as u64) > scalar[3];

if b4 {
// Copy the generated random scalar in the first 32 bytes of the
// destination slice (scalars are stored in little endian).
for (i, integer) in scalar.iter().enumerate() {
let bytes = integer.to_le_bytes();
dest[i * 8..(i + 1) * 8].copy_from_slice(&bytes);
}
Comment on lines +92 to +95

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto; unnecessary usage of iterators


// Zero the remaining bytes (if any).
if dest.len() > 32 {
dest[32..].fill(0);
}
return;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The returned value is a raw little-endian representation of the scalar limbs. This is not the expected serialized format, I wonder why we are doing it.

If we are to generate the limbs, we should probably send them directly to the scalar. We only have to return "bytes" because we opted to create a new RNG, when in fact what we need is to sample uniform 4-tuple u64.

}
}
}

fn try_fill_bytes(
&mut self,
dest: &mut [u8],
) -> Result<(), rand_core::Error> {
self.0.try_fill_bytes(dest)
}
}

impl<R> SeedableRng for UniScalarRng<R>
where
R: SeedableRng,
{
type Seed = <R as SeedableRng>::Seed;

fn from_seed(seed: Self::Seed) -> Self {
Self(R::from_seed(seed))
}
}

impl Fr {
/// SHR impl: shifts bits n times, equivalent to division by 2^n.
#[inline]
pub fn divn(&mut self, mut n: u32) {
Expand Down Expand Up @@ -303,3 +393,30 @@ fn w_naf_2() {
});
assert_eq!(scalar, recomputed);
}

#[test]
fn test_uni_rng() {
use rand::rngs::StdRng;
let mut rng: UniScalarRng<StdRng> = UniScalarRng::seed_from_u64(0xbeef);

let mut buf32 = [0u8; 32];
let mut buf64 = [0u8; 64];

for _ in 0..100000 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more suitable approach would be to use a fuzzing library. Consider quickcheck

// fill an array of 64 bytes with our random scalar generator
rng.fill_bytes(&mut buf64);

// copy the first 32 bytes into another buffer and check that these
// bytes are the canonical encoding of a scalar
buf32.copy_from_slice(&buf64[..32]);
let scalar1: Option<Fr> = Fr::from_bytes(&buf32).into();
assert!(scalar1.is_some());

// create a second scalar from the 64 bytes wide array and check that it
// generates the same scalar as generated from the 32 bytes wide
// array
let scalar2: Fr = Fr::from_bytes_wide(&buf64);
let scalar1 = scalar1.unwrap();
assert_eq!(scalar1, scalar2);
}
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub use dusk::{
dhke, GENERATOR, GENERATOR_EXTENDED, GENERATOR_NUMS,
GENERATOR_NUMS_EXTENDED,
};
pub use fr::dusk::UniScalarRng;
/// An alias for [`AffinePoint`]
pub type JubJubAffine = AffinePoint;
/// An alias for [`ExtendedPoint`]
Expand Down