-
Notifications
You must be signed in to change notification settings - Fork 39
control num_instances
in one place to improve soundness check
#901
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
Conversation
pub raw_pi: Vec<Vec<E::BaseField>>, | ||
// the evaluation of raw_pi. | ||
pub pi_evals: Vec<E>, | ||
opcode_proofs: BTreeMap<String, (usize, ZKVMOpcodeProof<E, PCS>)>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we have
- String: circuit name
- usize: circuit index
to carry information in proof. However a malicious prover can make both inconsistent.
The new fixed avoid duplicated information appear in different place
e41a67c
to
1025644
Compare
}) | ||
.collect_vec(); | ||
|
||
// (non uniform) collect dynamic address hints as witness for verifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this information from table proof to centralized place, and aligned with opcode proof with same terminology: num_instance
1025644
to
005efdc
Compare
num_instances
in one place to improve soundness check
let mpcs_opcode_commitment = self | ||
.opcode_proofs | ||
.iter() | ||
.map(|(circuit_name, (_, proof))| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These change make less readibility since now we lost circuit_name and instead only circuit index. But it's ok we fix it later, for now mpcs still be the major bottleneck of proof size
Build on top of #901. ### Design rationale zkvm + mpcs api: use `batch_commit/batch_opening` to commit/open **ALL** opcode together. In more detail, we commit `fixed_commit` and `witin_commit` separately with just one opening. basefold: batch opcodes with rmm with different "height", in other words, different "num_vars". For basefold to batch opcodes with different variables, for sumcheck part we apply suffix alignment techniques based on #870. For FRI part, we also apply suffix alignment techniques. So the smaller codeword will "involved" into the folding process when current length match. During implementation, there is key principle in mind: verifier only rely on one untrusted information "num_instances" from prover, and other information should derived from verifier key. ### Working items - [x] mpcs batch api + batch basefold prover - [x] batch basefold verifier ### Fibonacci Benchmark Results | Size | Branch | Proof Size | E2E Latency Change | Prover Speed (kHz) | |----------|----------------------|------------|--------------------|---------------------| | 2²⁰ | Master | 14.68 MB | - | 363 | | 2²⁰ | Batched + conjecture | 1.16 MB | -14.195% | 422 | | 2²⁰ | Batched | 2.02 MB | -11.599% | 411 | | 2²¹ | Master | 15.53 MB | - | 411 | | 2²¹ | Batched + conjecture | 1.24 MB | -11.178% | 451 | | 2²¹ | Batched | 2.16 MB | -6.4525% | 428 | | 2²² | Master | 16.42 MB | - | 406 | | 2²² | Batched + conjecture | 1.31 MB | -11.178% | 433 | | 2²² | Batched | 2.31 MB | -13.691% | 448 |
covered via #894 |
This features is preliminary support to batched all opcode/table in one mpcs proof #894
Previously we got few issues
num_instance
are specify in each opcode proofrw_hint_var
are specify in each table proofwhile both are describe the same thing in different place, which make thing hard to control and make room for malicious prover to crack based on inconsistency.
In this PR, we unify num_instance in one central place, and do a strictly check of it's consistency.
This also addressed feature in #648