Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

Commit 1d190ba

Browse files
darth-cylispckunxian-xiaroynalnaruto
authored
refactor: remove keccak constants from aggregator (#752)
* add row counting interface for keccak * add class level capacity calculator for keccak * remove f capacity from core * remove capacity calculator in aggregator util * remove unnecessary imports * replace max keccak round in core * replace reference for max keccak * remove unnecessary keccak imports and constants * remove max keccak constant * remove constants in hash cell parsing * remove constant column sanity check * add state column usage log * adjust input bytes column * add long column padding * correct fmt * fix fmt * minor fixes * fix * Fix: allow skipping of L1Msg tx part 2 (calculate num_all_txs in tx circuit) (#778) * calculate num_l1_msgs and num_l2_txs in tx circuit * fix * fmt and clippy * fix: non-last tx requires next is calldata * add NumAllTxs in block table and copy it from pi to block table * add lookup for NumAllTxs in tx circuit * clippy * add block num diff check to avoid two real block have same num * clippy * address comments * Fix the bugs in RLP/Tx/PI circuit which are reported by Zellic & KALOS auditors (#572) * fix finding 3 (#575) * Fix zellic finding 4 (#576) * fix finding 3 (#575) * fix finding 4 --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * add range check on diffs (#586) * Fix finding 10 (#578) * fix finding 3 (#575) * fix finding 10 * Fix finding 13 (#579) * fix finding 3 (#575) * fix finding 13 * Fix zellic finding 14 (#580) * fix finding 3 (#575) * fix finding 14 * Fix zellic finding 5 (#584) * fix finding 3 (#575) * fix finding 5 * refine comments * fmt * Fix finding 17 (#602) * add q_last * fix * add more diff range check * fix finding 7 (#625) * tx_id = 1 when sm starts * Fix finding 11 : use length for rlc in rlp table (#719) * fix: use tag_bytes_rlc and tag_length to copy tag's bytes around * fix lookup input for Len & RLC & GasCost fields in tx circuit * refactor * fix * refactor * fix col phase issue * refactor bytes_rlc type * Fix the bugs in Tx & PI circuits reported by Zellic & KALOS auditors (#612) * lookup chain_id to RLP table * fix finding 22 (#614) * fix finding 21 (#613) * fix finding 23 (#618) * fix finding 26 (#622) * fix finding 28 (#624) Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * fix finding 29 (#623) Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * enforce is_final is true at the last row and fix RLC related vul (#735) * Fix finding 30 (#733) * enforce all txs in a block are included in the tx table * clippy --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * Fix Zellic / Kalos finding25 (#619) * fix finding 25 * add comment --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * fix conflicts --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com> * use q_first instead * fmt --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com> * add pi comments * rename preimage col idx * add keccak rows check * rename input bytes col finder fn * modify keccak row env constaint * modify keccak row env constaint * add named constant setup vars * modify keccak row check * clippy advised * add comments on chunk hash * fmt * avoid constant lookup table * avoid repetitive computation of input_bytes_col_idx --------- Co-authored-by: Zhuo Zhang <mycinbrin@gmail.com> Co-authored-by: xkx <xiakunxian130@gmail.com> Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
1 parent 964ae09 commit 1d190ba

File tree

9 files changed

+142
-78
lines changed

9 files changed

+142
-78
lines changed

‎aggregator/src/aggregation/config.rs

+1-9
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,10 @@ impl AggregationConfig {
8383
params.degree as usize,
8484
);
8585

86-
// The current code base is hardcoded for KeccakCircuit configured
87-
// with 300 rows and 87 columns per hash call.
8886
let columns = keccak_circuit_config.cell_manager.columns();
8987

90-
assert_eq!(
91-
columns.len(),
92-
87,
93-
"cell manager configuration does not match the hard coded setup"
94-
);
95-
9688
// enabling equality for preimage column
97-
meta.enable_equality(columns[6].advice);
89+
meta.enable_equality(columns[keccak_circuit_config.preimage_column_index].advice);
9890
// enable equality for the digest column
9991
meta.enable_equality(columns.last().unwrap().advice);
10092
// enable equality for the data RLC column

‎aggregator/src/constants.rs

-15
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,6 @@ pub(crate) const DIGEST_LEN: usize = 32;
1111
/// Input length per round
1212
pub(crate) const INPUT_LEN_PER_ROUND: usize = 136;
1313

14-
// Each round requires (NUM_ROUNDS+1) * DEFAULT_KECCAK_ROWS = 300 rows.
15-
// This library is hard coded for this parameter.
16-
// Modifying the following parameters may result into bugs.
17-
// Adopted from keccak circuit
18-
pub(crate) const DEFAULT_KECCAK_ROWS: usize = 12;
19-
// Adopted from keccak circuit
20-
pub(crate) const NUM_ROUNDS: usize = 24;
21-
pub(crate) const ROWS_PER_ROUND: usize = (NUM_ROUNDS + 1) * DEFAULT_KECCAK_ROWS;
22-
2314
// TODO(ZZ): update to the right degree
2415
#[allow(dead_code)]
2516
pub(crate) const LOG_DEGREE: u32 = 19;
@@ -58,9 +49,3 @@ pub(crate) const BITS: usize = 88;
5849
/// will be padded.
5950
// TODO: update me(?)
6051
pub const MAX_AGG_SNARKS: usize = 10;
61-
62-
/// The number of keccak rounds is the sum of
63-
/// - batch public input hash: 2 rounds
64-
/// - chunk's public input hash: 2 * MAX_AGG_SNARKS
65-
/// - batch data hash: (32 * MAX_AGG_SNARKS)/136 = 3
66-
pub(crate) const MAX_KECCAK_ROUNDS: usize = 2 * MAX_AGG_SNARKS + 5;

‎aggregator/src/core.rs

+19-14
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ use snark_verifier_sdk::{
2323
Snark,
2424
};
2525
use zkevm_circuits::{
26-
keccak_circuit::{keccak_packed_multi::multi_keccak, KeccakCircuitConfig},
27-
table::LookupTable,
26+
keccak_circuit::{
27+
keccak_packed_multi::{self, multi_keccak},
28+
KeccakCircuit, KeccakCircuitConfig,
29+
},
30+
table::{KeccakTable, LookupTable},
2831
util::Challenges,
2932
};
3033

3134
use crate::{
32-
constants::{
33-
CHAIN_ID_LEN, DIGEST_LEN, INPUT_LEN_PER_ROUND, LOG_DEGREE, MAX_AGG_SNARKS,
34-
MAX_KECCAK_ROUNDS, ROWS_PER_ROUND,
35-
},
35+
constants::{CHAIN_ID_LEN, DIGEST_LEN, INPUT_LEN_PER_ROUND, LOG_DEGREE, MAX_AGG_SNARKS},
3636
util::{
37-
assert_conditional_equal, assert_equal, assert_exist, get_indices, keccak_round_capacity,
37+
assert_conditional_equal, assert_equal, assert_exist, get_indices, get_max_keccak_updates,
3838
parse_hash_digest_cells, parse_hash_preimage_cells, parse_pi_hash_rlc_cells,
3939
},
4040
AggregationConfig, RlcConfig, CHUNK_DATA_HASH_INDEX, POST_STATE_ROOT_INDEX,
@@ -185,7 +185,9 @@ pub(crate) fn extract_hash_cells(
185185
preimages: &[Vec<u8>],
186186
) -> Result<ExtractedHashCells, Error> {
187187
let mut is_first_time = true;
188-
let num_rows = 1 << LOG_DEGREE;
188+
let keccak_capacity = KeccakCircuit::<Fr>::capacity_for_row(1 << LOG_DEGREE);
189+
let max_keccak_updates = get_max_keccak_updates(MAX_AGG_SNARKS);
190+
let keccak_f_rows = keccak_packed_multi::get_num_rows_per_update();
189191

190192
let timer = start_timer!(|| ("multi keccak").to_string());
191193
// preimages consists of the following parts
@@ -202,7 +204,7 @@ pub(crate) fn extract_hash_cells(
202204
// (3) batchDataHash preimage =
203205
// (chunk[0].dataHash || ... || chunk[k-1].dataHash)
204206
// each part of the preimage is mapped to image by Keccak256
205-
let witness = multi_keccak(preimages, challenges, keccak_round_capacity(num_rows))
207+
let witness = multi_keccak(preimages, challenges, keccak_capacity)
206208
.map_err(|e| Error::AssertionFailure(format!("multi keccak assignment failed: {e:?}")))?;
207209
end_timer!(timer);
208210

@@ -237,21 +239,24 @@ pub(crate) fn extract_hash_cells(
237239

238240
let timer = start_timer!(|| "assign row");
239241
log::trace!("witness length: {}", witness.len());
242+
let input_bytes_col_idx =
243+
keccak_packed_multi::get_input_bytes_col_idx_in_cell_manager()
244+
+ <KeccakTable as LookupTable<Fr>>::columns(&keccak_config.keccak_table)
245+
.len()
246+
- 1;
240247
for (offset, keccak_row) in witness.iter().enumerate() {
241248
let row = keccak_config.set_row(&mut region, offset, keccak_row)?;
242249

243250
if cur_preimage_index.is_some() && *cur_preimage_index.unwrap() == offset {
244-
// 10-th column is Keccak input in Keccak circuit
245-
hash_input_cells.push(row[10].clone());
251+
hash_input_cells.push(row[input_bytes_col_idx].clone());
246252
cur_preimage_index = preimage_indices_iter.next();
247253
}
248254
if cur_digest_index.is_some() && *cur_digest_index.unwrap() == offset {
249255
// last column is Keccak output in Keccak circuit
250256
hash_output_cells.push(row.last().unwrap().clone()); // sage unwrap
251257
cur_digest_index = digest_indices_iter.next();
252258
}
253-
if offset % ROWS_PER_ROUND == 0 && offset / ROWS_PER_ROUND <= MAX_KECCAK_ROUNDS
254-
{
259+
if offset % keccak_f_rows == 0 && offset / keccak_f_rows <= max_keccak_updates {
255260
// first column is is_final
256261
is_final_cells.push(row[0].clone());
257262
// second column is data rlc
@@ -268,7 +273,7 @@ pub(crate) fn extract_hash_cells(
268273
// sanity
269274
assert_eq!(
270275
hash_input_cells.len(),
271-
MAX_KECCAK_ROUNDS * INPUT_LEN_PER_ROUND
276+
max_keccak_updates * INPUT_LEN_PER_ROUND
272277
);
273278
assert_eq!(hash_output_cells.len(), (MAX_AGG_SNARKS + 4) * DIGEST_LEN);
274279

‎aggregator/src/tests/rlc/dynamic_hashes.rs

+6-8
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,14 @@ use snark_verifier::loader::halo2::halo2_ecc::halo2_base::utils::fs::gen_srs;
99
use snark_verifier_sdk::{gen_pk, gen_snark_shplonk, verify_snark_shplonk, CircuitExt};
1010
use zkevm_circuits::{
1111
keccak_circuit::{
12-
keccak_packed_multi::multi_keccak, KeccakCircuitConfig, KeccakCircuitConfigArgs,
12+
keccak_packed_multi::{self, multi_keccak},
13+
KeccakCircuit, KeccakCircuitConfig, KeccakCircuitConfigArgs,
1314
},
1415
table::{KeccakTable, LookupTable},
1516
util::{Challenges, SubCircuitConfig},
1617
};
1718

18-
use crate::{
19-
aggregation::RlcConfig,
20-
constants::{LOG_DEGREE, ROWS_PER_ROUND},
21-
util::keccak_round_capacity,
22-
};
19+
use crate::{aggregation::RlcConfig, constants::LOG_DEGREE};
2320

2421
#[derive(Default, Debug, Clone)]
2522
struct DynamicHashCircuit {
@@ -78,6 +75,7 @@ impl Circuit<Fr> for DynamicHashCircuit {
7875
mut layouter: impl Layouter<Fr>,
7976
) -> Result<(), Error> {
8077
let (config, challenges) = config;
78+
let keccak_f_rows = keccak_packed_multi::get_num_rows_per_update();
8179

8280
config
8381
.keccak_circuit_config
@@ -95,7 +93,7 @@ impl Circuit<Fr> for DynamicHashCircuit {
9593
let witness = multi_keccak(
9694
&[hash_preimage.clone()],
9795
challenge,
98-
keccak_round_capacity(1 << LOG_DEGREE),
96+
KeccakCircuit::<Fr>::capacity_for_row(1 << LOG_DEGREE),
9997
)
10098
.unwrap();
10199

@@ -113,7 +111,7 @@ impl Circuit<Fr> for DynamicHashCircuit {
113111
config
114112
.keccak_circuit_config
115113
.set_row(&mut region, offset, keccak_row)?;
116-
if offset % ROWS_PER_ROUND == 0 && data_rlc_cells.len() < 4 {
114+
if offset % keccak_f_rows == 0 && data_rlc_cells.len() < 4 {
117115
// second element is data rlc
118116
data_rlc_cells.push(row[1].clone());
119117
}

‎aggregator/src/util.rs

+47-23
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
use crate::{
2+
aggregation::RlcConfig,
3+
constants::{DIGEST_LEN, INPUT_LEN_PER_ROUND, MAX_AGG_SNARKS},
4+
};
15
use eth_types::Field;
26
use halo2_proofs::{
37
circuit::{AssignedCell, Region},
@@ -9,29 +13,35 @@ use snark_verifier::loader::halo2::halo2_ecc::halo2_base::{
913
gates::{flex_gate::FlexGateConfig, GateInstructions},
1014
AssignedValue, Context,
1115
};
12-
13-
use crate::{
14-
aggregation::RlcConfig,
15-
constants::{DIGEST_LEN, INPUT_LEN_PER_ROUND, MAX_AGG_SNARKS},
16-
DEFAULT_KECCAK_ROWS, NUM_ROUNDS,
16+
use zkevm_circuits::keccak_circuit::keccak_packed_multi::{
17+
get_num_rows_per_round, get_num_rows_per_update,
1718
};
1819

19-
use std::env::var;
20+
// Calculates the maximum keccak updates (1 absorb, or 1 f-box invoke)
21+
// needed for the number of snarks
22+
pub(crate) fn get_max_keccak_updates(max_snarks: usize) -> usize {
23+
// The public input hash for the batch is derived from hashing
24+
// chain_id || chunk_0's prev_state || chunk_k-1's post_state ||
25+
// chunk_k-1's withdraw_root || batch_data_hash.
26+
// In total there're 168 bytes. Therefore 2 pi rounds are required.
27+
let pi_rounds = 2;
28+
// Hash for each chunk is derived from hashing the chunk's
29+
// chain_id || prev_state || post_state || withdraw_root || data_hash
30+
// Each chunk hash therefore also requires 2 keccak rounds for 168 bytes.
31+
let chunk_hash_rounds = 2 * max_snarks;
32+
let data_hash_rounds = get_data_hash_keccak_updates(max_snarks);
2033

21-
pub(crate) fn keccak_round_capacity(num_rows: usize) -> Option<usize> {
22-
if num_rows > 0 {
23-
// Subtract two for unusable rows
24-
Some(num_rows / ((NUM_ROUNDS + 1) * get_num_rows_per_round()) - 2)
25-
} else {
26-
None
27-
}
34+
pi_rounds + chunk_hash_rounds + data_hash_rounds
2835
}
36+
pub(crate) fn get_data_hash_keccak_updates(max_snarks: usize) -> usize {
37+
let data_hash_rounds = (32 * max_snarks) / INPUT_LEN_PER_ROUND;
38+
let padding_round = if data_hash_rounds * INPUT_LEN_PER_ROUND < 32 * max_snarks {
39+
1
40+
} else {
41+
0
42+
};
2943

30-
pub(crate) fn get_num_rows_per_round() -> usize {
31-
var("KECCAK_ROWS")
32-
.unwrap_or_else(|_| format!("{DEFAULT_KECCAK_ROWS}"))
33-
.parse()
34-
.expect("Cannot parse KECCAK_ROWS env var as usize")
44+
data_hash_rounds + padding_round
3545
}
3646

3747
/// Return
@@ -42,9 +52,14 @@ pub(crate) fn get_indices(preimages: &[Vec<u8>]) -> (Vec<usize>, Vec<usize>) {
4252
let mut digest_indices = vec![];
4353
let mut round_ctr = 0;
4454

55+
let keccak_f_rows = get_num_rows_per_update();
56+
let inner_round_rows = get_num_rows_per_round();
57+
4558
for preimage in preimages.iter().take(MAX_AGG_SNARKS + 1) {
4659
// 136 = 17 * 8 is the size in bytes of each
4760
// input chunk that can be processed by Keccak circuit using absorb
61+
62+
// For example, if num_rows_per_inner_round for Keccak is 12, then
4863
// each chunk of size 136 needs 300 Keccak circuit rows to prove
4964
// which consists of 12 Keccak rows for each of 24 + 1 Keccak circuit rounds
5065
// digest only happens at the end of the last input chunk with
@@ -53,38 +68,47 @@ pub(crate) fn get_indices(preimages: &[Vec<u8>]) -> (Vec<usize>, Vec<usize>) {
5368
let mut preimage_padded = preimage.clone();
5469
preimage_padded.resize(INPUT_LEN_PER_ROUND * num_rounds, 0);
5570
for (i, round) in preimage_padded.chunks(INPUT_LEN_PER_ROUND).enumerate() {
71+
let f_round_offset = round_ctr * keccak_f_rows;
5672
// indices for preimages
5773
for (j, _chunk) in round.chunks(8).into_iter().enumerate() {
74+
let inner_offset = f_round_offset + (j + 1) * inner_round_rows;
5875
for k in 0..8 {
59-
preimage_indices.push(round_ctr * 300 + j * 12 + k + 12)
76+
preimage_indices.push(inner_offset + k);
6077
}
6178
}
6279
// indices for digests
6380
if i == num_rounds - 1 {
6481
for j in 0..4 {
82+
let inner_offset = f_round_offset
83+
+ j * inner_round_rows
84+
+ (keccak_f_rows - inner_round_rows * (DIGEST_LEN / 8));
6585
for k in 0..8 {
66-
digest_indices.push(round_ctr * 300 + j * 12 + k + 252)
86+
digest_indices.push(inner_offset + k);
6787
}
6888
}
6989
}
7090
round_ctr += 1;
7191
}
7292
}
7393
// last hash is for data_hash and has various length, so we output all the possible cells
74-
for _i in 0..3 {
94+
for _i in 0..get_data_hash_keccak_updates(MAX_AGG_SNARKS) {
7595
for (j, _) in (0..INPUT_LEN_PER_ROUND)
7696
.into_iter()
7797
.chunks(8)
7898
.into_iter()
7999
.enumerate()
80100
{
101+
let inner_offset = round_ctr * keccak_f_rows + (j + 1) * inner_round_rows;
81102
for k in 0..8 {
82-
preimage_indices.push(round_ctr * 300 + j * 12 + k + 12)
103+
preimage_indices.push(inner_offset + k);
83104
}
84105
}
85106
for j in 0..4 {
107+
let inner_offset = round_ctr * keccak_f_rows
108+
+ j * inner_round_rows
109+
+ (keccak_f_rows - inner_round_rows * (DIGEST_LEN / 8));
86110
for k in 0..8 {
87-
digest_indices.push(round_ctr * 300 + j * 12 + k + 252)
111+
digest_indices.push(inner_offset + k);
88112
}
89113
}
90114
round_ctr += 1;

‎zkevm-circuits/src/keccak_circuit.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ pub struct KeccakCircuitConfig<F> {
6565
normalize_6: [TableColumn; 2],
6666
chi_base_table: [TableColumn; 2],
6767
pack_table: [TableColumn; 2],
68+
/// The column for enabling copy constraints in aggregator
69+
pub preimage_column_index: usize,
6870
_marker: PhantomData<F>,
6971
}
7072

@@ -138,6 +140,8 @@ impl<F: Field> SubCircuitConfig<F> for KeccakCircuitConfig<F> {
138140
s_next[i][j] = cell.at_offset(meta, get_num_rows_per_round() as i32).expr();
139141
}
140142
}
143+
log::debug!("- Post states:");
144+
log::debug!("Columns: {}", cell_manager.get_width());
141145
// Absorb data
142146
let absorb_from = cell_manager.query_cell(meta);
143147
let absorb_data = cell_manager.query_cell(meta);
@@ -185,6 +189,7 @@ impl<F: Field> SubCircuitConfig<F> for KeccakCircuitConfig<F> {
185189
log::debug!("- Post absorb:");
186190
log::debug!("Lookups: {}", lookup_counter);
187191
log::debug!("Columns: {}", cell_manager.get_width());
192+
let preimage_column_index: usize = cell_manager.get_width() + 1;
188193
total_lookup_counter += lookup_counter;
189194

190195
// Process inputs.
@@ -863,6 +868,7 @@ impl<F: Field> SubCircuitConfig<F> for KeccakCircuitConfig<F> {
863868
normalize_6,
864869
chi_base_table,
865870
pack_table,
871+
preimage_column_index,
866872
_marker: PhantomData,
867873
}
868874
}
@@ -1065,9 +1071,15 @@ impl<F: Field> KeccakCircuit<F> {
10651071

10661072
/// The number of keccak_f's that can be done in this circuit
10671073
pub fn capacity(&self) -> Option<usize> {
1068-
if self.num_rows > 0 {
1074+
Self::capacity_for_row(self.num_rows)
1075+
}
1076+
1077+
/// The number of keccak_f's that can be done for
1078+
/// a particular row number depending on current Keccak params
1079+
pub fn capacity_for_row(num_rows: usize) -> Option<usize> {
1080+
if num_rows > 0 {
10691081
// Subtract two for unusable rows
1070-
Some(self.num_rows / ((NUM_ROUNDS + 1) * get_num_rows_per_round()) - 2)
1082+
Some(num_rows / ((NUM_ROUNDS + 1) * get_num_rows_per_round()) - 2)
10711083
} else {
10721084
None
10731085
}

0 commit comments

Comments
 (0)