QA: PI circuit

[pinkiebell]

The following points need clarification regarding the zkevm-specs and the pi circuit implementation.

• In the zkevm-specs we encode each value with FQ(v) = v % 0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47 to fit into the field. In circuits we take the raw value for values that fit safely and otherwise doing rlc(value, randomness). Which is king?

  • This issue also applies to rlc_rpi that isn't using mul,add-mod in the circuit impl.

• In the specs we define rand_rpi to be keccak(raw_public_inputs). Is there a reason that the circuit implementation doesn;t build it by default instead of requiring this in the constructor?

• Related to the above, why do we have an additional randomness value. Isn't that supposed to be rand_rpi?

• Do we mul,add-mod rand_rpi too or just safely mask off one byte?

• Another issue is tx_sign_hash which is handled quite differently here.

I did like to know which one of the implementations I should take as reference 🙂

[han0110]

For the first bullet, I think we should not encode values that don't fit in field with rlc(value, randomness) when calculating rlc_rpi, since it just adds more complexity :( Perhaps what we should do is to split these 256-bits values into hi and lo as input of rlc_rpi.

• In the specs we define rand_rpi to be keccak(raw_public_inputs). Is there a reason that the circuit implementation doesn;t build it by default instead of requiring this in the constructor?

I think it's just for convenience, but @ed255 might know better.

• Related to the above, why do we have an additional randomness value. Isn't that supposed to be rand_rpi?

The randomness is a challenge inside PLONK to do RLC for EVM words. rand_rpi is more like a challenge outside PLONK, but used as public input of PLONK.

• Do we mul,add-mod rand_rpi too or just safely mask off one byte?

I think we need, and it seems we also need to check each item is less than modulus, otherwise one can just use alias to alter the keccak256(raw_public_inputs) result (not sure how bad this is, but definitely looks not good).

• Another issue is tx_sign_hash which is handled quite differently here.

I think this is following the spec of ECDSA, taking the leftmost bits of hash output as sign message.

[ed255]

* In the zkevm-specs we encode each value with `FQ(v) = v % 0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47` to fit into the field.
  In circuits we take the raw value for values that fit safely and otherwise doing `rlc(value, randomness)`.
  Which is king?
  
  * This issue also applies to `rlc_rpi` that isn't using mul,add-mod in the circuit impl.

I'm responsible for this and I'm sorry about it! When I was writing the spec of the public inputs circuit, we had talked about not using the RLC for word and instead splitting words into 2 values (hi and lo). See https://github.com/privacy-scaling-explorations/zkevm-specs/issues/216 . So I thought we would soon have that, so as a shortcut I just took words that are used as public inputs and did % p. Thinking that we would replace it soon with v_hi, v_lo. But then we decided to postpone this change, and this shortcut stayed in the design.

The correct thing to be consistent with the rest of the circuits should be to do RLC(v, r). That requires accessing the randomness before starting the proof computation (before doing any commitment), which is very tricky, and may be not possible (there's a cyclic dependency). A solution that we know should work and is less complex is splitting those values into hi an lo, as Han says. But I'm afraid we can't just do this change in the public inputs circuit, we'd need to propagate it everywhere (and remove the word RLC from all circuits).

* In the specs we define `rand_rpi` to be `keccak(raw_public_inputs)`. Is there a reason that the circuit implementation doesn;t build it by default instead of requiring this in the constructor?

Technically the rand_rpi should be keccak(raw_public_inputs | commitment(raw_public_inputs)). The reason is because we calculate this RLC twice:

1. In the verifier, using the freshly calculated raw_public_inputs values.
2. In the circuit, using the committed raw_public_inputs advice column values.

We need to make sure that once the rand_rpi is known, the raw_public_inputs in both 1 and 2 can't be changed. So the rand_rpi must depend on a commitment to both raw_public_inputs vectors. And that will guarantee that they are the same; otherwise the prover can generate valid proofs where the two raw_public_inputs are different.

Calculating the commitment(raw_public_inputs) is not done yet because:

1. It requires this to be resolved: https://github.com/privacy-scaling-explorations/halo2/issues/105 . Alternatively Han thought of a shortcut to get that without doing a change in halo2
2. We need to isolate the code to generate a commitment outside of a circuit proof.

I think there's no particular reason why the circuit takes a rand_rpi in the constructor instead of calculating that value internally.

* Related to the above, why do we have an additional `randomness` value. Isn't that supposed to be `rand_rpi`?

As Han replied, we require different randomness for different stuff. AFAIK in all the circuits we require 3 randomnesses:

1. for Word RLC
2. for dynamic length byte array input for keccak hashing
3. rand_rpi

* Do we mul,add-mod `rand_rpi` too or just safely mask off one byte?

rand_rpi should be in the field. And it's used like this (using addmod and mulmod):

$p = \sum_i rand\_rpi^i \times raw\_public\_inputs[i] \text{ mod } p$

* Another issue is `tx_sign_hash` which is handled quite differently [here](https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/main/eth-types/src/geth_types.rs#L215).

Just what Han said, that's part of the ECDSA spec. An in particular msg_hash % q will not fit in the field used in the circuit. So it would need to be RLC'ed.

I did like to know which one of the implementations I should take as reference slightly_smiling_face

I think the biggest concern is the first point. It's not clear to me which path to take in the short term. I see three options:

1. Move forward with a mocked randomness. This means that we can't have a sound public inputs circuit yet.
2. Figure out a way to take the word_challenge (the randomness required for word_rlc) outside of the circuit so that the verifier can do RLC(v, word_challenge). It's unclear if this is even possible
3. Resolve https://github.com/privacy-scaling-explorations/zkevm-specs/issues/216 . This requires a big refactor.

[pinkiebell]

Thanks! I will take a chance with option 1 first. That means I first have to adapt the yul code to do the rlc dance 👍

[kunxian-xia]

@ed255 Why don't we just use the challenge generated when we finished the synthesize of first phase columns(word bytes) and second phase columns (Word RLC) as the rand_rpi ? And there might be some issue with the current pi circuit which does not make the randomness for word RLC public, since the contract needs to use it to get the full raw_public_inputs.

[ed255]

@ed255 Why don't we just use the challenge generated when we finished the synthesize of first phase columns(word bytes) and second phase columns (Word RLC) as the rand_rpi?

That would be ideal, but from a conversation with Han, I learned that the halo2 proving system first writes the instance (public inputs) to the transcript, and then commits the advice columns which are also written to the transcript. To calculate the public inputs we need the randomness, which is taken by hashing the transcript after the advice columns have been committed. So this creates a circular dependency. If I remember correctly, writing the public inputs to the transcript after committing the advice columns may be unsound.

And there might be some issue with the current pi circuit which does not make the randomness for word RLC public, since the contract needs to use it to get the full raw_public_inputs.

Definitely, the current pi circuit design can't be implemented :( because there's no way to expose the randomness via public inputs for the verifier to use it.