Skip to content

Commit 29b152a

Browse files
DreamWuGitDreamWuGit
authored
Prepare bytecode refactor (#1397)
* add BytecodeLengthGadget BytecodeLookupGadget * remove program_counter_offset * update create/invalid_jump/precompile_fail * update return_revert/stop/extcodecopy * remove lookup * add debug helper * return code length in BytecodeLengthGadget::assign * fix clippy * remove extra code_hash * add algorithm * more test * fix clippy from upstream * edge test for algorithm
1 parent 1d31ee8 commit 29b152a

File tree

14 files changed

+347
-146
lines changed

14 files changed

+347
-146
lines changed

‎aggregator/src/chunk.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ impl ChunkInfo {
6565
b.transactions
6666
.iter()
6767
.enumerate()
68-
.filter(|(idx, tx)| !tx.is_l1_tx())
68+
.filter(|(_idx, tx)| !tx.is_l1_tx())
6969
.flat_map(|(idx, tx)| {
7070
tx.to_eth_tx(
7171
b.header.hash,

‎bus-mapping/src/circuit_input_builder/execution.rs

+18
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,24 @@ pub enum NumberOrHash {
368368
Hash(H256),
369369
}
370370

371+
impl NumberOrHash {
372+
/// get hash value for NumberOrHash::Hash type
373+
pub fn get_hash(&self) -> H256 {
374+
match self {
375+
Self::Hash(val) => *val,
376+
_ => panic!("not a hash type"),
377+
}
378+
}
379+
380+
/// get number value for NumberOrHash::Number type
381+
pub fn get_number(&self) -> usize {
382+
match self {
383+
Self::Number(val) => *val,
384+
_ => panic!("not a number type"),
385+
}
386+
}
387+
}
388+
371389
/// Represents all bytes related in one copy event.
372390
///
373391
/// - When the source is memory, `bytes` is the memory content, including masked areas. The

‎zkevm-circuits/src/evm_circuit/execution/callop.rs

+21-13
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ use crate::{
55
step::ExecutionState,
66
util::{
77
and,
8-
common_gadget::{CommonCallGadget, TransferGadget, TransferGadgetInfo},
8+
common_gadget::{
9+
BytecodeLookupGadget, CommonCallGadget, TransferGadget, TransferGadgetInfo,
10+
},
911
constraint_builder::{
1012
ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition,
1113
Transition::{Delta, To},
@@ -42,7 +44,7 @@ use std::cmp::min;
4244
#[derive(Clone, Debug)]
4345

4446
pub(crate) struct CallOpGadget<F> {
45-
opcode: Cell<F>,
47+
opcode_gadget: BytecodeLookupGadget<F>,
4648
is_call: IsZeroGadget<F>,
4749
is_callcode: IsZeroGadget<F>,
4850
is_delegatecall: IsZeroGadget<F>,
@@ -91,14 +93,20 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
9193
const EXECUTION_STATE: ExecutionState = ExecutionState::CALL_OP;
9294

9395
fn configure(cb: &mut EVMConstraintBuilder<F>) -> Self {
94-
let opcode = cb.query_cell();
95-
cb.opcode_lookup(opcode.expr(), 1.expr());
96-
let is_call = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALL.expr());
97-
let is_callcode = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALLCODE.expr());
98-
let is_delegatecall =
99-
IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::DELEGATECALL.expr());
100-
let is_staticcall =
101-
IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::STATICCALL.expr());
96+
let opcode_gadget = BytecodeLookupGadget::construct(cb);
97+
98+
let is_call =
99+
IsZeroGadget::construct(cb, opcode_gadget.opcode.expr() - OpcodeId::CALL.expr());
100+
let is_callcode =
101+
IsZeroGadget::construct(cb, opcode_gadget.opcode.expr() - OpcodeId::CALLCODE.expr());
102+
let is_delegatecall = IsZeroGadget::construct(
103+
cb,
104+
opcode_gadget.opcode.expr() - OpcodeId::DELEGATECALL.expr(),
105+
);
106+
let is_staticcall = IsZeroGadget::construct(
107+
cb,
108+
opcode_gadget.opcode.expr() - OpcodeId::STATICCALL.expr(),
109+
);
102110

103111
// Use rw_counter of the step which triggers next call as its call_id.
104112
let callee_call_id = cb.curr.state.rw_counter.clone();
@@ -708,7 +716,7 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
708716
);
709717

710718
Self {
711-
opcode,
719+
opcode_gadget,
712720
is_call,
713721
is_callcode,
714722
is_delegatecall,
@@ -841,8 +849,8 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
841849
}
842850
}
843851

844-
self.opcode
845-
.assign(region, offset, Value::known(F::from(opcode.as_u64())))?;
852+
self.opcode_gadget
853+
.assign(region, offset, block, call, step)?;
846854
self.is_call.assign(
847855
region,
848856
offset,

‎zkevm-circuits/src/evm_circuit/execution/codecopy.rs

+15-9
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
param::{N_BYTES_MEMORY_WORD_SIZE, N_BYTES_U64},
1010
step::ExecutionState,
1111
util::{
12-
common_gadget::{SameContextGadget, WordByteCapGadget},
12+
common_gadget::{BytecodeLengthGadget, SameContextGadget, WordByteCapGadget},
1313
constraint_builder::{
1414
ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition,
1515
},
@@ -46,6 +46,8 @@ pub(crate) struct CodeCopyGadget<F> {
4646
/// RW inverse counter from the copy table at the start of related copy
4747
/// steps.
4848
copy_rwc_inc: Cell<F>,
49+
/// Wraps the bytecode length and lookup.
50+
code_len_gadget: BytecodeLengthGadget<F>,
4951
}
5052

5153
impl<F: Field> ExecutionGadget<F> for CodeCopyGadget<F> {
@@ -73,9 +75,6 @@ impl<F: Field> ExecutionGadget<F> for CodeCopyGadget<F> {
7375
// Fetch the hash of bytecode running in current environment.
7476
let code_hash = cb.curr.state.code_hash.clone();
7577

76-
// Fetch the bytecode length from the bytecode table.
77-
cb.bytecode_length(code_hash.expr(), code_size.expr());
78-
7978
// Calculate the next memory size and the gas cost for this memory
8079
// access. This also accounts for the dynamic gas required to copy bytes to
8180
// memory.
@@ -128,6 +127,14 @@ impl<F: Field> ExecutionGadget<F> for CodeCopyGadget<F> {
128127
};
129128
let same_context = SameContextGadget::construct(cb, opcode, step_state_transition);
130129

130+
// Fetch the bytecode length from the bytecode table.
131+
let code_len_gadget = BytecodeLengthGadget::construct(cb, cb.curr.state.code_hash.clone());
132+
cb.require_equal(
133+
"code_size == code_len_gadget::code_length",
134+
code_size.expr(),
135+
code_len_gadget.code_length.expr(),
136+
);
137+
131138
Self {
132139
same_context,
133140
code_offset,
@@ -136,6 +143,7 @@ impl<F: Field> ExecutionGadget<F> for CodeCopyGadget<F> {
136143
memory_expansion,
137144
memory_copier_gas,
138145
copy_rwc_inc,
146+
code_len_gadget,
139147
}
140148
}
141149

@@ -160,12 +168,10 @@ impl<F: Field> ExecutionGadget<F> for CodeCopyGadget<F> {
160168
let [dest_offset, code_offset, size] =
161169
[0, 1, 2].map(|i| block.rws[step.rw_indices[i]].stack_value());
162170

163-
let bytecode = block
164-
.bytecodes
165-
.get(&call.code_hash)
166-
.expect("could not find current environment's bytecode");
171+
let code_size = self
172+
.code_len_gadget
173+
.assign(region, offset, block, &call.code_hash)?;
167174

168-
let code_size = bytecode.bytes.len() as u64;
169175
self.code_size
170176
.assign(region, offset, Value::known(F::from(code_size)))?;
171177

‎zkevm-circuits/src/evm_circuit/execution/codesize.rs

+12-13
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
evm_circuit::{
88
step::ExecutionState,
99
util::{
10-
common_gadget::SameContextGadget,
10+
common_gadget::{BytecodeLengthGadget, SameContextGadget},
1111
constraint_builder::{
1212
ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition,
1313
},
@@ -24,7 +24,7 @@ use super::ExecutionGadget;
2424
pub(crate) struct CodesizeGadget<F> {
2525
same_context: SameContextGadget<F>,
2626
codesize_bytes: [Cell<F>; 8],
27-
codesize: Cell<F>,
27+
code_len_gadget: BytecodeLengthGadget<F>,
2828
}
2929

3030
impl<F: Field> ExecutionGadget<F> for CodesizeGadget<F> {
@@ -38,14 +38,6 @@ impl<F: Field> ExecutionGadget<F> for CodesizeGadget<F> {
3838
let codesize_bytes = array_init(|_| cb.query_byte());
3939

4040
let code_hash = cb.curr.state.code_hash.clone();
41-
let codesize = cb.query_cell();
42-
cb.bytecode_length(code_hash.expr(), codesize.expr());
43-
44-
cb.require_equal(
45-
"Constraint: bytecode length lookup == codesize",
46-
from_bytes::expr(&codesize_bytes),
47-
codesize.expr(),
48-
);
4941

5042
cb.stack_push(cb.word_rlc(codesize_bytes.clone().map(|c| c.expr())));
5143

@@ -57,11 +49,18 @@ impl<F: Field> ExecutionGadget<F> for CodesizeGadget<F> {
5749
..Default::default()
5850
};
5951
let same_context = SameContextGadget::construct(cb, opcode, step_state_transition);
52+
let code_len_gadget = BytecodeLengthGadget::construct(cb, code_hash);
53+
54+
cb.require_equal(
55+
"Constraint: bytecode length lookup == codesize",
56+
from_bytes::expr(&codesize_bytes),
57+
code_len_gadget.code_length.expr(),
58+
);
6059

6160
Self {
6261
same_context,
6362
codesize_bytes,
64-
codesize,
63+
code_len_gadget,
6564
}
6665
}
6766

@@ -87,8 +86,8 @@ impl<F: Field> ExecutionGadget<F> for CodesizeGadget<F> {
8786
c.assign(region, offset, Value::known(F::from(*b as u64)))?;
8887
}
8988

90-
self.codesize
91-
.assign(region, offset, Value::known(F::from(codesize)))?;
89+
self.code_len_gadget
90+
.assign(region, offset, block, &call.code_hash)?;
9291

9392
Ok(())
9493
}

‎zkevm-circuits/src/evm_circuit/execution/create.rs

+7-8
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
},
88
step::ExecutionState,
99
util::{
10-
common_gadget::{get_copy_bytes, TransferGadget},
10+
common_gadget::{get_copy_bytes, BytecodeLookupGadget, TransferGadget},
1111
constraint_builder::{
1212
ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition,
1313
Transition::{Delta, To},
@@ -41,7 +41,7 @@ use std::iter::once;
4141
/// Gadget for CREATE and CREATE2 opcodes
4242
#[derive(Clone, Debug)]
4343
pub(crate) struct CreateGadget<F, const IS_CREATE2: bool, const S: ExecutionState> {
44-
opcode: Cell<F>,
44+
opcode_gadget: BytecodeLookupGadget<F>,
4545
tx_id: Cell<F>,
4646
reversion_info: ReversionInfo<F>,
4747
depth: Cell<F>,
@@ -89,13 +89,12 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
8989
const EXECUTION_STATE: ExecutionState = S;
9090

9191
fn configure(cb: &mut EVMConstraintBuilder<F>) -> Self {
92-
let opcode = cb.query_cell();
9392
let copy_rw_increase = cb.query_cell();
93+
let opcode_gadget = BytecodeLookupGadget::construct(cb);
9494

95-
cb.opcode_lookup(opcode.expr(), 1.expr());
9695
cb.require_equal(
9796
"Opcode is CREATE or CREATE2",
98-
opcode.expr(),
97+
opcode_gadget.opcode.expr(),
9998
if IS_CREATE2 {
10099
OpcodeId::CREATE2
101100
} else {
@@ -535,7 +534,7 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
535534
);
536535

537536
Self {
538-
opcode,
537+
opcode_gadget,
539538
tx_id,
540539
reversion_info,
541540
depth,
@@ -578,9 +577,9 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
578577
) -> Result<(), Error> {
579578
let opcode = step.opcode.unwrap();
580579
let is_create2 = opcode == OpcodeId::CREATE2;
581-
self.opcode
582-
.assign(region, offset, Value::known(F::from(opcode.as_u64())))?;
583580

581+
self.opcode_gadget
582+
.assign(region, offset, block, call, step)?;
584583
self.tx_id
585584
.assign(region, offset, Value::known(tx.id.to_scalar().unwrap()))?;
586585
self.depth.assign(

‎zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs

+9-11
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
param::N_BYTES_PROGRAM_COUNTER,
55
step::ExecutionState,
66
util::{
7-
common_gadget::{CommonErrorGadget, WordByteCapGadget},
7+
common_gadget::{BytecodeLengthGadget, CommonErrorGadget, WordByteCapGadget},
88
constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder},
99
math_gadget::{IsEqualGadget, IsZeroGadget},
1010
CachedRegion, Cell,
@@ -21,7 +21,6 @@ use halo2_proofs::{circuit::Value, plonk::Error};
2121
pub(crate) struct ErrorInvalidJumpGadget<F> {
2222
opcode: Cell<F>,
2323
dest: WordByteCapGadget<F, N_BYTES_PROGRAM_COUNTER>,
24-
code_len: Cell<F>,
2524
value: Cell<F>,
2625
is_code: Cell<F>,
2726
push_rlc: Cell<F>,
@@ -30,6 +29,7 @@ pub(crate) struct ErrorInvalidJumpGadget<F> {
3029
phase2_condition: Cell<F>,
3130
is_condition_zero: IsZeroGadget<F>,
3231
common_error_gadget: CommonErrorGadget<F>,
32+
code_len_gadget: BytecodeLengthGadget<F>,
3333
}
3434

3535
impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
@@ -38,8 +38,9 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
3838
const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorInvalidJump;
3939

4040
fn configure(cb: &mut EVMConstraintBuilder<F>) -> Self {
41-
let code_len = cb.query_cell();
42-
let dest = WordByteCapGadget::construct(cb, code_len.expr());
41+
// Look up bytecode length
42+
let code_len_gadget = BytecodeLengthGadget::construct(cb, cb.curr.state.code_hash.clone());
43+
let dest = WordByteCapGadget::construct(cb, code_len_gadget.code_length.expr());
4344

4445
let opcode = cb.query_cell();
4546
let value = cb.query_cell();
@@ -71,9 +72,6 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
7172
cb.require_zero("condition is not zero", is_condition_zero.expr());
7273
});
7374

74-
// Look up bytecode length
75-
cb.bytecode_length(cb.curr.state.code_hash.expr(), code_len.expr());
76-
7775
// If destination is in valid range, lookup for the value.
7876
cb.condition(dest.lt_cap(), |cb| {
7977
cb.bytecode_lookup(
@@ -95,7 +93,6 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
9593
Self {
9694
opcode,
9795
dest,
98-
code_len,
9996
value,
10097
is_code,
10198
push_rlc,
@@ -104,6 +101,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
104101
phase2_condition,
105102
is_condition_zero,
106103
common_error_gadget,
104+
code_len_gadget,
107105
}
108106
}
109107

@@ -132,9 +130,9 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
132130
.bytecodes
133131
.get(&call.code_hash)
134132
.expect("could not find current environment's bytecode");
135-
let code_len = code.bytes.len() as u64;
136-
self.code_len
137-
.assign(region, offset, Value::known(F::from(code_len)))?;
133+
let code_len = self
134+
.code_len_gadget
135+
.assign(region, offset, block, &call.code_hash)?;
138136

139137
let dest = block.rws[step.rw_indices[0]].stack_value();
140138
self.dest.assign(region, offset, dest, F::from(code_len))?;

0 commit comments

Comments
 (0)