[precompile] part2 integrate keccak precompile into e2e flow #980
Conversation
hero78119
force-pushed
the
feat/gkr_iop_integration
branch
2 times, most recently
from
f81759d to
3f77797
Compare
hero78119
force-pushed
the
feat/gkr_iop_integration
branch
from
a4bdf61 to
45e98d8
Compare
hero78119
force-pushed
the
feat/gkr_iop_integration
branch
from
766666e to
9dd5207
Compare
| @@ -879,8 +881,7 @@ pub fn verify<E: ExtensionField, PCS: PolynomialCommitmentScheme<E> + serde::Ser | |||
| transcript, | |||
| zkvm_proof.has_halt(&verifier.vk), | |||
| )?; | |||
| // print verification statistics like proof size and hash count | |||
| tracing::info!("e2e proof stat: {}", zkvm_proof); | |||
There was a problem hiding this comment.
this was move to proof ready before verify, so even verify failed we still can see proof size data
| @@ -115,12 +96,10 @@ impl<E: ExtensionField> MockProver<E> { | |||
| &layer.expr_names, | |||
| &layer.out_eq_and_eval_exprs | |||
| ) { | |||
| if !expect.is_equal(&got) { | |||
| if expect != got { | |||
There was a problem hiding this comment.
Is there a case that expect and got are equal, but one is saved as FieldType::Base(..) but the other is saved as FieldType::Ext(..)?
There was a problem hiding this comment.
yes you are right and I verified it's syntatic equal.
I added this commit, moving is_equal into PartialEq trait definition
| Some(Expression::StructuralWitIn(id, ..)) => Expression::WitIn(offset_eq_id + *id), | ||
| invalid => panic!("invalid eq format {:?}", invalid), | ||
| }; | ||
| zero_check_exprs.push(eq_expr * zero_check_expr); |
There was a problem hiding this comment.
Is there any reason to change it from Expression::WitIn(..) to Expression::StructuralWitIn(..)?
There was a problem hiding this comment.
Do you mean reverse? what it actually do here is convert StructuralWitIn to WitIn, because gkr-iop backend only recognize Witin
| num_instances.push(( | ||
| index, | ||
| num_instance >> vk.get_cs().rotation_vars().unwrap_or(0), | ||
| )); | ||
| num_instances_with_rotation.push((index, num_instance)) |
There was a problem hiding this comment.
Why compute num_instances from num_instance >> vk.get_cs().rotation_vars().unwrap_or(0),))?
There was a problem hiding this comment.
because what rmm.num_instances() return is height, or number of rows. With rotation, the relation between number of rows and real instance are got rotation factor. So to derive real num instances, we need to divide rotation size, thus we do right shit toward rotation num_vars
gkr-iop circuit
This PR complete keccak precompile circuit construction, and try to separate clearly: for zkvm related circuit are construct in "keccak" opcode, while precompile only handle dedicate logic. For witness assignment also same principle, phase 1 witness are also assign separately for better modulariztion
circuit builder
lookup keccak circuit switch to new circuit builder and not depends on
layer_constrain_systemdependency.e2e integration
keccak opcode are integrated into prover flow. It can be invoke via guest program:
cargo run --package ceno_zkvm --release --bin e2e -- --platform=ceno --hints=1,6 examples/target/riscv32im-ceno-zkvm-elf/release/examples/ceno_rt_keccakfixed RC processing
In this PR RC was refactor to use witness first, because I realized the design of expanding fixed poly (and commitment) according to num_instance is very complex because fixed poly in our codebase are assume immutable everywhere. Mutability involve quite of change which also seems unnessesary. In particular, fixed poly immutable should be common sense. Thus, I will try to figure out how to deal with RC (and fixed poly) properly later
Next steps (in next PR)
Integrate gkr-iop verifier into verifler flow
benchmark
on fibonacci against master remain no change