Skip to content

Commit

Permalink
Bug: fix to not truncate external_address for EXTCODECOPY (privac…
Browse files Browse the repository at this point in the history
…y-scaling-explorations#1260)

### Description

Fix to add stack-read operation of whole `external_address` word to RW
table in bus-mapping, and lookup that word in circuit.

### Issue Link

Close
privacy-scaling-explorations#1252

### Type of change

- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

### How Has This Been Tested?

Could be tested via test cases of `ExtcodecopyGadget`.
  • Loading branch information
silathdiir authored Feb 27, 2023
1 parent 35c4f5c commit 09ba860
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
5 changes: 3 additions & 2 deletions bus-mapping/src/evm/opcodes/extcodecopy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ fn gen_extcodecopy_step(
) -> Result<ExecStep, Error> {
let mut exec_step = state.new_step(geth_step)?;

let external_address = geth_step.stack.nth_last(0)?.to_address();
let external_address_word = geth_step.stack.nth_last(0)?;
let external_address = external_address_word.to_address();
let dest_offset = geth_step.stack.nth_last(1)?;
let offset = geth_step.stack.nth_last(2)?;
let length = geth_step.stack.nth_last(3)?;
Expand All @@ -71,7 +72,7 @@ fn gen_extcodecopy_step(
state.stack_read(
&mut exec_step,
geth_step.stack.nth_last_filled(0),
external_address.to_word(),
external_address_word,
)?;
state.stack_read(
&mut exec_step,
Expand Down
26 changes: 13 additions & 13 deletions zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ use crate::{
},
from_bytes,
memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget},
not, select, CachedRegion, Cell, MemoryAddress, RandomLinearCombination,
not, select, CachedRegion, Cell, MemoryAddress, Word,
},
witness::{Block, Call, ExecStep, Transaction},
},
table::{AccountFieldTag, CallContextFieldTag},
};
use bus_mapping::circuit_input_builder::CopyDataType;
use eth_types::{evm_types::GasCost, Field, ToAddress, ToLittleEndian, ToScalar};
use eth_types::{evm_types::GasCost, Field, ToLittleEndian, ToScalar};
use gadgets::util::Expr;
use halo2_proofs::{circuit::Value, plonk::Error};

Expand All @@ -25,7 +25,7 @@ use super::ExecutionGadget;
#[derive(Clone, Debug)]
pub(crate) struct ExtcodecopyGadget<F> {
same_context: SameContextGadget<F>,
external_address: RandomLinearCombination<F, N_BYTES_ACCOUNT_ADDRESS>,
external_address_word: Word<F>,
memory_address: MemoryAddressGadget<F>,
data_offset: MemoryAddress<F>,
tx_id: Cell<F>,
Expand All @@ -46,12 +46,15 @@ impl<F: Field> ExecutionGadget<F> for ExtcodecopyGadget<F> {
fn configure(cb: &mut ConstraintBuilder<F>) -> Self {
let opcode = cb.query_cell();

let external_address = cb.query_word_rlc();
let external_address_word = cb.query_word_rlc();
let external_address =
from_bytes::expr(&external_address_word.cells[..N_BYTES_ACCOUNT_ADDRESS]);

let memory_offset = cb.query_cell_phase2();
let data_offset = cb.query_word_rlc();
let memory_length = cb.query_word_rlc();

cb.stack_pop(external_address.expr());
cb.stack_pop(external_address_word.expr());
cb.stack_pop(memory_offset.expr());
cb.stack_pop(data_offset.expr());
cb.stack_pop(memory_length.expr());
Expand All @@ -63,15 +66,15 @@ impl<F: Field> ExecutionGadget<F> for ExtcodecopyGadget<F> {
let is_warm = cb.query_bool();
cb.account_access_list_write(
tx_id.expr(),
from_bytes::expr(&external_address.cells),
external_address.expr(),
1.expr(),
is_warm.expr(),
Some(&mut reversion_info),
);

let code_hash = cb.query_cell_phase2();
cb.account_read(
from_bytes::expr(&external_address.cells),
external_address.expr(),
AccountFieldTag::CodeHash,
code_hash.expr(),
);
Expand Down Expand Up @@ -126,7 +129,7 @@ impl<F: Field> ExecutionGadget<F> for ExtcodecopyGadget<F> {

Self {
same_context,
external_address,
external_address_word,
memory_address,
data_offset,
tx_id,
Expand All @@ -153,11 +156,8 @@ impl<F: Field> ExecutionGadget<F> for ExtcodecopyGadget<F> {

let [external_address, memory_offset, data_offset, memory_length] =
[0, 1, 2, 3].map(|idx| block.rws[step.rw_indices[idx]].stack_value());
let mut le_bytes = external_address.to_address().0;
le_bytes.reverse();
self.external_address
.assign(region, offset, Some(le_bytes))?;

self.external_address_word
.assign(region, offset, Some(external_address.to_le_bytes()))?;
let memory_address =
self.memory_address
.assign(region, offset, memory_offset, memory_length)?;
Expand Down

0 comments on commit 09ba860

Please sign in to comment.