zkevm-circuits
zkevm-circuits copied to clipboard
cb.rw_counter_offset() is not good style
some opcodes use cb.rw_counter_offset() to automatically calculating rw counter change. but it is not good style. cb.rw_counter_offset() is always right, and apparently hide the how many rw changes under which conditions. let compare two ways of it:
rw_counter: Transition::Delta(cb.rw_counter_offset())rw_counter: Delta( 10.expr() - is_first_tx.expr() + coinbase_code_hash_is_zero.expr(), ),
for 1. code, can't get any useful info. for 2. can know exactly rw increase and in which cases, good for code readability and understanding.
Additional context
No response
We had an offline discussion with @hero78119 and @DreamWuGit. Let me try to summarize the discussion.
The conclusion is to support computing rw counter automatically.
As Dream mentioned in the issue description, the benefit of manual computations is the visibility of what operations bumps rw counter by how many.
The arguments for computing automatically are
- When a change is introduced to the code, only the relevant lines are affected. In the manual approach, we also need to bump the rw counter of the irrelevant lines.
- The manual approach spreads magic numbers across the codebase, which is hard to track.
The compromise of the two approaches is to improve the debug message that shows what conditions affect the changes of the rw counter.