state circuit update part1: change RwTable according to latest spec; finish stack/storage constraints #380
Conversation
+ q_target -> tag + flag -> is_write move address and storagekey to keys prune padding col
github-actions
bot
added
T-opcode
lispc
changed the title
yes. Old implementation uses lookup, but we need finally use range check. This modification needs some work. So I disable it temporarily |
Cool, thanks! |
There was a problem hiding this comment.
This looks very good! Thanks for the nice work :)
I've added some comments and questions. Maybe you're planning to address them in the next PR, that's totally OK by me! But if that's the case I would just ask if you can add some small comments stating the missing tasks with TODO.
|
|
||
| for (i, diff_is_zero_chip) in diff_is_zero_chips.iter().enumerate() { | ||
| let (value, diff) = match i { | ||
| // FIXME: find a better way here |
There was a problem hiding this comment.
I think this is good enough :)
The only think that would come to my mind to improve this would be to change RwRow to have a field keys: [F, 5].
| region.assign_advice(|| "rw counter", self.rw_counter, offset, || Ok(rw_counter))?; | ||
| region.assign_advice(|| "value", self.value, offset, || Ok(value))?; | ||
| region.assign_advice(|| "is_write", self.is_write, offset, || Ok(is_write))?; | ||
|
|
There was a problem hiding this comment.
I noticed that the following values are not assigned:
value_prevaux1aux2
Why is that?
There was a problem hiding this comment.
yes, I will add "aux1" and "aux2"! While I think value_prev has been removed already in the spec?
There was a problem hiding this comment.
ah that's right, value_prev is no longer in the state circuit spec.
|
@ed255 Hi Edu, I addressed the comments above~ You can have a look. Thanks! |
| region.assign_advice(|| "rw counter", self.rw_counter, offset, || Ok(rw_counter))?; | ||
| region.assign_advice(|| "value", self.value, offset, || Ok(value))?; | ||
| region.assign_advice(|| "is_write", self.is_write, offset, || Ok(is_write))?; | ||
|
|
There was a problem hiding this comment.
ah that's right, value_prev is no longer in the state circuit spec.
…acy-scaling-explorations#380) * Constrain `CALLDATALOAD` stack push result must be 0 if stack pop offset is Uint64 overflow. * Fix `valid_bytes` to a const generic parameter in `WordRangeGadget`. * Refactor memory `copy_from` in bus-mapping. * Fix a mistake in bus-mapping `CALLDATACOPY`. * Refactor to use `WordByteCapGadget`. * Fix lint.
resolving issue: #356
The reason why I submit this WIP PR now is that RwTable is updated according to the latest spec here. Another key column is added, so the overall keys become: tag,key1,key2,key3,key4.
If we can merge this PR early, we may encounter less conflicts on RwTable. So i think it is better to split the whole state circuit refactor PR into several smaller ones, merge them one by one periodically