Remove minimal_rows in the Bytecode circuit

[ChihChengLiang]

Description

• Remove minimal_rows
• Configure the bytecode circuit with the circuit degree instead of size.

Issue Link

#1607

Type of change

[x] Bug fix (non-breaking change which fixes an issue)

Note

Please review this after #1632 is merged.

[hero78119]

LGTM

[ed255]

Please take a look at my long discussion :P

[ed255]

I think that parametrizing the circuit with just the degree has some downsides, in particular for testing with the MockProver.
Most of the times when running the MockProver we want the verification to only apply to rows that contain actual data. The only case where we want more than that would be to test the padding strategy that the circuit applies to support a fixed size circuit with a smaller than max capacity data; but that's usually a single test.

By configuring with degree, when running the MockProver we'll be checking more rows than strictly necessary. Let's say we have some bytecodes that need 1200 rows. We would need to verify 2048 rows with the current approach.

Also this is specially important when testing the SuperCircuit with the MockProver. Let's imagine that for a block we need 4000 rows for the TxCircuit and 100 for the block circuit. Then the SuperCircuit degree would be k=12 (4096 rows). Would you pass degree=12 to the TxCircuit? Maybe you'd pass degree=7 (128 rows) to have a better verification time, but that feels strange because that degree is not the real one.

We have other circuits that don't receive the degree, and instead receive how many rows to enable, or the maximum number of operations to verify.

• CopyCircuit takes max_copy_rows
• KeccakCircuit takes num_rows
• ExpCircuit takes max_exp_steps

To avoid dynamic circuits on real proofs, we have the DynamicCParams and FixedCParams types in bus-mapping/src/circuit_input_builder.rs. The first allows the circuit input builder to chose the circuit parameters dynamically based on a specific block input; and the second allows us to pick static parameters regardless of the input (so we get fast MockProver verification, and constant circuits for the real prover).

[ed255]

On the other hand: I agree that saying:

let last_row_offset = 2usize.pow(degree) - unusable_rows - 1;

Is correct and clear.
Whereas if we only have the parameters max_rows and unusable_rows, I would argue that

let last_row_offset = max_rows - 1;

But then we don't know if last_row_offset <= 2^k - unusable_rows - 1, which is a nice check to have as early as possible.

As a possible solution, 2 ideas come to my mind:

• a) Pass the max_rows and degree to each circuit so that we can have the full view and do all the early checks, while supporting fast MockProver.verify. Note that for a real proof we would need to set max_rows = 2^degree - unusable_rows(), but we already have that API in the SubCircuit trait
• b) Extend halo2 (either the Layouter or the ConstraintSystem) to expose the circuit degree, so that we can get that value during configuration or synthesis time, and do all the checks we want with it.

[ChihChengLiang]

I see the reasoning now.

• To save the testing time on Mock prover, I'll change the parameter from "degree" back to "size." Or even better, rename "size" to "max_rows."
• I'll comment on the rationale for using size.
• I'll comment on DynamicCParams and FixedCParams

For validating max_rows, I like the idea for Halo2 to expose the circuit degree. For the quick fix, I'll use the SubCircuit trait.

[ChihChengLiang]

Hi @ed255,
I've addressed the feedback, can you check again?

[ed255]

Following the change in assign_internal, now line 23 (in from_bytes) should be:

Suggested change

	let prover = MockProver::<F>::run(log2_ceil(self.max_rows), self, Vec::new()).unwrap();
	Self::new(bytecodes.into(), 2usize.pow(k) - Self::<Fr>::unusable_rows())

I think this is the reason why tests are failing.

[ed255]

LGTM!