From de85dea2fff7a153c8b06170ba0787a77bddf240 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Mon, 9 Jan 2023 16:54:55 +0700 Subject: [PATCH 01/18] Bytecode circuit refactor (not passing tests) --- .../src/bytecode_circuit/bytecode_unroller.rs | 748 ++++++++++-------- .../evm_circuit/util/constraint_builder.rs | 2 +- zkevm-circuits/src/table.rs | 6 +- zkevm-circuits/src/witness/bytecode.rs | 2 +- 4 files changed, 402 insertions(+), 356 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index a5f82d68a5..88a2b81d33 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -44,18 +44,14 @@ pub struct BytecodeCircuitConfig { minimum_rows: usize, q_enable: Column, q_first: Column, - q_last: Selector, + q_last: Column, bytecode_table: BytecodeTable, - push_rindex: Column, - hash_input_rlc: Column, - code_length: Column, - byte_push_size: Column, - is_final: Column, - padding: Column, - push_rindex_inv: Column, - push_rindex_is_zero: IsZeroConfig, - length_inv: Column, - length_is_zero: IsZeroConfig, + push_data_left: Column, + value_rlc: Column, + length: Column, + push_data_size: Column, + push_data_left_inv: Column, + push_data_left_is_zero: IsZeroConfig, push_table: [Column; PUSH_TABLE_WIDTH], // External tables pub(crate) keccak_table: KeccakTable, @@ -85,146 +81,155 @@ impl SubCircuitConfig for BytecodeCircuitConfig { ) -> Self { let q_enable = meta.fixed_column(); let q_first = meta.fixed_column(); - let q_last = meta.selector(); + let q_last = meta.fixed_column(); let value = bytecode_table.value; - let push_rindex = meta.advice_column(); - let hash_input_rlc = meta.advice_column_in(SecondPhase); - let code_length = meta.advice_column(); - let byte_push_size = meta.advice_column(); - let is_final = meta.advice_column(); - let padding = meta.advice_column(); - let push_rindex_inv = meta.advice_column(); - let length_inv = meta.advice_column(); + let push_data_left = meta.advice_column(); + let value_rlc = meta.advice_column_in(SecondPhase); + let length = meta.advice_column(); + let push_data_size = meta.advice_column(); + let push_data_left_inv = meta.advice_column(); let push_table = array_init::array_init(|_| meta.fixed_column()); - // A byte is an opcode when `push_rindex == 0` on the previous row, - // else it's push data. - let push_rindex_is_zero = IsZeroChip::configure( - meta, - |meta| { - // Conditions: - // - Not on the first row - meta.query_fixed(q_enable, Rotation::cur()) - * not::expr(meta.query_fixed(q_first, Rotation::cur())) - }, - |meta| meta.query_advice(push_rindex, Rotation::prev()), - push_rindex_inv, - ); + let is_header_to_header = |meta: &mut VirtualCells| { + and::expr(vec![ + not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())), + not::expr(meta.query_advice(bytecode_table.tag, Rotation::next())), + ]) + }; - // Does the current row have bytecode field tag == Length? - let is_row_tag_length = |meta: &mut VirtualCells| { + let is_header_to_byte = |meta: &mut VirtualCells| { and::expr(vec![ - not::expr(meta.query_advice(padding, Rotation::cur())), not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())), + meta.query_advice(bytecode_table.tag, Rotation::next()), ]) }; - // Does the current row have bytecode field tag == Byte? - let is_row_tag_byte = |meta: &mut VirtualCells| { + let is_byte_to_header = |meta: &mut VirtualCells| { and::expr(vec![ - not::expr(meta.query_advice(padding, Rotation::cur())), meta.query_advice(bytecode_table.tag, Rotation::cur()), + not::expr(meta.query_advice(bytecode_table.tag, Rotation::next())), ]) }; - // For a row tagged Length, is the length (value) zero? - let length_is_zero = IsZeroChip::configure( - meta, - |meta| meta.query_fixed(q_enable, Rotation::cur()) * is_row_tag_length(meta), - |meta| meta.query_advice(value, Rotation::cur()), - length_inv, - ); - - let q_continue = |meta: &mut VirtualCells| { - // When - // - Not on the first row - // - The previous row did not contain the last byte + let is_byte_to_byte = |meta: &mut VirtualCells| { and::expr(vec![ - not::expr(meta.query_fixed(q_first, Rotation::cur())), - not::expr(meta.query_advice(is_final, Rotation::prev())), + meta.query_advice(bytecode_table.tag, Rotation::cur()), + meta.query_advice(bytecode_table.tag, Rotation::next()), ]) }; - meta.create_gate("continue", |meta| { + let is_header = |meta: &mut VirtualCells| { + not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())) + }; + + let is_byte = + |meta: &mut VirtualCells| meta.query_advice(bytecode_table.tag, Rotation::cur()); + + // A byte is an opcode when `push_data_left == 0` on the current row, + // else it's push data. + let push_data_left_is_zero = IsZeroChip::configure( + meta, + |meta| meta.query_fixed(q_enable, Rotation::cur()), + |meta| meta.query_advice(push_data_left, Rotation::cur()), + push_data_left_inv, + ); + + // When q_first || q_last -> + // assert cur.tag == Header + meta.create_gate("first and last row", |meta| { let mut cb = BaseConstraintBuilder::default(); - // Did the previous row have bytecode field tag == Length? - let is_prev_row_tag_length = |meta: &mut VirtualCells| { - and::expr(vec![ - not::expr(meta.query_fixed(q_first, Rotation::cur())), - not::expr(meta.query_advice(padding, Rotation::prev())), - not::expr(meta.query_advice(bytecode_table.tag, Rotation::prev())), - ]) - }; + cb.require_zero( + "cur.tag == Header", + meta.query_advice(bytecode_table.tag, Rotation::cur()), + ); - cb.require_equal( - "if prev_row.tag == Length: index == 0 else index == index + 1", + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + or::expr(vec![ + meta.query_fixed(q_first, Rotation::cur()), + meta.query_fixed(q_last, Rotation::cur()), + ]), + ])) + }); + + // When is_header -> + // assert cur.index == 0 + // assert cur.value == cur.length + meta.create_gate("Header row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_zero( + "cur.index == 0", meta.query_advice(bytecode_table.index, Rotation::cur()), - select::expr( - is_prev_row_tag_length(meta), - 0.expr(), - meta.query_advice(bytecode_table.index, Rotation::prev()) + 1.expr(), - ), - ); - cb.require_equal( - "is_code := push_rindex_prev == 0", - meta.query_advice(bytecode_table.is_code, Rotation::cur()), - select::expr( - is_prev_row_tag_length(meta), - 1.expr(), - push_rindex_is_zero.clone().is_zero_expression, - ), - ); - cb.require_equal( - "hash_input_rlc := hash_input_rlc_prev * challenges.keccak_input + byte", - meta.query_advice(hash_input_rlc, Rotation::cur()), - meta.query_advice(hash_input_rlc, Rotation::prev()) * challenges.keccak_input() - + meta.query_advice(value, Rotation::cur()), - ); - cb.require_equal( - "code_hash needs to remain the same", - meta.query_advice(bytecode_table.code_hash, Rotation::cur()), - meta.query_advice(bytecode_table.code_hash, Rotation::prev()), - ); - cb.require_equal( - "code_length needs to remain the same", - meta.query_advice(code_length, Rotation::cur()), - meta.query_advice(code_length, Rotation::prev()), ); + cb.require_equal( - "padding needs to remain the same", - meta.query_advice(padding, Rotation::cur()), - meta.query_advice(padding, Rotation::prev()), + "cur.value == cur.length", + meta.query_advice(bytecode_table.value, Rotation::cur()), + meta.query_advice(length, Rotation::cur()), ); - // Conditions: - // - Continuing cb.gate(and::expr(vec![ meta.query_fixed(q_enable, Rotation::cur()), - q_continue(meta), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_header(meta), ])) }); - meta.create_gate("start of bytecode", |meta| { + // When is_byte -> + // assert push_data_size_table_lookup(cur.value, cur.push_data_size) + // assert cur.is_code == (cur.push_data_left == 0) + meta.create_gate("Byte row", |meta| { let mut cb = BaseConstraintBuilder::default(); + cb.require_equal( - "next_row.tag == (tag.Length or tag.Padding) if length == 0 else tag.Byte", - meta.query_advice(bytecode_table.tag, Rotation::next()), - select::expr( - length_is_zero.clone().is_zero_expression, - select::expr( - meta.query_advice(padding, Rotation::next()), - BytecodeFieldTag::Padding.expr(), - BytecodeFieldTag::Length.expr(), - ), - BytecodeFieldTag::Byte.expr(), - ), + "cur.is_code == (cur.push_data_left == 0)", + meta.query_advice(bytecode_table.is_code, Rotation::cur()), + push_data_left_is_zero.clone().is_zero_expression, ); - cb.require_equal( - "if row.tag == tag.Length: value == row.code_length", - meta.query_advice(value, Rotation::cur()), - meta.query_advice(code_length, Rotation::cur()), + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte(meta), + ])) + }); + meta.lookup_any( + "push_data_size_table_lookup(cur.value, cur.push_data_size)", + |meta| { + let enable = and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte(meta), + ]); + + let lookup_columns = vec![value, push_data_size]; + + let mut constraints = vec![]; + + for i in 0..PUSH_TABLE_WIDTH { + constraints.push(( + enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), + meta.query_fixed(push_table[i], Rotation::cur()), + )) + } + constraints + }, + ); + + // When is_header_to_header or q_last -> + // assert cur.length == 0 + // assert cur.hash == EMPTY_HASH + meta.create_gate("Header to header row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_zero( + "cur.length == 0", + meta.query_advice(length, Rotation::cur()), ); + + // TODO: assert cur.hash == EMPTY_HASH // FIXME: Since randomness is only known at synthesis time, the RLC of empty // code_hash is not constant. Consider doing a lookup to the empty code_hash // value? cb.condition(length_is_zero.clone().is_zero_expression, @@ -235,150 +240,161 @@ impl SubCircuitConfig for BytecodeCircuitConfig { // ); // }); - // Conditions: - // - Not Continuing - // - This is the start of a new bytecode cb.gate(and::expr(vec![ meta.query_fixed(q_enable, Rotation::cur()), - is_row_tag_length(meta), - not::expr(q_continue(meta)), + or::expr(vec![ + is_header_to_header(meta), + meta.query_fixed(q_last, Rotation::cur()), + ]), ])) }); - meta.create_gate("start of padding", |meta| { + // When is_header_to_byte -> + // assert next.length == cur.length + // assert next.index == 0 + // assert next.is_code == 1 + // assert next.hash == cur.hash + // assert next.value_rlc == next.value + meta.create_gate("Header to byte row", |meta| { let mut cb = BaseConstraintBuilder::default(); + cb.require_equal( - "row needs to be marked as padding", - meta.query_advice(padding, Rotation::cur()), + "next.length == cur.length", + meta.query_advice(length, Rotation::next()), + meta.query_advice(length, Rotation::cur()), + ); + + cb.require_zero( + "next.index == 0", + meta.query_advice(bytecode_table.index, Rotation::next()), + ); + + cb.require_equal( + "next.is_code == 1", + meta.query_advice(bytecode_table.is_code, Rotation::next()), 1.expr(), ); - // Conditions: - // - Not Continuing - // - This is not the start of a new bytecode - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(is_row_tag_length(meta)), - not::expr(q_continue(meta)), - ])) - }); - meta.create_gate("length needs to be correct", |meta| { - let mut cb = BaseConstraintBuilder::default(); - cb.condition(1.expr() - length_is_zero.clone().is_zero_expression, |cb| { - cb.require_equal( - "index + 1 needs to equal code_length", - meta.query_advice(bytecode_table.index, Rotation::cur()) + 1.expr(), - meta.query_advice(code_length, Rotation::cur()), - ); - }); - // Conditions: - // - On the row with the last byte (`is_final == 1`) - // - Of bytecode with length > 0 - // - Not padding + cb.require_equal( + "next.hash == cur.hash", + meta.query_advice(bytecode_table.code_hash, Rotation::next()), + meta.query_advice(bytecode_table.code_hash, Rotation::cur()), + ); + + cb.require_equal( + "next.value_rlc == next.value", + meta.query_advice(value_rlc, Rotation::next()), + meta.query_advice(bytecode_table.value, Rotation::next()), + ); + cb.gate(and::expr(vec![ meta.query_fixed(q_enable, Rotation::cur()), - meta.query_advice(is_final, Rotation::cur()), - not::expr(meta.query_advice(padding, Rotation::cur())), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_header_to_byte(meta), ])) }); - meta.create_gate("always", |meta| { + // When is_byte_to_byte -> + // assert next.length == cur.length + // assert next.index == cur.index + 1 + // assert next.hash == cur.hash + // assert next.value_rlc == cur.value_rlc * randomness + next.value + // if cur.is_code: + // assert next.push_data_left == cur.push_data_size + // else: + // assert next.push_data_left == cur.push_data_left - 1 + meta.create_gate("Byte to Byte row", |meta| { let mut cb = BaseConstraintBuilder::default(); - cb.require_boolean( - "is_final needs to be boolean", - meta.query_advice(is_final, Rotation::cur()), + + cb.require_equal( + "next.length == cur.length", + meta.query_advice(length, Rotation::next()), + meta.query_advice(length, Rotation::cur()), + ); + + cb.require_equal( + "next.index == cur.index + 1", + meta.query_advice(bytecode_table.index, Rotation::next()), + meta.query_advice(bytecode_table.index, Rotation::cur()) + 1.expr(), ); - cb.require_boolean( - "padding needs to be boolean", - meta.query_advice(padding, Rotation::cur()), + + cb.require_equal( + "next.hash == cur.hash", + meta.query_advice(bytecode_table.code_hash, Rotation::next()), + meta.query_advice(bytecode_table.code_hash, Rotation::cur()), ); - cb.condition(is_row_tag_byte(meta), |cb| { - cb.require_equal( - "push_rindex := is_code ? byte_push_size : push_rindex_prev - 1", - meta.query_advice(push_rindex, Rotation::cur()), - select::expr( - meta.query_advice(bytecode_table.is_code, Rotation::cur()), - meta.query_advice(byte_push_size, Rotation::cur()), - meta.query_advice(push_rindex, Rotation::prev()) - 1.expr(), - ), - ); - }); - // Conditions: Always - cb.gate(meta.query_fixed(q_enable, Rotation::cur())) - }); - meta.create_gate("padding", |meta| { - let mut cb = BaseConstraintBuilder::default(); - cb.require_boolean( - "padding can only go 0 -> 1 once", - meta.query_advice(padding, Rotation::cur()) - - meta.query_advice(padding, Rotation::prev()), + // TODO: check this + cb.require_equal( + "next.value_rlc == cur.value_rlc * randomness + next.value", + meta.query_advice(value_rlc, Rotation::next()), + meta.query_advice(value_rlc, Rotation::cur()) * challenges.keccak_input() + + meta.query_advice(value, Rotation::cur()), ); - // Conditions: - // - Not on the first row + + cb.require_equal( + "next.push_data_left == cur.is_code ? cur.push_data_size : cur.push_data_left - 1", + meta.query_advice(push_data_left, Rotation::next()), + select::expr( + meta.query_advice(bytecode_table.is_code, Rotation::cur()), + meta.query_advice(push_data_size, Rotation::cur()), + meta.query_advice(push_data_left, Rotation::cur()) - 1.expr(), + ), + ); + cb.gate(and::expr(vec![ meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_first, Rotation::cur())), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte_to_byte(meta), ])) }); - // The code_hash is checked on the latest row because only then have - // we accumulated all the bytes. We also have to go through the bytes - // in a forward manner because that's the only way we can know which - // bytes are op codes and which are push data. - meta.create_gate("last row", |meta| { + // When is_byte_to_header -> + // assert cur.index + 1 == cur.length + // assert keccak256_table_lookup(cur.hash, cur.length, cur.value_rlc) + meta.create_gate("Byte to Header row", |meta| { let mut cb = BaseConstraintBuilder::default(); + cb.require_equal( - "padding needs to be enabled OR the last row needs to be the last byte", - or::expr(vec![ - meta.query_advice(padding, Rotation::cur()), - meta.query_advice(is_final, Rotation::cur()), - ]), - 1.expr(), + "cur.index + 1 == cur.length", + meta.query_advice(bytecode_table.index, Rotation::next()) + 1.expr(), + meta.query_advice(length, Rotation::cur()), ); - // Conditions: - // - On the last row - cb.gate(meta.query_selector(q_last)) - }); - // Lookup how many bytes the current opcode pushes - // (also indirectly range checks `byte` to be in [0, 255]) - meta.lookup_any("Range bytes", |meta| { - // Conditions: Always - let q_enable = meta.query_fixed(q_enable, Rotation::cur()) * is_row_tag_byte(meta); - let lookup_columns = vec![value, byte_push_size]; - let mut constraints = vec![]; - for i in 0..PUSH_TABLE_WIDTH { - constraints.push(( - q_enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), - meta.query_fixed(push_table[i], Rotation::cur()), - )) - } - constraints + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte_to_header(meta), + ])) }); + meta.lookup_any( + "keccak256_table_lookup(cur.hash, cur.length, cur.value_rlc)", + |meta| { + let enable = and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte_to_header(meta), + ]); + + let lookup_columns = vec![value_rlc, length, bytecode_table.code_hash]; + + let mut constraints = vec![( + enable.clone(), + meta.query_advice(keccak_table.is_enabled, Rotation::cur()), + )]; + + // TODO: perhaps write this explicitly so it is more readable the matching + // between collumns + for (i, column) in keccak_table.columns().iter().skip(1).enumerate() { + constraints.push(( + enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), + meta.query_advice(*column, Rotation::cur()), + )) + } - // keccak lookup - meta.lookup_any("keccak", |meta| { - // Conditions: - // - On the row with the last byte (`is_final == 1`) - // - Not padding - let enable = and::expr(vec![ - meta.query_advice(is_final, Rotation::cur()), - not::expr(meta.query_advice(padding, Rotation::cur())), - ]); - let lookup_columns = vec![hash_input_rlc, code_length, bytecode_table.code_hash]; - let mut constraints = vec![( - enable.clone(), - meta.query_advice(keccak_table.is_enabled, Rotation::cur()), - )]; - for (i, column) in keccak_table.columns().iter().skip(1).enumerate() { - constraints.push(( - enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), - meta.query_advice(*column, Rotation::cur()), - )) - } - constraints - }); + constraints + }, + ); BytecodeCircuitConfig { minimum_rows: meta.minimum_rows(), @@ -386,16 +402,12 @@ impl SubCircuitConfig for BytecodeCircuitConfig { q_first, q_last, bytecode_table, - push_rindex, - hash_input_rlc, - code_length, - byte_push_size, - is_final, - padding, - push_rindex_inv, - push_rindex_is_zero, - length_inv, - length_is_zero, + push_data_left, + value_rlc, + length, + push_data_size, + push_data_left_inv, + push_data_left_is_zero, push_table, keccak_table, } @@ -421,8 +433,8 @@ impl BytecodeCircuitConfig { challenges: &Challenges>, fail_fast: bool, ) -> Result<(), Error> { - let push_rindex_is_zero_chip = IsZeroChip::construct(self.push_rindex_is_zero.clone()); - let length_is_zero_chip = IsZeroChip::construct(self.length_is_zero.clone()); + let push_data_left_is_zero_chip = + IsZeroChip::construct(self.push_data_left_is_zero.clone()); // Subtract the unusable rows from the size assert!(size > self.minimum_rows); @@ -432,98 +444,26 @@ impl BytecodeCircuitConfig { || "assign bytecode", |mut region| { let mut offset = 0; - let mut push_rindex_prev = 0; for bytecode in witness.iter() { - // Run over all the bytes - let mut push_rindex = 0; - let mut byte_push_size = 0; - let mut hash_input_rlc = challenges.keccak_input().map(|_| F::zero()); - let code_length = F::from(bytecode.bytes.len() as u64); - for (idx, row) in bytecode.rows.iter().enumerate() { - if fail_fast && offset > last_row_offset { - log::error!( - "Bytecode Circuit: offset={} > last_row_offset={}", - offset, - last_row_offset - ); - return Err(Error::Synthesis); - } - - let code_hash = challenges.evm_word().map(|challenge| { - RandomLinearCombination::::random_linear_combine( - row.code_hash.to_le_bytes(), - challenge, - ) - }); - - // Track which byte is an opcode and which is push - // data - let is_code = push_rindex == 0; - if idx > 0 { - byte_push_size = get_push_size(row.value.get_lower_128() as u8); - push_rindex = if is_code { - byte_push_size - } else { - push_rindex - 1 - }; - hash_input_rlc.as_mut().zip(challenges.keccak_input()).map( - |(hash_input_rlc, challenge)| { - *hash_input_rlc = *hash_input_rlc * challenge + row.value - }, - ); - } - - // Set the data for this row - if offset <= last_row_offset { - self.set_row( - &mut region, - &push_rindex_is_zero_chip, - &length_is_zero_chip, - offset, - true, - offset == last_row_offset, - code_hash, - row.tag, - row.index, - row.is_code, - row.value, - push_rindex, - hash_input_rlc, - code_length, - F::from(byte_push_size as u64), - idx == bytecode.bytes.len(), - false, - F::from(push_rindex_prev), - )?; - push_rindex_prev = push_rindex; - offset += 1; - } - } + self.assign_bytecode( + &mut region, + bytecode, + challenges, + &push_data_left_is_zero_chip, + &mut offset, + last_row_offset, + fail_fast, + )?; } // Padding for idx in offset..=last_row_offset { - self.set_row( + self.set_padding_row( &mut region, - &push_rindex_is_zero_chip, - &length_is_zero_chip, + &push_data_left_is_zero_chip, idx, - idx < last_row_offset, - idx == last_row_offset, - Value::known(F::zero()), - F::from(BytecodeFieldTag::Padding as u64), - F::zero(), - F::one(), - F::zero(), - 0, - Value::known(F::zero()), - F::zero(), - F::zero(), - true, - true, - F::from(push_rindex_prev), + last_row_offset, )?; - push_rindex_prev = 0; } Ok(()) }, @@ -531,11 +471,116 @@ impl BytecodeCircuitConfig { } #[allow(clippy::too_many_arguments)] - fn set_row( + fn assign_bytecode( &self, region: &mut Region<'_, F>, + bytecode: &UnrolledBytecode, + challenges: &Challenges>, push_rindex_is_zero_chip: &IsZeroChip, - length_is_zero_chip: &IsZeroChip, + offset: &mut usize, + last_row_offset: usize, + fail_fast: bool, + ) -> Result<(), Error> { + // Run over all the bytes + let mut push_data_left = 0; + let mut push_data_size = 0; + let mut value_rlc = challenges.keccak_input().map(|_| F::zero()); + let length = F::from(bytecode.bytes.len() as u64); + + for (idx, row) in bytecode.rows.iter().enumerate() { + if fail_fast && *offset > last_row_offset { + log::error!( + "Bytecode Circuit: offset={} > last_row_offset={}", + offset, + last_row_offset + ); + return Err(Error::Synthesis); + } + + // TODO: why different code_hash for each row? Is this going to produce the same + // result for every row? + let code_hash = challenges.evm_word().map(|challenge| { + RandomLinearCombination::::random_linear_combine( + row.code_hash.to_le_bytes(), + challenge, + ) + }); + + // Track which byte is an opcode and which is push + // data + if idx > 0 { + let is_code = push_data_left == 0; + assert_eq!(F::from(is_code as u64), row.is_code, "is_code must match"); + + push_data_size = get_push_size(row.value.get_lower_128() as u8); + + push_data_left = if is_code { + push_data_size + } else { + push_data_left - 1 + }; + + value_rlc + .as_mut() + .zip(challenges.keccak_input()) + .map(|(value_rlc, challenge)| *value_rlc = *value_rlc * challenge + row.value); + } + + // Set the data for this row + if *offset <= last_row_offset { + self.set_row( + region, + push_rindex_is_zero_chip, + *offset, + true, + *offset == last_row_offset, + code_hash, + row.tag, + row.index, + row.is_code, + row.value, + push_data_left, + value_rlc, + length, + F::from(push_data_size as u64), + )?; + *offset += 1; + } + } + + Ok(()) + } + + fn set_padding_row( + &self, + region: &mut Region<'_, F>, + push_data_left_is_zero_chip: &IsZeroChip, + offset: usize, + last_row_offset: usize, + ) -> Result<(), Error> { + self.set_row( + region, + push_data_left_is_zero_chip, + offset, + offset < last_row_offset, + offset == last_row_offset, + Value::known(F::zero()), + F::from(BytecodeFieldTag::Header as u64), + F::zero(), + F::zero(), + F::zero(), + 0, + Value::known(F::zero()), + F::zero(), + F::zero(), + ) + } + + #[allow(clippy::too_many_arguments)] + fn set_row( + &self, + region: &mut Region<'_, F>, + push_data_left_is_zero_chip: &IsZeroChip, offset: usize, enable: bool, last: bool, @@ -544,13 +589,10 @@ impl BytecodeCircuitConfig { index: F, is_code: F, value: F, - push_rindex: u64, - hash_input_rlc: Value, - code_length: F, - byte_push_size: F, - is_final: bool, - padding: bool, - push_rindex_prev: F, + push_data_left: u64, + value_rlc: Value, + length: F, + push_data_size: F, ) -> Result<(), Error> { // q_enable region.assign_fixed( @@ -569,9 +611,13 @@ impl BytecodeCircuitConfig { )?; // q_last - if last { - self.q_last.enable(region, offset)?; - } + let q_last_value = if last { F::one() } else { F::zero() }; + region.assign_fixed( + || format!("assign q_first {}", offset), + self.q_last, + offset, + || Value::known(q_last_value), + )?; // Advices for (name, column, value) in [ @@ -579,11 +625,13 @@ impl BytecodeCircuitConfig { ("index", self.bytecode_table.index, index), ("is_code", self.bytecode_table.is_code, is_code), ("value", self.bytecode_table.value, value), - ("push_rindex", self.push_rindex, F::from(push_rindex)), - ("code_length", self.code_length, code_length), - ("byte_push_size", self.byte_push_size, byte_push_size), - ("is_final", self.is_final, F::from(is_final as u64)), - ("padding", self.padding, F::from(padding as u64)), + ( + "push_data_left", + self.push_data_left, + F::from(push_data_left), + ), + ("length", self.length, length), + ("push_data_size", self.push_data_size, push_data_size), ] { region.assign_advice( || format!("assign {} {}", name, offset), @@ -594,7 +642,7 @@ impl BytecodeCircuitConfig { } for (name, column, value) in [ ("code_hash", self.bytecode_table.code_hash, code_hash), - ("hash_input_rlc", self.hash_input_rlc, hash_input_rlc), + ("value_rlc", self.value_rlc, value_rlc), ] { region.assign_advice( || format!("assign {} {}", name, offset), @@ -604,11 +652,11 @@ impl BytecodeCircuitConfig { )?; } - // push_rindex_is_zero_chip - push_rindex_is_zero_chip.assign(region, offset, Value::known(push_rindex_prev))?; - - // length_is_zero chip - length_is_zero_chip.assign(region, offset, Value::known(code_length))?; + push_data_left_is_zero_chip.assign( + region, + offset, + Value::known(F::from(push_data_left)), + )?; Ok(()) } @@ -649,7 +697,7 @@ pub fn unroll(bytes: Vec) -> UnrolledBytecode { let code_hash = keccak(&bytes[..]); let mut rows = vec![BytecodeRow:: { code_hash, - tag: F::from(BytecodeFieldTag::Length as u64), + tag: F::from(BytecodeFieldTag::Header as u64), index: F::zero(), is_code: F::zero(), value: F::from(bytes.len() as u64), @@ -832,7 +880,7 @@ mod tests { 0, BytecodeRow { code_hash, - tag: Fr::from(BytecodeFieldTag::Length as u64), + tag: Fr::from(BytecodeFieldTag::Header as u64), index: Fr::zero(), is_code: Fr::zero(), value: Fr::from(bytecode.to_vec().len() as u64), diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index a6ea091cb5..53d93478d8 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -601,7 +601,7 @@ impl<'a, F: Field> ConstraintBuilder<'a, F> { "Bytecode (length)", Lookup::Bytecode { hash: code_hash, - tag: BytecodeFieldTag::Length.expr(), + tag: BytecodeFieldTag::Header.expr(), index: 0.expr(), is_code: 0.expr(), value: cell.expr(), diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index 1b7d9a1667..c22bb269c8 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -545,12 +545,10 @@ impl MptTable { /// Tag to identify the field in a Bytecode Table row #[derive(Clone, Copy, Debug)] pub enum BytecodeFieldTag { - /// Length field - Length, + /// Header field + Header, /// Byte field Byte, - /// Padding field - Padding, } impl_expr!(BytecodeFieldTag); diff --git a/zkevm-circuits/src/witness/bytecode.rs b/zkevm-circuits/src/witness/bytecode.rs index 7c5c92f9fb..6c913af698 100644 --- a/zkevm-circuits/src/witness/bytecode.rs +++ b/zkevm-circuits/src/witness/bytecode.rs @@ -36,7 +36,7 @@ impl Bytecode { rows.push([ hash, - Value::known(F::from(BytecodeFieldTag::Length as u64)), + Value::known(F::from(BytecodeFieldTag::Header as u64)), Value::known(F::zero()), Value::known(F::zero()), Value::known(F::from(self.bytes.len() as u64)), From 55e24fdaf7cef3217f23cfd640b5779977ee8890 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Mon, 9 Jan 2023 21:53:59 +0700 Subject: [PATCH 02/18] Fix but in witness generation --- zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index 88a2b81d33..77fc8802be 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -483,6 +483,7 @@ impl BytecodeCircuitConfig { ) -> Result<(), Error> { // Run over all the bytes let mut push_data_left = 0; + let mut next_push_data_left = 0; let mut push_data_size = 0; let mut value_rlc = challenges.keccak_input().map(|_| F::zero()); let length = F::from(bytecode.bytes.len() as u64); @@ -514,7 +515,7 @@ impl BytecodeCircuitConfig { push_data_size = get_push_size(row.value.get_lower_128() as u8); - push_data_left = if is_code { + next_push_data_left = if is_code { push_data_size } else { push_data_left - 1 @@ -545,6 +546,7 @@ impl BytecodeCircuitConfig { F::from(push_data_size as u64), )?; *offset += 1; + push_data_left = next_push_data_left } } From acdfd775451dbe5950c7d373a807601e8168b178 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Wed, 11 Jan 2023 10:07:37 +0700 Subject: [PATCH 03/18] Fix small bug --- .../src/bytecode_circuit/bytecode_unroller.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index 77fc8802be..3ac051c487 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -18,6 +18,7 @@ use halo2_proofs::{ poly::Rotation, }; use keccak256::plain::Keccak; +use log::trace; use std::vec; use super::param::PUSH_TABLE_WIDTH; @@ -357,7 +358,7 @@ impl SubCircuitConfig for BytecodeCircuitConfig { cb.require_equal( "cur.index + 1 == cur.length", - meta.query_advice(bytecode_table.index, Rotation::next()) + 1.expr(), + meta.query_advice(bytecode_table.index, Rotation::cur()) + 1.expr(), meta.query_advice(length, Rotation::cur()), ); @@ -545,6 +546,22 @@ impl BytecodeCircuitConfig { length, F::from(push_data_size as u64), )?; + + trace!( + "bytecode.set_row({}): last:{} h:{:?} t:{:?} i:{:?} c:{:?} v:{:?} pdl:{} rlc:{:?} l:{:?} pds:{:?}", + offset, + *offset == last_row_offset, + code_hash, + row.tag.get_lower_32(), + row.index.get_lower_32(), + row.is_code.get_lower_32(), + row.value.get_lower_32(), + push_data_left, + value_rlc, + length.get_lower_32(), + push_data_size + ); + *offset += 1; push_data_left = next_push_data_left } From fc865ae347f3dab11005c5965ca4e0c3c4d9399e Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Wed, 11 Jan 2023 15:07:02 +0700 Subject: [PATCH 04/18] Fix small bug --- zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index 3ac051c487..ce080f3c7a 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -330,7 +330,7 @@ impl SubCircuitConfig for BytecodeCircuitConfig { "next.value_rlc == cur.value_rlc * randomness + next.value", meta.query_advice(value_rlc, Rotation::next()), meta.query_advice(value_rlc, Rotation::cur()) * challenges.keccak_input() - + meta.query_advice(value, Rotation::cur()), + + meta.query_advice(value, Rotation::next()), ); cb.require_equal( From bcb3ac6c1a6a27e00203e1adabb02a387d3b3a53 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Wed, 11 Jan 2023 15:18:45 +0700 Subject: [PATCH 05/18] Removing assert from wit-gen that should actually cause invalid proof --- zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index ce080f3c7a..6f93da9381 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -512,7 +512,6 @@ impl BytecodeCircuitConfig { // data if idx > 0 { let is_code = push_data_left == 0; - assert_eq!(F::from(is_code as u64), row.is_code, "is_code must match"); push_data_size = get_push_size(row.value.get_lower_128() as u8); From 8d58a91242c9e5bc6f8a7307f02492100423f117 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Thu, 12 Jan 2023 11:12:17 +0700 Subject: [PATCH 06/18] Modify tests because new circuit does not allow for bytecode in last row --- .../src/bytecode_circuit/bytecode_unroller.rs | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index 6f93da9381..79f179acd9 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -12,8 +12,7 @@ use gadgets::is_zero::{IsZeroChip, IsZeroConfig, IsZeroInstruction}; use halo2_proofs::{ circuit::{Layouter, Region, Value}, plonk::{ - Advice, Column, ConstraintSystem, Error, Expression, Fixed, SecondPhase, Selector, - VirtualCells, + Advice, Column, ConstraintSystem, Error, Expression, Fixed, SecondPhase, VirtualCells, }, poly::Rotation, }; @@ -441,6 +440,13 @@ impl BytecodeCircuitConfig { assert!(size > self.minimum_rows); let last_row_offset = size - self.minimum_rows + 1; + trace!( + "size: {}, minimum_rows: {}, last_row_offset:{}", + size, + self.minimum_rows, + last_row_offset + ); + layouter.assign_region( || "assign bytecode", |mut region| { @@ -528,7 +534,7 @@ impl BytecodeCircuitConfig { } // Set the data for this row - if *offset <= last_row_offset { + if *offset < last_row_offset { self.set_row( region, push_rindex_is_zero_chip, @@ -564,6 +570,9 @@ impl BytecodeCircuitConfig { *offset += 1; push_data_left = next_push_data_left } + if *offset == last_row_offset { + self.set_padding_row(region, push_rindex_is_zero_chip, *offset, last_row_offset)?; + } } Ok(()) @@ -831,7 +840,7 @@ impl SubCircuit for BytecodeCircuit { layouter: &mut impl Layouter, ) -> Result<(), Error> { config.load_aux_tables(layouter)?; - config.assign_internal(layouter, self.size, &self.bytecodes, challenges, false) + config.assign_internal(layouter, self.size, &self.bytecodes, challenges, true) } } @@ -936,7 +945,14 @@ mod tests { #[test] fn bytecode_full() { let k = 9; - test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 7])], true); + test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 8])], true); + } + + #[test] + fn bytecode_full_last_row() { + let k = 9; + // Last row must be a padding row, so we have one row less for actual bytecode + test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 7])], false); } /// Tests a circuit with incomplete bytecode From eafe3d5cd10bfde4879c4262bae0ccf2fa73510b Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Fri, 13 Jan 2023 14:54:18 +0700 Subject: [PATCH 07/18] Trying empty hash matching --- .../src/bytecode_circuit/bytecode_unroller.rs | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index 79f179acd9..bdc320f712 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -16,7 +16,7 @@ use halo2_proofs::{ }, poly::Rotation, }; -use keccak256::plain::Keccak; +use keccak256::{plain::Keccak, EMPTY_HASH_LE}; use log::trace; use std::vec; @@ -229,16 +229,17 @@ impl SubCircuitConfig for BytecodeCircuitConfig { meta.query_advice(length, Rotation::cur()), ); - // TODO: assert cur.hash == EMPTY_HASH - // FIXME: Since randomness is only known at synthesis time, the RLC of empty - // code_hash is not constant. Consider doing a lookup to the empty code_hash - // value? cb.condition(length_is_zero.clone().is_zero_expression, - // |cb| { cb.require_equal( - // "if length == 0: code_hash == RLC(EMPTY_HASH, randomness)", - // meta.query_advice(bytecode_table.code_hash, Rotation::cur()), - // Expression::Constant(keccak(&[], randomness)), - // ); - // }); + // TODO: the following does not match + let empty_hash = RandomLinearCombination::::random_linear_combine_expr( + EMPTY_HASH_LE.map(|v| Expression::Constant(F::from(v as u64))), + &core::array::from_fn::, 32, _>(|_| challenges.evm_word()), + ); + + cb.require_equal( + "assert cur.hash == EMPTY_HASH", + meta.query_advice(bytecode_table.code_hash, Rotation::cur()), + empty_hash, + ); cb.gate(and::expr(vec![ meta.query_fixed(q_enable, Rotation::cur()), @@ -447,6 +448,10 @@ impl BytecodeCircuitConfig { last_row_offset ); + let empty_hash = challenges.evm_word().map(|challenge| { + RandomLinearCombination::::random_linear_combine(*EMPTY_HASH_LE, challenge) + }); + layouter.assign_region( || "assign bytecode", |mut region| { @@ -457,6 +462,7 @@ impl BytecodeCircuitConfig { bytecode, challenges, &push_data_left_is_zero_chip, + empty_hash, &mut offset, last_row_offset, fail_fast, @@ -468,6 +474,7 @@ impl BytecodeCircuitConfig { self.set_padding_row( &mut region, &push_data_left_is_zero_chip, + empty_hash, idx, last_row_offset, )?; @@ -484,6 +491,7 @@ impl BytecodeCircuitConfig { bytecode: &UnrolledBytecode, challenges: &Challenges>, push_rindex_is_zero_chip: &IsZeroChip, + empty_hash: Value, offset: &mut usize, last_row_offset: usize, fail_fast: bool, @@ -571,7 +579,13 @@ impl BytecodeCircuitConfig { push_data_left = next_push_data_left } if *offset == last_row_offset { - self.set_padding_row(region, push_rindex_is_zero_chip, *offset, last_row_offset)?; + self.set_padding_row( + region, + push_rindex_is_zero_chip, + empty_hash, + *offset, + last_row_offset, + )?; } } @@ -582,6 +596,7 @@ impl BytecodeCircuitConfig { &self, region: &mut Region<'_, F>, push_data_left_is_zero_chip: &IsZeroChip, + empty_hash: Value, offset: usize, last_row_offset: usize, ) -> Result<(), Error> { @@ -591,7 +606,7 @@ impl BytecodeCircuitConfig { offset, offset < last_row_offset, offset == last_row_offset, - Value::known(F::zero()), + empty_hash, F::from(BytecodeFieldTag::Header as u64), F::zero(), F::zero(), From 323d2816fca4e1ddef4ad90de3aa5a9eba17400f Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Fri, 13 Jan 2023 17:35:12 +0700 Subject: [PATCH 08/18] Rename test to avoid confusion --- zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index bdc320f712..b1a62ac2d0 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -964,7 +964,7 @@ mod tests { } #[test] - fn bytecode_full_last_row() { + fn bytecode_last_row_with_byte() { let k = 9; // Last row must be a padding row, so we have one row less for actual bytecode test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 7])], false); From 7746689ac62c5f689a18cf1b6523208ff7afc8f0 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Sun, 15 Jan 2023 19:08:23 +0700 Subject: [PATCH 09/18] Match empty hash --- zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index b1a62ac2d0..c14bde88a9 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -1,6 +1,7 @@ use crate::{ evm_circuit::util::{ - and, constraint_builder::BaseConstraintBuilder, not, or, select, RandomLinearCombination, + and, constraint_builder::BaseConstraintBuilder, not, or, rlc, select, + RandomLinearCombination, }, table::{BytecodeFieldTag, BytecodeTable, DynamicTableColumns, KeccakTable}, util::{Challenges, Expr, SubCircuit, SubCircuitConfig}, @@ -18,6 +19,7 @@ use halo2_proofs::{ }; use keccak256::{plain::Keccak, EMPTY_HASH_LE}; use log::trace; +use rand::seq::index; use std::vec; use super::param::PUSH_TABLE_WIDTH; @@ -229,10 +231,9 @@ impl SubCircuitConfig for BytecodeCircuitConfig { meta.query_advice(length, Rotation::cur()), ); - // TODO: the following does not match let empty_hash = RandomLinearCombination::::random_linear_combine_expr( EMPTY_HASH_LE.map(|v| Expression::Constant(F::from(v as u64))), - &core::array::from_fn::, 32, _>(|_| challenges.evm_word()), + &challenges.evm_word_powers_of_randomness::<32>(), ); cb.require_equal( From f146022c7e4212cfb49c101d1972c9eb44361c84 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Sun, 15 Jan 2023 19:19:16 +0700 Subject: [PATCH 10/18] Clippy --- zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index c14bde88a9..fdc5d19942 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -1,7 +1,6 @@ use crate::{ evm_circuit::util::{ - and, constraint_builder::BaseConstraintBuilder, not, or, rlc, select, - RandomLinearCombination, + and, constraint_builder::BaseConstraintBuilder, not, or, select, RandomLinearCombination, }, table::{BytecodeFieldTag, BytecodeTable, DynamicTableColumns, KeccakTable}, util::{Challenges, Expr, SubCircuit, SubCircuitConfig}, @@ -19,7 +18,6 @@ use halo2_proofs::{ }; use keccak256::{plain::Keccak, EMPTY_HASH_LE}; use log::trace; -use rand::seq::index; use std::vec; use super::param::PUSH_TABLE_WIDTH; From bd5296582c33a23bc2366d2f464898d48545a6d7 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Sun, 15 Jan 2023 19:40:48 +0700 Subject: [PATCH 11/18] Refactor location of bytecode circuit --- circuit-benchmarks/src/bytecode_circuit.rs | 6 +- .../src/integration_test_circuits.rs | 2 +- zkevm-circuits/src/bytecode_circuit.rs | 2 + .../src/bytecode_circuit/bytecode_unroller.rs | 1077 +---------------- .../src/bytecode_circuit/circuit.rs | 1041 ++++++++++++++++ zkevm-circuits/src/bytecode_circuit/dev.rs | 5 +- zkevm-circuits/src/evm_circuit/util.rs | 7 +- zkevm-circuits/src/super_circuit.rs | 2 +- zkevm-circuits/src/util.rs | 22 +- 9 files changed, 1086 insertions(+), 1078 deletions(-) create mode 100644 zkevm-circuits/src/bytecode_circuit/circuit.rs diff --git a/circuit-benchmarks/src/bytecode_circuit.rs b/circuit-benchmarks/src/bytecode_circuit.rs index bb6e0e9de3..f4487db60c 100644 --- a/circuit-benchmarks/src/bytecode_circuit.rs +++ b/circuit-benchmarks/src/bytecode_circuit.rs @@ -19,9 +19,9 @@ mod tests { use rand::SeedableRng; use rand_xorshift::XorShiftRng; use std::env::var; - use zkevm_circuits::bytecode_circuit::bytecode_unroller::{ - unroll, BytecodeCircuit, UnrolledBytecode, - }; + use zkevm_circuits::bytecode_circuit::bytecode_unroller::{unroll, UnrolledBytecode}; + + use zkevm_circuits::bytecode_circuit::circuit::BytecodeCircuit; #[cfg_attr(not(feature = "benches"), ignore)] #[test] diff --git a/integration-tests/src/integration_test_circuits.rs b/integration-tests/src/integration_test_circuits.rs index 2faa9a49b4..efb15fc6ae 100644 --- a/integration-tests/src/integration_test_circuits.rs +++ b/integration-tests/src/integration_test_circuits.rs @@ -23,7 +23,7 @@ use rand_core::RngCore; use rand_xorshift::XorShiftRng; use std::collections::HashMap; use std::sync::Mutex; -use zkevm_circuits::bytecode_circuit::bytecode_unroller::BytecodeCircuit; +use zkevm_circuits::bytecode_circuit::circuit::BytecodeCircuit; use zkevm_circuits::copy_circuit::CopyCircuit; use zkevm_circuits::evm_circuit::test::{get_test_degree, get_test_instance}; use zkevm_circuits::evm_circuit::{test::get_test_cicuit_from_block, witness::block_convert}; diff --git a/zkevm-circuits/src/bytecode_circuit.rs b/zkevm-circuits/src/bytecode_circuit.rs index ff10723438..a9db23040f 100644 --- a/zkevm-circuits/src/bytecode_circuit.rs +++ b/zkevm-circuits/src/bytecode_circuit.rs @@ -2,6 +2,8 @@ /// Bytecode unroller pub mod bytecode_unroller; +/// Bytecode circuit +pub mod circuit; /// Bytecode circuit tester #[cfg(any(feature = "test", test))] pub mod dev; diff --git a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs index fdc5d19942..e032461a1c 100644 --- a/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs +++ b/zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs @@ -1,736 +1,25 @@ use crate::{ - evm_circuit::util::{ - and, constraint_builder::BaseConstraintBuilder, not, or, select, RandomLinearCombination, - }, - table::{BytecodeFieldTag, BytecodeTable, DynamicTableColumns, KeccakTable}, - util::{Challenges, Expr, SubCircuit, SubCircuitConfig}, - witness, + table::BytecodeFieldTag, + util::{get_push_size, keccak}, }; -use bus_mapping::evm::OpcodeId; -use eth_types::{Field, ToLittleEndian, Word}; -use gadgets::is_zero::{IsZeroChip, IsZeroConfig, IsZeroInstruction}; -use halo2_proofs::{ - circuit::{Layouter, Region, Value}, - plonk::{ - Advice, Column, ConstraintSystem, Error, Expression, Fixed, SecondPhase, VirtualCells, - }, - poly::Rotation, -}; -use keccak256::{plain::Keccak, EMPTY_HASH_LE}; -use log::trace; +use eth_types::{Field, Word}; use std::vec; -use super::param::PUSH_TABLE_WIDTH; /// Public data for the bytecode #[derive(Clone, Debug, PartialEq)] pub(crate) struct BytecodeRow { - code_hash: Word, - tag: F, - index: F, - is_code: F, - value: F, + pub(crate) code_hash: Word, + pub(crate) tag: F, + pub(crate) index: F, + pub(crate) is_code: F, + pub(crate) value: F, } /// Unrolled bytecode #[derive(Clone, Debug, PartialEq)] pub struct UnrolledBytecode { pub(crate) bytes: Vec, - rows: Vec>, -} - -#[derive(Clone, Debug)] -/// Bytecode circuit configuration -pub struct BytecodeCircuitConfig { - minimum_rows: usize, - q_enable: Column, - q_first: Column, - q_last: Column, - bytecode_table: BytecodeTable, - push_data_left: Column, - value_rlc: Column, - length: Column, - push_data_size: Column, - push_data_left_inv: Column, - push_data_left_is_zero: IsZeroConfig, - push_table: [Column; PUSH_TABLE_WIDTH], - // External tables - pub(crate) keccak_table: KeccakTable, -} - -/// Circuit configuration arguments -pub struct BytecodeCircuitConfigArgs { - /// BytecodeTable - pub bytecode_table: BytecodeTable, - /// KeccakTable - pub keccak_table: KeccakTable, - /// Challenges - pub challenges: Challenges>, -} - -impl SubCircuitConfig for BytecodeCircuitConfig { - type ConfigArgs = BytecodeCircuitConfigArgs; - - /// Return a new BytecodeCircuitConfig - fn new( - meta: &mut ConstraintSystem, - Self::ConfigArgs { - bytecode_table, - keccak_table, - challenges, - }: Self::ConfigArgs, - ) -> Self { - let q_enable = meta.fixed_column(); - let q_first = meta.fixed_column(); - let q_last = meta.fixed_column(); - let value = bytecode_table.value; - let push_data_left = meta.advice_column(); - let value_rlc = meta.advice_column_in(SecondPhase); - let length = meta.advice_column(); - let push_data_size = meta.advice_column(); - let push_data_left_inv = meta.advice_column(); - let push_table = array_init::array_init(|_| meta.fixed_column()); - - let is_header_to_header = |meta: &mut VirtualCells| { - and::expr(vec![ - not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())), - not::expr(meta.query_advice(bytecode_table.tag, Rotation::next())), - ]) - }; - - let is_header_to_byte = |meta: &mut VirtualCells| { - and::expr(vec![ - not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())), - meta.query_advice(bytecode_table.tag, Rotation::next()), - ]) - }; - - let is_byte_to_header = |meta: &mut VirtualCells| { - and::expr(vec![ - meta.query_advice(bytecode_table.tag, Rotation::cur()), - not::expr(meta.query_advice(bytecode_table.tag, Rotation::next())), - ]) - }; - - let is_byte_to_byte = |meta: &mut VirtualCells| { - and::expr(vec![ - meta.query_advice(bytecode_table.tag, Rotation::cur()), - meta.query_advice(bytecode_table.tag, Rotation::next()), - ]) - }; - - let is_header = |meta: &mut VirtualCells| { - not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())) - }; - - let is_byte = - |meta: &mut VirtualCells| meta.query_advice(bytecode_table.tag, Rotation::cur()); - - // A byte is an opcode when `push_data_left == 0` on the current row, - // else it's push data. - let push_data_left_is_zero = IsZeroChip::configure( - meta, - |meta| meta.query_fixed(q_enable, Rotation::cur()), - |meta| meta.query_advice(push_data_left, Rotation::cur()), - push_data_left_inv, - ); - - // When q_first || q_last -> - // assert cur.tag == Header - meta.create_gate("first and last row", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_zero( - "cur.tag == Header", - meta.query_advice(bytecode_table.tag, Rotation::cur()), - ); - - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - or::expr(vec![ - meta.query_fixed(q_first, Rotation::cur()), - meta.query_fixed(q_last, Rotation::cur()), - ]), - ])) - }); - - // When is_header -> - // assert cur.index == 0 - // assert cur.value == cur.length - meta.create_gate("Header row", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_zero( - "cur.index == 0", - meta.query_advice(bytecode_table.index, Rotation::cur()), - ); - - cb.require_equal( - "cur.value == cur.length", - meta.query_advice(bytecode_table.value, Rotation::cur()), - meta.query_advice(length, Rotation::cur()), - ); - - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_last, Rotation::cur())), - is_header(meta), - ])) - }); - - // When is_byte -> - // assert push_data_size_table_lookup(cur.value, cur.push_data_size) - // assert cur.is_code == (cur.push_data_left == 0) - meta.create_gate("Byte row", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_equal( - "cur.is_code == (cur.push_data_left == 0)", - meta.query_advice(bytecode_table.is_code, Rotation::cur()), - push_data_left_is_zero.clone().is_zero_expression, - ); - - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_last, Rotation::cur())), - is_byte(meta), - ])) - }); - meta.lookup_any( - "push_data_size_table_lookup(cur.value, cur.push_data_size)", - |meta| { - let enable = and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_last, Rotation::cur())), - is_byte(meta), - ]); - - let lookup_columns = vec![value, push_data_size]; - - let mut constraints = vec![]; - - for i in 0..PUSH_TABLE_WIDTH { - constraints.push(( - enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), - meta.query_fixed(push_table[i], Rotation::cur()), - )) - } - constraints - }, - ); - - // When is_header_to_header or q_last -> - // assert cur.length == 0 - // assert cur.hash == EMPTY_HASH - meta.create_gate("Header to header row", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_zero( - "cur.length == 0", - meta.query_advice(length, Rotation::cur()), - ); - - let empty_hash = RandomLinearCombination::::random_linear_combine_expr( - EMPTY_HASH_LE.map(|v| Expression::Constant(F::from(v as u64))), - &challenges.evm_word_powers_of_randomness::<32>(), - ); - - cb.require_equal( - "assert cur.hash == EMPTY_HASH", - meta.query_advice(bytecode_table.code_hash, Rotation::cur()), - empty_hash, - ); - - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - or::expr(vec![ - is_header_to_header(meta), - meta.query_fixed(q_last, Rotation::cur()), - ]), - ])) - }); - - // When is_header_to_byte -> - // assert next.length == cur.length - // assert next.index == 0 - // assert next.is_code == 1 - // assert next.hash == cur.hash - // assert next.value_rlc == next.value - meta.create_gate("Header to byte row", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_equal( - "next.length == cur.length", - meta.query_advice(length, Rotation::next()), - meta.query_advice(length, Rotation::cur()), - ); - - cb.require_zero( - "next.index == 0", - meta.query_advice(bytecode_table.index, Rotation::next()), - ); - - cb.require_equal( - "next.is_code == 1", - meta.query_advice(bytecode_table.is_code, Rotation::next()), - 1.expr(), - ); - - cb.require_equal( - "next.hash == cur.hash", - meta.query_advice(bytecode_table.code_hash, Rotation::next()), - meta.query_advice(bytecode_table.code_hash, Rotation::cur()), - ); - - cb.require_equal( - "next.value_rlc == next.value", - meta.query_advice(value_rlc, Rotation::next()), - meta.query_advice(bytecode_table.value, Rotation::next()), - ); - - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_last, Rotation::cur())), - is_header_to_byte(meta), - ])) - }); - - // When is_byte_to_byte -> - // assert next.length == cur.length - // assert next.index == cur.index + 1 - // assert next.hash == cur.hash - // assert next.value_rlc == cur.value_rlc * randomness + next.value - // if cur.is_code: - // assert next.push_data_left == cur.push_data_size - // else: - // assert next.push_data_left == cur.push_data_left - 1 - meta.create_gate("Byte to Byte row", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_equal( - "next.length == cur.length", - meta.query_advice(length, Rotation::next()), - meta.query_advice(length, Rotation::cur()), - ); - - cb.require_equal( - "next.index == cur.index + 1", - meta.query_advice(bytecode_table.index, Rotation::next()), - meta.query_advice(bytecode_table.index, Rotation::cur()) + 1.expr(), - ); - - cb.require_equal( - "next.hash == cur.hash", - meta.query_advice(bytecode_table.code_hash, Rotation::next()), - meta.query_advice(bytecode_table.code_hash, Rotation::cur()), - ); - - // TODO: check this - cb.require_equal( - "next.value_rlc == cur.value_rlc * randomness + next.value", - meta.query_advice(value_rlc, Rotation::next()), - meta.query_advice(value_rlc, Rotation::cur()) * challenges.keccak_input() - + meta.query_advice(value, Rotation::next()), - ); - - cb.require_equal( - "next.push_data_left == cur.is_code ? cur.push_data_size : cur.push_data_left - 1", - meta.query_advice(push_data_left, Rotation::next()), - select::expr( - meta.query_advice(bytecode_table.is_code, Rotation::cur()), - meta.query_advice(push_data_size, Rotation::cur()), - meta.query_advice(push_data_left, Rotation::cur()) - 1.expr(), - ), - ); - - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_last, Rotation::cur())), - is_byte_to_byte(meta), - ])) - }); - - // When is_byte_to_header -> - // assert cur.index + 1 == cur.length - // assert keccak256_table_lookup(cur.hash, cur.length, cur.value_rlc) - meta.create_gate("Byte to Header row", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_equal( - "cur.index + 1 == cur.length", - meta.query_advice(bytecode_table.index, Rotation::cur()) + 1.expr(), - meta.query_advice(length, Rotation::cur()), - ); - - cb.gate(and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_last, Rotation::cur())), - is_byte_to_header(meta), - ])) - }); - meta.lookup_any( - "keccak256_table_lookup(cur.hash, cur.length, cur.value_rlc)", - |meta| { - let enable = and::expr(vec![ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_fixed(q_last, Rotation::cur())), - is_byte_to_header(meta), - ]); - - let lookup_columns = vec![value_rlc, length, bytecode_table.code_hash]; - - let mut constraints = vec![( - enable.clone(), - meta.query_advice(keccak_table.is_enabled, Rotation::cur()), - )]; - - // TODO: perhaps write this explicitly so it is more readable the matching - // between collumns - for (i, column) in keccak_table.columns().iter().skip(1).enumerate() { - constraints.push(( - enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), - meta.query_advice(*column, Rotation::cur()), - )) - } - - constraints - }, - ); - - BytecodeCircuitConfig { - minimum_rows: meta.minimum_rows(), - q_enable, - q_first, - q_last, - bytecode_table, - push_data_left, - value_rlc, - length, - push_data_size, - push_data_left_inv, - push_data_left_is_zero, - push_table, - keccak_table, - } - } -} - -impl BytecodeCircuitConfig { - pub(crate) fn assign( - &self, - layouter: &mut impl Layouter, - size: usize, - witness: &[UnrolledBytecode], - challenges: &Challenges>, - ) -> Result<(), Error> { - self.assign_internal(layouter, size, witness, challenges, true) - } - - pub(crate) fn assign_internal( - &self, - layouter: &mut impl Layouter, - size: usize, - witness: &[UnrolledBytecode], - challenges: &Challenges>, - fail_fast: bool, - ) -> Result<(), Error> { - let push_data_left_is_zero_chip = - IsZeroChip::construct(self.push_data_left_is_zero.clone()); - - // Subtract the unusable rows from the size - assert!(size > self.minimum_rows); - let last_row_offset = size - self.minimum_rows + 1; - - trace!( - "size: {}, minimum_rows: {}, last_row_offset:{}", - size, - self.minimum_rows, - last_row_offset - ); - - let empty_hash = challenges.evm_word().map(|challenge| { - RandomLinearCombination::::random_linear_combine(*EMPTY_HASH_LE, challenge) - }); - - layouter.assign_region( - || "assign bytecode", - |mut region| { - let mut offset = 0; - for bytecode in witness.iter() { - self.assign_bytecode( - &mut region, - bytecode, - challenges, - &push_data_left_is_zero_chip, - empty_hash, - &mut offset, - last_row_offset, - fail_fast, - )?; - } - - // Padding - for idx in offset..=last_row_offset { - self.set_padding_row( - &mut region, - &push_data_left_is_zero_chip, - empty_hash, - idx, - last_row_offset, - )?; - } - Ok(()) - }, - ) - } - - #[allow(clippy::too_many_arguments)] - fn assign_bytecode( - &self, - region: &mut Region<'_, F>, - bytecode: &UnrolledBytecode, - challenges: &Challenges>, - push_rindex_is_zero_chip: &IsZeroChip, - empty_hash: Value, - offset: &mut usize, - last_row_offset: usize, - fail_fast: bool, - ) -> Result<(), Error> { - // Run over all the bytes - let mut push_data_left = 0; - let mut next_push_data_left = 0; - let mut push_data_size = 0; - let mut value_rlc = challenges.keccak_input().map(|_| F::zero()); - let length = F::from(bytecode.bytes.len() as u64); - - for (idx, row) in bytecode.rows.iter().enumerate() { - if fail_fast && *offset > last_row_offset { - log::error!( - "Bytecode Circuit: offset={} > last_row_offset={}", - offset, - last_row_offset - ); - return Err(Error::Synthesis); - } - - // TODO: why different code_hash for each row? Is this going to produce the same - // result for every row? - let code_hash = challenges.evm_word().map(|challenge| { - RandomLinearCombination::::random_linear_combine( - row.code_hash.to_le_bytes(), - challenge, - ) - }); - - // Track which byte is an opcode and which is push - // data - if idx > 0 { - let is_code = push_data_left == 0; - - push_data_size = get_push_size(row.value.get_lower_128() as u8); - - next_push_data_left = if is_code { - push_data_size - } else { - push_data_left - 1 - }; - - value_rlc - .as_mut() - .zip(challenges.keccak_input()) - .map(|(value_rlc, challenge)| *value_rlc = *value_rlc * challenge + row.value); - } - - // Set the data for this row - if *offset < last_row_offset { - self.set_row( - region, - push_rindex_is_zero_chip, - *offset, - true, - *offset == last_row_offset, - code_hash, - row.tag, - row.index, - row.is_code, - row.value, - push_data_left, - value_rlc, - length, - F::from(push_data_size as u64), - )?; - - trace!( - "bytecode.set_row({}): last:{} h:{:?} t:{:?} i:{:?} c:{:?} v:{:?} pdl:{} rlc:{:?} l:{:?} pds:{:?}", - offset, - *offset == last_row_offset, - code_hash, - row.tag.get_lower_32(), - row.index.get_lower_32(), - row.is_code.get_lower_32(), - row.value.get_lower_32(), - push_data_left, - value_rlc, - length.get_lower_32(), - push_data_size - ); - - *offset += 1; - push_data_left = next_push_data_left - } - if *offset == last_row_offset { - self.set_padding_row( - region, - push_rindex_is_zero_chip, - empty_hash, - *offset, - last_row_offset, - )?; - } - } - - Ok(()) - } - - fn set_padding_row( - &self, - region: &mut Region<'_, F>, - push_data_left_is_zero_chip: &IsZeroChip, - empty_hash: Value, - offset: usize, - last_row_offset: usize, - ) -> Result<(), Error> { - self.set_row( - region, - push_data_left_is_zero_chip, - offset, - offset < last_row_offset, - offset == last_row_offset, - empty_hash, - F::from(BytecodeFieldTag::Header as u64), - F::zero(), - F::zero(), - F::zero(), - 0, - Value::known(F::zero()), - F::zero(), - F::zero(), - ) - } - - #[allow(clippy::too_many_arguments)] - fn set_row( - &self, - region: &mut Region<'_, F>, - push_data_left_is_zero_chip: &IsZeroChip, - offset: usize, - enable: bool, - last: bool, - code_hash: Value, - tag: F, - index: F, - is_code: F, - value: F, - push_data_left: u64, - value_rlc: Value, - length: F, - push_data_size: F, - ) -> Result<(), Error> { - // q_enable - region.assign_fixed( - || format!("assign q_enable {}", offset), - self.q_enable, - offset, - || Value::known(F::from(enable as u64)), - )?; - - // q_first - region.assign_fixed( - || format!("assign q_first {}", offset), - self.q_first, - offset, - || Value::known(F::from((offset == 0) as u64)), - )?; - - // q_last - let q_last_value = if last { F::one() } else { F::zero() }; - region.assign_fixed( - || format!("assign q_first {}", offset), - self.q_last, - offset, - || Value::known(q_last_value), - )?; - - // Advices - for (name, column, value) in [ - ("tag", self.bytecode_table.tag, tag), - ("index", self.bytecode_table.index, index), - ("is_code", self.bytecode_table.is_code, is_code), - ("value", self.bytecode_table.value, value), - ( - "push_data_left", - self.push_data_left, - F::from(push_data_left), - ), - ("length", self.length, length), - ("push_data_size", self.push_data_size, push_data_size), - ] { - region.assign_advice( - || format!("assign {} {}", name, offset), - column, - offset, - || Value::known(value), - )?; - } - for (name, column, value) in [ - ("code_hash", self.bytecode_table.code_hash, code_hash), - ("value_rlc", self.value_rlc, value_rlc), - ] { - region.assign_advice( - || format!("assign {} {}", name, offset), - column, - offset, - || value, - )?; - } - - push_data_left_is_zero_chip.assign( - region, - offset, - Value::known(F::from(push_data_left)), - )?; - - Ok(()) - } - - /// load fixed tables - pub(crate) fn load_aux_tables(&self, layouter: &mut impl Layouter) -> Result<(), Error> { - // push table: BYTE -> NUM_PUSHED: - // [0, OpcodeId::PUSH1] -> 0 - // [OpcodeId::PUSH1, OpcodeId::PUSH32] -> [1..32] - // [OpcodeId::PUSH32, 256] -> 0 - layouter.assign_region( - || "push table", - |mut region| { - for byte in 0usize..256 { - let push_size = get_push_size(byte as u8); - for (name, column, value) in &[ - ("byte", self.push_table[0], byte as u64), - ("push_size", self.push_table[1], push_size), - ] { - region.assign_fixed( - || format!("Push table assign {} {}", name, byte), - *column, - byte, - || Value::known(F::from(*value)), - )?; - } - } - Ok(()) - }, - )?; - - Ok(()) - } + pub(crate) rows: Vec>, } /// Get unrolled bytecode from raw bytes @@ -764,351 +53,3 @@ pub fn unroll(bytes: Vec) -> UnrolledBytecode { } UnrolledBytecode { bytes, rows } } - -fn is_push(byte: u8) -> bool { - OpcodeId::from(byte).is_push() -} - -fn get_push_size(byte: u8) -> u64 { - if is_push(byte) { - byte as u64 - OpcodeId::PUSH1.as_u64() + 1 - } else { - 0u64 - } -} - -fn keccak(msg: &[u8]) -> Word { - let mut keccak = Keccak::default(); - keccak.update(msg); - Word::from_big_endian(keccak.digest().as_slice()) -} - -fn into_words(message: &[u8]) -> Vec { - let words_total = message.len() / 8; - let mut words: Vec = vec![0; words_total]; - - for i in 0..words_total { - let mut word_bits: [u8; 8] = Default::default(); - word_bits.copy_from_slice(&message[i * 8..i * 8 + 8]); - words[i] = u64::from_le_bytes(word_bits); - } - - words -} - -/// BytecodeCircuit -#[derive(Clone, Default, Debug)] -pub struct BytecodeCircuit { - /// Unrolled bytecodes - pub bytecodes: Vec>, - /// Circuit size - pub size: usize, -} - -impl BytecodeCircuit { - /// new BytecodeCircuitTester - pub fn new(bytecodes: Vec>, size: usize) -> Self { - BytecodeCircuit { bytecodes, size } - } - - /// Creates bytecode circuit from block and bytecode_size. - pub fn new_from_block_sized(block: &witness::Block, bytecode_size: usize) -> Self { - let bytecodes: Vec> = block - .bytecodes - .iter() - .map(|(_, b)| unroll(b.bytes.clone())) - .collect(); - Self::new(bytecodes, bytecode_size) - } -} - -impl SubCircuit for BytecodeCircuit { - type Config = BytecodeCircuitConfig; - - fn new_from_block(block: &witness::Block) -> Self { - // TODO: Find a nicer way to add the extra `128`. Is this to account for - // unusable rows? Then it could be calculated like this: - // fn unusable_rows>() -> usize { - // let mut cs = ConstraintSystem::default(); - // C::configure(&mut cs); - // cs.blinding_factors() - // } - let bytecode_size = block.circuits_params.max_bytecode + 128; - Self::new_from_block_sized(block, bytecode_size) - } - - /// Return the minimum number of rows required to prove the block - fn min_num_rows_block(block: &witness::Block) -> usize { - block - .bytecodes - .values() - .map(|bytecode| bytecode.bytes.len() + 1) - .sum() - } - - /// Make the assignments to the TxCircuit - fn synthesize_sub( - &self, - config: &Self::Config, - challenges: &Challenges>, - layouter: &mut impl Layouter, - ) -> Result<(), Error> { - config.load_aux_tables(layouter)?; - config.assign_internal(layouter, self.size, &self.bytecodes, challenges, true) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::bytecode_circuit::dev::test_bytecode_circuit_unrolled; - use eth_types::Bytecode; - use halo2_proofs::halo2curves::bn256::Fr; - - fn get_randomness() -> F { - F::from(123456) - } - - /// Verify unrolling code - #[test] - fn bytecode_unrolling() { - let k = 10; - let mut rows = vec![]; - let mut bytecode = Bytecode::default(); - // First add all non-push bytes, which should all be seen as code - for byte in 0u8..=255u8 { - if !is_push(byte) { - bytecode.write(byte, true); - rows.push(BytecodeRow { - code_hash: Word::zero(), - tag: Fr::from(BytecodeFieldTag::Byte as u64), - index: Fr::from(rows.len() as u64), - is_code: Fr::from(true as u64), - value: Fr::from(byte as u64), - }); - } - } - // Now add the different push ops - for n in 1..=32 { - let data_byte = OpcodeId::PUSH32.as_u8(); - bytecode.push( - n, - Word::from_little_endian(&vec![data_byte; n as usize][..]), - ); - rows.push(BytecodeRow { - code_hash: Word::zero(), - tag: Fr::from(BytecodeFieldTag::Byte as u64), - index: Fr::from(rows.len() as u64), - is_code: Fr::from(true as u64), - value: Fr::from(OpcodeId::PUSH1.as_u64() + ((n - 1) as u64)), - }); - for _ in 0..n { - rows.push(BytecodeRow { - code_hash: Word::zero(), - tag: Fr::from(BytecodeFieldTag::Byte as u64), - index: Fr::from(rows.len() as u64), - is_code: Fr::from(false as u64), - value: Fr::from(data_byte as u64), - }); - } - } - // Set the code_hash of the complete bytecode in the rows - let code_hash = keccak(&bytecode.to_vec()[..]); - for row in rows.iter_mut() { - row.code_hash = code_hash; - } - rows.insert( - 0, - BytecodeRow { - code_hash, - tag: Fr::from(BytecodeFieldTag::Header as u64), - index: Fr::zero(), - is_code: Fr::zero(), - value: Fr::from(bytecode.to_vec().len() as u64), - }, - ); - // Unroll the bytecode - let unrolled = unroll(bytecode.to_vec()); - // Check if the bytecode was unrolled correctly - assert_eq!( - UnrolledBytecode { - bytes: bytecode.to_vec(), - rows, - }, - unrolled, - ); - // Verify the unrolling in the circuit - test_bytecode_circuit_unrolled::(k, vec![unrolled], true); - } - - /// Tests a fully empty circuit - #[test] - fn bytecode_empty() { - let k = 9; - test_bytecode_circuit_unrolled::(k, vec![unroll(vec![])], true); - } - - #[test] - fn bytecode_simple() { - let k = 9; - let bytecodes = vec![unroll(vec![7u8]), unroll(vec![6u8]), unroll(vec![5u8])]; - test_bytecode_circuit_unrolled::(k, bytecodes, true); - } - - /// Tests a fully full circuit - #[test] - fn bytecode_full() { - let k = 9; - test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 8])], true); - } - - #[test] - fn bytecode_last_row_with_byte() { - let k = 9; - // Last row must be a padding row, so we have one row less for actual bytecode - test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 7])], false); - } - - /// Tests a circuit with incomplete bytecode - #[test] - fn bytecode_incomplete() { - let k = 9; - test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) + 1])], false); - } - - /// Tests multiple bytecodes in a single circuit - #[test] - fn bytecode_push() { - let k = 9; - test_bytecode_circuit_unrolled::( - k, - vec![ - unroll(vec![]), - unroll(vec![OpcodeId::PUSH32.as_u8()]), - unroll(vec![OpcodeId::PUSH32.as_u8(), OpcodeId::ADD.as_u8()]), - unroll(vec![OpcodeId::ADD.as_u8(), OpcodeId::PUSH32.as_u8()]), - unroll(vec![ - OpcodeId::ADD.as_u8(), - OpcodeId::PUSH32.as_u8(), - OpcodeId::ADD.as_u8(), - ]), - ], - true, - ); - } - - /// Test invalid code_hash data - #[test] - fn bytecode_invalid_hash_data() { - let k = 9; - let bytecode = vec![8u8, 2, 3, 8, 9, 7, 128]; - let unrolled = unroll(bytecode); - test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); - // Change the code_hash on the first position - { - let mut invalid = unrolled.clone(); - invalid.rows[0].code_hash += Word::one(); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Change the code_hash on another position - { - let mut invalid = unrolled.clone(); - invalid.rows[4].code_hash += Word::one(); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Change all the hashes so it doesn't match the keccak lookup code_hash - { - let mut invalid = unrolled; - for row in invalid.rows.iter_mut() { - row.code_hash = Word::one(); - } - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - } - - /// Test invalid index - #[test] - #[ignore] - fn bytecode_invalid_index() { - let k = 9; - let bytecode = vec![8u8, 2, 3, 8, 9, 7, 128]; - let unrolled = unroll(bytecode); - test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); - // Start the index at 1 - { - let mut invalid = unrolled.clone(); - for row in invalid.rows.iter_mut() { - row.index += Fr::one(); - } - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Don't increment an index once - { - let mut invalid = unrolled; - invalid.rows.last_mut().unwrap().index -= Fr::one(); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - } - - /// Test invalid byte data - #[test] - fn bytecode_invalid_byte_data() { - let k = 9; - let bytecode = vec![8u8, 2, 3, 8, 9, 7, 128]; - let unrolled = unroll(bytecode); - test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); - // Change the first byte - { - let mut invalid = unrolled.clone(); - invalid.rows[1].value = Fr::from(9u64); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Change a byte on another position - { - let mut invalid = unrolled.clone(); - invalid.rows[5].value = Fr::from(6u64); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Set a byte value out of range - { - let mut invalid = unrolled; - invalid.rows[3].value = Fr::from(256u64); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - } - - /// Test invalid is_code data - #[test] - fn bytecode_invalid_is_code() { - let k = 9; - let bytecode = vec![ - OpcodeId::ADD.as_u8(), - OpcodeId::PUSH1.as_u8(), - OpcodeId::PUSH1.as_u8(), - OpcodeId::SUB.as_u8(), - OpcodeId::PUSH7.as_u8(), - OpcodeId::ADD.as_u8(), - OpcodeId::PUSH6.as_u8(), - ]; - let unrolled = unroll(bytecode); - test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); - // Mark the 3rd byte as code (is push data from the first PUSH1) - { - let mut invalid = unrolled.clone(); - invalid.rows[3].is_code = Fr::one(); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Mark the 4rd byte as data (is code) - { - let mut invalid = unrolled.clone(); - invalid.rows[4].is_code = Fr::zero(); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Mark the 7th byte as code (is data for the PUSH7) - { - let mut invalid = unrolled; - invalid.rows[7].is_code = Fr::one(); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - } -} diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs new file mode 100644 index 0000000000..45f27d5262 --- /dev/null +++ b/zkevm-circuits/src/bytecode_circuit/circuit.rs @@ -0,0 +1,1041 @@ +use crate::{ + evm_circuit::util::{ + and, constraint_builder::BaseConstraintBuilder, not, or, select, RandomLinearCombination, + }, + table::{BytecodeFieldTag, BytecodeTable, DynamicTableColumns, KeccakTable}, + util::{get_push_size, Challenges, Expr, SubCircuit, SubCircuitConfig}, + witness, +}; +use eth_types::{Field, ToLittleEndian}; +use gadgets::is_zero::{IsZeroChip, IsZeroConfig, IsZeroInstruction}; +use halo2_proofs::{ + circuit::{Layouter, Region, Value}, + plonk::{ + Advice, Column, ConstraintSystem, Error, Expression, Fixed, SecondPhase, VirtualCells, + }, + poly::Rotation, +}; +use keccak256::EMPTY_HASH_LE; +use log::trace; +use std::vec; + +use super::{ + bytecode_unroller::{unroll, UnrolledBytecode}, + param::PUSH_TABLE_WIDTH, +}; + +#[derive(Clone, Debug)] +/// Bytecode circuit configuration +pub struct BytecodeCircuitConfig { + minimum_rows: usize, + q_enable: Column, + q_first: Column, + q_last: Column, + bytecode_table: BytecodeTable, + push_data_left: Column, + value_rlc: Column, + length: Column, + push_data_size: Column, + push_data_left_inv: Column, + push_data_left_is_zero: IsZeroConfig, + push_table: [Column; PUSH_TABLE_WIDTH], + // External tables + pub(crate) keccak_table: KeccakTable, +} + +/// Circuit configuration arguments +pub struct BytecodeCircuitConfigArgs { + /// BytecodeTable + pub bytecode_table: BytecodeTable, + /// KeccakTable + pub keccak_table: KeccakTable, + /// Challenges + pub challenges: Challenges>, +} + +impl SubCircuitConfig for BytecodeCircuitConfig { + type ConfigArgs = BytecodeCircuitConfigArgs; + + /// Return a new BytecodeCircuitConfig + fn new( + meta: &mut ConstraintSystem, + Self::ConfigArgs { + bytecode_table, + keccak_table, + challenges, + }: Self::ConfigArgs, + ) -> Self { + let q_enable = meta.fixed_column(); + let q_first = meta.fixed_column(); + let q_last = meta.fixed_column(); + let value = bytecode_table.value; + let push_data_left = meta.advice_column(); + let value_rlc = meta.advice_column_in(SecondPhase); + let length = meta.advice_column(); + let push_data_size = meta.advice_column(); + let push_data_left_inv = meta.advice_column(); + let push_table = array_init::array_init(|_| meta.fixed_column()); + + let is_header_to_header = |meta: &mut VirtualCells| { + and::expr(vec![ + not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())), + not::expr(meta.query_advice(bytecode_table.tag, Rotation::next())), + ]) + }; + + let is_header_to_byte = |meta: &mut VirtualCells| { + and::expr(vec![ + not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())), + meta.query_advice(bytecode_table.tag, Rotation::next()), + ]) + }; + + let is_byte_to_header = |meta: &mut VirtualCells| { + and::expr(vec![ + meta.query_advice(bytecode_table.tag, Rotation::cur()), + not::expr(meta.query_advice(bytecode_table.tag, Rotation::next())), + ]) + }; + + let is_byte_to_byte = |meta: &mut VirtualCells| { + and::expr(vec![ + meta.query_advice(bytecode_table.tag, Rotation::cur()), + meta.query_advice(bytecode_table.tag, Rotation::next()), + ]) + }; + + let is_header = |meta: &mut VirtualCells| { + not::expr(meta.query_advice(bytecode_table.tag, Rotation::cur())) + }; + + let is_byte = + |meta: &mut VirtualCells| meta.query_advice(bytecode_table.tag, Rotation::cur()); + + // A byte is an opcode when `push_data_left == 0` on the current row, + // else it's push data. + let push_data_left_is_zero = IsZeroChip::configure( + meta, + |meta| meta.query_fixed(q_enable, Rotation::cur()), + |meta| meta.query_advice(push_data_left, Rotation::cur()), + push_data_left_inv, + ); + + // When q_first || q_last -> + // assert cur.tag == Header + meta.create_gate("first and last row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_zero( + "cur.tag == Header", + meta.query_advice(bytecode_table.tag, Rotation::cur()), + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + or::expr(vec![ + meta.query_fixed(q_first, Rotation::cur()), + meta.query_fixed(q_last, Rotation::cur()), + ]), + ])) + }); + + // When is_header -> + // assert cur.index == 0 + // assert cur.value == cur.length + meta.create_gate("Header row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_zero( + "cur.index == 0", + meta.query_advice(bytecode_table.index, Rotation::cur()), + ); + + cb.require_equal( + "cur.value == cur.length", + meta.query_advice(bytecode_table.value, Rotation::cur()), + meta.query_advice(length, Rotation::cur()), + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_header(meta), + ])) + }); + + // When is_byte -> + // assert push_data_size_table_lookup(cur.value, cur.push_data_size) + // assert cur.is_code == (cur.push_data_left == 0) + meta.create_gate("Byte row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_equal( + "cur.is_code == (cur.push_data_left == 0)", + meta.query_advice(bytecode_table.is_code, Rotation::cur()), + push_data_left_is_zero.clone().is_zero_expression, + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte(meta), + ])) + }); + meta.lookup_any( + "push_data_size_table_lookup(cur.value, cur.push_data_size)", + |meta| { + let enable = and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte(meta), + ]); + + let lookup_columns = vec![value, push_data_size]; + + let mut constraints = vec![]; + + for i in 0..PUSH_TABLE_WIDTH { + constraints.push(( + enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), + meta.query_fixed(push_table[i], Rotation::cur()), + )) + } + constraints + }, + ); + + // When is_header_to_header or q_last -> + // assert cur.length == 0 + // assert cur.hash == EMPTY_HASH + meta.create_gate("Header to header row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_zero( + "cur.length == 0", + meta.query_advice(length, Rotation::cur()), + ); + + let empty_hash = RandomLinearCombination::::random_linear_combine_expr( + EMPTY_HASH_LE.map(|v| Expression::Constant(F::from(v as u64))), + &challenges.evm_word_powers_of_randomness::<32>(), + ); + + cb.require_equal( + "assert cur.hash == EMPTY_HASH", + meta.query_advice(bytecode_table.code_hash, Rotation::cur()), + empty_hash, + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + or::expr(vec![ + is_header_to_header(meta), + meta.query_fixed(q_last, Rotation::cur()), + ]), + ])) + }); + + // When is_header_to_byte -> + // assert next.length == cur.length + // assert next.index == 0 + // assert next.is_code == 1 + // assert next.hash == cur.hash + // assert next.value_rlc == next.value + meta.create_gate("Header to byte row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_equal( + "next.length == cur.length", + meta.query_advice(length, Rotation::next()), + meta.query_advice(length, Rotation::cur()), + ); + + cb.require_zero( + "next.index == 0", + meta.query_advice(bytecode_table.index, Rotation::next()), + ); + + cb.require_equal( + "next.is_code == 1", + meta.query_advice(bytecode_table.is_code, Rotation::next()), + 1.expr(), + ); + + cb.require_equal( + "next.hash == cur.hash", + meta.query_advice(bytecode_table.code_hash, Rotation::next()), + meta.query_advice(bytecode_table.code_hash, Rotation::cur()), + ); + + cb.require_equal( + "next.value_rlc == next.value", + meta.query_advice(value_rlc, Rotation::next()), + meta.query_advice(bytecode_table.value, Rotation::next()), + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_header_to_byte(meta), + ])) + }); + + // When is_byte_to_byte -> + // assert next.length == cur.length + // assert next.index == cur.index + 1 + // assert next.hash == cur.hash + // assert next.value_rlc == cur.value_rlc * randomness + next.value + // if cur.is_code: + // assert next.push_data_left == cur.push_data_size + // else: + // assert next.push_data_left == cur.push_data_left - 1 + meta.create_gate("Byte to Byte row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_equal( + "next.length == cur.length", + meta.query_advice(length, Rotation::next()), + meta.query_advice(length, Rotation::cur()), + ); + + cb.require_equal( + "next.index == cur.index + 1", + meta.query_advice(bytecode_table.index, Rotation::next()), + meta.query_advice(bytecode_table.index, Rotation::cur()) + 1.expr(), + ); + + cb.require_equal( + "next.hash == cur.hash", + meta.query_advice(bytecode_table.code_hash, Rotation::next()), + meta.query_advice(bytecode_table.code_hash, Rotation::cur()), + ); + + // TODO: check this + cb.require_equal( + "next.value_rlc == cur.value_rlc * randomness + next.value", + meta.query_advice(value_rlc, Rotation::next()), + meta.query_advice(value_rlc, Rotation::cur()) * challenges.keccak_input() + + meta.query_advice(value, Rotation::next()), + ); + + cb.require_equal( + "next.push_data_left == cur.is_code ? cur.push_data_size : cur.push_data_left - 1", + meta.query_advice(push_data_left, Rotation::next()), + select::expr( + meta.query_advice(bytecode_table.is_code, Rotation::cur()), + meta.query_advice(push_data_size, Rotation::cur()), + meta.query_advice(push_data_left, Rotation::cur()) - 1.expr(), + ), + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte_to_byte(meta), + ])) + }); + + // When is_byte_to_header -> + // assert cur.index + 1 == cur.length + // assert keccak256_table_lookup(cur.hash, cur.length, cur.value_rlc) + meta.create_gate("Byte to Header row", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_equal( + "cur.index + 1 == cur.length", + meta.query_advice(bytecode_table.index, Rotation::cur()) + 1.expr(), + meta.query_advice(length, Rotation::cur()), + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte_to_header(meta), + ])) + }); + meta.lookup_any( + "keccak256_table_lookup(cur.hash, cur.length, cur.value_rlc)", + |meta| { + let enable = and::expr(vec![ + meta.query_fixed(q_enable, Rotation::cur()), + not::expr(meta.query_fixed(q_last, Rotation::cur())), + is_byte_to_header(meta), + ]); + + let lookup_columns = vec![value_rlc, length, bytecode_table.code_hash]; + + let mut constraints = vec![( + enable.clone(), + meta.query_advice(keccak_table.is_enabled, Rotation::cur()), + )]; + + // TODO: perhaps write this explicitly so it is more readable the matching + // between collumns + for (i, column) in keccak_table.columns().iter().skip(1).enumerate() { + constraints.push(( + enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), + meta.query_advice(*column, Rotation::cur()), + )) + } + + constraints + }, + ); + + BytecodeCircuitConfig { + minimum_rows: meta.minimum_rows(), + q_enable, + q_first, + q_last, + bytecode_table, + push_data_left, + value_rlc, + length, + push_data_size, + push_data_left_inv, + push_data_left_is_zero, + push_table, + keccak_table, + } + } +} + +impl BytecodeCircuitConfig { + pub(crate) fn assign( + &self, + layouter: &mut impl Layouter, + size: usize, + witness: &[UnrolledBytecode], + challenges: &Challenges>, + ) -> Result<(), Error> { + self.assign_internal(layouter, size, witness, challenges, true) + } + + pub(crate) fn assign_internal( + &self, + layouter: &mut impl Layouter, + size: usize, + witness: &[UnrolledBytecode], + challenges: &Challenges>, + fail_fast: bool, + ) -> Result<(), Error> { + let push_data_left_is_zero_chip = + IsZeroChip::construct(self.push_data_left_is_zero.clone()); + + // Subtract the unusable rows from the size + assert!(size > self.minimum_rows); + let last_row_offset = size - self.minimum_rows + 1; + + trace!( + "size: {}, minimum_rows: {}, last_row_offset:{}", + size, + self.minimum_rows, + last_row_offset + ); + + let empty_hash = challenges.evm_word().map(|challenge| { + RandomLinearCombination::::random_linear_combine(*EMPTY_HASH_LE, challenge) + }); + + layouter.assign_region( + || "assign bytecode", + |mut region| { + let mut offset = 0; + for bytecode in witness.iter() { + self.assign_bytecode( + &mut region, + bytecode, + challenges, + &push_data_left_is_zero_chip, + empty_hash, + &mut offset, + last_row_offset, + fail_fast, + )?; + } + + // Padding + for idx in offset..=last_row_offset { + self.set_padding_row( + &mut region, + &push_data_left_is_zero_chip, + empty_hash, + idx, + last_row_offset, + )?; + } + Ok(()) + }, + ) + } + + #[allow(clippy::too_many_arguments)] + fn assign_bytecode( + &self, + region: &mut Region<'_, F>, + bytecode: &UnrolledBytecode, + challenges: &Challenges>, + push_rindex_is_zero_chip: &IsZeroChip, + empty_hash: Value, + offset: &mut usize, + last_row_offset: usize, + fail_fast: bool, + ) -> Result<(), Error> { + // Run over all the bytes + let mut push_data_left = 0; + let mut next_push_data_left = 0; + let mut push_data_size = 0; + let mut value_rlc = challenges.keccak_input().map(|_| F::zero()); + let length = F::from(bytecode.bytes.len() as u64); + + for (idx, row) in bytecode.rows.iter().enumerate() { + if fail_fast && *offset > last_row_offset { + log::error!( + "Bytecode Circuit: offset={} > last_row_offset={}", + offset, + last_row_offset + ); + return Err(Error::Synthesis); + } + + // TODO: why different code_hash for each row? Is this going to produce the same + // result for every row? + let code_hash = challenges.evm_word().map(|challenge| { + RandomLinearCombination::::random_linear_combine( + row.code_hash.to_le_bytes(), + challenge, + ) + }); + + // Track which byte is an opcode and which is push + // data + if idx > 0 { + let is_code = push_data_left == 0; + + push_data_size = get_push_size(row.value.get_lower_128() as u8); + + next_push_data_left = if is_code { + push_data_size + } else { + push_data_left - 1 + }; + + value_rlc + .as_mut() + .zip(challenges.keccak_input()) + .map(|(value_rlc, challenge)| *value_rlc = *value_rlc * challenge + row.value); + } + + // Set the data for this row + if *offset < last_row_offset { + self.set_row( + region, + push_rindex_is_zero_chip, + *offset, + true, + *offset == last_row_offset, + code_hash, + row.tag, + row.index, + row.is_code, + row.value, + push_data_left, + value_rlc, + length, + F::from(push_data_size as u64), + )?; + + trace!( + "bytecode.set_row({}): last:{} h:{:?} t:{:?} i:{:?} c:{:?} v:{:?} pdl:{} rlc:{:?} l:{:?} pds:{:?}", + offset, + *offset == last_row_offset, + code_hash, + row.tag.get_lower_32(), + row.index.get_lower_32(), + row.is_code.get_lower_32(), + row.value.get_lower_32(), + push_data_left, + value_rlc, + length.get_lower_32(), + push_data_size + ); + + *offset += 1; + push_data_left = next_push_data_left + } + if *offset == last_row_offset { + self.set_padding_row( + region, + push_rindex_is_zero_chip, + empty_hash, + *offset, + last_row_offset, + )?; + } + } + + Ok(()) + } + + fn set_padding_row( + &self, + region: &mut Region<'_, F>, + push_data_left_is_zero_chip: &IsZeroChip, + empty_hash: Value, + offset: usize, + last_row_offset: usize, + ) -> Result<(), Error> { + self.set_row( + region, + push_data_left_is_zero_chip, + offset, + offset < last_row_offset, + offset == last_row_offset, + empty_hash, + F::from(BytecodeFieldTag::Header as u64), + F::zero(), + F::zero(), + F::zero(), + 0, + Value::known(F::zero()), + F::zero(), + F::zero(), + ) + } + + #[allow(clippy::too_many_arguments)] + fn set_row( + &self, + region: &mut Region<'_, F>, + push_data_left_is_zero_chip: &IsZeroChip, + offset: usize, + enable: bool, + last: bool, + code_hash: Value, + tag: F, + index: F, + is_code: F, + value: F, + push_data_left: u64, + value_rlc: Value, + length: F, + push_data_size: F, + ) -> Result<(), Error> { + // q_enable + region.assign_fixed( + || format!("assign q_enable {}", offset), + self.q_enable, + offset, + || Value::known(F::from(enable as u64)), + )?; + + // q_first + region.assign_fixed( + || format!("assign q_first {}", offset), + self.q_first, + offset, + || Value::known(F::from((offset == 0) as u64)), + )?; + + // q_last + let q_last_value = if last { F::one() } else { F::zero() }; + region.assign_fixed( + || format!("assign q_first {}", offset), + self.q_last, + offset, + || Value::known(q_last_value), + )?; + + // Advices + for (name, column, value) in [ + ("tag", self.bytecode_table.tag, tag), + ("index", self.bytecode_table.index, index), + ("is_code", self.bytecode_table.is_code, is_code), + ("value", self.bytecode_table.value, value), + ( + "push_data_left", + self.push_data_left, + F::from(push_data_left), + ), + ("length", self.length, length), + ("push_data_size", self.push_data_size, push_data_size), + ] { + region.assign_advice( + || format!("assign {} {}", name, offset), + column, + offset, + || Value::known(value), + )?; + } + for (name, column, value) in [ + ("code_hash", self.bytecode_table.code_hash, code_hash), + ("value_rlc", self.value_rlc, value_rlc), + ] { + region.assign_advice( + || format!("assign {} {}", name, offset), + column, + offset, + || value, + )?; + } + + push_data_left_is_zero_chip.assign( + region, + offset, + Value::known(F::from(push_data_left)), + )?; + + Ok(()) + } + + /// load fixed tables + pub(crate) fn load_aux_tables(&self, layouter: &mut impl Layouter) -> Result<(), Error> { + // push table: BYTE -> NUM_PUSHED: + // [0, OpcodeId::PUSH1] -> 0 + // [OpcodeId::PUSH1, OpcodeId::PUSH32] -> [1..32] + // [OpcodeId::PUSH32, 256] -> 0 + layouter.assign_region( + || "push table", + |mut region| { + for byte in 0usize..256 { + let push_size = get_push_size(byte as u8); + for (name, column, value) in &[ + ("byte", self.push_table[0], byte as u64), + ("push_size", self.push_table[1], push_size), + ] { + region.assign_fixed( + || format!("Push table assign {} {}", name, byte), + *column, + byte, + || Value::known(F::from(*value)), + )?; + } + } + Ok(()) + }, + )?; + + Ok(()) + } +} + +/// BytecodeCircuit +#[derive(Clone, Default, Debug)] +pub struct BytecodeCircuit { + /// Unrolled bytecodes + pub bytecodes: Vec>, + /// Circuit size + pub size: usize, +} + +impl BytecodeCircuit { + /// new BytecodeCircuitTester + pub fn new(bytecodes: Vec>, size: usize) -> Self { + BytecodeCircuit { bytecodes, size } + } + + /// Creates bytecode circuit from block and bytecode_size. + pub fn new_from_block_sized(block: &witness::Block, bytecode_size: usize) -> Self { + let bytecodes: Vec> = block + .bytecodes + .iter() + .map(|(_, b)| unroll(b.bytes.clone())) + .collect(); + Self::new(bytecodes, bytecode_size) + } +} + +impl SubCircuit for BytecodeCircuit { + type Config = BytecodeCircuitConfig; + + fn new_from_block(block: &witness::Block) -> Self { + // TODO: Find a nicer way to add the extra `128`. Is this to account for + // unusable rows? Then it could be calculated like this: + // fn unusable_rows>() -> usize { + // let mut cs = ConstraintSystem::default(); + // C::configure(&mut cs); + // cs.blinding_factors() + // } + let bytecode_size = block.circuits_params.max_bytecode + 128; + Self::new_from_block_sized(block, bytecode_size) + } + + /// Return the minimum number of rows required to prove the block + fn min_num_rows_block(block: &witness::Block) -> usize { + block + .bytecodes + .values() + .map(|bytecode| bytecode.bytes.len() + 1) + .sum() + } + + /// Make the assignments to the TxCircuit + fn synthesize_sub( + &self, + config: &Self::Config, + challenges: &Challenges>, + layouter: &mut impl Layouter, + ) -> Result<(), Error> { + config.load_aux_tables(layouter)?; + config.assign_internal(layouter, self.size, &self.bytecodes, challenges, true) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + bytecode_circuit::{bytecode_unroller::BytecodeRow, dev::test_bytecode_circuit_unrolled}, + util::{is_push, keccak}, + }; + use bus_mapping::evm::OpcodeId; + use eth_types::{Bytecode, Word}; + use halo2_proofs::halo2curves::bn256::Fr; + + fn get_randomness() -> F { + F::from(123456) + } + + /// Verify unrolling code + #[test] + fn bytecode_unrolling() { + let k = 10; + let mut rows = vec![]; + let mut bytecode = Bytecode::default(); + // First add all non-push bytes, which should all be seen as code + for byte in 0u8..=255u8 { + if !is_push(byte) { + bytecode.write(byte, true); + rows.push(BytecodeRow { + code_hash: Word::zero(), + tag: Fr::from(BytecodeFieldTag::Byte as u64), + index: Fr::from(rows.len() as u64), + is_code: Fr::from(true as u64), + value: Fr::from(byte as u64), + }); + } + } + // Now add the different push ops + for n in 1..=32 { + let data_byte = OpcodeId::PUSH32.as_u8(); + bytecode.push( + n, + Word::from_little_endian(&vec![data_byte; n as usize][..]), + ); + rows.push(BytecodeRow { + code_hash: Word::zero(), + tag: Fr::from(BytecodeFieldTag::Byte as u64), + index: Fr::from(rows.len() as u64), + is_code: Fr::from(true as u64), + value: Fr::from(OpcodeId::PUSH1.as_u64() + ((n - 1) as u64)), + }); + for _ in 0..n { + rows.push(BytecodeRow { + code_hash: Word::zero(), + tag: Fr::from(BytecodeFieldTag::Byte as u64), + index: Fr::from(rows.len() as u64), + is_code: Fr::from(false as u64), + value: Fr::from(data_byte as u64), + }); + } + } + // Set the code_hash of the complete bytecode in the rows + let code_hash = keccak(&bytecode.to_vec()[..]); + for row in rows.iter_mut() { + row.code_hash = code_hash; + } + rows.insert( + 0, + BytecodeRow { + code_hash, + tag: Fr::from(BytecodeFieldTag::Header as u64), + index: Fr::zero(), + is_code: Fr::zero(), + value: Fr::from(bytecode.to_vec().len() as u64), + }, + ); + // Unroll the bytecode + let unrolled = unroll(bytecode.to_vec()); + // Check if the bytecode was unrolled correctly + assert_eq!( + UnrolledBytecode { + bytes: bytecode.to_vec(), + rows, + }, + unrolled, + ); + // Verify the unrolling in the circuit + test_bytecode_circuit_unrolled::(k, vec![unrolled], true); + } + + /// Tests a fully empty circuit + #[test] + fn bytecode_empty() { + let k = 9; + test_bytecode_circuit_unrolled::(k, vec![unroll(vec![])], true); + } + + #[test] + fn bytecode_simple() { + let k = 9; + let bytecodes = vec![unroll(vec![7u8]), unroll(vec![6u8]), unroll(vec![5u8])]; + test_bytecode_circuit_unrolled::(k, bytecodes, true); + } + + /// Tests a fully full circuit + #[test] + fn bytecode_full() { + let k = 9; + test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 8])], true); + } + + #[test] + fn bytecode_last_row_with_byte() { + let k = 9; + // Last row must be a padding row, so we have one row less for actual bytecode + test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) - 7])], false); + } + + /// Tests a circuit with incomplete bytecode + #[test] + fn bytecode_incomplete() { + let k = 9; + test_bytecode_circuit_unrolled::(k, vec![unroll(vec![7u8; 2usize.pow(k) + 1])], false); + } + + /// Tests multiple bytecodes in a single circuit + #[test] + fn bytecode_push() { + let k = 9; + test_bytecode_circuit_unrolled::( + k, + vec![ + unroll(vec![]), + unroll(vec![OpcodeId::PUSH32.as_u8()]), + unroll(vec![OpcodeId::PUSH32.as_u8(), OpcodeId::ADD.as_u8()]), + unroll(vec![OpcodeId::ADD.as_u8(), OpcodeId::PUSH32.as_u8()]), + unroll(vec![ + OpcodeId::ADD.as_u8(), + OpcodeId::PUSH32.as_u8(), + OpcodeId::ADD.as_u8(), + ]), + ], + true, + ); + } + + /// Test invalid code_hash data + #[test] + fn bytecode_invalid_hash_data() { + let k = 9; + let bytecode = vec![8u8, 2, 3, 8, 9, 7, 128]; + let unrolled = unroll(bytecode); + test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); + // Change the code_hash on the first position + { + let mut invalid = unrolled.clone(); + invalid.rows[0].code_hash += Word::one(); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + // Change the code_hash on another position + { + let mut invalid = unrolled.clone(); + invalid.rows[4].code_hash += Word::one(); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + // Change all the hashes so it doesn't match the keccak lookup code_hash + { + let mut invalid = unrolled; + for row in invalid.rows.iter_mut() { + row.code_hash = Word::one(); + } + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + } + + /// Test invalid index + #[test] + #[ignore] + fn bytecode_invalid_index() { + let k = 9; + let bytecode = vec![8u8, 2, 3, 8, 9, 7, 128]; + let unrolled = unroll(bytecode); + test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); + // Start the index at 1 + { + let mut invalid = unrolled.clone(); + for row in invalid.rows.iter_mut() { + row.index += Fr::one(); + } + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + // Don't increment an index once + { + let mut invalid = unrolled; + invalid.rows.last_mut().unwrap().index -= Fr::one(); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + } + + /// Test invalid byte data + #[test] + fn bytecode_invalid_byte_data() { + let k = 9; + let bytecode = vec![8u8, 2, 3, 8, 9, 7, 128]; + let unrolled = unroll(bytecode); + test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); + // Change the first byte + { + let mut invalid = unrolled.clone(); + invalid.rows[1].value = Fr::from(9u64); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + // Change a byte on another position + { + let mut invalid = unrolled.clone(); + invalid.rows[5].value = Fr::from(6u64); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + // Set a byte value out of range + { + let mut invalid = unrolled; + invalid.rows[3].value = Fr::from(256u64); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + } + + /// Test invalid is_code data + #[test] + fn bytecode_invalid_is_code() { + let k = 9; + let bytecode = vec![ + OpcodeId::ADD.as_u8(), + OpcodeId::PUSH1.as_u8(), + OpcodeId::PUSH1.as_u8(), + OpcodeId::SUB.as_u8(), + OpcodeId::PUSH7.as_u8(), + OpcodeId::ADD.as_u8(), + OpcodeId::PUSH6.as_u8(), + ]; + let unrolled = unroll(bytecode); + test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); + // Mark the 3rd byte as code (is push data from the first PUSH1) + { + let mut invalid = unrolled.clone(); + invalid.rows[3].is_code = Fr::one(); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + // Mark the 4rd byte as data (is code) + { + let mut invalid = unrolled.clone(); + invalid.rows[4].is_code = Fr::zero(); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + // Mark the 7th byte as code (is data for the PUSH7) + { + let mut invalid = unrolled; + invalid.rows[7].is_code = Fr::one(); + test_bytecode_circuit_unrolled::(k, vec![invalid], false); + } + } +} diff --git a/zkevm-circuits/src/bytecode_circuit/dev.rs b/zkevm-circuits/src/bytecode_circuit/dev.rs index 54c2578648..291cda00e2 100644 --- a/zkevm-circuits/src/bytecode_circuit/dev.rs +++ b/zkevm-circuits/src/bytecode_circuit/dev.rs @@ -1,6 +1,5 @@ -use super::bytecode_unroller::{ - unroll, BytecodeCircuit, BytecodeCircuitConfig, BytecodeCircuitConfigArgs, UnrolledBytecode, -}; +use super::bytecode_unroller::{unroll, UnrolledBytecode}; +use super::circuit::{BytecodeCircuit, BytecodeCircuitConfig, BytecodeCircuitConfigArgs}; use crate::table::{BytecodeTable, KeccakTable}; use crate::util::{Challenges, SubCircuit, SubCircuitConfig}; use eth_types::Field; diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index a979cf3f4b..652c5b9f05 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -476,7 +476,12 @@ pub(crate) mod rlc { expressions: &[E], power_of_randomness: &[E], ) -> Expression { - debug_assert!(expressions.len() <= power_of_randomness.len() + 1); + debug_assert!( + expressions.len() <= power_of_randomness.len() + 1, + "{} <= {}", + expressions.len(), + power_of_randomness.len() + 1 + ); let mut rlc = expressions[0].expr(); for (expression, randomness) in expressions[1..].iter().zip(power_of_randomness.iter()) { diff --git a/zkevm-circuits/src/super_circuit.rs b/zkevm-circuits/src/super_circuit.rs index 6af4a80ba3..600a7715e9 100644 --- a/zkevm-circuits/src/super_circuit.rs +++ b/zkevm-circuits/src/super_circuit.rs @@ -51,7 +51,7 @@ //! - [x] Tx Circuit //! - [ ] MPT Circuit -use crate::bytecode_circuit::bytecode_unroller::{ +use crate::bytecode_circuit::circuit::{ BytecodeCircuit, BytecodeCircuitConfig, BytecodeCircuitConfigArgs, }; use crate::copy_circuit::{CopyCircuit, CopyCircuitConfig, CopyCircuitConfigArgs}; diff --git a/zkevm-circuits/src/util.rs b/zkevm-circuits/src/util.rs index eff206056b..21768a17b2 100644 --- a/zkevm-circuits/src/util.rs +++ b/zkevm-circuits/src/util.rs @@ -1,14 +1,16 @@ //! Common utility traits and functions. +use bus_mapping::evm::OpcodeId; use halo2_proofs::{ arithmetic::FieldExt, circuit::{Layouter, Value}, plonk::{Challenge, ConstraintSystem, Error, Expression, FirstPhase, VirtualCells}, poly::Rotation, }; +use keccak256::plain::Keccak; use crate::table::TxLogFieldTag; use crate::witness; -use eth_types::{Field, ToAddress}; +use eth_types::{Field, ToAddress, Word}; pub use ethers_core::types::{Address, U256}; pub use gadgets::util::Expr; @@ -173,3 +175,21 @@ pub trait SubCircuitConfig { pub fn log2_ceil(n: usize) -> u32 { u32::BITS - (n as u32).leading_zeros() - (n & (n - 1) == 0) as u32 } + +pub(crate) fn keccak(msg: &[u8]) -> Word { + let mut keccak = Keccak::default(); + keccak.update(msg); + Word::from_big_endian(keccak.digest().as_slice()) +} + +pub(crate) fn is_push(byte: u8) -> bool { + OpcodeId::from(byte).is_push() +} + +pub(crate) fn get_push_size(byte: u8) -> u64 { + if is_push(byte) { + byte as u64 - OpcodeId::PUSH1.as_u64() + 1 + } else { + 0u64 + } +} From 5d160a1df5457c07cc2b0061cdb1e4438c4b8f24 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Thu, 19 Jan 2023 04:45:17 +0100 Subject: [PATCH 12/18] Update zkevm-circuits/src/bytecode_circuit/circuit.rs Co-authored-by: Han --- zkevm-circuits/src/bytecode_circuit/circuit.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs index 45f27d5262..7b1f8cf381 100644 --- a/zkevm-circuits/src/bytecode_circuit/circuit.rs +++ b/zkevm-circuits/src/bytecode_circuit/circuit.rs @@ -792,9 +792,6 @@ mod tests { use eth_types::{Bytecode, Word}; use halo2_proofs::halo2curves::bn256::Fr; - fn get_randomness() -> F { - F::from(123456) - } /// Verify unrolling code #[test] From ab644eeb5d3e81b5f77823a0865c9512a3610c59 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Thu, 19 Jan 2023 12:17:20 +0100 Subject: [PATCH 13/18] Rename argument --- zkevm-circuits/src/bytecode_circuit/circuit.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs index 887b67ec46..39dc8774aa 100644 --- a/zkevm-circuits/src/bytecode_circuit/circuit.rs +++ b/zkevm-circuits/src/bytecode_circuit/circuit.rs @@ -475,7 +475,7 @@ impl BytecodeCircuitConfig { region: &mut Region<'_, F>, bytecode: &UnrolledBytecode, challenges: &Challenges>, - push_rindex_is_zero_chip: &IsZeroChip, + push_data_left_is_zero_chip: &IsZeroChip, empty_hash: Value, offset: &mut usize, last_row_offset: usize, @@ -530,7 +530,7 @@ impl BytecodeCircuitConfig { if *offset < last_row_offset { self.set_row( region, - push_rindex_is_zero_chip, + push_data_left_is_zero_chip, *offset, true, *offset == last_row_offset, @@ -566,7 +566,7 @@ impl BytecodeCircuitConfig { if *offset == last_row_offset { self.set_padding_row( region, - push_rindex_is_zero_chip, + push_data_left_is_zero_chip, empty_hash, *offset, last_row_offset, From 65feca9faf20e81823aa2f0afd69fa520409b426 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Thu, 19 Jan 2023 14:29:53 +0100 Subject: [PATCH 14/18] Better wit gen for code_hash column --- .../src/bytecode_circuit/circuit.rs | 38 +++++++------------ zkevm-circuits/src/bytecode_circuit/dev.rs | 3 +- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs index 39dc8774aa..2d5df36df0 100644 --- a/zkevm-circuits/src/bytecode_circuit/circuit.rs +++ b/zkevm-circuits/src/bytecode_circuit/circuit.rs @@ -488,6 +488,15 @@ impl BytecodeCircuitConfig { let mut value_rlc = challenges.keccak_input().map(|_| F::zero()); let length = F::from(bytecode.bytes.len() as u64); + // Code hash with challenge is calculated only using the first row of the + // bytecode (header row), the rest of the code_hash in other rows are ignored. + let code_hash = challenges.evm_word().map(|challenge| { + RandomLinearCombination::::random_linear_combine( + bytecode.rows[0].code_hash.to_le_bytes(), + challenge, + ) + }); + for (idx, row) in bytecode.rows.iter().enumerate() { if fail_fast && *offset > last_row_offset { log::error!( @@ -498,15 +507,6 @@ impl BytecodeCircuitConfig { return Err(Error::Synthesis); } - // TODO: why different code_hash for each row? Is this going to produce the same - // result for every row? - let code_hash = challenges.evm_word().map(|challenge| { - RandomLinearCombination::::random_linear_combine( - row.code_hash.to_le_bytes(), - challenge, - ) - }); - // Track which byte is an opcode and which is push // data if idx > 0 { @@ -930,26 +930,16 @@ mod tests { let bytecode = vec![8u8, 2, 3, 8, 9, 7, 128]; let unrolled = unroll(bytecode); test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); - // Change the code_hash on the first position + // Change the code_hash on the first position (header row) { let mut invalid = unrolled.clone(); invalid.rows[0].code_hash += Word::one(); + trace!("bytecode_invalid_hash_data: Change the code_hash on the first position"); test_bytecode_circuit_unrolled::(k, vec![invalid], false); } - // Change the code_hash on another position - { - let mut invalid = unrolled.clone(); - invalid.rows[4].code_hash += Word::one(); - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } - // Change all the hashes so it doesn't match the keccak lookup code_hash - { - let mut invalid = unrolled; - for row in invalid.rows.iter_mut() { - row.code_hash = Word::one(); - } - test_bytecode_circuit_unrolled::(k, vec![invalid], false); - } + // TODO: other rows code_hash are ignored by the witness generation, to + // test other rows invalid code_hash, we would need to inject an evil + // witness. } /// Test invalid index diff --git a/zkevm-circuits/src/bytecode_circuit/dev.rs b/zkevm-circuits/src/bytecode_circuit/dev.rs index 291cda00e2..a8ac0b55a2 100644 --- a/zkevm-circuits/src/bytecode_circuit/dev.rs +++ b/zkevm-circuits/src/bytecode_circuit/dev.rs @@ -91,5 +91,6 @@ pub fn test_bytecode_circuit_unrolled( error!("{}", failure); } } - assert_eq!(result.is_ok(), success); + let error_msg = if success { "valid" } else { "invalid" }; + assert_eq!(result.is_ok(), success, "proof must be {}", error_msg); } From 4bf2fec084406426147095f64cd6a87feaddee95 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Thu, 19 Jan 2023 14:32:22 +0100 Subject: [PATCH 15/18] Clean up --- zkevm-circuits/src/bytecode_circuit/circuit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs index 2d5df36df0..60744c12e3 100644 --- a/zkevm-circuits/src/bytecode_circuit/circuit.rs +++ b/zkevm-circuits/src/bytecode_circuit/circuit.rs @@ -932,7 +932,7 @@ mod tests { test_bytecode_circuit_unrolled::(k, vec![unrolled.clone()], true); // Change the code_hash on the first position (header row) { - let mut invalid = unrolled.clone(); + let mut invalid = unrolled; invalid.rows[0].code_hash += Word::one(); trace!("bytecode_invalid_hash_data: Change the code_hash on the first position"); test_bytecode_circuit_unrolled::(k, vec![invalid], false); From 3081961f9e086545168bb8459dd398425512ce9c Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Wed, 1 Feb 2023 09:05:08 +0100 Subject: [PATCH 16/18] Update zkevm-circuits/src/bytecode_circuit/circuit.rs Co-authored-by: Chih Cheng Liang --- zkevm-circuits/src/bytecode_circuit/circuit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs index 60744c12e3..9ac94d62f7 100644 --- a/zkevm-circuits/src/bytecode_circuit/circuit.rs +++ b/zkevm-circuits/src/bytecode_circuit/circuit.rs @@ -354,7 +354,7 @@ impl SubCircuitConfig for BytecodeCircuitConfig { ])) }); meta.lookup_any( - "keccak256_table_lookup(cur.hash, cur.length, cur.value_rlc)", + "keccak256_table_lookup(cur.value_rlc, cur.length, cur.hash)", |meta| { let enable = and::expr(vec![ meta.query_fixed(q_enable, Rotation::cur()), From 2136a57425fdaf41c871f649b3b63a2c6e34604c Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Wed, 1 Feb 2023 09:24:45 +0100 Subject: [PATCH 17/18] Matching columns more readable --- zkevm-circuits/src/bytecode_circuit/circuit.rs | 13 +++++-------- zkevm-circuits/src/table.rs | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs index 9ac94d62f7..0d168f5051 100644 --- a/zkevm-circuits/src/bytecode_circuit/circuit.rs +++ b/zkevm-circuits/src/bytecode_circuit/circuit.rs @@ -310,7 +310,6 @@ impl SubCircuitConfig for BytecodeCircuitConfig { meta.query_advice(bytecode_table.code_hash, Rotation::cur()), ); - // TODO: check this cb.require_equal( "next.value_rlc == cur.value_rlc * randomness + next.value", meta.query_advice(value_rlc, Rotation::next()), @@ -362,19 +361,17 @@ impl SubCircuitConfig for BytecodeCircuitConfig { is_byte_to_header(meta), ]); - let lookup_columns = vec![value_rlc, length, bytecode_table.code_hash]; - let mut constraints = vec![( enable.clone(), meta.query_advice(keccak_table.is_enabled, Rotation::cur()), )]; - // TODO: perhaps write this explicitly so it is more readable the matching - // between collumns - for (i, column) in keccak_table.columns().iter().skip(1).enumerate() { + for (circuit_column, table_column) in + keccak_table.match_columns(value_rlc, length, bytecode_table.code_hash) + { constraints.push(( - enable.clone() * meta.query_advice(lookup_columns[i], Rotation::cur()), - meta.query_advice(*column, Rotation::cur()), + enable.clone() * meta.query_advice(circuit_column, Rotation::cur()), + meta.query_advice(table_column, Rotation::cur()), )) } diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index aee16f2806..3ecc3e0530 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -831,6 +831,21 @@ impl KeccakTable { }, ) } + + /// returns matchings between the circuit columns passed as parameters and + /// the table collumns + pub fn match_columns( + &self, + value_rlc: Column, + length: Column, + code_hash: Column, + ) -> Vec<(Column, Column)> { + vec![ + (value_rlc, self.input_rlc), + (length, self.input_len), + (code_hash, self.output_rlc), + ] + } } impl DynamicTableColumns for KeccakTable { From 18d68779df205b9430b08b8c9b6781e4d80334a7 Mon Sep 17 00:00:00 2001 From: Leo Lara Date: Wed, 1 Feb 2023 10:54:54 +0100 Subject: [PATCH 18/18] Small fix --- integration-tests/tests/circuits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/tests/circuits.rs b/integration-tests/tests/circuits.rs index 7ffd1a7801..ce0a7bd4e2 100644 --- a/integration-tests/tests/circuits.rs +++ b/integration-tests/tests/circuits.rs @@ -66,7 +66,7 @@ macro_rules! unroll_tests { super_circuit::SuperCircuit, tx_circuit::TxCircuit, evm_circuit::EvmCircuit, - bytecode_circuit::bytecode_unroller::BytecodeCircuit, + bytecode_circuit::circuit::BytecodeCircuit, copy_circuit::CopyCircuit }; use halo2_proofs::halo2curves::bn256::Fr;