Skip to content

Commit

Permalink
fix rws padding logic and error handling (privacy-scaling-exploration…
Browse files Browse the repository at this point in the history
…s#1515)

### Description

Fixed rws padding logic and padding error handling.

### Issue Link


privacy-scaling-explorations#1507

### Type of change

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

### Contents

- more documentation on padding logic
- fix max_rws calculation
- de-duplicated startop in end_block.tx by skip second one.
- panic instead of confused error log in padding calculation function.
  • Loading branch information
hero78119 authored Jul 6, 2023
1 parent dd1e1c4 commit 515a2bc
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 16 deletions.
22 changes: 18 additions & 4 deletions bus-mapping/src/circuit_input_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ impl CircuitInputBuilder<FixedCParams> {
step.bus_mapping_instance.push(op_ref);
};

// rwc index start from 1
let total_rws = state.block_ctx.rwc.0 - 1;
// We need at least 1 extra Start row
#[allow(clippy::int_plus_one)]
Expand All @@ -306,13 +307,21 @@ impl CircuitInputBuilder<FixedCParams> {
max_rws
);
}
push_op(&mut end_block_last, RWCounter(1), RW::READ, StartOp {});
let (padding_start, padding_end) = (1, max_rws - total_rws); // rw counter start from 1
push_op(
&mut end_block_last,
RWCounter(max_rws - total_rws),
RWCounter(padding_start),
RW::READ,
StartOp {},
);
if padding_end != padding_start {
push_op(
&mut end_block_last,
RWCounter(padding_end),
RW::READ,
StartOp {},
);
}

self.block.block_steps.end_block_not_last = end_block_not_last;
self.block.block_steps.end_block_last = end_block_last;
Expand Down Expand Up @@ -380,7 +389,12 @@ impl CircuitInputBuilder<DynamicCParams> {
.fold(0, |acc, c| acc + c.bytes.len())
* 2
+ 2;
let max_rws: usize = self.block_ctx.rwc.into();

let total_rws_before_padding: usize =
<RWCounter as Into<usize>>::into(self.block_ctx.rwc) - 1; // -1 since rwc start from index `1`
let max_rws_after_padding = total_rws_before_padding
+ 1 // fill 1 to have exactly one StartOp padding in below `set_end_block`
+ if total_rws_before_padding > 0 { 1 /*end_block -> CallContextFieldTag::TxId lookup*/ } else { 0 };
// Computing the number of rows for the EVM circuit requires the size of ExecStep,
// which is determined in the code of zkevm-circuits and cannot be imported here.
// When the evm circuit receives a 0 value it dynamically computes the minimum
Expand All @@ -392,7 +406,7 @@ impl CircuitInputBuilder<DynamicCParams> {
// needed.
let max_keccak_rows = 0;
FixedCParams {
max_rws: max_rws + 3,
max_rws: max_rws_after_padding,
max_txs,
max_calldata,
max_copy_rows,
Expand Down
14 changes: 9 additions & 5 deletions zkevm-circuits/src/evm_circuit/execution/end_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
util::{word::Word, Expr},
};
use eth_types::Field;
use gadgets::util::select;
use halo2_proofs::{circuit::Value, plonk::Error};

#[derive(Clone, Debug)]
Expand All @@ -41,10 +42,13 @@ impl<F: Field> ExecutionGadget<F> for EndBlockGadget<F> {
// Note that rw_counter starts at 1
let is_empty_block =
IsZeroGadget::construct(cb, cb.curr.state.rw_counter.clone().expr() - 1.expr());
// If the block is empty, we do 0 rw_table lookups
// If the block is not empty, we will do 1 call_context lookup
let total_rws = not::expr(is_empty_block.expr())
* (cb.curr.state.rw_counter.clone().expr() - 1.expr() + 1.expr());

let total_rws_before_padding = cb.curr.state.rw_counter.clone().expr() - 1.expr()
+ select::expr(
is_empty_block.expr(),
0.expr(),
1.expr(), // If the block is not empty, we will do 1 call_context lookup below
);

// 1. Constraint total_rws and total_txs witness values depending on the empty
// block case.
Expand Down Expand Up @@ -85,7 +89,7 @@ impl<F: Field> ExecutionGadget<F> for EndBlockGadget<F> {
// rw_table to ensure there is no malicious insertion.
// Verify that there are at most total_rws meaningful entries in the rw_table
cb.rw_table_start_lookup(1.expr());
cb.rw_table_start_lookup(max_rws.expr() - total_rws.expr());
cb.rw_table_start_lookup(max_rws.expr() - total_rws_before_padding.expr());
// Since every lookup done in the EVM circuit must succeed and uses
// a unique rw_counter, we know that at least there are
// total_rws meaningful entries in the rw_table.
Expand Down
14 changes: 10 additions & 4 deletions zkevm-circuits/src/state_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,12 +500,18 @@ impl<F: Field> SubCircuit<F> for StateCircuit<F> {
config.assign_with_region(&mut region, &self.rows, &self.updates, self.n_rows)?;
#[cfg(any(feature = "test", test, feature = "test-circuits"))]
{
let padding_length = RwMap::padding_len(self.rows.len(), self.n_rows);
let first_non_padding_index = if self.rows.len() < self.n_rows {
RwMap::padding_len(self.rows.len(), self.n_rows)
} else {
1 // at least 1 StartOp padding in idx 0, so idx 1 is first non-padding row
};

for ((column, row_offset), &f) in &self.overrides {
let advice_column = column.value(config);
let offset =
usize::try_from(isize::try_from(padding_length).unwrap() + *row_offset)
.unwrap();
let offset = usize::try_from(
isize::try_from(first_non_padding_index).unwrap() + *row_offset,
)
.unwrap();
region.assign_advice(
|| "override",
advice_column,
Expand Down
5 changes: 2 additions & 3 deletions zkevm-circuits/src/witness/rw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,9 @@ impl RwMap {
target_len - rows_len
} else {
if target_len != 0 {
log::error!(
panic!(
"RwMap::padding_len overflow, target_len: {}, rows_len: {}",
target_len,
rows_len
target_len, rows_len
);
}
1
Expand Down

0 comments on commit 515a2bc

Please sign in to comment.