Skip to content

Conversation

@maramihali
Copy link
Collaborator

Implemented the polynomial optimisation excluding the MIPP

@maramihali maramihali requested a review from nikkolasg January 11, 2023 15:52
@maramihali maramihali self-assigned this Jan 11, 2023
@maramihali maramihali force-pushed the mipp branch 3 times, most recently from b92a53f to 5380870 Compare January 11, 2023 15:56
// commitment list to the satisfying witness polynomial list
let (comm_list, t) = PolyList::commit(&pl, &gens.gens_pc.ck);

let mut bytes = Vec::new();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Right now this is OK but we need a more serious full Transcript trait for handling correctly and sanely all types of inputs.

.map(|i| {
let z: Vec<Scalar> = (0..pow_m)
.into_par_iter()
.map(|j| Z[(j << m) | i])
Copy link
Collaborator

Choose a reason for hiding this comment

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

A small comments explaining we're selecting by row i and then column j would make the reader more at ease ;)

let mut prod = Scalar::one();
for j in 0..m {
let b_j = b[j];
if i >> j & 1 == 1 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here for comments please - i'm afraid of binary expressions left unexplained :D

@@ -0,0 +1,273 @@
use ark_bls12_377::{Bls12_377 as I, G1Affine};
Copy link

Choose a reason for hiding this comment

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

Single letter uppercase names are usually used for generics. When I read the source below, I was a bit confused where the generic I was defined. Then I found out it's a concrete type. If Bls12_377 is a too long name, then perhaps just Bls?

Copy link

Choose a reason for hiding this comment

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

Or Bls12, this is what blstrs is using as type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We will move this codebase to generic later on. This requires a larger effort accross the whole initial spartan codebase.
In other words, we'll have to make a "beautify and productify" pass over the whole codebase at some point.

@maramihali
Copy link
Collaborator Author

maramihali commented Feb 6, 2023

Closing this and opening a PR with the full scheme and addressed comments from here (#12).

@maramihali maramihali closed this Feb 6, 2023
@maramihali maramihali deleted the mipp branch February 10, 2023 10:24
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.

4 participants