[CREATE Part A] Modification to Copy Circuit #1356
Conversation
github-actions
bot
added
crate-mock
| @@ -94,11 +95,12 @@ impl<F: Field> ExecutionGadget<F> for ReturnRevertGadget<F> { | |||
|
|
|||
| let is_contract_deployment = | |||
| is_create.clone() * is_success.expr() * not::expr(copy_rw_increase_is_zero.expr()); | |||
| let (caller_id, address, reversion_info, code_hash) = | |||
| let (caller_id, address, reversion_info, code_hash, init_code_rlc) = | |||
| cb.condition(is_contract_deployment.clone(), |cb| { | |||
| // We don't need to place any additional constraints on code_hash because the | |||
| // copy circuit enforces that it is the hash of the bytes in the copy lookup. | |||
There was a problem hiding this comment.
since intruduce init_code_rlc i think the comment also need to adjust a bit.
There was a problem hiding this comment.
Not sure what comment I should add. Could you provide an example?
| @@ -47,6 +47,8 @@ pub struct CopyCircuitConfig<F> { | |||
| pub is_last: Column<Advice>, | |||
| /// The value copied in this copy step. | |||
| pub value: Column<Advice>, | |||
| /// Random linear combination accumulator value. | |||
| pub value_acc: Column<Advice>, | |||
There was a problem hiding this comment.
Can we rename value_acc into value_keccak_acc for more self-explanation
There was a problem hiding this comment.
I would prefer to rename to value_acc_rlc. Most of our code and docs are using _rlc as suffix to represent rlc variables and it looks good to me without keyword, keccak.
|
|
||
| cb.gate(meta.query_fixed(q_enable, Rotation::cur())) | ||
| }); | ||
|
|
||
| meta.create_gate( |
There was a problem hiding this comment.
Seems this gate and next gate can be combine into just 1 gate ?
| cb.gate(and::expr([ | ||
| meta.query_fixed(q_enable, Rotation::cur()), | ||
| meta.query_advice(is_last, Rotation::next()), | ||
| and::expr([ |
There was a problem hiding this comment.
Here nested and::expr seems unnessarary, is it for readibility?
There was a problem hiding this comment.
agree, I don't think we need it, but it has better readability for me. I prefer to keep the way it is. How do you think?
|
Also raise privacy-scaling-explorations/zkevm-specs#411 to align copy-circuit specs |
|
Moved to #1419 |
### Description NOTE: This is an updated version of #1356 This is Part A of a 3 part pull request to add support for `CREATE`/`CREATE2` opcodes. Part A: #1356 Part B: #1357 Part C: #1358 ### Issue Link #1130 ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Contents As part of the bigger additions needed for the `CREATE`/`CREATE2` opcodes' gadget, this PR adds support for the copy circuit to "always" have a value accumulator field `value_acc`. ### Rationale We need a value accumulator (of the random linear combination) in order to get the `RLC(bytes)` for the bytes copied from `Memory` to `Bytecode` (specifically the init code). This RLC is later used to do a lookup to the Keccak table to check the value of `keccak256(init_code)`. ### How Has This Been Tested? The existing tests for copy circuit pass for the updated constraints on the copy circuit. --------- Co-authored-by: Rohit Narurkar <[email protected]> Co-authored-by: KimiWu <[email protected]>
Description
This is Part A of a 3 part pull request to add support for
CREATE/CREATE2opcodes.Part A: #1356
Part B: #1357
Part C: #1358
Issue Link
#1130
Type of change
Contents
As part of the bigger additions needed for the
CREATE/CREATE2opcodes' gadget, this PR adds support for the copy circuit to "always" have a value accumulator fieldvalue_acc.Rationale
We need a value accumulator (of the random linear combination) in order to get the
RLC(bytes)for the bytes copied fromMemorytoBytecode(specifically the init code). This RLC is later used to do a lookup to the Keccak table to check the value ofkeccak256(init_code).How Has This Been Tested?
The existing tests for copy circuit pass for the updated constraints on the copy circuit.