bug: some fixed columns are not fixed at all

[lispc]

In super circuit, copy_table.q_enable has no padding mechanism so it is not fixed in fact.

So using empty block and real block can result with different vks. Even if proof from real block can be verified with vk from empty block, it is not sound since q_enable is almost disabled in (allmost?) all offsets..

Another smaller problem is that evm circuit has tx_table.tag, which is also a fixed column, but in evm circuit it is not fixed (while in super circuit tx_table.tag is fixed). If we treat evm circuit as a "sub" circuit then it may be acceptable. But again, vk from evm circuit is not universal anyway...

I remember we have encounter this problem (sveral times?) before.. So maybe better to setup a test case to ensure gen_vk(empty_block) == gen_vk(a_real_block)..

[han0110]

But again, vk from evm circuit is not universal anyway...

I think we can update it to be fixed just by move TxCircuit::assign_tx_table logic to TxTable::load, and it shouldn't affect the testing time. Then also need to update this

zkevm-circuits/zkevm-circuits/src/evm_circuit.rs

Lines 427 to 428 in 9400c62

	let num_rows_required_for_tx_table: usize =
	block.txs.iter().map(|tx| 9 + tx.call_data.len()).sum();

to use max_calldata instead of tx.call_data.len().

So maybe better to setup a test case to ensure gen_vk(empty_block) == gen_vk(a_real_block)..

I think this is a really nice idea if we have this test in integration test.

[aguzmant103]

Meeting notes:

• Not only fixing but adding a new test to the code base to verify
• Concern this test will take a lot of time because it will need a real prover
• Key generation not much but supercircuit maybe yes
• We want to address all circuits not just the ones identified above

[CPerezz]

@han0110 I think that this wasn't a testing issue but rather an absence of padding for the CopyCircuit.
At least we've been with it with @ed255 and seems that we have discovered a couple of things that were missing!

Let us know what you think if you have time!

[ed255]

Maybe related to this issue (@CPerezz pointed me to it): privacy-scaling-explorations/zkevm-specs#359 as all circuits will require a padding strategy to have constant fixed columns regardless of the input.

[CPerezz]

This won't be closed until #1062 is merged.

[aguzmant103]

Note: the remaining circuit is exp circuit.
After working on the alternative PR we can close this PR as duplicate #1062

[ed255]

Haichen will check with the author of the spec PR privacy-scaling-explorations/zkevm-specs#359 to determine when is the circuit PR is expected to be done in order to coordinate with PSE. We would like to address this issue (non-constant fixed columns) with high priority, so if the expected date for the circuit is too far away we (PSE) can work on on the circuit in parallel while the spec PR is being done (considering that the strategy on how to resolve the Exp Circuit padding has already been discussed and won't change the circuit constraints)