Make EVM circuit work with multi-phase MockProver

[han0110]

This PR aims to fix EVM circuit to have all advice columns in correct phase to work with multi-phase MockProver privacy-scaling-explorations/halo2#129, and should also work with real prover.

Base on #1087.

[lispc]

another related problem: i guess this line may be wrong. Need to be less than?

https://github.com/privacy-scaling-explorations/halo2/blob/99795dd4dfd48478fcbbd86a6610df814c282ad4/halo2_proofs/src/plonk/prover.rs#L207

for example, in evm circuit, the stored_expressions in CachedRegion need to "pick" assigned values to do RLC and assign the results. It seems if we don't assign phase1 columns in phase2/phase3, it picks a zero value, and do incorrect RLC..

[han0110]

another related problem: i guess this line may be wrong. Need to be less than?

hmm I kind of forget the reason to be that strict (I thought it's because the to is FnMut but it seems not). This is the original discussion on this topic zcash/halo2#593 (comment). Mainly I might just wanted to keep the convention that in unrelated phase we don't call the to (for instance in keygen the to in assign_advice will not be called, and in create_proof the to in assign_fixed will not be called as well).

But this indeed causes the trouble you said (in later phase we can't get the value from AssignedCell of previous phase), and that's the reason I did the change like this (note that the to is called again to always get the value):

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

Lines 148 to 174 in 97491b2

	pub fn assign_advice<'v, V, VR, A, AR>(
	&'v mut self,
	annotation: A,
	column: Column<Advice>,
	offset: usize,
	to: V,
	) -> Result<AssignedCell<VR, F>, Error>
	where
	V: Fn() -> Value<VR> + 'v,
	for<'vr> Assigned<F>: From<&'vr VR>,
	A: Fn() -> AR,
	AR: Into<String>,
	{
	// Actually set the value
	let res = self.region.assign_advice(annotation, column, offset, &to);
	// Cache the value
	// Note that the `value_field` in `AssignedCell` might be `Value::unkonwn` if
	// the column has different phase than current one, so we call to `to`
	// again here to cache the value.
	if res.is_ok() {
	to().map(|f| {
	self.advice[column.index() - self.width_start][offset - self.height_start] =
	Assigned::from(&f).evaluate();
	});
	}
	res
	}

[lispc]

another related problem: i guess this line may be wrong. Need to be less than?

hmm I kind of forget the reason to be that strict (I thought it's because the to is FnMut but it seems not). This is the original discussion on this topic zcash/halo2#593 (comment). Mainly I might just wanted to keep the convention that in unrelated phase we don't call the to (for instance in keygen the to in assign_advice will not be called, and in create_proof the to in assign_fixed will not be called as well).

But this indeed causes the trouble you said (in later phase we can't get the value from AssignedCell of previous phase), and that's the reason I did the change like this (note that the to is called again to always get the value):

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

Lines 148 to 174 in 97491b2

	pub fn assign_advice<'v, V, VR, A, AR>(
	&'v mut self,
	annotation: A,
	column: Column<Advice>,
	offset: usize,
	to: V,
	) -> Result<AssignedCell<VR, F>, Error>
	where
	V: Fn() -> Value<VR> + 'v,
	for<'vr> Assigned<F>: From<&'vr VR>,
	A: Fn() -> AR,
	AR: Into<String>,
	{
	// Actually set the value
	let res = self.region.assign_advice(annotation, column, offset, &to);
	// Cache the value
	// Note that the `value_field` in `AssignedCell` might be `Value::unkonwn` if
	// the column has different phase than current one, so we call to `to`
	// again here to cache the value.
	if res.is_ok() {
	to().map(|f| {
	self.advice[column.index() - self.width_start][offset - self.height_start] =
	Assigned::from(&f).evaluate();
	});
	}
	res
	}

thanks! this seems a better solution!

[ed255]

LGTM!