Skip to content

Commit

Permalink
Fix to handle successful run with Uint64 overflow for multiple opcodes (
Browse files Browse the repository at this point in the history 
privacy-scaling-explorations#1317)

### Description

Fix successful run cases with Uint64 overflow for multiple opcodes.

1. Add `WordByteRangeGadget` to constrain if Word is within the
specified byte range.

2. Add `WordByteCapGadget` to constrain if Word is within the specified
byte range (implemented by WordByteRangeGadget) and less than a maximum
cap (used to replace a WordByteRangeGadget and LtGadget).

3. Fix bus-mapping and zkevm-circuits to handle overflow cases. And add
unit-tests for these cases.

TODO: will try to handle memory offset overflow with zero length in
another PR (try to rebase for this local PR
scroll-tech#393) and related
issue
privacy-scaling-explorations#1301.

### Rationale

Reference detailed code in `go-etherum` as:

.
[BLOCKHASH](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L438)
.
[CALLDATALOAD](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L285)
.
[CALLDATACOPY](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L306)
.
[CODECOPY](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L364)
.
[EXTCODECOPY](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L382)
.
[JUMPI](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L550)

### Issue Link

Close
privacy-scaling-explorations#1276

### Type of change

- [X] Bug fix (non-breaking change which fixes an issue)

### How Has This Been Tested?

Add unit-test cases for Uint64 overflow values.

---------

Co-authored-by: Zhang Zhuo <[email protected]>
  • Loading branch information
@silathdiir @lispc
silathdiir and lispc authored Apr 28, 2023
1 parent ab6a91a commit 22dd263
Show file tree
Hide file tree
Showing 16 changed files with 734 additions and 555 deletions.
61 changes: 60 additions & 1 deletion bus-mapping/src/circuit_input_builder/input_state_ref.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use eth_types::{
evm_types::{
gas_utils::memory_expansion_gas_cost, Gas, GasCost, MemoryAddress, OpcodeId, StackAddress,
},
Address, GethExecStep, ToAddress, ToBigEndian, ToWord, Word, H256, U256,
Address, Bytecode, GethExecStep, ToAddress, ToBigEndian, ToWord, Word, H256, U256,
};
use ethers_core::utils::{get_contract_address, get_create2_address};
use std::cmp::max;
Expand Down Expand Up @@ -1348,4 +1348,63 @@ impl<'a> CircuitInputStateRef<'a> {
}
Ok(())
}

/// Generate copy steps for bytecode.
pub(crate) fn gen_copy_steps_for_bytecode(
&mut self,
exec_step: &mut ExecStep,
bytecode: &Bytecode,
src_addr: u64,
dst_addr: u64,
src_addr_end: u64,
bytes_left: u64,
) -> Result<Vec<(u8, bool)>, Error> {
let mut copy_steps = Vec::with_capacity(bytes_left as usize);
for idx in 0..bytes_left {
let addr = src_addr.checked_add(idx).unwrap_or(src_addr_end);
let step = if addr < src_addr_end {
let code = bytecode.code.get(addr as usize).unwrap();
(code.value, code.is_code)
} else {
(0, false)
};
copy_steps.push(step);
self.memory_write(exec_step, (dst_addr + idx).into(), step.0)?;
}

Ok(copy_steps)
}

/// Generate copy steps for call data.
pub(crate) fn gen_copy_steps_for_call_data(
&mut self,
exec_step: &mut ExecStep,
src_addr: u64,
dst_addr: u64,
src_addr_end: u64,
bytes_left: u64,
) -> Result<Vec<(u8, bool)>, Error> {
let mut copy_steps = Vec::with_capacity(bytes_left as usize);
for idx in 0..bytes_left {
let addr = src_addr.checked_add(idx).unwrap_or(src_addr_end);
let value = if addr < src_addr_end {
let byte =
self.call_ctx()?.call_data[(addr - self.call()?.call_data_offset) as usize];
if !self.call()?.is_root {
self.push_op(
exec_step,
RW::READ,
MemoryOp::new(self.call()?.caller_id, addr.into(), byte),
);
}
byte
} else {
0
};
copy_steps.push((value, false));
self.memory_write(exec_step, (dst_addr + idx).into(), value)?;
}

Ok(copy_steps)
}
}
70 changes: 21 additions & 49 deletions bus-mapping/src/evm/opcodes/calldatacopy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
circuit_input_builder::{
CircuitInputStateRef, CopyDataType, CopyEvent, ExecStep, NumberOrHash,
},
operation::{CallContextField, MemoryOp, RW},
operation::CallContextField,
Error,
};
use eth_types::GethExecStep;
Expand All @@ -20,13 +20,13 @@ impl Opcode for Calldatacopy {
let mut exec_steps = vec![gen_calldatacopy_step(state, geth_step)?];

// reconstruction
let memory_offset = geth_step.stack.nth_last(0)?.as_u64();
let data_offset = geth_step.stack.nth_last(1)?.as_u64();
let length = geth_step.stack.nth_last(2)?.as_usize();
let memory_offset = geth_step.stack.nth_last(0)?;
let data_offset = geth_step.stack.nth_last(1)?;
let length = geth_step.stack.nth_last(2)?;
let call_ctx = state.call_ctx_mut()?;
let memory = &mut call_ctx.memory;

memory.copy_from(memory_offset, &call_ctx.call_data, data_offset, length);
memory.copy_from(memory_offset, data_offset, length, &call_ctx.call_data);

let copy_event = gen_copy_event(state, geth_step)?;
state.push_copy(&mut exec_steps[0], copy_event);
Expand Down Expand Up @@ -92,64 +92,36 @@ fn gen_calldatacopy_step(
Ok(exec_step)
}

fn gen_copy_steps(
state: &mut CircuitInputStateRef,
exec_step: &mut ExecStep,
src_addr: u64,
dst_addr: u64,
src_addr_end: u64,
bytes_left: u64,
is_root: bool,
) -> Result<Vec<(u8, bool)>, Error> {
let mut copy_steps = Vec::with_capacity(bytes_left as usize);
for idx in 0..bytes_left {
let addr = src_addr + idx;
let value = if addr < src_addr_end {
let byte =
state.call_ctx()?.call_data[(addr - state.call()?.call_data_offset) as usize];
if !is_root {
state.push_op(
exec_step,
RW::READ,
MemoryOp::new(state.call()?.caller_id, addr.into(), byte),
);
}
byte
} else {
0
};
copy_steps.push((value, false));
state.memory_write(exec_step, (dst_addr + idx).into(), value)?;
}

Ok(copy_steps)
}

fn gen_copy_event(
state: &mut CircuitInputStateRef,
geth_step: &GethExecStep,
) -> Result<CopyEvent, Error> {
let rw_counter_start = state.block_ctx.rwc;
let memory_offset = geth_step.stack.nth_last(0)?.as_u64();
let data_offset = geth_step.stack.nth_last(1)?.as_u64();

let memory_offset = geth_step.stack.nth_last(0)?;
let data_offset = geth_step.stack.nth_last(1)?;
let length = geth_step.stack.nth_last(2)?.as_u64();

let call_data_offset = state.call()?.call_data_offset;
let call_data_length = state.call()?.call_data_length;
let (src_addr, src_addr_end) = (
call_data_offset + data_offset,
call_data_offset + call_data_length,
);

let dst_addr = memory_offset.as_u64();
let src_addr_end = call_data_offset.checked_add(call_data_length).unwrap();

// Reset start offset to end offset if overflow.
let src_addr = u64::try_from(data_offset)
.ok()
.and_then(|offset| offset.checked_add(call_data_offset))
.unwrap_or(src_addr_end)
.min(src_addr_end);

let mut exec_step = state.new_step(geth_step)?;
let copy_steps = gen_copy_steps(
state,
let copy_steps = state.gen_copy_steps_for_call_data(
&mut exec_step,
src_addr,
memory_offset,
dst_addr,
src_addr_end,
length,
state.call()?.is_root,
)?;

let (src_type, src_id) = if state.call()?.is_root {
Expand All @@ -165,7 +137,7 @@ fn gen_copy_event(
src_addr_end,
dst_type: CopyDataType::Memory,
dst_id: NumberOrHash::Number(state.call()?.call_id),
dst_addr: memory_offset,
dst_addr,
log_id: None,
rw_counter_start,
bytes: copy_steps,
Expand Down
132 changes: 69 additions & 63 deletions bus-mapping/src/evm/opcodes/calldataload.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,73 +23,79 @@ impl Opcode for Calldataload {
let offset = geth_step.stack.nth_last(0)?;
state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(0), offset)?;

let is_root = state.call()?.is_root;
if is_root {
state.call_context_read(
&mut exec_step,
state.call()?.call_id,
CallContextField::TxId,
state.tx_ctx.id().into(),
// Check if offset is Uint64 overflow.
let calldata_word = if let Ok(offset) = u64::try_from(offset) {
let is_root = state.call()?.is_root;
let call_id = state.call()?.call_id;
if is_root {
state.call_context_read(
&mut exec_step,
call_id,
CallContextField::TxId,
state.tx_ctx.id().into(),
);
state.call_context_read(
&mut exec_step,
call_id,
CallContextField::CallDataLength,
state.call()?.call_data_length.into(),
);
} else {
state.call_context_read(
&mut exec_step,
call_id,
CallContextField::CallerId,
state.call()?.caller_id.into(),
);
state.call_context_read(
&mut exec_step,
call_id,
CallContextField::CallDataLength,
state.call()?.call_data_length.into(),
);
state.call_context_read(
&mut exec_step,
call_id,
CallContextField::CallDataOffset,
state.call()?.call_data_offset.into(),
);
}

let call_data_offset = state.call()?.call_data_offset;
let call_data_length = state.call()?.call_data_length;
let (src_addr, src_addr_end, caller_id, call_data) = (
call_data_offset + offset.min(call_data_length),
call_data_offset + call_data_length,
state.call()?.caller_id,
state.call_ctx()?.call_data.to_vec(),
);
state.call_context_read(
&mut exec_step,
state.call()?.call_id,
CallContextField::CallDataLength,
state.call()?.call_data_length.into(),
);
} else {
state.call_context_read(
&mut exec_step,
state.call()?.call_id,
CallContextField::CallerId,
state.call()?.caller_id.into(),
);
state.call_context_read(
&mut exec_step,
state.call()?.call_id,
CallContextField::CallDataLength,
state.call()?.call_data_length.into(),
);
state.call_context_read(
&mut exec_step,
state.call()?.call_id,
CallContextField::CallDataOffset,
state.call()?.call_data_offset.into(),
);
}

let call = state.call()?.clone();
let (src_addr, src_addr_end, caller_id, call_data) = (
call.call_data_offset as usize + offset.as_usize(),
call.call_data_offset as usize + call.call_data_length as usize,
call.caller_id,
state.call_ctx()?.call_data.to_vec(),
);
let calldata_word = (0..32)
.map(|idx| {
let addr = src_addr + idx;
if addr < src_addr_end {
let byte = call_data[addr - call.call_data_offset as usize];
if !is_root {
// caller id as call_id
state.push_op(
&mut exec_step,
RW::READ,
MemoryOp::new(caller_id, (src_addr + idx).into(), byte),
);
let calldata: Vec<_> = (0..32)
.map(|idx| {
let addr = src_addr.checked_add(idx).unwrap_or(src_addr_end);
if addr < src_addr_end {
let byte = call_data[(addr - call_data_offset) as usize];
if !is_root {
state.push_op(
&mut exec_step,
RW::READ,
MemoryOp::new(caller_id, (src_addr + idx).into(), byte),
);
}
byte
} else {
0
}
byte
} else {
0
}
})
.collect::<Vec<u8>>();
})
.collect();

U256::from_big_endian(&calldata)
} else {
// Stack push `0` as result if overflow.
U256::zero()
};

state.stack_write(
&mut exec_step,
geth_step.stack.last_filled(),
U256::from_big_endian(&calldata_word),
)?;
state.stack_write(&mut exec_step, geth_step.stack.last_filled(), calldata_word)?;

Ok(vec![exec_step])
}
Expand Down
Loading

0 comments on commit 22dd263

Please sign in to comment.