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

Remove powers-of-2 restriction on cohort size #134

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 0 additions & 1 deletion ferveo-python/ferveo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
SharedSecret,
ValidatorMessage,
ThresholdEncryptionError,
InvalidShareNumberParameter,
InvalidDkgStateToDeal,
InvalidDkgStateToAggregate,
InvalidDkgStateToVerify,
Expand Down
4 changes: 0 additions & 4 deletions ferveo-python/ferveo/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,6 @@ class ThresholdEncryptionError(Exception):
pass


class InvalidShareNumberParameter(Exception):
pass


class InvalidDkgStateToDeal(Exception):
pass

Expand Down
1 change: 1 addition & 0 deletions ferveo/benches/bench_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ criterion_main! {
// bench_batch_inverse,
// benchmarks::pairing::ec,
benchmarks::validity_checks::validity_checks,
benchmarks::eval_domain::eval_domain,
}
57 changes: 57 additions & 0 deletions ferveo/benches/benchmarks/eval_domain.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#![allow(clippy::redundant_closure)]
#![allow(clippy::unit_arg)]

pub use ark_bls12_381::Bls12_381 as EllipticCurve;
use ark_ff::Field;
use ark_poly::EvaluationDomain;
use criterion::{black_box, criterion_group, BenchmarkId, Criterion};
use digest::crypto_common::rand_core::SeedableRng;
use ferveo_pre_release::*;
use rand::prelude::StdRng;

const NUM_SHARES_CASES: [usize; 6] = [2, 4, 8, 16, 32, 64];

pub fn bench_eval_domain(c: &mut Criterion) {
let mut group = c.benchmark_group("EVAL DOMAIN");
group.sample_size(10);

let rng = &mut StdRng::seed_from_u64(0);
let s = ark_bls12_381::Fr::from_random_bytes(&[0u8; 32]).unwrap();

for shares_num in NUM_SHARES_CASES {
Copy link
Member

Choose a reason for hiding this comment

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

So this is benchmarking the speed of num shares that are powers of 2 for Radix2 vs MixedRadix? Cool.

As an aside. Do we have a test somewhere for a non-power of 2 number of shares? Basically, a test that confirms that the non-powers of 2 work appropriately.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't consider this kind of test to be interesting before. I did a manual test (in REPL) before submitting the PR and that lulled me into a false sense of correctness. Now that I followed up on your suggestion, it turns out only the simple variant works for a non-power-of-two number of shares.

I will post the changes here and investigate the issue with the precomputed variant.

Thanks for catching this.

Copy link
Author

Choose a reason for hiding this comment

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

The bug has been found and fixed. Please see my comment below.

let eval_radix2_eval_domain = {
let domain =
ark_poly::MixedRadixEvaluationDomain::new(shares_num).unwrap();
let phi = SecretPolynomial::<ark_bls12_381::Bls12_381>::new(
&s, shares_num, rng,
);

move || {
black_box(phi.0.evaluate_over_domain_by_ref(domain));
}
};

let eval_mixed_eval_domain = {
let domain =
ark_poly::MixedRadixEvaluationDomain::new(shares_num).unwrap();
let phi = SecretPolynomial::<ark_bls12_381::Bls12_381>::new(
&s, shares_num, rng,
);

move || {
black_box(phi.0.evaluate_over_domain_by_ref(domain));
}
};

group.bench_function(
BenchmarkId::new("eval_radix2_eval_domain", shares_num),
|b| b.iter(|| eval_radix2_eval_domain()),
);
group.bench_function(
BenchmarkId::new("eval_mixed_eval_domain", shares_num),
|b| b.iter(|| eval_mixed_eval_domain()),
);
}
}

criterion_group!(eval_domain, bench_eval_domain);
1 change: 1 addition & 0 deletions ferveo/benches/benchmarks/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//pub mod block_proposer;
// pub mod pairing;
pub mod eval_domain;
pub mod validity_checks;
Loading
Loading