Skip to content

Commit

Permalink
fix soundness: include public input to transcript (#355)
Browse files Browse the repository at this point in the history
public input should be included to transcript.

we may also need to include verifier key digest, which involve widely
change to derive Serialize/Deserializa on
ConstrainSystem/Expression/etc. ~~Will raise a ticket for that~~
#356
  • Loading branch information
hero78119 authored Oct 11, 2024
1 parent a9cd143 commit 8e2214d
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
35 changes: 31 additions & 4 deletions ceno_zkvm/examples/riscv_opcodes.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{iter, time::Instant};
use std::{iter, panic, time::Instant};

use ceno_zkvm::{
instructions::riscv::{arith::AddInstruction, branch::BltuInstruction},
Expand Down Expand Up @@ -256,8 +256,35 @@ fn main() {
// change public input maliciously should cause verifier to reject proof
zkvm_proof.pv[0] = <GoldilocksExt2 as ff_ext::ExtensionField>::BaseField::ONE;
zkvm_proof.pv[1] = <GoldilocksExt2 as ff_ext::ExtensionField>::BaseField::ONE;
verifier
.verify_proof(zkvm_proof, transcript)
.expect_err("verify proof should return with error");

// capture panic message, if have
let default_hook = panic::take_hook();
panic::set_hook(Box::new(|_info| {
// by default it will print msg to stdout/stderr
// we override it to avoid print msg since we will capture the msg by our own
}));
let result = panic::catch_unwind(|| verifier.verify_proof(zkvm_proof, transcript));
panic::set_hook(default_hook);
match result {
Ok(res) => {
res.expect_err("verify proof should return with error");
}
Err(err) => {
let msg: String = if let Some(message) = err.downcast_ref::<&str>() {
message.to_string()
} else if let Some(message) = err.downcast_ref::<String>() {
message.to_string()
} else if let Some(message) = err.downcast_ref::<&String>() {
message.to_string()
} else {
unreachable!()
};

if !msg.starts_with("0th round's prover message is not consistent with the claim") {
println!("unknown panic {msg:?}");
panic::resume_unwind(err);
};
}
};
}
}
3 changes: 3 additions & 0 deletions ceno_zkvm/src/scheme/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ impl<E: ExtensionField, PCS: PolynomialCommitmentScheme<E>> ZKVMProver<E, PCS> {
let mut vm_proof = ZKVMProof::empty(pi);
let pi = &vm_proof.pv;

// including public input to transcript
pi.iter().for_each(|v| transcript.append_field_element(v));

// commit to fixed commitment
for (_, pk) in self.pk.circuit_pks.iter() {
if let Some(fixed_commit) = &pk.vk.fixed_commit {
Expand Down
3 changes: 3 additions & 0 deletions ceno_zkvm/src/scheme/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ impl<E: ExtensionField, PCS: PolynomialCommitmentScheme<E>> ZKVMVerifier<E, PCS>
}
}

// including public input to transcript
pi.iter().for_each(|v| transcript.append_field_element(v));

// write fixed commitment to transcript
for (_, vk) in self.vk.circuit_vks.iter() {
if let Some(fixed_commit) = vk.fixed_commit.as_ref() {
Expand Down

0 comments on commit 8e2214d

Please sign in to comment.