Make VerifyFailure::emit public

[georgwiese]

This way, users can pretty-print the error while still catching the verification failure. As far as I can see, printing the error is otherwise only exposed via MockProver::assert_satisfied.

[CPerezz]

Hi!

Well, the idea is that the errors of the MockProver are there only to test that they will happen.

In other words, MockProver is designed such that the failure is asserted to be equal to a particular one that you provide OR to be printed (Debug or Display-wise) directly after a panic in the verify function.

But you should never need to call emit from the consumer perspective.

Can you share an example where you actually need to have this? The "pretty-print" will look the same as the Debugimpl used in the assertion. You can better format it using"{:#?}"`.

[georgwiese]

Thanks for the quick response :)

The "pretty-print" will look the same as the Debug impl used in the assertion. You can better format it using"{:#?}"`.

I don't think so, it looks different for me! Also, looking at the code, functions like render_cell_not_assigned don't seem to be called anywhere else except from emit().

My use-case is that I have a wrapper around MockProver, and usually I want to print the error and panic, as assert_satisfied() would do. But, as you mentioned, in tests I might want to assert that constraints are not satisfied. So it would be better for my wrapper to just return the result of MockProver::verify(), but then there doesn't seem to be a way to get the pretty error in the "typical" case,

[georgwiese]

(I'm on v0.3.0, but looking at the code, looks like this would be the same problem with the current main)

[CPerezz]

Hey @georgwiese does any of these examples work for what you're trying to do?

• halo2/halo2_frontend/src/dev.rs

  Lines 297 to 312 in 1650a0b

  	/// let prover = MockProver::<Fp>::run(K, &circuit, instance).unwrap();
  	/// assert_eq!(
  	/// prover.verify(),
  	/// Err(vec![VerifyFailure::ConstraintNotSatisfied {
  	/// constraint: ((0, "R1CS constraint").into(), 0, "buggy R1CS").into(),
  	/// location: FailureLocation::InRegion {
  	/// region: (0, "Example region").into(),
  	/// offset: 0,
  	/// },
  	/// cell_values: vec![
  	/// (((Any::advice(), 0).into(), 0).into(), "0x2".to_string()),
  	/// (((Any::advice(), 1).into(), 0).into(), "0x4".to_string()),
  	/// (((Any::advice(), 2).into(), 0).into(), "0x8".to_string()),
  	/// ],
  	/// }])
  	/// );

• halo2/halo2_proofs/examples/shuffle.rs

  Lines 250 to 268 in 1650a0b

  	let prover = MockProver::run(k, &circuit, vec![]).unwrap();
  	match (prover.verify(), expected) {
  	(Ok(_), Ok(_)) => {}
  	(Err(err), Err(expected)) => {
  	assert_eq!(
  	err.into_iter()
  	.map(|failure| match failure {
  	VerifyFailure::ConstraintNotSatisfied {
  	constraint,
  	location,
  	..
  	} => (constraint, location),
  	_ => panic!("MockProver::verify has result unmatching expected"),
  	})
  	.collect::<Vec<_>>(),
  	expected
  	)
  	}
  	(_, _) => panic!("MockProver::verify has result unmatching expected"),

Notice that VerifyFailure is indeed pub such that the matching against it can be done.
In any case, if the issue is a panic on MockProver errors (as for the issue you've linked) which seems solvable without the emit method being pub.
See

halo2/halo2_frontend/src/dev/failure.rs

Line 127 in 1650a0b

pub enum VerifyFailure {

[georgwiese]

Hmm, I don't see how. The behavior I'd like is to be able to pretty-print the error (same as when calling assert_satisfied()) but not panic, and as far as I can see there is no way to do that now.

Basically, we have a trait for each prover which returns a Result, and one implementation of this trait wraps MockProver. If we call validate(), we get the Result but no pretty error message; if we call assert_satisfied() we get the message but we're not able to catch the error.

[CPerezz]

I don't see how you can't get the error.

If you call

halo2/halo2_frontend/src/dev.rs

Lines 773 to 778 in 1650a0b

	/// Returns `Ok(())` if this `MockProver` is satisfied, or a list of errors indicating
	/// the reasons that the circuit is not satisfied.
	/// Constraints and lookup are checked at `usable_rows`, parallelly.
	pub fn verify(&self) -> Result<(), Vec<VerifyFailure>> {
	self.verify_at_rows(self.usable_rows.clone(), self.usable_rows.clone())
	}

you should be able to invoke the Display impl for the vector of failures after calling .err().unwrap().

Apologies if I'm missing how this doesn't solve your issue.
If this doesn't allow it, then I'm happy to include this after someone else agrees to it too!

Edit:
Ohhh I see now.

You don't want the regular error but rather the one that gives you the constraint details with the parrot message etc..
So the one that writes the table.

Fair enough. Apologies for the insistence.

[CPerezz]

LGTM! This looks like the easiest and simplest way to go.

[ed255]

LGTM! After reading the discussion I believe the use case described by @georgwiese is totally reasonable!