Hi, I am confident that the public signals are handled incorrectly in the Groth16 circom adapter.

First, lets check the vitalik integration test here. It belongs to the circuit:

template Test() {
	signal input x;
	signal output out;

	signal sym_1;
	signal y;

	sym_1 <== x * x;
	out <== (sym_1 * x) + (x + 5);
}

The circuit code above shows that we have 1 output (which is a public signal), 0 public inputs and 1 private inputs; these are also confirmed by the r1cs file here.

As the test asserts the witness returned from circom_to_lambda is as follows:

["1", "3", "23", "9"] // these are all hexadecimal btw

As per the README & the poseidon test here, the public signals are returned by the slice &witness[..qap.num_of_public_inputs]. There are two things wrong with this:

1. The public output was not considered, so num_of_public_inputs is only 1 due to the constant 1 added in here.
2. Even if the outputs were considered, we still have to consider two slices, e.g. one for the constant 1 and other for the 23 that skips over the private inputs, in other words we care about: ["1", ..., "23", ...] within our witness but we have no way to do this without knowing the number of non-intermediate private inputs.

I think there should be two things done:

• I believe the correct form of pub input calculation should be:

let num_of_pub_inputs =
    circom_r1cs["nPubInputs"].as_u64().unwrap() as usize +
    circom_r1cs["nOutputs"].as_u64().unwrap() as usize +
    1;

• The public signals should not be &witness[..qap.num_of_public_inputs] but instead slice in such a way that it skips over private inputs. Perhaps a QAP method can take a reference to the witness and do this, without letting the user do the slicing manually like this.

However, the QAP is edited in such a way that the proof verification actually passes with the wrong public inputs, as the passing tests show. I have not yet looked deep into what might change there, but would love to contribute if shown a hint 🙏🏻