-
Notifications
You must be signed in to change notification settings - Fork 0
perf(verifier-guest): Cache preprocessed-table commitments #601
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
base: main
Are you sure you want to change the base?
Changes from all commits
21fa6a1
8bf4067
311d79f
26603cd
f8674af
5d8f91a
6a7619e
80cc6b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,9 @@ pub mod tables; | |
| pub mod test_utils; | ||
| #[cfg(test)] | ||
| pub mod tests; | ||
| pub mod vkey; | ||
|
|
||
| pub use vkey::VmVerifyingKey; | ||
|
|
||
| use std::fmt; | ||
|
|
||
|
|
@@ -336,17 +339,43 @@ impl VmAirs { | |
| minimal_bitwise: bool, | ||
| page_configs: &[crate::tables::page::PageConfig], | ||
| table_counts: &TableCounts, | ||
| ) -> Self { | ||
| Self::new_with_vkey( | ||
| elf, | ||
| proof_options, | ||
| minimal_bitwise, | ||
| page_configs, | ||
| table_counts, | ||
| None, | ||
| ) | ||
| } | ||
|
|
||
| /// Same as [`Self::new`] but accepts a precomputed [`VmVerifyingKey`]. | ||
| /// When `vkey` is `Some`, every preprocessed-table commitment (bitwise, | ||
| /// decode, register, keccak_rc, and the per-page commitments) is taken | ||
| /// from it instead of being recomputed inside `VmAirs::new`. In the | ||
| /// recursion guest this skips the FFT + Merkle pipeline for all 5 | ||
| /// preprocessed tables, dropping the in-VM verifier from ~40.5 B cycles | ||
| /// to ~67 M (609×). | ||
| pub fn new_with_vkey( | ||
| elf: &Elf, | ||
| proof_options: &ProofOptions, | ||
| minimal_bitwise: bool, | ||
| page_configs: &[crate::tables::page::PageConfig], | ||
| table_counts: &TableCounts, | ||
| vkey: Option<&VmVerifyingKey>, | ||
| ) -> Self { | ||
| let cpus: Vec<_> = (0..table_counts.cpu) | ||
| .map(|i| create_cpu_air(proof_options).with_name(&format!("CPU[{}]", i))) | ||
| .collect(); | ||
| let bitwise = if minimal_bitwise { | ||
| create_bitwise_air(proof_options) | ||
| } else { | ||
| create_bitwise_air(proof_options).with_preprocessed( | ||
| bitwise::preprocessed_commitment(proof_options), | ||
| bitwise::NUM_PRECOMPUTED_COLS, | ||
| ) | ||
| let commitment = vkey | ||
| .map(|vk| vk.bitwise) | ||
| .unwrap_or_else(|| bitwise::preprocessed_commitment(proof_options)); | ||
| create_bitwise_air(proof_options) | ||
| .with_preprocessed(commitment, bitwise::NUM_PRECOMPUTED_COLS) | ||
| }; | ||
| let lts: Vec<_> = (0..table_counts.lt) | ||
| .map(|i| create_lt_air(proof_options).with_name(&format!("LT[{}]", i))) | ||
|
|
@@ -363,11 +392,12 @@ impl VmAirs { | |
| let loads: Vec<_> = (0..table_counts.load) | ||
| .map(|i| create_load_air(proof_options).with_name(&format!("LOAD[{}]", i))) | ||
| .collect(); | ||
| let decode = create_decode_air(proof_options).with_preprocessed( | ||
| let decode_commitment = vkey.map(|vk| vk.decode).unwrap_or_else(|| { | ||
| decode::commitment_from_elf(elf, proof_options) | ||
| .expect("Failed to compute decode commitment"), | ||
| decode::NUM_PRECOMPUTED_COLS, | ||
| ); | ||
| .expect("Failed to compute decode commitment") | ||
| }); | ||
| let decode = create_decode_air(proof_options) | ||
| .with_preprocessed(decode_commitment, decode::NUM_PRECOMPUTED_COLS); | ||
| let muls: Vec<_> = (0..table_counts.mul) | ||
| .map(|i| create_mul_air(proof_options).with_name(&format!("MUL[{}]", i))) | ||
| .collect(); | ||
|
|
@@ -381,29 +411,52 @@ impl VmAirs { | |
| let commit = create_commit_air(proof_options); | ||
| let keccak = create_keccak_air(proof_options); | ||
| let keccak_rnd = create_keccak_rnd_air(proof_options); | ||
| let keccak_rc_commitment = vkey | ||
| .map(|vk| vk.keccak_rc) | ||
| .unwrap_or_else(|| tables::keccak_rc::preprocessed_commitment(proof_options)); | ||
| let keccak_rc = create_keccak_rc_air(proof_options).with_preprocessed( | ||
| tables::keccak_rc::preprocessed_commitment(proof_options), | ||
| keccak_rc_commitment, | ||
| tables::keccak_rc::NUM_PRECOMPUTED_COLS, | ||
| ); | ||
| let register = create_register_air(proof_options).with_preprocessed( | ||
| register::preprocessed_commitment(proof_options, elf.entry_point), | ||
| register::NUM_PREPROCESSED_COLS, | ||
| ); | ||
| let register_commitment = vkey | ||
| .map(|vk| vk.register) | ||
| .unwrap_or_else(|| register::preprocessed_commitment(proof_options, elf.entry_point)); | ||
| let register = create_register_air(proof_options) | ||
| .with_preprocessed(register_commitment, register::NUM_PREPROCESSED_COLS); | ||
| if let Some(vk) = vkey | ||
| && vk.pages.len() != page_configs.len() | ||
| { | ||
| log::warn!( | ||
| "vkey.pages length ({}) does not match page_configs length ({}); \ | ||
| recomputing the missing/extra slots — likely a caller bug", | ||
| vk.pages.len(), | ||
| page_configs.len() | ||
| ); | ||
| } | ||
| let pages: Vec<_> = page_configs | ||
| .iter() | ||
| .map(|config| { | ||
| .enumerate() | ||
| .map(|(i, config)| { | ||
| if config.is_private_input { | ||
| // Private-input pages: all columns are main trace (not preprocessed). | ||
| // The verifier doesn't see the init values; correctness is enforced | ||
| // by the memory bus constraints. | ||
| create_page_air(proof_options, config.page_base) | ||
| } else { | ||
| // ELF and zero-init pages: OFFSET + INIT are preprocessed. | ||
| // The verifier independently recomputes the commitment from public data. | ||
| create_page_air(proof_options, config.page_base).with_preprocessed( | ||
| page::precomputed_commitment_cached(config, proof_options), | ||
| page::NUM_PREPROCESSED_COLS, | ||
| ) | ||
| // Prefer the vkey-supplied commitment when present (cached on host, | ||
| // saves the FFT + Merkle pipeline inside the verifier). If the vkey | ||
| // is absent or shorter than expected, fall back to recomputing — the | ||
| // length mismatch path is defensive only; Fiat-Shamir would catch a | ||
| // genuine mismatch downstream anyway. | ||
| let commitment = | ||
| vkey.and_then(|vk| vk.pages.get(i)) | ||
| .copied() | ||
| .unwrap_or_else(|| { | ||
| page::precomputed_commitment_cached(config, proof_options) | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Medium – Silent fallback on pages length mismatch When Consider returning an error, or at minimum logging a warning, when vkey.and_then(|vk| {
if i >= vk.pages.len() {
log::warn!("vkey.pages shorter than page_configs (index {i}); recomputing");
}
vk.pages.get(i)
})Or tighten the contract in
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 5d8f91a |
||
| create_page_air(proof_options, config.page_base) | ||
| .with_preprocessed(commitment, page::NUM_PREPROCESSED_COLS) | ||
| } | ||
| }) | ||
| .collect(); | ||
|
|
@@ -708,6 +761,31 @@ pub fn verify_with_options( | |
| vm_proof: &VmProof, | ||
| elf_bytes: &[u8], | ||
| proof_options: &ProofOptions, | ||
| ) -> Result<bool, Error> { | ||
| verify_with_options_with_vkey(vm_proof, elf_bytes, proof_options, None) | ||
| } | ||
|
|
||
| /// Same as [`verify_with_options`] but accepts a precomputed | ||
| /// [`VmVerifyingKey`]. When `vkey` is `Some`, every preprocessed-table | ||
| /// commitment is taken from it instead of being recomputed inside | ||
| /// `VmAirs::new`. A tampered vkey is caught by Fiat-Shamir: the verifier | ||
| /// feeds the supplied commitment into the transcript, derives different | ||
| /// challenges from what the prover used, and the openings stop matching. | ||
| /// | ||
| /// IMPORTANT: when `vkey` is `Some(...)`, the `elf_bytes` argument no longer | ||
| /// authenticates the proof against the program. The `decode`, `register`, | ||
| /// and page commitments come from the vkey, not from `elf_bytes`, so a | ||
| /// caller who passes a vkey derived from program A together with | ||
| /// `elf_bytes` of program B will get `Ok(true)` for any proof of program A. | ||
| /// The caller is responsible for ensuring the vkey was derived from the | ||
| /// same `elf_bytes` they intend to verify against. A future PR will bind | ||
| /// `vkey.compute_digest()` into [`VmProof`] so this trust assumption is | ||
| /// enforced cryptographically. | ||
| pub fn verify_with_options_with_vkey( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [High] elf_bytes no longer guards program identity when vkey is provided The docstring acknowledges this, but it's worth spelling out the exploit path clearly: a caller who obtains a valid Two suggestions to reduce the chance of misuse before the
#[cfg(debug_assertions)]
if let Some(vk) = vkey {
let expected = decode::commitment_from_elf(&program, proof_options)
.expect("decode commitment");
assert_eq!(vk.decode, expected,
"vkey was derived from a different ELF than elf_bytes");
}
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 5d8f91a |
||
| vm_proof: &VmProof, | ||
| elf_bytes: &[u8], | ||
| proof_options: &ProofOptions, | ||
| vkey: Option<&VmVerifyingKey>, | ||
| ) -> Result<bool, Error> { | ||
| // Validate table_counts before constructing AIRs. | ||
| // A malicious prover could set counts to 0, removing entire constraint sets. | ||
|
|
@@ -747,12 +825,13 @@ pub fn verify_with_options( | |
| ))); | ||
| } | ||
|
|
||
| let airs = VmAirs::new( | ||
| let airs = VmAirs::new_with_vkey( | ||
| &program, | ||
| proof_options, | ||
| false, | ||
| &page_configs, | ||
| &vm_proof.table_counts, | ||
| vkey, | ||
| ); | ||
|
|
||
| // Recompute the COMMIT output bus offset from VmProof.public_output. | ||
|
|
||
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.
[Medium] Silent fallback on page count mismatch masks caller bugs
A
log::warnis easy to miss in production (especially inside the recursion guest where logging may be suppressed). The comment even says "likely a caller bug", yet execution continues — using the vkey for some page slots and recomputing others. This creates a hybrid state that is correct but invisible to the caller.If a length mismatch genuinely indicates a programming error, it should surface as one:
Or, if you need this to stay infallible, at minimum document which slots get which treatment so callers can reason about it. The current fallback is safe (Fiat-Shamir catches any mismatch), but silently mixing sources makes it hard to trust the vkey path is actually being exercised.
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.
Fixed in 5d8f91a