[CREATE Part A (updated)] Modification to Copy Circuit #1419
Conversation
github-actions
bot
added
crate-mock
|
This PR is a continuation of #1356 |
github-actions
bot
added
the
crate-bus-mapping
github-actions
bot
removed
the
crate-bus-mapping
There was a problem hiding this comment.
Currently the tests are not passing due to a degree increase of the gates expressions, take a look at my comment for a possible fix. Following Han observation about the missing constraints on is_first and is_last I think a careful review of this circuit could be really useful! I suggest that we resolve the underconstraints in a future PR (Kimi already opened an issue for that), and move on with this PR once the tests pass to unblock the CREATE.
| @@ -50,6 +50,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_rlc: Column<Advice>, | |||
There was a problem hiding this comment.
Is this related to #971 ?
I see that now the rlc is accumulated in value_acc_rlc and then exposed in the table via rlc_acc, so rlc values never appear in the value column. So probably the issue is solved here.
There was a problem hiding this comment.
This was @roynalnaruto's fix and not sure the reason behind. But it seems solved the issue you mentioned.
There was a problem hiding this comment.
I think to have an extra value_acc_rlc is to support memory <-> bytecode copy and rlc at the same time without changing other parts too much.
And it seems to also resolve #971, so we could try to move the phase of value from second to first and see if it works as expected.
zkevm-circuits/src/copy_circuit.rs
Outdated
| or::expr([ | ||
| and::expr([ | ||
| tag.value_equals(CopyDataType::Memory, Rotation::cur())(meta), | ||
| tag.value_equals(CopyDataType::Bytecode, Rotation::next())(meta), | ||
| ]), | ||
| tag.value_equals(CopyDataType::RlcAcc, Rotation::next())(meta), | ||
| ]), |
There was a problem hiding this comment.
The expression from here may be causing the final gate degree ti increase more than 9 (and that's why tests are not passing). A simple solution would be to split this into 2 constraints. Following this pattern:
if A or B then X -> if A then X, if B then X.
Another thing that comes to my mind is that or::expr returns a boolean, but to build a selector expression just having 0 when false and != 0 when true is enough, so perhaps you could replace the or by a +. This would give 2 when both expressions are true, but that's ok.
There was a problem hiding this comment.
Nice idea! It looks nice and clean now, fixed in 52d480a
…adgets (#1425) ### Description **NOTE: This is an updated version of #1357 This PR is actually based on top of #1419 ### 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 Error types for insufficient balance and nonce overflow. These are supporting changes required for the CREATE/CREATE2 opcodes' gadget. ### Rationale The above errors will be handled within the CREATE/CREATE2 opcodes' gadget. In case of insufficient balance, it is also handled in the CallOp gadget for call related opcodes. --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Description
NOTE: This is an updated version of #1356
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.