From 2fc02c29b81f1eba42c49e7ecb016a293186a866 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Thu, 15 Aug 2024 10:42:50 +0800 Subject: [PATCH 01/13] add BytecodeLengthGadget BytecodeLookupGadget --- .../src/circuit_input_builder/execution.rs | 18 +++++ .../src/evm_circuit/execution/callop.rs | 34 +++++---- .../src/evm_circuit/execution/codecopy.rs | 19 ++++- .../src/evm_circuit/execution/codesize.rs | 25 +++---- .../src/evm_circuit/util/common_gadget.rs | 73 +++++++++++++++++++ 5 files changed, 139 insertions(+), 30 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index 4e022339e8..d5404c6f1e 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -368,6 +368,24 @@ pub enum NumberOrHash { Hash(H256), } +impl NumberOrHash { + /// get hash value for NumberOrHash::Hash type + pub fn get_hash(&self) -> H256 { + match self { + Self::Hash(val) => *val, + _ => panic!("not a hash type"), + } + } + + /// get number value for NumberOrHash::Number type + pub fn get_number(&self) -> usize { + match self { + Self::Number(val) => *val, + _ => panic!("not a number type"), + } + } +} + /// Represents all bytes related in one copy event. /// /// - When the source is memory, `bytes` is the memory content, including masked areas. The diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs index fb19aa291b..17e78198e9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/callop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/callop.rs @@ -5,7 +5,9 @@ use crate::{ step::ExecutionState, util::{ and, - common_gadget::{CommonCallGadget, TransferGadget, TransferGadgetInfo}, + common_gadget::{ + BytecodeLookupGadget, CommonCallGadget, TransferGadget, TransferGadgetInfo, + }, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::{Delta, To}, @@ -42,7 +44,7 @@ use std::cmp::min; #[derive(Clone, Debug)] pub(crate) struct CallOpGadget { - opcode: Cell, + opcode_gadget: BytecodeLookupGadget, is_call: IsZeroGadget, is_callcode: IsZeroGadget, is_delegatecall: IsZeroGadget, @@ -91,14 +93,20 @@ impl ExecutionGadget for CallOpGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::CALL_OP; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let opcode = cb.query_cell(); - cb.opcode_lookup(opcode.expr(), 1.expr()); - let is_call = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALL.expr()); - let is_callcode = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALLCODE.expr()); - let is_delegatecall = - IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::DELEGATECALL.expr()); - let is_staticcall = - IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::STATICCALL.expr()); + let opcode_gadget = BytecodeLookupGadget::construct(cb); + + let is_call = + IsZeroGadget::construct(cb, opcode_gadget.opcode.expr() - OpcodeId::CALL.expr()); + let is_callcode = + IsZeroGadget::construct(cb, opcode_gadget.opcode.expr() - OpcodeId::CALLCODE.expr()); + let is_delegatecall = IsZeroGadget::construct( + cb, + opcode_gadget.opcode.expr() - OpcodeId::DELEGATECALL.expr(), + ); + let is_staticcall = IsZeroGadget::construct( + cb, + opcode_gadget.opcode.expr() - OpcodeId::STATICCALL.expr(), + ); // Use rw_counter of the step which triggers next call as its call_id. let callee_call_id = cb.curr.state.rw_counter.clone(); @@ -708,7 +716,7 @@ impl ExecutionGadget for CallOpGadget { ); Self { - opcode, + opcode_gadget, is_call, is_callcode, is_delegatecall, @@ -841,8 +849,8 @@ impl ExecutionGadget for CallOpGadget { } } - self.opcode - .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + self.opcode_gadget + .assign(region, offset, block, call, step)?; self.is_call.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index bc8ecef115..2a4812f87b 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -9,7 +9,7 @@ use crate::{ param::{N_BYTES_MEMORY_WORD_SIZE, N_BYTES_U64}, step::ExecutionState, util::{ - common_gadget::{SameContextGadget, WordByteCapGadget}, + common_gadget::{BytecodeLengthGadget, SameContextGadget, WordByteCapGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition, }, @@ -46,6 +46,8 @@ pub(crate) struct CodeCopyGadget { /// RW inverse counter from the copy table at the start of related copy /// steps. copy_rwc_inc: Cell, + /// Wraps the bytecode length and lookup. + code_len_gadget: BytecodeLengthGadget, } impl ExecutionGadget for CodeCopyGadget { @@ -73,9 +75,6 @@ impl ExecutionGadget for CodeCopyGadget { // Fetch the hash of bytecode running in current environment. let code_hash = cb.curr.state.code_hash.clone(); - // Fetch the bytecode length from the bytecode table. - cb.bytecode_length(code_hash.expr(), code_size.expr()); - // Calculate the next memory size and the gas cost for this memory // access. This also accounts for the dynamic gas required to copy bytes to // memory. @@ -128,6 +127,14 @@ impl ExecutionGadget for CodeCopyGadget { }; let same_context = SameContextGadget::construct(cb, opcode, step_state_transition); + // Fetch the bytecode length from the bytecode table. + let code_len_gadget = BytecodeLengthGadget::construct(cb, cb.curr.state.code_hash.clone()); + cb.require_equal( + "code_size == code_len_gadget::code_length", + code_size.expr(), + code_len_gadget.code_length.expr(), + ); + Self { same_context, code_offset, @@ -136,6 +143,7 @@ impl ExecutionGadget for CodeCopyGadget { memory_expansion, memory_copier_gas, copy_rwc_inc, + code_len_gadget, } } @@ -197,6 +205,9 @@ impl ExecutionGadget for CodeCopyGadget { ), )?; + self.code_len_gadget + .assign(region, offset, block, &call.code_hash)?; + Ok(()) } } diff --git a/zkevm-circuits/src/evm_circuit/execution/codesize.rs b/zkevm-circuits/src/evm_circuit/execution/codesize.rs index 34e00fedfd..09428e88af 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codesize.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codesize.rs @@ -7,7 +7,7 @@ use crate::{ evm_circuit::{ step::ExecutionState, util::{ - common_gadget::SameContextGadget, + common_gadget::{BytecodeLengthGadget, SameContextGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition, }, @@ -24,7 +24,7 @@ use super::ExecutionGadget; pub(crate) struct CodesizeGadget { same_context: SameContextGadget, codesize_bytes: [Cell; 8], - codesize: Cell, + code_len_gadget: BytecodeLengthGadget, } impl ExecutionGadget for CodesizeGadget { @@ -38,14 +38,6 @@ impl ExecutionGadget for CodesizeGadget { let codesize_bytes = array_init(|_| cb.query_byte()); let code_hash = cb.curr.state.code_hash.clone(); - let codesize = cb.query_cell(); - cb.bytecode_length(code_hash.expr(), codesize.expr()); - - cb.require_equal( - "Constraint: bytecode length lookup == codesize", - from_bytes::expr(&codesize_bytes), - codesize.expr(), - ); cb.stack_push(cb.word_rlc(codesize_bytes.clone().map(|c| c.expr()))); @@ -57,11 +49,18 @@ impl ExecutionGadget for CodesizeGadget { ..Default::default() }; let same_context = SameContextGadget::construct(cb, opcode, step_state_transition); + let code_len_gadget = BytecodeLengthGadget::construct(cb, code_hash); + + cb.require_equal( + "Constraint: bytecode length lookup == codesize", + from_bytes::expr(&codesize_bytes), + code_len_gadget.code_length.expr(), + ); Self { same_context, codesize_bytes, - codesize, + code_len_gadget, } } @@ -87,8 +86,8 @@ impl ExecutionGadget for CodesizeGadget { c.assign(region, offset, Value::known(F::from(*b as u64)))?; } - self.codesize - .assign(region, offset, Value::known(F::from(codesize)))?; + self.code_len_gadget + .assign(region, offset, block, &call.code_hash)?; Ok(()) } diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs index e38fd69053..4af0e0215d 100644 --- a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs @@ -112,6 +112,79 @@ impl SameContextGadget { } } +// this helper does bytecode length lookup. +#[derive(Clone, Debug)] +pub(crate) struct BytecodeLengthGadget { + pub(crate) code_length: Cell, + // more fields here later +} + +impl BytecodeLengthGadget { + pub(crate) fn construct(cb: &mut EVMConstraintBuilder, code_hash: Cell) -> Self { + // Fetch the bytecode length from the bytecode table. + let code_length = cb.query_cell(); + cb.bytecode_length(code_hash.expr(), code_length.expr()); + + Self { code_length } + } + + pub(crate) fn assign( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + block: &Block, + code_hash: &U256, + ) -> Result<(), Error> { + let code_length = if code_hash.is_zero() { + 0 + } else { + block + .bytecodes + .get(code_hash) + .expect("could not find bytecode") + .bytes + .len() as u64 + }; + + self.code_length + .assign(region, offset, Value::known(F::from(code_length)))?; + + Ok(()) + } +} + +// this helper does opcode lookup for some opcodes(callop, create, return_revert etc.), +// which don't use SameContextGadget. +#[derive(Clone, Debug)] +pub(crate) struct BytecodeLookupGadget { + pub(crate) opcode: Cell, + // more fields here later +} + +impl BytecodeLookupGadget { + pub(crate) fn construct(cb: &mut EVMConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + cb.opcode_lookup(opcode.expr(), 1.expr()); + + Self { opcode } + } + + pub(crate) fn assign( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + block: &Block, + call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + let opcode = step.opcode.unwrap(); + self.opcode + .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + + Ok(()) + } +} + /// Construction of step state transition that restores caller's state. #[derive(Clone, Debug)] pub(crate) struct RestoreContextGadget { From 8c75cd058392562099c2995c948b60e9fad487ad Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Thu, 15 Aug 2024 11:29:08 +0800 Subject: [PATCH 02/13] remove program_counter_offset --- .../src/evm_circuit/util/constraint_builder.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index a009687417..743dbd275f 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -303,7 +303,6 @@ pub(crate) struct EVMConstraintBuilder<'a, F> { execution_state: ExecutionState, constraints: Constraints, rw_counter_offset: Expression, - program_counter_offset: usize, stack_pointer_offset: Expression, log_id_offset: usize, in_next_step: bool, @@ -404,10 +403,6 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { self.rw_counter_offset.clone() } - pub(crate) fn program_counter_offset(&self) -> usize { - self.program_counter_offset - } - pub(crate) fn stack_pointer_offset(&self) -> Expression { self.stack_pointer_offset.clone() } @@ -667,12 +662,7 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { } pub(crate) fn opcode_lookup_rlc(&mut self, opcode: Expression, push_rlc: Expression) { - self.opcode_lookup_at_rlc( - self.curr.state.program_counter.expr() + self.program_counter_offset.expr(), - opcode, - push_rlc, - ); - self.program_counter_offset += 1; + self.opcode_lookup_at_rlc(self.curr.state.program_counter.expr(), opcode, push_rlc); } pub(crate) fn opcode_lookup_at_rlc( From 39278b343c8e0bdbf91704b6298f24b9b822d4fa Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Thu, 15 Aug 2024 11:46:34 +0800 Subject: [PATCH 03/13] update create/invalid_jump/precompile_fail --- .../src/evm_circuit/execution/create.rs | 16 ++++----- .../execution/error_invalid_jump.rs | 18 +++++----- .../execution/error_precompile_failed.rs | 35 +++++++++++-------- .../evm_circuit/util/constraint_builder.rs | 1 - 4 files changed, 36 insertions(+), 34 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/create.rs b/zkevm-circuits/src/evm_circuit/execution/create.rs index 2d292fb456..0ae4e891d2 100644 --- a/zkevm-circuits/src/evm_circuit/execution/create.rs +++ b/zkevm-circuits/src/evm_circuit/execution/create.rs @@ -7,7 +7,7 @@ use crate::{ }, step::ExecutionState, util::{ - common_gadget::{get_copy_bytes, TransferGadget}, + common_gadget::{get_copy_bytes, BytecodeLookupGadget, TransferGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::{Delta, To}, @@ -41,7 +41,7 @@ use std::iter::once; /// Gadget for CREATE and CREATE2 opcodes #[derive(Clone, Debug)] pub(crate) struct CreateGadget { - opcode: Cell, + opcode_gadget: BytecodeLookupGadget, tx_id: Cell, reversion_info: ReversionInfo, depth: Cell, @@ -89,13 +89,13 @@ impl ExecutionGadget< const EXECUTION_STATE: ExecutionState = S; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let opcode = cb.query_cell(); let copy_rw_increase = cb.query_cell(); + let opcode_gadget = BytecodeLookupGadget::construct(cb); - cb.opcode_lookup(opcode.expr(), 1.expr()); + cb.opcode_lookup(opcode_gadget.opcode.expr(), 1.expr()); cb.require_equal( "Opcode is CREATE or CREATE2", - opcode.expr(), + opcode_gadget.opcode.expr(), if IS_CREATE2 { OpcodeId::CREATE2 } else { @@ -535,7 +535,7 @@ impl ExecutionGadget< ); Self { - opcode, + opcode_gadget, tx_id, reversion_info, depth, @@ -578,9 +578,9 @@ impl ExecutionGadget< ) -> Result<(), Error> { let opcode = step.opcode.unwrap(); let is_create2 = opcode == OpcodeId::CREATE2; - self.opcode - .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + self.opcode_gadget + .assign(region, offset, block, call, step)?; self.tx_id .assign(region, offset, Value::known(tx.id.to_scalar().unwrap()))?; self.depth.assign( diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs index 46fe5d9284..2e5111456c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs @@ -4,7 +4,7 @@ use crate::{ param::N_BYTES_PROGRAM_COUNTER, step::ExecutionState, util::{ - common_gadget::{CommonErrorGadget, WordByteCapGadget}, + common_gadget::{BytecodeLengthGadget, CommonErrorGadget, WordByteCapGadget}, constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, math_gadget::{IsEqualGadget, IsZeroGadget}, CachedRegion, Cell, @@ -21,7 +21,6 @@ use halo2_proofs::{circuit::Value, plonk::Error}; pub(crate) struct ErrorInvalidJumpGadget { opcode: Cell, dest: WordByteCapGadget, - code_len: Cell, value: Cell, is_code: Cell, push_rlc: Cell, @@ -30,6 +29,7 @@ pub(crate) struct ErrorInvalidJumpGadget { phase2_condition: Cell, is_condition_zero: IsZeroGadget, common_error_gadget: CommonErrorGadget, + code_len_gadget: BytecodeLengthGadget, } impl ExecutionGadget for ErrorInvalidJumpGadget { @@ -38,8 +38,9 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorInvalidJump; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let code_len = cb.query_cell(); - let dest = WordByteCapGadget::construct(cb, code_len.expr()); + // Look up bytecode length + let code_len_gadget = BytecodeLengthGadget::construct(cb, cb.curr.state.code_hash.clone()); + let dest = WordByteCapGadget::construct(cb, code_len_gadget.code_length.expr()); let opcode = cb.query_cell(); let value = cb.query_cell(); @@ -71,9 +72,6 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { cb.require_zero("condition is not zero", is_condition_zero.expr()); }); - // Look up bytecode length - cb.bytecode_length(cb.curr.state.code_hash.expr(), code_len.expr()); - // If destination is in valid range, lookup for the value. cb.condition(dest.lt_cap(), |cb| { cb.bytecode_lookup( @@ -95,7 +93,6 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { Self { opcode, dest, - code_len, value, is_code, push_rlc, @@ -104,6 +101,7 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { phase2_condition, is_condition_zero, common_error_gadget, + code_len_gadget, } } @@ -133,8 +131,8 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { .get(&call.code_hash) .expect("could not find current environment's bytecode"); let code_len = code.bytes.len() as u64; - self.code_len - .assign(region, offset, Value::known(F::from(code_len)))?; + self.code_len_gadget + .assign(region, offset, block, &call.code_hash)?; let dest = block.rws[step.rw_indices[0]].stack_value(); self.dest.assign(region, offset, dest, F::from(code_len))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs b/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs index c0cee751bd..85b27ba77e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs @@ -3,6 +3,7 @@ use crate::{ execution::ExecutionGadget, step::ExecutionState, util::{ + common_gadget::BytecodeLookupGadget, constraint_builder::EVMConstraintBuilder, math_gadget::IsZeroGadget, memory_gadget::{CommonMemoryAddressGadget, MemoryAddressGadget}, @@ -19,7 +20,7 @@ use halo2_proofs::{circuit::Value, plonk::Error}; #[derive(Clone, Debug)] pub(crate) struct ErrorPrecompileFailedGadget { - opcode: Cell, + opcode_gadget: BytecodeLookupGadget, is_call: IsZeroGadget, is_callcode: IsZeroGadget, is_delegatecall: IsZeroGadget, @@ -37,15 +38,20 @@ impl ExecutionGadget for ErrorPrecompileFailedGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorPrecompileFailed; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let opcode = cb.query_cell(); - cb.opcode_lookup(opcode.expr(), 1.expr()); - - let is_call = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALL.expr()); - let is_callcode = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALLCODE.expr()); - let is_delegatecall = - IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::DELEGATECALL.expr()); - let is_staticcall = - IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::STATICCALL.expr()); + let opcode_gadget = BytecodeLookupGadget::construct(cb); + + let is_call = + IsZeroGadget::construct(cb, opcode_gadget.opcode.expr() - OpcodeId::CALL.expr()); + let is_callcode = + IsZeroGadget::construct(cb, opcode_gadget.opcode.expr() - OpcodeId::CALLCODE.expr()); + let is_delegatecall = IsZeroGadget::construct( + cb, + opcode_gadget.opcode.expr() - OpcodeId::DELEGATECALL.expr(), + ); + let is_staticcall = IsZeroGadget::construct( + cb, + opcode_gadget.opcode.expr() - OpcodeId::STATICCALL.expr(), + ); // constrain op code // NOTE: this precompile gadget is for dummy use at the moment, the real error handling for @@ -97,7 +103,7 @@ impl ExecutionGadget for ErrorPrecompileFailedGadget { let rd_address = MemoryAddressGadget::construct(cb, rd_offset, rd_length); Self { - opcode, + opcode_gadget, is_call, is_callcode, is_delegatecall, @@ -116,7 +122,7 @@ impl ExecutionGadget for ErrorPrecompileFailedGadget { offset: usize, block: &Block, _tx: &Transaction, - _call: &Call, + call: &Call, step: &ExecStep, ) -> Result<(), Error> { let opcode = step.opcode.unwrap(); @@ -137,9 +143,8 @@ impl ExecutionGadget for ErrorPrecompileFailedGadget { step.rw_indices[is_call_or_callcode + 5], ] .map(|idx| block.rws[idx].stack_value()); - - self.opcode - .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + self.opcode_gadget + .assign(region, offset, block, call, step)?; self.is_call.assign( region, offset, diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 743dbd275f..caa58353d8 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -350,7 +350,6 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { not_step_last: Vec::new(), }, rw_counter_offset: 0.expr(), - program_counter_offset: 0, stack_pointer_offset: 0.expr(), log_id_offset: 0, in_next_step: false, From 8bd3b17907bbea68e67edb89714d489a43ea2b69 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Thu, 15 Aug 2024 15:37:57 +0800 Subject: [PATCH 04/13] update return_revert/stop/extcodecopy --- .../src/evm_circuit/execution/extcodecopy.rs | 37 +++++++++------- .../evm_circuit/execution/return_revert.rs | 17 +++---- .../src/evm_circuit/execution/stop.rs | 44 +++++++++---------- 3 files changed, 52 insertions(+), 46 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index f7b93246b9..dc1c6027a5 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -3,7 +3,7 @@ use crate::{ param::{N_BYTES_ACCOUNT_ADDRESS, N_BYTES_MEMORY_WORD_SIZE, N_BYTES_U64}, step::ExecutionState, util::{ - common_gadget::{SameContextGadget, WordByteCapGadget}, + common_gadget::{BytecodeLengthGadget, SameContextGadget, WordByteCapGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition, @@ -40,7 +40,7 @@ pub(crate) struct ExtcodecopyGadget { is_warm: Cell, code_hash: Cell, not_exists: IsZeroGadget, - code_size: Cell, + code_len_gadget: BytecodeLengthGadget, copy_rwc_inc: Cell, memory_expansion: MemoryExpansionGadget, memory_copier_gas: MemoryCopierGasGadget, @@ -58,12 +58,17 @@ impl ExecutionGadget for ExtcodecopyGadget { let external_address = from_bytes::expr(&external_address_word.cells[..N_BYTES_ACCOUNT_ADDRESS]); - let code_size = cb.query_cell(); - let memory_length = cb.query_word_rlc(); let memory_offset = cb.query_cell_phase2(); - let code_offset = WordByteCapGadget::construct(cb, code_size.expr()); + let code_hash = cb.query_cell_phase2(); + let not_exists = IsZeroGadget::construct(cb, code_hash.clone().expr()); + let exists = not::expr(not_exists.expr()); + let code_len_gadget = cb.condition(exists.expr(), |cb| { + BytecodeLengthGadget::construct(cb, code_hash.clone()) + }); + + let code_offset = WordByteCapGadget::construct(cb, code_len_gadget.code_length.expr()); cb.stack_pop(external_address_word.expr()); cb.stack_pop(memory_offset.expr()); cb.stack_pop(code_offset.original_word()); @@ -86,13 +91,12 @@ impl ExecutionGadget for ExtcodecopyGadget { AccountFieldTag::CodeHash, code_hash.expr(), ); - let not_exists = IsZeroGadget::construct(cb, code_hash.expr()); - let exists = not::expr(not_exists.expr()); - cb.condition(exists.expr(), |cb| { - cb.bytecode_length(code_hash.expr(), code_size.expr()); - }); + cb.condition(not_exists.expr(), |cb| { - cb.require_zero("code_size is zero when non_exists", code_size.expr()); + cb.require_zero( + "code_size is zero when non_exists", + code_len_gadget.code_length.expr(), + ); }); let memory_address = MemoryAddressGadget::construct(cb, memory_offset, memory_length); @@ -115,7 +119,7 @@ impl ExecutionGadget for ExtcodecopyGadget { let src_addr = select::expr( code_offset.lt_cap(), code_offset.valid_value(), - code_size.expr(), + code_len_gadget.code_length.expr(), ); cb.copy_table_lookup( @@ -124,7 +128,7 @@ impl ExecutionGadget for ExtcodecopyGadget { cb.curr.state.call_id.expr(), CopyDataType::Memory.expr(), src_addr, - code_size.expr(), + code_len_gadget.code_length.expr(), memory_address.offset(), memory_address.length(), 0.expr(), @@ -159,7 +163,7 @@ impl ExecutionGadget for ExtcodecopyGadget { is_warm, code_hash, not_exists, - code_size, + code_len_gadget, copy_rwc_inc, memory_expansion, memory_copier_gas, @@ -215,8 +219,9 @@ impl ExecutionGadget for ExtcodecopyGadget { .bytes .len() as u64 }; - self.code_size - .assign(region, offset, Value::known(F::from(code_size)))?; + + self.code_len_gadget + .assign(region, offset, block, &code_hash)?; self.code_offset .assign(region, offset, code_offset, F::from(code_size))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs index 3fe6f3782d..0b6a815fe7 100644 --- a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs +++ b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs @@ -4,7 +4,7 @@ use crate::{ param::{N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE, STACK_CAPACITY}, step::ExecutionState, util::{ - common_gadget::{get_copy_bytes, RestoreContextGadget}, + common_gadget::{get_copy_bytes, BytecodeLookupGadget, RestoreContextGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition, Transition::{Delta, To}, @@ -31,7 +31,7 @@ use halo2_proofs::{circuit::Value, plonk::Error}; #[derive(Clone, Debug)] pub(crate) struct ReturnRevertGadget { - opcode: Cell, + opcode_gadget: BytecodeLookupGadget, // check if it is REVERT opcode is_revert: IsEqualGadget, @@ -66,15 +66,15 @@ impl ExecutionGadget for ReturnRevertGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::RETURN_REVERT; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let opcode = cb.query_cell(); + let opcode_gadget = BytecodeLookupGadget::construct(cb); - cb.opcode_lookup(opcode.expr(), 1.expr()); - let is_revert = IsEqualGadget::construct(cb, opcode.expr(), OpcodeId::REVERT.expr()); + let is_revert = + IsEqualGadget::construct(cb, opcode_gadget.opcode.expr(), OpcodeId::REVERT.expr()); // constrain op codes cb.require_in_set( "RETURN_REVERT state is for RETURN or REVERT", - opcode.expr(), + opcode_gadget.opcode.expr(), vec![OpcodeId::RETURN.expr(), OpcodeId::REVERT.expr()], ); @@ -326,7 +326,7 @@ impl ExecutionGadget for ReturnRevertGadget { }); Self { - opcode, + opcode_gadget, is_revert, range, deployed_bytecode_rlc, @@ -359,7 +359,8 @@ impl ExecutionGadget for ReturnRevertGadget { step: &ExecStep, ) -> Result<(), Error> { let opcode = F::from(step.opcode.unwrap().as_u64()); - self.opcode.assign(region, offset, Value::known(opcode))?; + self.opcode_gadget + .assign(region, offset, block, call, step)?; self.is_revert .assign(region, offset, opcode, F::from(OpcodeId::REVERT.as_u64()))?; diff --git a/zkevm-circuits/src/evm_circuit/execution/stop.rs b/zkevm-circuits/src/evm_circuit/execution/stop.rs index 3c4b7c3490..2b64b058d9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/stop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/stop.rs @@ -4,7 +4,7 @@ use crate::{ param::N_BYTES_PROGRAM_COUNTER, step::ExecutionState, util::{ - common_gadget::RestoreContextGadget, + common_gadget::{BytecodeLengthGadget, BytecodeLookupGadget, RestoreContextGadget}, constraint_builder::{ ConstrainBuilderCommon, EVMConstraintBuilder, StepStateTransition, Transition::{Delta, Same, To}, @@ -22,10 +22,11 @@ use halo2_proofs::{circuit::Value, plonk::Error}; #[derive(Clone, Debug)] pub(crate) struct StopGadget { - code_length: Cell, is_within_range: LtGadget, - opcode: Cell, + opcode_gadget: BytecodeLookupGadget, restore_context: RestoreContextGadget, + /// Wraps the bytecode length and lookup. + code_len_gadget: BytecodeLengthGadget, } impl ExecutionGadget for StopGadget { @@ -34,20 +35,22 @@ impl ExecutionGadget for StopGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::STOP; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - let code_length = cb.query_cell(); - cb.bytecode_length(cb.curr.state.code_hash.expr(), code_length.expr()); - let is_within_range = - LtGadget::construct(cb, cb.curr.state.program_counter.expr(), code_length.expr()); - let opcode = cb.query_cell(); - cb.condition(is_within_range.expr(), |cb| { - cb.opcode_lookup(opcode.expr(), 1.expr()); - }); + let code_len_gadget = BytecodeLengthGadget::construct(cb, cb.curr.state.code_hash.clone()); + + let is_within_range = LtGadget::construct( + cb, + cb.curr.state.program_counter.expr(), + code_len_gadget.code_length.expr(), + ); + let opcode_gadget = cb.condition(is_within_range.expr(), |cb| { + BytecodeLookupGadget::construct(cb) + }); // We do the responsible opcode check explicitly here because we're not using // the `SameContextGadget` for `STOP`. cb.require_equal( "Opcode should be STOP", - opcode.expr(), + opcode_gadget.opcode.expr(), OpcodeId::STOP.expr(), ); @@ -86,10 +89,10 @@ impl ExecutionGadget for StopGadget { }); Self { - code_length, is_within_range, - opcode, + opcode_gadget, restore_context, + code_len_gadget, } } @@ -106,11 +109,9 @@ impl ExecutionGadget for StopGadget { .bytecodes .get(&call.code_hash) .expect("could not find current environment's bytecode"); - self.code_length.assign( - region, - offset, - Value::known(F::from(code.bytes.len() as u64)), - )?; + + self.code_len_gadget + .assign(region, offset, block, &call.code_hash)?; self.is_within_range.assign( region, @@ -119,9 +120,8 @@ impl ExecutionGadget for StopGadget { F::from(code.bytes.len() as u64), )?; - let opcode = step.opcode.unwrap(); - self.opcode - .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + self.opcode_gadget + .assign(region, offset, block, call, step)?; if !call.is_root { self.restore_context From 53bc22c923bc251b0969208befedbef103993865 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Thu, 15 Aug 2024 15:57:13 +0800 Subject: [PATCH 05/13] remove lookup --- zkevm-circuits/src/evm_circuit/execution/create.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/create.rs b/zkevm-circuits/src/evm_circuit/execution/create.rs index 0ae4e891d2..62332f4d65 100644 --- a/zkevm-circuits/src/evm_circuit/execution/create.rs +++ b/zkevm-circuits/src/evm_circuit/execution/create.rs @@ -92,7 +92,6 @@ impl ExecutionGadget< let copy_rw_increase = cb.query_cell(); let opcode_gadget = BytecodeLookupGadget::construct(cb); - cb.opcode_lookup(opcode_gadget.opcode.expr(), 1.expr()); cb.require_equal( "Opcode is CREATE or CREATE2", opcode_gadget.opcode.expr(), From a2eff9545195d455e663871fdb5179d980f3966b Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Thu, 15 Aug 2024 16:26:14 +0800 Subject: [PATCH 06/13] add debug helper --- zkevm-circuits/src/evm_circuit/util/constraint_builder.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index caa58353d8..a229d364a4 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -402,6 +402,12 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { self.rw_counter_offset.clone() } + // Warn: this is a debug helper,only used in debug purpose: if we want to diagnose which `copy_table_lookup` failed, can comment that + // `copy_table_lookup` codes, and call this method to increase rw_counter correctly, thus make other constraints succeed. + pub(crate) fn add_counter_offset(&mut self, offset: Expression) { + self.rw_counter_offset = self.rw_counter_offset.clone() + self.condition_expr() * offset; + } + pub(crate) fn stack_pointer_offset(&self) -> Expression { self.stack_pointer_offset.clone() } From d1bbfa544d091bcde536cc67f2d29e554830af0c Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Thu, 15 Aug 2024 17:31:21 +0800 Subject: [PATCH 07/13] return code length in BytecodeLengthGadget::assign --- .../src/evm_circuit/execution/codecopy.rs | 11 +++-------- .../evm_circuit/execution/error_invalid_jump.rs | 4 ++-- .../src/evm_circuit/execution/extcodecopy.rs | 14 ++------------ zkevm-circuits/src/evm_circuit/execution/stop.rs | 10 +++------- .../src/evm_circuit/util/common_gadget.rs | 4 ++-- 5 files changed, 12 insertions(+), 31 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index 2a4812f87b..58879cb04c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -168,12 +168,10 @@ impl ExecutionGadget for CodeCopyGadget { let [dest_offset, code_offset, size] = [0, 1, 2].map(|i| block.rws[step.rw_indices[i]].stack_value()); - let bytecode = block - .bytecodes - .get(&call.code_hash) - .expect("could not find current environment's bytecode"); + let code_size = self + .code_len_gadget + .assign(region, offset, block, &call.code_hash)?; - let code_size = bytecode.bytes.len() as u64; self.code_size .assign(region, offset, Value::known(F::from(code_size)))?; @@ -205,9 +203,6 @@ impl ExecutionGadget for CodeCopyGadget { ), )?; - self.code_len_gadget - .assign(region, offset, block, &call.code_hash)?; - Ok(()) } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs index 2e5111456c..4b82a82bfe 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs @@ -130,8 +130,8 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { .bytecodes .get(&call.code_hash) .expect("could not find current environment's bytecode"); - let code_len = code.bytes.len() as u64; - self.code_len_gadget + let code_len = self + .code_len_gadget .assign(region, offset, block, &call.code_hash)?; let dest = block.rws[step.rw_indices[0]].stack_value(); diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index dc1c6027a5..5ac46de974 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -209,18 +209,8 @@ impl ExecutionGadget for ExtcodecopyGadget { self.not_exists .assign_value(region, offset, region.code_hash(code_hash))?; - let code_size = if code_hash.is_zero() { - 0 - } else { - block - .bytecodes - .get(&code_hash) - .expect("could not find external bytecode") - .bytes - .len() as u64 - }; - - self.code_len_gadget + let code_size = self + .code_len_gadget .assign(region, offset, block, &code_hash)?; self.code_offset diff --git a/zkevm-circuits/src/evm_circuit/execution/stop.rs b/zkevm-circuits/src/evm_circuit/execution/stop.rs index 2b64b058d9..34c9fe4de9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/stop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/stop.rs @@ -105,19 +105,15 @@ impl ExecutionGadget for StopGadget { call: &Call, step: &ExecStep, ) -> Result<(), Error> { - let code = block - .bytecodes - .get(&call.code_hash) - .expect("could not find current environment's bytecode"); - - self.code_len_gadget + let code_len = self + .code_len_gadget .assign(region, offset, block, &call.code_hash)?; self.is_within_range.assign( region, offset, F::from(step.program_counter), - F::from(code.bytes.len() as u64), + F::from(code_len), )?; self.opcode_gadget diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs index 4af0e0215d..e28d4a2f87 100644 --- a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs @@ -134,7 +134,7 @@ impl BytecodeLengthGadget { offset: usize, block: &Block, code_hash: &U256, - ) -> Result<(), Error> { + ) -> Result { let code_length = if code_hash.is_zero() { 0 } else { @@ -149,7 +149,7 @@ impl BytecodeLengthGadget { self.code_length .assign(region, offset, Value::known(F::from(code_length)))?; - Ok(()) + Ok(code_length) } } From 3bbdda5c61c3818b86c8dcf16eb76da08f722328 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 16 Aug 2024 09:53:20 +0800 Subject: [PATCH 08/13] fix clippy --- .../src/evm_circuit/execution/error_precompile_failed.rs | 4 ++-- zkevm-circuits/src/evm_circuit/execution/stop.rs | 4 ++-- zkevm-circuits/src/evm_circuit/util/common_gadget.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs b/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs index 85b27ba77e..ed6c3fc055 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs @@ -7,7 +7,7 @@ use crate::{ constraint_builder::EVMConstraintBuilder, math_gadget::IsZeroGadget, memory_gadget::{CommonMemoryAddressGadget, MemoryAddressGadget}, - sum, CachedRegion, Cell, Word, + sum, CachedRegion, Word, }, }, table::CallContextFieldTag, @@ -16,7 +16,7 @@ use crate::{ }; use bus_mapping::evm::OpcodeId; use eth_types::{ToLittleEndian, U256}; -use halo2_proofs::{circuit::Value, plonk::Error}; +use halo2_proofs::plonk::Error; #[derive(Clone, Debug)] pub(crate) struct ErrorPrecompileFailedGadget { diff --git a/zkevm-circuits/src/evm_circuit/execution/stop.rs b/zkevm-circuits/src/evm_circuit/execution/stop.rs index 34c9fe4de9..08eaf4bbee 100644 --- a/zkevm-circuits/src/evm_circuit/execution/stop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/stop.rs @@ -10,7 +10,7 @@ use crate::{ Transition::{Delta, Same, To}, }, math_gadget::LtGadget, - CachedRegion, Cell, + CachedRegion, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -18,7 +18,7 @@ use crate::{ util::{Expr, Field}, }; use bus_mapping::evm::OpcodeId; -use halo2_proofs::{circuit::Value, plonk::Error}; +use halo2_proofs::plonk::Error; #[derive(Clone, Debug)] pub(crate) struct StopGadget { diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs index e28d4a2f87..be35a54a9a 100644 --- a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs @@ -173,8 +173,8 @@ impl BytecodeLookupGadget { &self, region: &mut CachedRegion<'_, '_, F>, offset: usize, - block: &Block, - call: &Call, + _block: &Block, + _call: &Call, step: &ExecStep, ) -> Result<(), Error> { let opcode = step.opcode.unwrap(); From 2f7a729b9c030601589828eb313ec2d738e8f4ae Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 16 Aug 2024 11:36:22 +0800 Subject: [PATCH 09/13] remove extra code_hash --- zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index 5ac46de974..b1cb0a2052 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -85,7 +85,6 @@ impl ExecutionGadget for ExtcodecopyGadget { Some(&mut reversion_info), ); - let code_hash = cb.query_cell_phase2(); cb.account_read( external_address.expr(), AccountFieldTag::CodeHash, From 3ee8efb7888f3c2d261315059d8ca4906c7452b8 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 16 Aug 2024 15:07:59 +0800 Subject: [PATCH 10/13] add algorithm --- zkevm-circuits/src/evm_circuit/util.rs | 81 ++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index de1d561319..d20165eff5 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -778,3 +778,84 @@ impl Inverter { } } } + +/// The function of this algorithm:Split a vec into two subsets such that +/// the sums of the two subsets are as close as possible。 +pub(crate) fn find_two_closest_subset(vec: &[usize]) -> (Vec, Vec) { + let total_sum: usize = vec.iter().sum(); + let n = vec.len(); + + // dp[i][j]:indicates whether it is possible to achieve a sum of j using the first i elements. + let mut dp = vec![vec![false; total_sum / 2 + 1]; n + 1]; + + // initialization: first sum zero can be always reached. + // for i in 0..=n { + // dp[i][0] = true; + // } + for dp_inner in dp.iter_mut().take(n + 1) { + dp_inner[0] = true; + } + + // fill dp table + for i in 1..=n { + for j in 1..=(total_sum / 2) { + if j >= vec[i - 1] { + dp[i][j] = dp[i - 1][j] || dp[i - 1][j - vec[i - 1]]; + } else { + dp[i][j] = dp[i - 1][j]; + } + } + } + + // find closest sum + let mut sum1 = 0; + for j in (0..=(total_sum / 2)).rev() { + if dp[n][j] { + sum1 = j; + break; + } + } + + // construct two sub set + let mut subset1 = Vec::new(); + let mut subset2 = Vec::new(); + let mut current_sum = sum1; + for i in (1..=n).rev() { + if current_sum >= vec[i - 1] && dp[i - 1][current_sum - vec[i - 1]] { + subset1.push(vec[i - 1]); + current_sum -= vec[i - 1]; + } else { + subset2.push(vec[i - 1]); + } + } + + (subset1, subset2) +} + +// tests for algorithm of `find_two_closest_subset` +#[test] +fn test_find_two_closest_subset() { + let mut nums = vec![80, 100, 10, 20]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 100, set2's sum: 110, diff = 10 + assert_eq!(set1, [20, 80]); + assert_eq!(set2, [10, 100]); + + nums = vec![80, 20, 50, 110, 32]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 142, set2's sum: 150, diff = 8 + assert_eq!(set1, [32, 110]); + assert_eq!(set2, [50, 20, 80]); + + nums = vec![1, 5, 11, 5, 10]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 16, set2's sum: 16, diff = 0 + assert_eq!(set1, [10, 5, 1]); + assert_eq!(set2, [11, 5]); + + nums = vec![1, 5, 11, 5, 10, 20, 4]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 27, set2's sum: 29, diff = 2 + assert_eq!(set1, [10, 5, 11, 1]); + assert_eq!(set2, [4, 20, 5]); +} From cff5bd6a1ef9350197a322bd2ffd58c432190125 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 16 Aug 2024 15:21:30 +0800 Subject: [PATCH 11/13] more test --- zkevm-circuits/src/evm_circuit/util.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index d20165eff5..4dbc375307 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -858,4 +858,10 @@ fn test_find_two_closest_subset() { // set1's sum: 27, set2's sum: 29, diff = 2 assert_eq!(set1, [10, 5, 11, 1]); assert_eq!(set2, [4, 20, 5]); + + nums = vec![100, 105, 110, 50, 100, 40, 200]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 350, set2's sum: 355, diff = 5 + assert_eq!(set1, [200, 40, 110]); + assert_eq!(set2, [100, 50, 105, 100]); } From 6a48d01fb6c5b5a732b56b2c2ee4d2873f8b5fc3 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 16 Aug 2024 15:43:00 +0800 Subject: [PATCH 12/13] fix clippy from upstream --- aggregator/src/chunk.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aggregator/src/chunk.rs b/aggregator/src/chunk.rs index c5cb6c2422..5734db069b 100644 --- a/aggregator/src/chunk.rs +++ b/aggregator/src/chunk.rs @@ -65,7 +65,7 @@ impl ChunkInfo { b.transactions .iter() .enumerate() - .filter(|(idx, tx)| !tx.is_l1_tx()) + .filter(|(_idx, tx)| !tx.is_l1_tx()) .flat_map(|(idx, tx)| { tx.to_eth_tx( b.header.hash, From e6f0839dd7324d24063c20166c69100b38abaac0 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Fri, 16 Aug 2024 17:37:37 +0800 Subject: [PATCH 13/13] edge test for algorithm --- zkevm-circuits/src/evm_circuit/util.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index 4dbc375307..2eb75a11e9 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -864,4 +864,26 @@ fn test_find_two_closest_subset() { // set1's sum: 350, set2's sum: 355, diff = 5 assert_eq!(set1, [200, 40, 110]); assert_eq!(set2, [100, 50, 105, 100]); + + // only one number in set + nums = vec![100]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 0, set2's sum: 100, diff = 100 + let empty_vec: [usize; 0] = []; + assert_eq!(set1, empty_vec); + assert_eq!(set2, [100]); + + // only two numbers in set + nums = vec![10, 11]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 10, set2's sum: 11, diff = 1 + assert_eq!(set1, [10]); + assert_eq!(set2, [11]); + + // empty set + nums = vec![]; + let (set1, set2) = find_two_closest_subset(&nums); + // set1's sum: 0, set2's sum: 0, diff = 0 + assert_eq!(set1, empty_vec); + assert_eq!(set2, empty_vec); }