Skip to content

Commit

Permalink
fix bug in the decoding procedure for Vec<PartialAuthenticationPath>
Browse files Browse the repository at this point in the history
The culprit was this line in `BFieldCodec`:
```
if (vec_len != 0 || mask != 0) && index == str.len() {
```

This if-statement generates an error if the predicate evaluates to true.
The idea is that if the vector length (`vec_len`, number of
`Option<Digest>`s in this partial authentication path) is nonzero, or if
the mask (`mask`, whose bits indicate whether the option is `Some(..)`
or `None`) is nonzero, then we should be expecting at least one digest
to follow; and so in this case the index cannot point to the end of the
string.

The logic is flawed because the vector length can nonzero with no
following digests. This occurs in partial authentication paths
consisting entirely of `None`s. The `mask` variable will be zero,
thus triggering the predicate erroneously.

The fix is to change the offending line to this:
```
if vec_len != 0 && mask != 0 && index == str.len() {
```

In other words, we are only expecting to read another digest if
a) the vector has nonzero length, _and_
b) the mask is nonzero.

Also, remove profiling from test. Use the benchmark for profiling.
  • Loading branch information
aszepieniec authored and jan-ferdinand committed Nov 18, 2022
1 parent cc15b18 commit e7fd6cc
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 11 deletions.
22 changes: 19 additions & 3 deletions triton-vm/src/bfield_codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ impl BFieldCodec for Vec<PartialAuthenticationPath<Digest>> {
fn decode(str: &[BFieldElement]) -> Result<Box<Self>, Box<dyn Error>> {
let mut index = 0;
let mut vector = vec![];

// while there is at least one partial auth path left, parse it
while index < str.len() {
let len_remaining = str[index].value() as usize;
index += 1;
Expand All @@ -330,8 +332,22 @@ impl BFieldCodec for Vec<PartialAuthenticationPath<Digest>> {
let mask = str[index + 1].value() as u32;
index += 2;

if (vec_len != 0 || mask != 0) && index == str.len() {
return Err(BFieldCodecError::boxed("cannot decode string of BFieldElements as Vec of PartialAuthenticationPaths due to length mismatch (2)"));
// if the vector length and mask indicates some digests are following
// and we are already at the end of the buffer
if vec_len != 0 && mask != 0 && index == str.len() {
return Err(BFieldCodecError::boxed(
&format!("Cannot decode string of BFieldElements as Vec of PartialAuthenticationPaths due to length mismatch (2).\n\
vec_len: {}\n\
mask: {}\n\
index: {}\n\
str.len(): {}\n\
str[0]: {}",
vec_len,
mask,
index,
str.len(),
str[0])
));
}

if (len_remaining - 2) % DIGEST_LENGTH != 0 {
Expand All @@ -347,7 +363,7 @@ impl BFieldCodec for Vec<PartialAuthenticationPath<Digest>> {
pap.push(Some(*Digest::decode(chunk)?));
index += DIGEST_LENGTH;
} else {
return Err(BFieldCodecError::boxed("cannot decode string of BFieldElements as Vec of PartialAuthenticationPaths due to length mismatch (3)"));
return Err(BFieldCodecError::boxed("cannot decode string of BFieldElements as Vec of PartialAuthenticationPaths due to length mismatch (4)"));
}
}

Expand Down
61 changes: 53 additions & 8 deletions triton-vm/src/stark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1749,19 +1749,12 @@ pub(crate) mod triton_stark_tests {
#[test]
fn triton_prove_verify_halt_test() {
let code_with_input = test_halt();
let mut profiler = Some(TritonProfiler::new("prove halt"));
let (stark, proof) = parse_simulate_prove(
&code_with_input.source_code,
code_with_input.input.clone(),
code_with_input.secret_input.clone(),
&mut profiler,
&mut None,
);
let mut profiler = profiler.unwrap();
profiler.finish();
let report = profiler.report();
println!("{report}");

println!("between prove and verify");

let result = stark.verify(proof, &mut None);
if let Err(e) = result {
Expand All @@ -1770,6 +1763,58 @@ pub(crate) mod triton_stark_tests {
assert!(result.unwrap());
}

#[test]
#[ignore = "used for tracking&debugging deserialization errors"]
fn triton_prove_halt_save_error_test() {
let code_with_input = test_halt();

for _ in 0..100 {
let (stark, proof) = parse_simulate_prove(
&code_with_input.source_code,
code_with_input.input.clone(),
code_with_input.secret_input.clone(),
&mut None,
);

let filename = "halt_error.tsp";
let result = stark.verify(proof.clone(), &mut None);
if let Err(e) = result {
if let Err(e) = save_proof(filename, proof) {
panic!("Unsyntactical proof and can't save! {}", e);
}
panic!(
"Saved proof to {} because verifier is unhappy! {}",
filename, e
);
}
assert!(result.unwrap());
}
}

#[test]
#[ignore = "used for tracking&debugging deserialization errors"]
fn triton_load_verify_halt_test() {
let code_with_input = test_halt();
let (stark, _) = parse_simulate_prove(
&code_with_input.source_code,
code_with_input.input.clone(),
code_with_input.secret_input.clone(),
&mut None,
);

let filename = "halt_error.tsp";
let proof = match load_proof(filename) {
Ok(p) => p,
Err(e) => panic!("Could not load proof from disk at {}: {}", filename, e),
};

let result = stark.verify(proof, &mut None);
if let Err(e) = result {
panic!("Verifier is unhappy! {}", e);
}
assert!(result.unwrap());
}

#[test]
fn prove_verify_fibonacci_100_test() {
let source_code = sample_programs::FIBONACCI_VIT;
Expand Down

0 comments on commit e7fd6cc

Please sign in to comment.