Blackbox range function (WIP)#18
Conversation
benjaminwilson
left a comment
benjaminwilson
left a comment
There was a problem hiding this comment.
looks great!! btw if you pull on fiat-shamir there are some tests that would serve as a useful template!
| }; | ||
|
|
||
| const NUM_WITNESS_THRESHOLD_FOR_LOOKUP_TABLE: usize = 5; | ||
| const NUM_BITS_THRESHOLD_FOR_DIGITAL_DECOMP: u32 = 32; |
There was a problem hiding this comment.
Feels like maybe 32 might be too large for NUM_BITS_THRESHOLD_FOR_DIGITAL_DECOMP?
|
|
||
| const NUM_WITNESS_THRESHOLD_FOR_LOOKUP_TABLE: usize = 5; | ||
| const NUM_BITS_THRESHOLD_FOR_DIGITAL_DECOMP: u32 = 32; | ||
| pub const LOG_BASE_DECOMPOSITION: u32 = 32; |
| let mut memory_blocks: BTreeMap<usize, ReadOnlyMemoryBlock> = BTreeMap::new(); | ||
| // Range blocks to map the number of bits threshold to the vector of values that | ||
| // are meant to be constrained within that range. | ||
| let mut range_blocks: BTreeMap<u32, Vec<usize>> = BTreeMap::new(); |
There was a problem hiding this comment.
As you mentioned, looks like we need to support also RANGE(T) for T not a power of two, as well, so will need to use T as a key to this map (not logT)
| let mut range_blocks: BTreeMap<u32, Vec<usize>> = BTreeMap::new(); | ||
| // Same as above, but for number of bits that are above the [NUM_BITS_THRESHOLD_FOR_DIGITAL_DECOMP]. | ||
| // Separated so that we can separate the witness values into digits to do smaller range checks. | ||
| let mut range_blocks_outside_threshold: BTreeMap<u32, &Vec<usize>> = BTreeMap::new(); |
There was a problem hiding this comment.
Nit: we don't actually populate this guy until later on, immediately before we use it - I think even perhaps it's redundant, since could just inline that case with the others in the if block starting line 257
| @@ -151,9 +164,36 @@ impl R1CS { | |||
| } | |||
|
|
|||
| // These are calls to built-in functions, for this we need to create. | |||
There was a problem hiding this comment.
missing some words here somehow :)
| .push(input_witness); | ||
| // Keep track of the different ranges we are checking in order | ||
| // to track and update multiplicities. | ||
| range_blocks.keys().for_each(|key| { |
There was a problem hiding this comment.
Doesn't feel like the right place to do this - currently this is happening for every RANGE opcode instance?
| }); | ||
| } | ||
| _ => { | ||
| println!("Other black box function: {:?}", black_box_func_call); |
There was a problem hiding this comment.
Let's panic or error?
| /// | ||
| /// Checks that both sums (LHS and RHS) are equal at the end. | ||
| fn add_logup_summations(&mut self, num_bits: u32, values_to_lookup: &[usize]) { | ||
| // Sample the Shwartz-Zippel challenge for the log derivative |
| current_product_witness = next_product_witness; | ||
| }); | ||
|
|
||
| self.matrices.add_constraint( |
There was a problem hiding this comment.
Don't think this constraint is correct? Shouldn't it be current_product_witness == 0?
| /// Products with linear operations on the witness indices. | ||
| /// Fields are Product((index, a, b), (index, c, d)) such that | ||
| /// we wish to compute (ax + b) * (cx + d). | ||
| ProductLinearOperation( |
There was a problem hiding this comment.
I wonder if witness * (another_witness + const) would be sufficiently general? (it's faster to solve for)
Blackbox range function (WIP)
2 participants
No description provided.