Feat/#498 integrate precompiles into callop #508
Conversation
KimiWu123
changed the title
|
|
|
@roynalnaruto , do you have time to give me some feedback? Thanks. 😅 |
Co-authored-by: Chih Cheng Liang <chihchengliang@gmail.com>
KimiWu123
force-pushed
the
feat/#498-integrate-precompiles-into-callop
branch
from
14a38d4
to
f3eee6f
Compare
| input_bytes.extend(sig_r.int_value().to_bytes(32, "little")) | ||
| input_bytes.extend(sig_s.int_value().to_bytes(32, "little")) | ||
| input_rlc = RLC(bytes(reversed(input_bytes)), keccak_randomness, n_bytes=128).expr() | ||
| instruction.constrain_equal(rlc_data.input_rlc, input_rlc) |
There was a problem hiding this comment.
Aren't these two values actually the same (I mean not equal, but the same)? Because rlc_data.input_rlc is computed when instantiating the auxiliary data and input_rlc is computed accessing the same values in aux_data?
There was a problem hiding this comment.
For me, data assignment in testing is like what assign_exec_step does at proving time in our circuit. So, yes, it looks the same (but not always the same).
At proving time, a prover assigns
- msg_hash
- sig_v, sig_r, sig_s
- RLC(msg_hash, sig_v, sig_r, sig_s) (a.k.a
input_rlchere, this field is to verify data consistency between calls)
In verification logic here, we have to gaurantee msg_hash, sig_v, sig_r and sig_s are the same pairs (sig_v, sig_r and sig_s are coming from the signature of msg_hash). However, a malious signer could sign msg_hash2 and have sig_v2, sig_r2 and sig_s2. It still can pass all the constraints if we don't have calculate input_rlc here. I'm assuming rlc_data.input_rlc is coming from previous step (we can pass data around between previous step and next step in our circuit, but it seems not doable in our spec), so that's why rlc_data.input_rlc and input_rlc look "the same".
Does it make sense to you? Or do you think we need to copy rlc_data.input_rlc from copy_table?
There was a problem hiding this comment.
I think I am missing something and would like to fix my understanding :). My understanding was that we need to ensure that instruction.curr.aux_data is the same as used in callop.py and that this should be done by checking the RLC, so to have a lookup call into copy_table (RLC field) here in ecrecover.py. Maybe this is not the case?
I agree on the assignment part, putting a link to the assignment was probably misleading from my side. What it seems to me here is that we have aux_data: PrecompileAuxData = instruction.curr.aux_data[0] and then both values, aux_data.input_rlc and input_rlc, are computed using aux_data. Or is it aux_data.input_rlc checked somewhere else?
There was a problem hiding this comment.
My understanding was that we need to ensure that
instruction.curr.aux_datais the same as used incallop.pyand that this should be done by checking the RLC, so to have a lookup call intocopy_table(RLC field) here inecrecover.py. Maybe this is not the case?
Yes, that's exactly what I want to do.
I agree on the assignment part, putting a link to the assignment was probably misleading from my side. What it seems to me here is that we have
aux_data: PrecompileAuxData = instruction.curr.aux_data[0]and then both values,aux_data.input_rlcandinput_rlc, are computed usingaux_data. Or is itaux_data.input_rlcchecked somewhere else?
No, aux_data.input_rlc was not check in other place.
In my implementation, I treated aux_data as witness inputs, a place I can assign my witnesses. Those data (e.g. sig_r, sig_v...etc) can't be retrieved from stack or memory like what other opcode gagdets implemented.
I might not explain it very well, but you can check Scroll's impl. The msg_hash_raw was converted into rlc formate and compared with msg_hash_keccak_rlc. In the assignment, you can see both of them come from the same source, here and here.
closed #498