Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
640b275
Fix to handle successful run with Uint64 overflow in multiple opcodes.
silathdiir Mar 2, 2023
12669db
Fix lint.
silathdiir Mar 13, 2023
78931f3
Fix failed cases.
silathdiir Mar 13, 2023
a3226b4
Merge branch 'main' into bug/some-successful-cases-with-overflow-inpu…
silathdiir Mar 22, 2023
6fe356e
Merge remote-tracking branch 'upstream/main' into bug/some-successful…
silathdiir Apr 18, 2023
82505e2
Fix failed tests.
silathdiir Apr 18, 2023
c17ba9d
Try to trigger CI for network failure.
silathdiir Apr 18, 2023
ee29e93
Revert "Try to trigger CI for network failure."
silathdiir Apr 18, 2023
c1d18a2
Merge remote-tracking branch 'upstream/main' into bug/some-successful…
silathdiir Apr 24, 2023
138378f
Fix lint.
silathdiir Apr 24, 2023
2092dc2
Merge remote-tracking branch 'upstream/main' into bug/some-successful…
silathdiir Apr 27, 2023
29cf64c
Delete `debug_assert!(VALID_BYTES < 32)` in `assign` and `valid_value…
silathdiir Apr 27, 2023
4419f2b
Merge branch 'main' into bug/some-successful-cases-with-overflow-inpu…
lispc Apr 27, 2023
787093f
Delete redundant constraint `JUMPI condition must be 0 if destination…
silathdiir Apr 27, 2023
11f8eb3
Rename `within_range` to `not_overflow`.
silathdiir Apr 27, 2023
594553d
Replace `checked_add` and `unwrap` with just adding.
silathdiir Apr 27, 2023
74a938b
Update some comments to retry CI.
silathdiir Apr 28, 2023
9d6579a
Merge [u64-overflow PR](https://github.com/privacy-scaling-exploratio…
silathdiir May 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions zkevm-circuits/src/evm_circuit/execution/calldataload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl<F: Field> ExecutionGadget<F> for CallDataLoadGadget<F> {
cb.stack_pop(data_offset.original_word());

cb.condition(
and::expr([data_offset.within_range(), cb.curr.state.is_root.expr()]),
and::expr([data_offset.not_overflow(), cb.curr.state.is_root.expr()]),
|cb| {
cb.call_context_lookup(
false.expr(),
Expand All @@ -92,7 +92,7 @@ impl<F: Field> ExecutionGadget<F> for CallDataLoadGadget<F> {

cb.condition(
and::expr([
data_offset.within_range(),
data_offset.not_overflow(),
not::expr(cb.curr.state.is_root.expr()),
]),
|cb| {
Expand Down Expand Up @@ -134,7 +134,7 @@ impl<F: Field> ExecutionGadget<F> for CallDataLoadGadget<F> {
// For a root call, the call data comes from tx's data field.
cb.condition(
and::expr([
data_offset.within_range(),
data_offset.not_overflow(),
buffer_reader.read_flag(idx),
cb.curr.state.is_root.expr(),
]),
Expand All @@ -150,7 +150,7 @@ impl<F: Field> ExecutionGadget<F> for CallDataLoadGadget<F> {
// For an internal call, the call data comes from memory.
cb.condition(
and::expr([
data_offset.within_range(),
data_offset.not_overflow(),
buffer_reader.read_flag(idx),
not::expr(cb.curr.state.is_root.expr()),
]),
Expand Down Expand Up @@ -227,34 +227,32 @@ impl<F: Field> ExecutionGadget<F> for CallDataLoadGadget<F> {
.assign(region, offset, Value::known(F::from(call_data_offset)))?;

let data_offset = block.rws[step.rw_indices[0]].stack_value();
let offset_within_range =
let offset_not_overflow =
self.data_offset
.assign(region, offset, data_offset, F::from(call_data_length))?;

let data_offset = if offset_within_range {
let data_offset = if offset_not_overflow {
data_offset.as_u64()
} else {
call_data_length
};
let src_addr_end = call_data_offset.checked_add(call_data_length).unwrap();
let src_addr_end = call_data_offset + call_data_length;
let src_addr = call_data_offset
.checked_add(data_offset)
.unwrap_or(src_addr_end)
.min(src_addr_end);

let mut calldata_bytes = vec![0u8; N_BYTES_WORD];
if offset_within_range {
if offset_not_overflow {
for (i, byte) in calldata_bytes.iter_mut().enumerate() {
if call.is_root {
// Fetch from tx call data.
if src_addr.checked_add(i as u64).unwrap() < tx.call_data_length as u64 {
if src_addr + (i as u64) < tx.call_data_length as u64 {
*byte = tx.call_data[src_addr as usize + i];
}
} else {
// Fetch from memory.
if src_addr.checked_add(i as u64).unwrap()
< call.call_data_offset + call.call_data_length
{
if src_addr + (i as u64) < call.call_data_offset + call.call_data_length {
*byte =
block.rws[step.rw_indices[OFFSET_RW_MEMORY_INDICES + i]].memory_value();
}
Expand Down
16 changes: 8 additions & 8 deletions zkevm-circuits/src/evm_circuit/execution/jumpi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use halo2_proofs::plonk::Error;
#[derive(Clone, Debug)]
pub(crate) struct JumpiGadget<F> {
same_context: SameContextGadget<F>,
dest_word: WordByteRangeGadget<F, N_BYTES_PROGRAM_COUNTER>,
dest: WordByteRangeGadget<F, N_BYTES_PROGRAM_COUNTER>,
phase2_condition: Cell<F>,
is_condition_zero: IsZeroGadget<F>,
}
Expand All @@ -33,11 +33,11 @@ impl<F: Field> ExecutionGadget<F> for JumpiGadget<F> {
const EXECUTION_STATE: ExecutionState = ExecutionState::JUMPI;

fn configure(cb: &mut EVMConstraintBuilder<F>) -> Self {
let dest_word = WordByteRangeGadget::construct(cb);
let dest = WordByteRangeGadget::construct(cb);
let phase2_condition = cb.query_cell_phase2();

// Pop the value from the stack
cb.stack_pop(dest_word.original_word());
cb.stack_pop(dest.original_word());
cb.stack_pop(phase2_condition.expr());

// Determine if the jump condition is met
Expand All @@ -48,18 +48,18 @@ impl<F: Field> ExecutionGadget<F> for JumpiGadget<F> {
cb.condition(should_jump.clone(), |cb| {
cb.require_equal(
"JUMPI destination must be within range if condition is non-zero",
dest_word.within_range(),
dest.not_overflow(),
1.expr(),
);

cb.opcode_lookup_at(dest_word.valid_value(), OpcodeId::JUMPDEST.expr(), 1.expr());
cb.opcode_lookup_at(dest.valid_value(), OpcodeId::JUMPDEST.expr(), 1.expr());
});

// Transit program_counter to destination when should_jump, otherwise by
// delta 1.
let next_program_counter = select::expr(
should_jump,
dest_word.valid_value(),
dest.valid_value(),
cb.curr.state.program_counter.expr() + 1.expr(),
);

Expand All @@ -76,7 +76,7 @@ impl<F: Field> ExecutionGadget<F> for JumpiGadget<F> {

Self {
same_context,
dest_word,
dest,
phase2_condition,
is_condition_zero,
}
Expand All @@ -97,7 +97,7 @@ impl<F: Field> ExecutionGadget<F> for JumpiGadget<F> {
[step.rw_indices[0], step.rw_indices[1]].map(|idx| block.rws[idx].stack_value());
let condition = region.word_rlc(condition);

self.dest_word.assign(region, offset, destination)?;
self.dest.assign(region, offset, destination)?;
self.phase2_condition.assign(region, offset, condition)?;
self.is_condition_zero
.assign_value(region, offset, condition)?;
Expand Down
40 changes: 18 additions & 22 deletions zkevm-circuits/src/evm_circuit/util/common_gadget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1105,8 +1105,8 @@ impl<F: Field> CommonErrorGadget<F> {
}
}

/// Check if the passed in word is within the specified byte range and less than
/// a maximum cap.
/// Check if the passed in word is within the specified byte range
/// (not overflow) and less than a maximum cap.
#[derive(Clone, Debug)]
pub(crate) struct WordByteCapGadget<F, const VALID_BYTES: usize> {
word: WordByteRangeGadget<F, VALID_BYTES>,
Expand All @@ -1116,24 +1116,24 @@ pub(crate) struct WordByteCapGadget<F, const VALID_BYTES: usize> {
impl<F: Field, const VALID_BYTES: usize> WordByteCapGadget<F, VALID_BYTES> {
pub(crate) fn construct(cb: &mut EVMConstraintBuilder<F>, cap: Expression<F>) -> Self {
let word = WordByteRangeGadget::construct(cb);
let value = select::expr(word.within_range(), word.valid_value(), cap.expr());
let value = select::expr(word.overflow(), cap.expr(), word.valid_value());
let lt_cap = LtGadget::construct(cb, value, cap);

Self { word, lt_cap }
}

/// Return true if within the specified byte range, false if overflow. No
/// matter whether it is less than the cap.
/// Return true if within the specified byte range (not overflow), false if
/// overflow. No matter whether it is less than the cap.
pub(crate) fn assign(
&self,
region: &mut CachedRegion<'_, '_, F>,
offset: usize,
original: U256,
cap: F,
) -> Result<bool, Error> {
let within_range = self.word.assign(region, offset, original)?;
let not_overflow = self.word.assign(region, offset, original)?;

let value = if within_range {
let value = if not_overflow {
let mut bytes = [0; 32];
bytes[0..VALID_BYTES].copy_from_slice(&original.to_le_bytes()[0..VALID_BYTES]);
F::from_repr(bytes).unwrap()
Expand All @@ -1143,7 +1143,7 @@ impl<F: Field, const VALID_BYTES: usize> WordByteCapGadget<F, VALID_BYTES> {

self.lt_cap.assign(region, offset, value, cap)?;

Ok(within_range)
Ok(not_overflow)
}

pub(crate) fn lt_cap(&self) -> Expression<F> {
Expand All @@ -1162,28 +1162,28 @@ impl<F: Field, const VALID_BYTES: usize> WordByteCapGadget<F, VALID_BYTES> {
self.word.valid_value()
}

pub(crate) fn within_range(&self) -> Expression<F> {
self.word.within_range()
pub(crate) fn not_overflow(&self) -> Expression<F> {
self.word.not_overflow()
}
}

/// Check if the passed in word is within the specified byte range.
/// Check if the passed in word is within the specified byte range (not overflow).
#[derive(Clone, Debug)]
pub(crate) struct WordByteRangeGadget<F, const VALID_BYTES: usize> {
original: Word<F>,
within_range: IsZeroGadget<F>,
not_overflow: IsZeroGadget<F>,
}

impl<F: Field, const VALID_BYTES: usize> WordByteRangeGadget<F, VALID_BYTES> {
pub(crate) fn construct(cb: &mut EVMConstraintBuilder<F>) -> Self {
debug_assert!(VALID_BYTES < 32);

let original = cb.query_word_rlc();
let within_range = IsZeroGadget::construct(cb, sum::expr(&original.cells[VALID_BYTES..]));
let not_overflow = IsZeroGadget::construct(cb, sum::expr(&original.cells[VALID_BYTES..]));

Self {
original,
within_range,
not_overflow,
}
}

Expand All @@ -1194,15 +1194,13 @@ impl<F: Field, const VALID_BYTES: usize> WordByteRangeGadget<F, VALID_BYTES> {
offset: usize,
original: U256,
) -> Result<bool, Error> {
debug_assert!(VALID_BYTES < 32);

self.original
.assign(region, offset, Some(original.to_le_bytes()))?;

let overflow_hi = original.to_le_bytes()[VALID_BYTES..]
.iter()
.fold(0, |acc, val| acc + u64::from(*val));
self.within_range
self.not_overflow
.assign(region, offset, F::from(overflow_hi))?;

Ok(overflow_hi == 0)
Expand All @@ -1213,16 +1211,14 @@ impl<F: Field, const VALID_BYTES: usize> WordByteRangeGadget<F, VALID_BYTES> {
}

pub(crate) fn overflow(&self) -> Expression<F> {
not::expr(self.within_range())
not::expr(self.not_overflow())
}

pub(crate) fn valid_value(&self) -> Expression<F> {
debug_assert!(VALID_BYTES < 32);

from_bytes::expr(&self.original.cells[..VALID_BYTES])
}

pub(crate) fn within_range(&self) -> Expression<F> {
self.within_range.expr()
pub(crate) fn not_overflow(&self) -> Expression<F> {
self.not_overflow.expr()
}
}
2 changes: 1 addition & 1 deletion zkevm-circuits/src/witness/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,12 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState {
OpcodeId::CODECOPY => ExecutionState::CODECOPY,
OpcodeId::CALLDATALOAD => ExecutionState::CALLDATALOAD,
OpcodeId::CODESIZE => ExecutionState::CODESIZE,
OpcodeId::EXTCODECOPY => ExecutionState::EXTCODECOPY,
OpcodeId::RETURN | OpcodeId::REVERT => ExecutionState::RETURN_REVERT,
OpcodeId::RETURNDATASIZE => ExecutionState::RETURNDATASIZE,
OpcodeId::RETURNDATACOPY => ExecutionState::RETURNDATACOPY,
OpcodeId::CREATE => ExecutionState::CREATE,
OpcodeId::CREATE2 => ExecutionState::CREATE2,
OpcodeId::EXTCODECOPY => ExecutionState::EXTCODECOPY,
// dummy ops
OpcodeId::SELFDESTRUCT => dummy!(ExecutionState::SELFDESTRUCT),
_ => unimplemented!("unimplemented opcode {:?}", op),
Expand Down