Add SplitBN254 machine, use queries in SplitGL machine
#716
Conversation
georgwiese
force-pushed
the
block-machine-queries
branch
5 times, most recently
from
f98e992 to
5345624
Compare
georgwiese
force-pushed
the
split-machines-with-queries
branch
3 times, most recently
from
fe0f2db to
4d6e811
Compare
georgwiese
force-pushed
the
split-machines-with-queries
branch
from
4d6e811 to
ec0dd52
Compare
| @@ -40,7 +40,7 @@ fn gen_halo2_proof(file_name: &str, inputs: Vec<Bn254Field>) { | |||
| inputs, | |||
| &mktemp::Temp::new_dir().unwrap(), | |||
| true, | |||
| Some(backend::BackendType::Halo2), | |||
| Some(backend::BackendType::Halo2Mock), | |||
There was a problem hiding this comment.
Otherwise, the test takes very long and eventually gets cancelled.
I think it's completely fine to use the mock prover here, as it checks that all constraints are satisfied and that's all we want to test here.
|
|
||
| // 1. Decompose the input into bytes | ||
|
|
||
| // The byte decomposition of the input, in little-endian order | ||
| // and shifted forward by one (to use the last row of the | ||
| // previous block) | ||
| col witness bytes; | ||
| // Puts the bytes together to form the input | ||
| col witness bytes(i) query ("hint", (in_acc' >> (((i + 1) % 8) * 8)) % 0xff); |
There was a problem hiding this comment.
Too confusing? 😄
((i + 1) % 8)is the index in the current block, shifted by 1(in_acc' >> (((i + 1) % 8) * 8))is the input shifted by that many bytes(in_acc' >> (((i + 1) % 8) * 8)) % 0xff)selects the least significant byte
So in the end, it does what the comment above says that it should, but let me know if you have an idea how to make it clearer :)
There was a problem hiding this comment.
Yea it's fine. If the prover passes malicious values now, witgen won't fail, right?
There was a problem hiding this comment.
I'd phrase it differently, if the constraints allow for more than one value, witgen won't fail anymore. For example, if we remove step 3 and fix #593. That's why I think it's always better to rely on automatic witgen, but in this case it's not possible.
georgwiese
force-pushed
the
split-machines-with-queries
branch
from
ec0dd52 to
f26a161
Compare
| @@ -135,7 +135,7 @@ where | |||
| } | |||
| } else { | |||
| Err(IncompleteCause::ExpressionEvaluationUnimplemented( | |||
| "Cannot evaluate arrays or next references.".to_string(), | |||
| "Cannot evaluate arrays.".to_string(), | |||
There was a problem hiding this comment.
Are these changes needed here?
There was a problem hiding this comment.
Yes, otherwise the machine doesn't work, but they are also in #735, so if we merge that first they'll go away.
| reg A7; | ||
| reg A8; | ||
|
|
||
| degree 65536; |
There was a problem hiding this comment.
I think we shouldn't have degree statements in std machines, because if a program with a longer ROM uses this, the compiler issues an error and the user needs to manually change it. Maybe we should have something like a "lower bound" thing, because this is what it is, right?
There was a problem hiding this comment.
Ah wait, this is a test file so it's fine
There was a problem hiding this comment.
I agree, but this is just the test!
| assert_eq A5, 0; | ||
| assert_eq A6, 0; | ||
| assert_eq A7, 0; | ||
| assert_eq A8, 0; |
Includes the changes from #735.
This PR does two small changes to the existing
SplitGLmachine:operation_idcolumn (possible since Optional latch and operation_id #594)Then, it adds an analogous machine & test for BN254. I recommend reviewing the diff to the Goldilocks machine.