From 8de5a6d1ca58f16e936484147822d42d5a69649f Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 29 Jun 2023 16:46:20 +0800 Subject: [PATCH 01/16] fix finding 3 (#575) --- zkevm-circuits/src/rlp_circuit_fsm.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index 691765017e..0253467d05 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -536,7 +536,9 @@ impl RlpCircuitConfig { }, ); - // if (tx_id' == tx_id and format' != format) or (tx_id' != tx_id and tx_id' != 0) + // These two cases are not the very last non-padding RLP instance. + // 1. (tx_id' == tx_id and format' != format) + // 2. (tx_id' != tx_id and tx_id' != 0) cb.condition( sum::expr([ // case 1 @@ -572,6 +574,23 @@ impl RlpCircuitConfig { }, ); + // For the very last non-padding RLP instance, we have + // tx_id' != tx_id && tx_id' == 0 + cb.condition( + and::expr([ + is_padding_in_dt.expr(Rotation::next())(meta), + not::expr(tx_id_check_in_dt.is_equal_expression.expr()), + ]), + |cb| { + // byte_rev_idx == 1 + cb.require_equal( + "byte_rev_idx is 1 at the last index", + meta.query_advice(data_table.byte_rev_idx, Rotation::cur()), + 1.expr(), + ); + }, + ); + cb.gate(meta.query_fixed(q_enabled, Rotation::cur())) }); From 014bdd3077fcab8942669fca0bf100e6398f4efd Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 6 Jul 2023 10:17:53 +0800 Subject: [PATCH 02/16] Fix zellic finding 4 (#576) * fix finding 3 (#575) * fix finding 4 --------- Co-authored-by: Rohit Narurkar --- zkevm-circuits/src/rlp_circuit_fsm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index 0253467d05..f86786fe6c 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -597,7 +597,7 @@ impl RlpCircuitConfig { meta.lookup_any("byte value check", |meta| { let cond = and::expr([ meta.query_fixed(q_enabled, Rotation::cur()), - is_padding_in_dt.expr(Rotation::cur())(meta), + not::expr(is_padding_in_dt.expr(Rotation::cur())(meta)), ]); vec![meta.query_advice(data_table.byte_value, Rotation::cur())] From cea4a876513774239db6cd4cb3518aa8e374fe13 Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 6 Jul 2023 10:50:35 +0800 Subject: [PATCH 03/16] add range check on diffs (#586) --- zkevm-circuits/src/rlp_circuit_fsm.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index f86786fe6c..f85c780dc2 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -16,7 +16,7 @@ use gadgets::{ util::{and, not, select, sum, Expr}, }; use halo2_proofs::{ - circuit::{Layouter, Region, Value}, + circuit::{Chip, Layouter, Region, Value}, plonk::{ Advice, Any, Column, ConstraintSystem, Error, Expression, Fixed, SecondPhase, VirtualCells, }, @@ -800,6 +800,28 @@ impl RlpCircuitConfig { |meta| meta.query_advice(format, Rotation::next()), ); + // constrain diff belong to range256 table for each comparator + let mut diffs = vec![]; + diffs.extend(byte_value_gte_0x80.lt_chip.config().diff); + diffs.extend(byte_value_gte_0xb8.lt_chip.config().diff); + diffs.extend(byte_value_gte_0xc0.lt_chip.config().diff); + diffs.extend(byte_value_gte_0xf8.lt_chip.config().diff); + diffs.extend(byte_value_lte_0x80.lt_chip.config().diff); + diffs.extend(byte_value_lte_0xb8.lt_chip.config().diff); + diffs.extend(byte_value_lte_0xc0.lt_chip.config().diff); + diffs.extend(byte_value_lte_0xf8.lt_chip.config().diff); + diffs.extend(tidx_lte_tlength.lt_chip.config().diff); + diffs.extend(mlength_lte_0x20.lt_chip.config().diff); + + for diff in diffs { + meta.lookup_any("diff in range256", |meta| { + let diff = meta.query_advice(diff, Rotation::cur()); + let range256 = meta.query_fixed(range256_table.0, Rotation::cur()); + + vec![(diff, range256)] + }); + } + // constraints on the booleans that we use to reduce degree meta.create_gate("booleans for reducing degree (part one)", |meta| { let mut cb = BaseConstraintBuilder::default(); From 37b273d6db0b1172a5ec055337a707800fd5416b Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 6 Jul 2023 10:52:01 +0800 Subject: [PATCH 04/16] Fix finding 10 (#578) * fix finding 3 (#575) * fix finding 10 --- zkevm-circuits/src/rlp_circuit_fsm.rs | 35 ++++++++++++++++++++------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index f85c780dc2..bf80742bc7 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -1055,20 +1055,13 @@ impl RlpCircuitConfig { cb.condition( meta.query_advice(transit_to_new_rlp_instance, Rotation::cur()), |cb| { - let tx_id = meta.query_advice(rlp_table.tx_id, Rotation::cur()); - let tx_id_next = meta.query_advice(rlp_table.tx_id, Rotation::next()); - let format = meta.query_advice(rlp_table.format, Rotation::cur()); - let format_next = meta.query_advice(rlp_table.format, Rotation::next()); - let tag_next = tag_next_expr(meta); + let tag_next = meta.query_advice(tag, Rotation::next()); // state transition. update_state!(meta, cb, byte_idx, 1); update_state!(meta, cb, depth, 0); update_state!(meta, cb, state, DecodeTagStart); - cb.require_zero( - "(tx_id' == tx_id + 1) or (format' == format + 1)", - (tx_id_next - tx_id - 1.expr()) * (format_next - format - 1.expr()), - ); + cb.require_zero( "tag == TxType or tag == BeginList", (tag_next.expr() - TxType.expr()) @@ -1076,6 +1069,30 @@ impl RlpCircuitConfig { ); }, ); + // tx_id' == tx_id => format' == format + 1 + cb.condition( + and::expr([ + meta.query_advice(transit_to_new_rlp_instance, Rotation::cur()), + tx_id_check_in_sm.is_equal_expression.expr(), + ]), + |cb| { + let format = meta.query_advice(rlp_table.format, Rotation::cur()); + let format_next = meta.query_advice(rlp_table.format, Rotation::next()); + cb.require_equal("format' == format + 1", format_next, format + 1.expr()); + }, + ); + // tx_id' != tx_id => tx_id' == tx_id + 1 + cb.condition( + and::expr([ + meta.query_advice(transit_to_new_rlp_instance, Rotation::cur()), + not::expr(tx_id_check_in_sm.is_equal_expression.expr()), + ]), + |cb| { + let tx_id = meta.query_advice(rlp_table.tx_id, Rotation::cur()); + let tx_id_next = meta.query_advice(rlp_table.tx_id, Rotation::next()); + cb.require_equal("tx_id' == tx_id + 1", tx_id_next, tx_id + 1.expr()); + }, + ); cb.condition( and::expr([ case_4.expr(), From a4d784be57d4bce8986603085239a296a9a70b82 Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 6 Jul 2023 10:53:46 +0800 Subject: [PATCH 05/16] Fix finding 13 (#579) * fix finding 3 (#575) * fix finding 13 --- zkevm-circuits/src/rlp_circuit_fsm.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index bf80742bc7..089867d289 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -1180,6 +1180,7 @@ impl RlpCircuitConfig { cb.condition(tidx_eq_tlen, |cb| { // assertions emit_rlp_tag!(meta, cb, tag_expr(meta), false); + constrain_eq!(meta, cb, rlp_table.tag_value, tag_value_acc_expr(meta)); // state transitions. update_state!(meta, cb, tag, tag_next_expr(meta)); From c48b94a7c177afca5012e8d7a60286e802b1ae06 Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 6 Jul 2023 10:54:02 +0800 Subject: [PATCH 06/16] Fix zellic finding 14 (#580) * fix finding 3 (#575) * fix finding 14 --- zkevm-circuits/src/rlp_circuit_fsm.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index 089867d289..2d93c490cf 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -1283,6 +1283,7 @@ impl RlpCircuitConfig { ]); cb.condition(cond.expr(), |cb| { // assertions. + do_not_emit!(meta, cb); constrain_eq!(meta, cb, is_tag_begin, true); // state transitions From 1243ef48be9001c8906c2a5a9209b135c5c20510 Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 6 Jul 2023 21:14:28 +0800 Subject: [PATCH 07/16] Fix zellic finding 5 (#584) * fix finding 3 (#575) * fix finding 5 * refine comments * fmt --- zkevm-circuits/src/rlp_circuit_fsm.rs | 30 ++++++++++++++++ zkevm-circuits/src/witness/l1_msg.rs | 4 +-- zkevm-circuits/src/witness/rlp_fsm.rs | 52 +++++++++++++++++---------- 3 files changed, 65 insertions(+), 21 deletions(-) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index 2d93c490cf..fc04e155d3 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -306,6 +306,8 @@ pub struct RlpCircuitConfig { tidx_lte_tlength: ComparatorConfig, /// Check for max_length <= 32 mlength_lte_0x20: ComparatorConfig, + /// Check for tag_length <= max_length + tlength_lte_mlength: ComparatorConfig, /// Check for depth == 0 depth_check: IsEqualConfig, /// Check for depth == 1 @@ -775,6 +777,12 @@ impl RlpCircuitConfig { |meta| meta.query_advice(max_length, Rotation::cur()), |_meta| 0x20.expr(), ); + let tlength_lte_mlength = ComparatorChip::configure( + meta, + cmp_enabled, + |meta| meta.query_advice(tag_length, Rotation::cur()), + |meta| meta.query_advice(max_length, Rotation::cur()), + ); let depth_check = IsEqualChip::configure( meta, cmp_enabled, @@ -1179,6 +1187,18 @@ impl RlpCircuitConfig { // Bytes => DecodeTagStart cb.condition(tidx_eq_tlen, |cb| { // assertions + let (lt, eq) = tlength_lte_mlength.expr(meta, Some(Rotation::cur())); + cb.require_equal( + "tag_length <= max_length", + // we can use `sum` instead of `or` for two reasons + // 1. both `lt` and `eq` are boolean and they cannot be true at the same time, + // therefore the result of `sum` is same as `or`. + // 2. sum has lower degree + sum::expr([ + lt, eq, + ]), + true.expr(), + ); emit_rlp_tag!(meta, cb, tag_expr(meta), false); constrain_eq!(meta, cb, rlp_table.tag_value, tag_value_acc_expr(meta)); @@ -1330,6 +1350,8 @@ impl RlpCircuitConfig { // LongList => DecodeTagStart cb.condition(tidx_eq_tlen.expr(), |cb| { // assertions + let (lt, eq) = tlength_lte_mlength.expr(meta, Some(Rotation::cur())); + cb.require_equal("tag_length <= max_length", sum::expr([lt, eq]), true.expr()); // state transitions update_state!(meta, cb, tag, tag_next_expr(meta)); @@ -1452,6 +1474,7 @@ impl RlpCircuitConfig { byte_value_gte_0xf8, tidx_lte_tlength, mlength_lte_0x20, + tlength_lte_mlength, depth_check, depth_eq_one, @@ -1673,6 +1696,13 @@ impl RlpCircuitConfig { F::from(witness.state_machine.tag_idx as u64), F::from(witness.state_machine.tag_length as u64), )?; + let tlength_lte_mlength_chip = ComparatorChip::construct(self.tlength_lte_mlength.clone()); + tlength_lte_mlength_chip.assign( + region, + row, + F::from(witness.state_machine.tag_length as u64), + F::from(witness.state_machine.max_length as u64), + )?; let depth_check_chip = IsEqualChip::construct(self.depth_check.clone()); depth_check_chip.assign( diff --git a/zkevm-circuits/src/witness/l1_msg.rs b/zkevm-circuits/src/witness/l1_msg.rs index 97c0633738..1eb13bb916 100644 --- a/zkevm-circuits/src/witness/l1_msg.rs +++ b/zkevm-circuits/src/witness/l1_msg.rs @@ -1,7 +1,7 @@ use crate::{ evm_circuit::param::{N_BYTES_ACCOUNT_ADDRESS, N_BYTES_U64, N_BYTES_WORD}, witness::{ - rlp_fsm::{N_BYTES_CALLDATA, N_BYTES_LIST}, + rlp_fsm::{MAX_TAG_LENGTH_OF_LIST, N_BYTES_CALLDATA}, Format::L1MsgHash, RomTableRow, Tag::{BeginList, Data, EndList, Gas, Nonce, Sender, To, TxType, Value as TxValue}, @@ -21,7 +21,7 @@ impl Encodable for L1MsgTx { pub fn rom_table_rows() -> Vec { let rows = vec![ (TxType, BeginList, 1, vec![1]), - (BeginList, Nonce, N_BYTES_LIST, vec![2]), + (BeginList, Nonce, MAX_TAG_LENGTH_OF_LIST, vec![2]), (Nonce, Gas, N_BYTES_U64, vec![3]), (Gas, To, N_BYTES_U64, vec![4]), (To, TxValue, N_BYTES_ACCOUNT_ADDRESS, vec![5]), diff --git a/zkevm-circuits/src/witness/rlp_fsm.rs b/zkevm-circuits/src/witness/rlp_fsm.rs index ed0ba00794..a6825dc030 100644 --- a/zkevm-circuits/src/witness/rlp_fsm.rs +++ b/zkevm-circuits/src/witness/rlp_fsm.rs @@ -151,13 +151,15 @@ use crate::{ }, }; -// The number of bytes of list can not larger than 2^24. -pub(crate) const N_BYTES_LIST: usize = 1 << 24; +// The number of bytes of list can not be larger than 2^24 = 2^(8*3). +// This const is meant to be the maximum of tag_length for representing a `LongList`. +// For example, [0xf9, 0xff, 0xff] has tag_length = 2 and has 0xffff bytes inside. +pub(crate) const MAX_TAG_LENGTH_OF_LIST: usize = 3; pub(crate) const N_BYTES_CALLDATA: usize = 1 << 24; fn eip155_tx_sign_rom_table_rows() -> Vec { let rows = vec![ - (BeginList, Nonce, N_BYTES_LIST, vec![1]), + (BeginList, Nonce, MAX_TAG_LENGTH_OF_LIST, vec![1]), (Nonce, GasPrice, N_BYTES_U64, vec![2]), (GasPrice, Gas, N_BYTES_WORD, vec![3]), (Gas, To, N_BYTES_U64, vec![4]), @@ -179,7 +181,7 @@ fn eip155_tx_sign_rom_table_rows() -> Vec { fn eip155_tx_hash_rom_table_rows() -> Vec { let rows = vec![ - (BeginList, Nonce, N_BYTES_LIST, vec![1]), + (BeginList, Nonce, MAX_TAG_LENGTH_OF_LIST, vec![1]), (Nonce, GasPrice, N_BYTES_U64, vec![2]), (GasPrice, Gas, N_BYTES_WORD, vec![3]), (Gas, To, N_BYTES_U64, vec![4]), @@ -201,7 +203,7 @@ fn eip155_tx_hash_rom_table_rows() -> Vec { pub fn pre_eip155_tx_sign_rom_table_rows() -> Vec { let rows = vec![ - (BeginList, Nonce, N_BYTES_LIST, vec![1]), + (BeginList, Nonce, MAX_TAG_LENGTH_OF_LIST, vec![1]), (Nonce, GasPrice, N_BYTES_U64, vec![2]), (GasPrice, Gas, N_BYTES_WORD, vec![3]), (Gas, To, N_BYTES_U64, vec![4]), @@ -220,7 +222,7 @@ pub fn pre_eip155_tx_sign_rom_table_rows() -> Vec { pub fn pre_eip155_tx_hash_rom_table_rows() -> Vec { let rows = vec![ - (BeginList, Nonce, N_BYTES_LIST, vec![1]), + (BeginList, Nonce, MAX_TAG_LENGTH_OF_LIST, vec![1]), (Nonce, GasPrice, N_BYTES_U64, vec![2]), (GasPrice, Gas, N_BYTES_WORD, vec![3]), (Gas, To, N_BYTES_U64, vec![4]), @@ -243,7 +245,7 @@ pub fn pre_eip155_tx_hash_rom_table_rows() -> Vec { pub fn eip1559_tx_hash_rom_table_rows() -> Vec { let rows = vec![ (TxType, BeginList, 1, vec![1]), - (BeginList, ChainId, N_BYTES_LIST, vec![2]), + (BeginList, ChainId, MAX_TAG_LENGTH_OF_LIST, vec![2]), (ChainId, Nonce, N_BYTES_U64, vec![3]), (Nonce, MaxPriorityFeePerGas, N_BYTES_U64, vec![4]), (MaxPriorityFeePerGas, MaxFeePerGas, N_BYTES_WORD, vec![5]), @@ -252,21 +254,26 @@ pub fn eip1559_tx_hash_rom_table_rows() -> Vec { (To, TxValue, N_BYTES_ACCOUNT_ADDRESS, vec![8]), (TxValue, Data, N_BYTES_WORD, vec![9]), (Data, BeginVector, N_BYTES_CALLDATA, vec![10, 11]), - (BeginVector, EndVector, N_BYTES_LIST, vec![21]), // access_list is none - (BeginVector, BeginList, N_BYTES_LIST, vec![12]), - (BeginList, AccessListAddress, N_BYTES_LIST, vec![13]), + (BeginVector, EndVector, MAX_TAG_LENGTH_OF_LIST, vec![21]), // access_list is none + (BeginVector, BeginList, MAX_TAG_LENGTH_OF_LIST, vec![12]), + ( + BeginList, + AccessListAddress, + MAX_TAG_LENGTH_OF_LIST, + vec![13], + ), ( AccessListAddress, BeginVector, N_BYTES_ACCOUNT_ADDRESS, vec![14, 15], ), - (BeginVector, EndVector, N_BYTES_LIST, vec![18]), /* access_list.storage_keys - * is none */ + (BeginVector, EndVector, MAX_TAG_LENGTH_OF_LIST, vec![18]), /* access_list.storage_keys + * is none */ ( BeginVector, AccessListStorageKey, - N_BYTES_LIST, + MAX_TAG_LENGTH_OF_LIST, vec![16, 17], ), (AccessListStorageKey, EndVector, N_BYTES_WORD, vec![18]), // finished parsing storage keys @@ -295,7 +302,7 @@ pub fn eip1559_tx_hash_rom_table_rows() -> Vec { pub fn eip1559_tx_sign_rom_table_rows() -> Vec { let rows = vec![ - (BeginList, ChainId, N_BYTES_LIST, vec![1]), + (BeginList, ChainId, MAX_TAG_LENGTH_OF_LIST, vec![1]), (ChainId, Nonce, N_BYTES_U64, vec![2]), (Nonce, MaxPriorityFeePerGas, N_BYTES_U64, vec![3]), (MaxPriorityFeePerGas, MaxFeePerGas, N_BYTES_WORD, vec![4]), @@ -304,20 +311,27 @@ pub fn eip1559_tx_sign_rom_table_rows() -> Vec { (To, TxValue, N_BYTES_ACCOUNT_ADDRESS, vec![7]), (TxValue, Data, N_BYTES_WORD, vec![8]), (Data, BeginVector, N_BYTES_CALLDATA, vec![9, 10]), - (BeginVector, EndVector, N_BYTES_LIST, vec![20]), // access_list is none - (BeginVector, BeginList, N_BYTES_LIST, vec![11]), - (BeginList, AccessListAddress, N_BYTES_LIST, vec![12]), + (BeginVector, EndVector, MAX_TAG_LENGTH_OF_LIST, vec![20]), // access_list is none + (BeginVector, BeginList, MAX_TAG_LENGTH_OF_LIST, vec![11]), + ( + BeginList, + AccessListAddress, + MAX_TAG_LENGTH_OF_LIST, + vec![12], + ), ( AccessListAddress, BeginVector, N_BYTES_ACCOUNT_ADDRESS, vec![13, 14], ), - (BeginVector, EndVector, N_BYTES_LIST, vec![17]), /* access_list.storage_keys is none */ + (BeginVector, EndVector, MAX_TAG_LENGTH_OF_LIST, vec![17]), /* access_list.storage_keys + * is + * none */ ( BeginVector, AccessListStorageKey, - N_BYTES_LIST, + MAX_TAG_LENGTH_OF_LIST, vec![15, 16], ), (AccessListStorageKey, EndVector, N_BYTES_WORD, vec![17]), // finished parsing storage keys From 3fd897712c85ba0bfc98820383326a3dbbca5eec Mon Sep 17 00:00:00 2001 From: xkx Date: Thu, 13 Jul 2023 11:21:54 +0800 Subject: [PATCH 08/16] Fix finding 17 (#602) * add q_last * fix --- zkevm-circuits/src/rlp_circuit_fsm.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index fc04e155d3..b0278ecd34 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -223,6 +223,8 @@ impl RlpFsmRomTable { pub struct RlpCircuitConfig { /// Whether the row is the first row. q_first: Column, + /// Whether the row is the last row. + q_last: Column, /// The state of RLP verifier at the current row. state: Column, /// A utility gadget to compare/query what state we are at. @@ -336,6 +338,7 @@ impl RlpCircuitConfig { let q_enabled = rlp_table.q_enable; let ( q_first, + q_last, byte_idx, byte_rev_idx, byte_value, @@ -354,6 +357,7 @@ impl RlpCircuitConfig { transit_to_new_rlp_instance, is_same_rlp_instance, ) = ( + meta.fixed_column(), meta.fixed_column(), meta.advice_column(), meta.advice_column(), @@ -1429,8 +1433,17 @@ impl RlpCircuitConfig { ])) }); + meta.create_gate("sm ends in End state", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + constrain_eq!(meta, cb, state, State::End); + + cb.gate(meta.query_fixed(q_last, Rotation::cur())) + }); + Self { q_first, + q_last, state, state_bits, rlp_table, @@ -1889,6 +1902,13 @@ impl RlpCircuitConfig { for i in sm_rows.len()..last_row { self.assign_sm_end_row(&mut region, i)?; } + region.assign_fixed(|| "q_first", self.q_first, 0, || Value::known(F::one()))?; + region.assign_fixed( + || "q_last", + self.q_last, + last_row - 1, + || Value::known(F::one()), + )?; Ok(()) }, From baa4a7be0a68e8a3da63b6b2caeb0902d81ec90f Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Thu, 13 Jul 2023 17:31:25 +0800 Subject: [PATCH 09/16] add more diff range check --- zkevm-circuits/src/rlp_circuit_fsm.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index b0278ecd34..94fe17aefc 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -824,6 +824,7 @@ impl RlpCircuitConfig { diffs.extend(byte_value_lte_0xf8.lt_chip.config().diff); diffs.extend(tidx_lte_tlength.lt_chip.config().diff); diffs.extend(mlength_lte_0x20.lt_chip.config().diff); + diffs.extend(tlength_lte_mlength.lt_chip.config().diff); for diff in diffs { meta.lookup_any("diff in range256", |meta| { From 158694342960353aa40a221c18dd8d28cff56706 Mon Sep 17 00:00:00 2001 From: xkx Date: Wed, 26 Jul 2023 20:11:13 +0800 Subject: [PATCH 10/16] fix finding 7 (#625) --- zkevm-circuits/src/rlp_circuit_fsm.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index 94fe17aefc..bcd9cb8273 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -951,6 +951,7 @@ impl RlpCircuitConfig { let mut cb = BaseConstraintBuilder::default(); let tag = tag_expr(meta); + constrain_eq!(meta, cb, state, DecodeTagStart.expr()); constrain_eq!(meta, cb, byte_idx, 1.expr()); cb.require_zero( "tag == TxType or tag == BeginList", From e22aba5d083cd3819bee3466641a573028d715e3 Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Wed, 26 Jul 2023 20:14:47 +0800 Subject: [PATCH 11/16] tx_id = 1 when sm starts --- zkevm-circuits/src/rlp_circuit_fsm.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index bcd9cb8273..dd5c5cc06c 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -952,6 +952,7 @@ impl RlpCircuitConfig { let tag = tag_expr(meta); constrain_eq!(meta, cb, state, DecodeTagStart.expr()); + constrain_eq!(meta, cb, tx_id, 1.expr()); constrain_eq!(meta, cb, byte_idx, 1.expr()); cb.require_zero( "tag == TxType or tag == BeginList", From 2ad2e75b762f63851c9e7cf81995c99323947c7d Mon Sep 17 00:00:00 2001 From: xkx Date: Wed, 16 Aug 2023 10:58:58 +0800 Subject: [PATCH 12/16] 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 --- zkevm-circuits/src/rlp_circuit_fsm.rs | 53 ++++++++--- zkevm-circuits/src/table.rs | 20 +++++ zkevm-circuits/src/tx_circuit.rs | 121 ++++++++++++++++++++++++-- zkevm-circuits/src/witness/rlp_fsm.rs | 60 ++++++++++++- zkevm-circuits/src/witness/tx.rs | 20 ++++- 5 files changed, 250 insertions(+), 24 deletions(-) diff --git a/zkevm-circuits/src/rlp_circuit_fsm.rs b/zkevm-circuits/src/rlp_circuit_fsm.rs index dd5c5cc06c..72d23ba69a 100644 --- a/zkevm-circuits/src/rlp_circuit_fsm.rs +++ b/zkevm-circuits/src/rlp_circuit_fsm.rs @@ -254,8 +254,6 @@ pub struct RlpCircuitConfig { /// When the tag occupies several bytes, this index denotes the /// incremental index of the byte within this tag instance. tag_idx: Column, - /// The length of bytes that hold this tag's value. - tag_length: Column, /// The accumulated value of the tag's bytes up to `tag_idx`. tag_value_acc: Column, /// The depth at this row. Since RLP encoded data can be nested, we use @@ -314,6 +312,8 @@ pub struct RlpCircuitConfig { depth_check: IsEqualConfig, /// Check for depth == 1 depth_eq_one: IsEqualConfig, + /// Check for byte_value == 0 + byte_value_is_zero: IsZeroConfig, /// Internal tables /// Data table @@ -335,6 +335,7 @@ impl RlpCircuitConfig { challenges: &Challenges>, ) -> Self { let (tx_id, format) = (rlp_table.tx_id, rlp_table.format); + let tag_length = rlp_table.tag_length; let q_enabled = rlp_table.q_enable; let ( q_first, @@ -349,7 +350,6 @@ impl RlpCircuitConfig { is_list, max_length, tag_idx, - tag_length, depth, is_tag_begin, is_tag_end, @@ -375,7 +375,6 @@ impl RlpCircuitConfig { meta.advice_column(), meta.advice_column(), meta.advice_column(), - meta.advice_column(), ); let tag_value_acc = meta.advice_column_in(SecondPhase); @@ -811,6 +810,12 @@ impl RlpCircuitConfig { |meta| meta.query_advice(format, Rotation::cur()), |meta| meta.query_advice(format, Rotation::next()), ); + let byte_value_is_zero = IsZeroChip::configure( + meta, + |meta| meta.query_fixed(q_enabled, Rotation::cur()), + byte_value, + |meta| meta.advice_column(), + ); // constrain diff belong to range256 table for each comparator let mut diffs = vec![]; @@ -920,6 +925,9 @@ impl RlpCircuitConfig { let tag_idx_expr = |meta: &mut VirtualCells| meta.query_advice(tag_idx, Rotation::cur()); let tag_value_acc_expr = |meta: &mut VirtualCells| meta.query_advice(tag_value_acc, Rotation::cur()); + let tag_bytes_rlc_expr = |meta: &mut VirtualCells| { + meta.query_advice(rlp_table.tag_bytes_rlc, Rotation::cur()) + }; let is_tag_next_end_expr = |meta: &mut VirtualCells| meta.query_advice(is_tag_end, Rotation::next()); let is_tag_end_expr = @@ -981,6 +989,8 @@ impl RlpCircuitConfig { // is_list = false, tag_value_acc = byte_value constrain_eq!(meta, cb, is_list, false); constrain_eq!(meta, cb, rlp_table.tag_value, byte_value_expr); + constrain_eq!(meta, cb, rlp_table.tag_bytes_rlc, byte_value_expr); + constrain_eq!(meta, cb, rlp_table.tag_length, 1); // state transitions. update_state!(meta, cb, tag, tag_next_expr(meta)); @@ -997,6 +1007,8 @@ impl RlpCircuitConfig { constrain_eq!(meta, cb, is_list, false); constrain_eq!(meta, cb, rlp_table.tag_value, 0); + constrain_eq!(meta, cb, rlp_table.tag_bytes_rlc, 0); + constrain_eq!(meta, cb, rlp_table.tag_length, 0); // state transitions. update_state!(meta, cb, tag, tag_next_expr(meta)); @@ -1149,6 +1161,7 @@ impl RlpCircuitConfig { update_state!(meta, cb, tag_idx, 1); update_state!(meta, cb, tag_length, byte_value_expr(meta) - 0x80.expr()); update_state!(meta, cb, tag_value_acc, byte_value_next_expr(meta)); + update_state!(meta, cb, rlp_table.tag_bytes_rlc, byte_value_next_expr(meta)); update_state!(meta, cb, state, State::Bytes); // depth is unchanged. @@ -1173,7 +1186,7 @@ impl RlpCircuitConfig { let b = select::expr( mlen_lt_0x20, 256.expr(), - select::expr(mlen_eq_0x20, evm_word_rand, keccak_input_rand), + select::expr(mlen_eq_0x20, evm_word_rand, keccak_input_rand.expr()), ); // Bytes => Bytes @@ -1185,6 +1198,8 @@ impl RlpCircuitConfig { update_state!(meta, cb, tag_idx, tag_idx_expr(meta) + 1.expr()); update_state!(meta, cb, tag_value_acc, tag_value_acc_expr(meta) * b.expr() + byte_value_next_expr(meta)); + update_state!(meta, cb, rlp_table.tag_bytes_rlc, + tag_bytes_rlc_expr(meta) * keccak_input_rand.expr() + byte_value_next_expr(meta)); update_state!(meta, cb, state, State::Bytes); // depth, tag_length unchanged. @@ -1286,6 +1301,8 @@ impl RlpCircuitConfig { // state transition. update_state!(meta, cb, tag_length, tag_value_acc_expr(meta)); update_state!(meta, cb, tag_idx, 1); + update_state!(meta, cb, rlp_table.tag_bytes_rlc, byte_value_next_expr(meta)); + update_state!(meta, cb, tag_value_acc, byte_value_next_expr(meta)); update_state!(meta, cb, state, State::Bytes); // depth is unchanged. @@ -1459,7 +1476,6 @@ impl RlpCircuitConfig { bytes_rlc, gas_cost_acc, tag_idx, - tag_length, tag_value_acc, is_list, max_length, @@ -1493,6 +1509,7 @@ impl RlpCircuitConfig { tlength_lte_mlength, depth_check, depth_eq_one, + byte_value_is_zero, // internal tables data_table, @@ -1544,6 +1561,18 @@ impl RlpCircuitConfig { row, || witness.rlp_table.tag_value, )?; + region.assign_advice( + || "rlp_table.tag_bytes_rlc", + self.rlp_table.tag_bytes_rlc, + row, + || witness.rlp_table.tag_bytes_rlc, + )?; + region.assign_advice( + || "rlp_table.tag_length", + self.rlp_table.tag_length, + row, + || Value::known(F::from(witness.rlp_table.tag_length as u64)), + )?; region.assign_advice( || "rlp_table.is_output", self.rlp_table.is_output, @@ -1594,12 +1623,6 @@ impl RlpCircuitConfig { row, || Value::known(F::from(witness.state_machine.tag_idx as u64)), )?; - region.assign_advice( - || "sm.tag_length", - self.tag_length, - row, - || Value::known(F::from(witness.state_machine.tag_length as u64)), - )?; region.assign_advice( || "sm.depth", self.depth, @@ -1710,13 +1733,13 @@ impl RlpCircuitConfig { region, row, F::from(witness.state_machine.tag_idx as u64), - F::from(witness.state_machine.tag_length as u64), + F::from(witness.rlp_table.tag_length as u64), )?; let tlength_lte_mlength_chip = ComparatorChip::construct(self.tlength_lte_mlength.clone()); tlength_lte_mlength_chip.assign( region, row, - F::from(witness.state_machine.tag_length as u64), + F::from(witness.rlp_table.tag_length as u64), F::from(witness.state_machine.max_length as u64), )?; @@ -1776,6 +1799,8 @@ impl RlpCircuitConfig { for (chip, lhs, rhs) in byte_value_checks { chip.assign(region, row, lhs, rhs)?; } + let bv_chip = IsZeroChip::construct(self.byte_value_is_zero.clone()); + bv_chip.assign(region, row, Value::known(byte_value))?; Ok(()) } diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index cfc297dcac..d29fd5f34b 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -2060,6 +2060,10 @@ pub struct RlpFsmRlpTable { pub rlp_tag: Column, /// The actual value of the current tag being decoded. pub tag_value: Column, + /// RLC of the tag's big-endian bytes + pub tag_bytes_rlc: Column, + /// The actual length of bytes of the current tag being decoded. + pub tag_length: Column, /// Whether or not the row emits an output value. pub is_output: Column, /// Whether or not the current tag's value was nil. @@ -2074,6 +2078,8 @@ impl LookupTable for RlpFsmRlpTable { self.format.into(), self.rlp_tag.into(), self.tag_value.into(), + self.tag_bytes_rlc.into(), + self.tag_length.into(), self.is_output.into(), self.is_none.into(), ] @@ -2086,6 +2092,8 @@ impl LookupTable for RlpFsmRlpTable { String::from("format"), String::from("rlp_tag"), String::from("tag_value_acc"), + String::from("tag_bytes_rlc"), + String::from("tag_length"), String::from("is_output"), String::from("is_none"), ] @@ -2101,6 +2109,8 @@ impl RlpFsmRlpTable { format: meta.advice_column(), rlp_tag: meta.advice_column(), tag_value: meta.advice_column_in(SecondPhase), + tag_bytes_rlc: meta.advice_column_in(SecondPhase), + tag_length: meta.advice_column(), is_output: meta.advice_column(), is_none: meta.advice_column(), } @@ -2154,6 +2164,16 @@ impl RlpFsmRlpTable { Value::known(F::from(usize::from(row.rlp_tag) as u64)), ), ("tag_value", self.tag_value.into(), row.tag_value), + ( + "tag_bytes_rlc", + self.tag_bytes_rlc.into(), + row.tag_bytes_rlc, + ), + ( + "tag_length", + self.tag_length.into(), + Value::known(F::from(row.tag_length as u64)), + ), ("is_output", self.is_output.into(), Value::known(F::one())), ( "is_none", diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 88289fbe14..d6f2a15b0b 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -24,7 +24,9 @@ use crate::{ witness::{rlp_fsm::Tag, RlpTag, Transaction}, }; use bus_mapping::circuit_input_builder::keccak_inputs_sign_verify; -use eth_types::{sign_types::SignData, Address, Field, ToAddress, ToLittleEndian, ToScalar}; +use eth_types::{ + sign_types::SignData, Address, Field, ToAddress, ToBigEndian, ToLittleEndian, ToScalar, +}; use gadgets::{ binary_number::{BinaryNumberChip, BinaryNumberConfig}, is_equal::{IsEqualChip, IsEqualConfig, IsEqualInstruction}, @@ -56,11 +58,13 @@ use halo2_proofs::plonk::FirstPhase as SecondPhase; #[cfg(not(feature = "onephase"))] use halo2_proofs::plonk::SecondPhase; use halo2_proofs::plonk::{Fixed, TableColumn}; +use itertools::Itertools; use crate::{ table::{BlockContextFieldTag::CumNumTxs, TxFieldTag::ChainID}, util::rlc_be_bytes, witness::{ + rlp_fsm::ValueTagLength, Format::{L1MsgHash, TxHashEip155, TxHashPreEip155, TxSignEip155, TxSignPreEip155}, RlpTag::{GasCost, Len, Null, RLC}, Tag::TxType as RLPTxType, @@ -105,6 +109,8 @@ pub struct TxCircuitConfig { rlp_tag: Column, // Whether tag's RLP-encoded value is 0x80 = rlp([]) is_none: Column, + tx_value_length: Column, + tx_value_rlc: Column, u16_table: TableColumn, @@ -189,6 +195,8 @@ impl SubCircuitConfig for TxCircuitConfig { // tag, rlp_tag, tx_type, is_none let tx_type = meta.advice_column(); let rlp_tag = meta.advice_column(); + let tx_value_rlc = meta.advice_column_in(SecondPhase); + let tx_value_length = meta.advice_column(); let is_none = meta.advice_column(); let tag_bits = BinaryNumberChip::configure(meta, q_enable, Some(tx_table.tag.into())); let tx_type_bits = BinaryNumberChip::configure(meta, q_enable, Some(tx_type.into())); @@ -630,6 +638,8 @@ impl SubCircuitConfig for TxCircuitConfig { meta, q_enable, rlp_tag, + tx_value_rlc, + tx_value_length, tx_type_bits, is_none, &lookup_conditions, @@ -875,6 +885,8 @@ impl SubCircuitConfig for TxCircuitConfig { tx_type_bits, rlp_tag, is_none, + tx_value_rlc, + tx_value_length, u16_table, tx_id_is_zero, value_is_zero, @@ -907,6 +919,8 @@ impl TxCircuitConfig { meta: &mut ConstraintSystem, q_enable: Column, rlp_tag: Column, + tx_value_rlc: Column, + tx_value_length: Column, tx_type_bits: BinaryNumberConfig, is_none: Column, lookup_conditions: &HashMap>, @@ -1010,6 +1024,8 @@ impl TxCircuitConfig { let enable = and::expr([meta.query_fixed(q_enable, Rotation::cur()), is_l1_msg(meta)]); let hash_format = L1MsgHash.expr(); let tag_value = 0x7E.expr(); + let tag_bytes_rlc = 0x7E.expr(); + let tag_length = 1.expr(); let input_exprs = vec![ 1.expr(), // q_enable = true @@ -1017,6 +1033,8 @@ impl TxCircuitConfig { hash_format, RLPTxType.expr(), tag_value, + tag_bytes_rlc, + tag_length, 1.expr(), // is_output = true 0.expr(), // is_none = false ]; @@ -1050,11 +1068,13 @@ impl TxCircuitConfig { sign_format, rlp_tag, meta.query_advice(tx_table.value, Rotation::cur()), + meta.query_advice(tx_value_rlc, Rotation::cur()), + meta.query_advice(tx_value_length, Rotation::cur()), 1.expr(), // is_output = true is_none, ] .into_iter() - .zip(rlp_table.table_exprs(meta).into_iter()) // tag_length_eq_one is the 6th column in rlp table + .zip_eq(rlp_table.table_exprs(meta).into_iter()) // tag_length_eq_one is the 6th column in rlp table .map(|(arg, table)| (enable.clone() * arg, table)) .collect() }); @@ -1086,11 +1106,13 @@ impl TxCircuitConfig { hash_format, rlp_tag, meta.query_advice(tx_table.value, Rotation::cur()), + meta.query_advice(tx_value_rlc, Rotation::cur()), + meta.query_advice(tx_value_length, Rotation::cur()), 1.expr(), // is_output = true is_none, ] .into_iter() - .zip(rlp_table.table_exprs(meta).into_iter()) + .zip_eq(rlp_table.table_exprs(meta).into_iter()) .map(|(arg, table)| (enable.clone() * arg, table)) .collect() }); @@ -1205,6 +1227,8 @@ impl TxCircuitConfig { tx_id_next: usize, tag: TxFieldTag, value: Value, + be_bytes_rlc: Option>, + be_bytes_length: Option, rlp_tag: Option, is_none: Option, is_padding_tx: Option, @@ -1236,6 +1260,18 @@ impl TxCircuitConfig { *offset, || Value::known(F::from(is_none.unwrap_or(false) as u64)), )?; + region.assign_advice( + || "value_be_bytes_rlc", + self.tx_value_rlc, + *offset, + || be_bytes_rlc.map_or(Value::known(F::zero()), |rlc| rlc), + )?; + region.assign_advice( + || "value_be_bytes_length", + self.tx_value_length, + *offset, + || Value::known(F::from(be_bytes_length.unwrap_or(0) as u64)), + )?; // assign to lookup condition columns let is_l1_msg = tx.map(|tx| tx.tx_type.is_l1_msg()).unwrap_or(false); @@ -1667,6 +1703,8 @@ impl TxCircuit { None, None, None, + None, + None, )?; // Assign all tx fields except for call data @@ -1700,24 +1738,39 @@ impl TxCircuit { }) }; log::debug!("calldata len: {}", tx.call_data.len()); - for (tag, rlp_tag, is_none, value) in [ + for (tag, rlp_tag, is_none, be_bytes_rlc, be_bytes_length, value) in [ // need to be in same order as that tx table load function uses ( Nonce, Some(Tag::Nonce.into()), Some(tx.nonce == 0), + Some(rlc_be_bytes( + &tx.nonce.to_be_bytes(), + challenges.keccak_input(), + )), + Some(tx.nonce.tag_length()), Value::known(F::from(tx.nonce)), ), ( Gas, Some(Tag::Gas.into()), Some(tx.gas == 0), + Some(rlc_be_bytes( + &tx.gas.to_be_bytes(), + challenges.keccak_input(), + )), + Some(tx.gas.tag_length()), Value::known(F::from(tx.gas)), ), ( GasPrice, Some(Tag::GasPrice.into()), Some(tx.gas_price.is_zero()), + Some(rlc_be_bytes( + &tx.gas_price.to_be_bytes(), + challenges.keccak_input(), + )), + Some(tx.gas_price.tag_length()), challenges .evm_word() .map(|challenge| rlc(tx.gas_price.to_le_bytes(), challenge)), @@ -1726,12 +1779,21 @@ impl TxCircuit { CallerAddress, Some(Tag::Sender.into()), None, + Some(rlc_be_bytes( + &tx.caller_address.to_fixed_bytes(), + challenges.keccak_input(), + )), + Some(tx.caller_address.tag_length()), Value::known(tx.caller_address.to_scalar().expect("tx.from too big")), ), ( CalleeAddress, Some(Tag::To.into()), Some(tx.callee_address.is_none()), + Some(tx.callee_address.map_or(Value::known(F::zero()), |callee| { + rlc_be_bytes(&callee.to_fixed_bytes(), challenges.keccak_input()) + })), + Some(tx.callee_address.tag_length()), Value::known( tx.callee_address .unwrap_or(Address::zero()) @@ -1743,12 +1805,19 @@ impl TxCircuit { IsCreate, None, None, + None, + None, Value::known(F::from(tx.is_create as u64)), ), ( TxFieldTag::Value, Some(Tag::Value.into()), Some(tx.value.is_zero()), + Some(rlc_be_bytes( + &tx.value.to_be_bytes(), + challenges.keccak_input(), + )), + Some(tx.value.tag_length()), challenges .evm_word() .map(|challenge| rlc(tx.value.to_le_bytes(), challenge)), @@ -1757,37 +1826,59 @@ impl TxCircuit { CallDataRLC, Some(Tag::Data.into()), Some(tx.call_data.is_empty()), + Some(rlc_be_bytes(&tx.call_data, challenges.keccak_input())), + Some(tx.call_data.tag_length()), rlc_be_bytes(&tx.call_data, challenges.keccak_input()), ), ( CallDataLength, None, None, + None, + None, Value::known(F::from(tx.call_data.len() as u64)), ), ( CallDataGasCost, None, None, + None, + None, Value::known(F::from(tx.call_data_gas_cost)), ), ( TxDataGasCost, Some(GasCost), None, + None, + None, Value::known(F::from(tx.tx_data_gas_cost)), ), - (ChainID, None, None, Value::known(F::from(tx.chain_id))), + ( + ChainID, + None, + None, + Some(rlc_be_bytes( + &tx.chain_id.to_be_bytes(), + challenges.keccak_input(), + )), + Some(tx.chain_id.tag_length()), + Value::known(F::from(tx.chain_id)), + ), ( SigV, Some(Tag::SigV.into()), Some(tx.v.is_zero()), + Some(rlc_be_bytes(&tx.v.to_be_bytes(), challenges.keccak_input())), + Some(tx.v.tag_length()), Value::known(F::from(tx.v)), ), ( SigR, Some(Tag::SigR.into()), Some(tx.r.is_zero()), + Some(rlc_be_bytes(&tx.r.to_be_bytes(), challenges.keccak_input())), + Some(tx.r.tag_length()), challenges .evm_word() .map(|challenge| rlc(tx.r.to_le_bytes(), challenge)), @@ -1796,6 +1887,8 @@ impl TxCircuit { SigS, Some(Tag::SigS.into()), Some(tx.s.is_zero()), + Some(rlc_be_bytes(&tx.s.to_be_bytes(), challenges.keccak_input())), + Some(tx.s.tag_length()), challenges .evm_word() .map(|challenge| rlc(tx.s.to_le_bytes(), challenge)), @@ -1804,29 +1897,37 @@ impl TxCircuit { TxSignLength, Some(Len), Some(false), + None, + Some(rlp_unsigned_tx_be_bytes.len().tag_length()), Value::known(F::from(rlp_unsigned_tx_be_bytes.len() as u64)), ), ( TxSignRLC, Some(RLC), Some(false), + None, + None, challenges.keccak_input().map(|rand| { rlp_unsigned_tx_be_bytes .iter() .fold(F::zero(), |acc, byte| acc * rand + F::from(*byte as u64)) }), ), - (TxSignHash, None, None, tx_sign_hash), + (TxSignHash, None, None, None, None, tx_sign_hash), ( TxHashLength, Some(Len), Some(false), + None, + Some(rlp_signed_tx_be_bytes.len().tag_length()), Value::known(F::from(rlp_signed_tx_be_bytes.len() as u64)), ), ( TxHashRLC, Some(RLC), Some(false), + None, + None, challenges.keccak_input().map(|rand| { rlp_signed_tx_be_bytes .iter() @@ -1837,6 +1938,8 @@ impl TxCircuit { TxFieldTag::TxHash, None, None, + None, + None, challenges.evm_word().map(|challenge| { tx.hash .to_fixed_bytes() @@ -1850,6 +1953,8 @@ impl TxCircuit { BlockNumber, None, None, + None, + None, Value::known(F::from(tx.block_number)), ), ] { @@ -1876,6 +1981,8 @@ impl TxCircuit { tx_id_next, // tx_id_next tag, value, + be_bytes_rlc, + be_bytes_length, rlp_tag, is_none, Some(is_padding_tx), @@ -1934,6 +2041,8 @@ impl TxCircuit { None, None, None, + None, + None, Some(is_final), Some(calldata_gas_cost), )?; diff --git a/zkevm-circuits/src/witness/rlp_fsm.rs b/zkevm-circuits/src/witness/rlp_fsm.rs index a6825dc030..487691fa8d 100644 --- a/zkevm-circuits/src/witness/rlp_fsm.rs +++ b/zkevm-circuits/src/witness/rlp_fsm.rs @@ -1,10 +1,59 @@ -use eth_types::Field; +use eth_types::{Address, Field, H160, U256}; use gadgets::{impl_expr, util::Expr}; use halo2_proofs::{arithmetic::FieldExt, circuit::Value, plonk::Expression}; use strum_macros::EnumIter; use crate::util::Challenges; +pub(crate) trait ValueTagLength { + fn tag_length(&self) -> u32; +} + +impl ValueTagLength for u64 { + fn tag_length(&self) -> u32 { + // note that 0_u64 is encoded as [0x80] in RLP + // see the relevant code at https://github.com/paritytech/parity-common/blob/master/rlp/src/impls.rs#L208 + (64 - self.leading_zeros() + 7) / 8 + } +} + +impl ValueTagLength for usize { + fn tag_length(&self) -> u32 { + // usize is treated as same as u64 + (*self as u64).tag_length() + } +} + +impl ValueTagLength for U256 { + fn tag_length(&self) -> u32 { + // note that U256::zero() is encoded as [0x80] in RLP + // see the relevant code at https://github.com/paritytech/parity-common/blob/impl-rlp-v0.3.0/primitive-types/src/lib.rs#L117 + (256 - self.leading_zeros() + 7) / 8 + } +} + +impl ValueTagLength for H160 { + fn tag_length(&self) -> u32 { + 20 + } +} + +impl ValueTagLength for Option
{ + fn tag_length(&self) -> u32 { + if self.is_none() { + 0 + } else { + self.unwrap().tag_length() + } + } +} + +impl ValueTagLength for Vec { + fn tag_length(&self) -> u32 { + self.len() as u32 + } +} + /// RLP tags #[derive(Default, Clone, Copy, Debug, EnumIter, PartialEq, Eq)] pub enum Tag { @@ -511,6 +560,12 @@ pub struct RlpTable { pub rlp_tag: RlpTag, /// The tag's value pub tag_value: Value, + /// RLC of the tag's big-endian bytes + pub tag_bytes_rlc: Value, + /// Length of the tag's big-endian bytes + /// Note that we use (tag_bytes_rlc, tag_length) to identify + /// the tag's dynamic-sized big-endian bytes + pub tag_length: usize, /// If current row is for output pub is_output: bool, /// If current tag's value is None. @@ -536,8 +591,6 @@ pub struct StateMachine { pub byte_value: u8, /// The index of the actual bytes of tag pub tag_idx: usize, - /// The length of the actual bytes of tag - pub tag_length: usize, /// The accumulated value of bytes up to `tag_idx` of tag /// In most cases, RlpTable.tag_value == StateMachine.tag_value_acc. /// However, for RlpTag::Len, we have @@ -581,4 +634,5 @@ pub(crate) struct SmState { pub(crate) tag_idx: usize, pub(crate) tag_length: usize, pub(crate) tag_value_acc: Value, + pub(crate) tag_bytes_rlc: Value, } diff --git a/zkevm-circuits/src/witness/tx.rs b/zkevm-circuits/src/witness/tx.rs index 6205353155..41ba15db36 100644 --- a/zkevm-circuits/src/witness/tx.rs +++ b/zkevm-circuits/src/witness/tx.rs @@ -385,6 +385,7 @@ impl Transaction { tag_idx: 0, tag_length: 0, tag_value_acc: Value::known(F::zero()), + tag_bytes_rlc: Value::known(F::zero()), byte_idx: 0, depth: 0, }; @@ -453,6 +454,8 @@ impl Transaction { assert!(!cur.tag.is_list()); is_output = true; cur.tag_value_acc = Value::known(F::from(byte_value as u64)); + cur.tag_bytes_rlc = cur.tag_value_acc; + cur.tag_length = 1; // state transitions next.state = DecodeTagStart; @@ -462,6 +465,8 @@ impl Transaction { is_output = true; is_none = true; cur.tag_value_acc = Value::known(F::zero()); + cur.tag_bytes_rlc = cur.tag_value_acc; + cur.tag_length = 0; // state transitions next.state = DecodeTagStart; @@ -474,6 +479,7 @@ impl Transaction { next.tag_length = (byte_value - 0x80) as usize; next.tag_value_acc = Value::known(F::from(rlp_bytes[cur.byte_idx + 1] as u64)); + next.tag_bytes_rlc = next.tag_value_acc; next.state = State::Bytes; } else if byte_value < 0xc0 { // assertions @@ -493,6 +499,7 @@ impl Transaction { rlp_tag = RlpTag::Len; } cur.tag_value_acc = Value::known(F::from(u64::from(byte_value - 0xc0))); + cur.tag_length = 1; // state transitions let num_bytes_of_new_list = usize::from(byte_value - 0xc0); @@ -537,6 +544,8 @@ impl Transaction { next.tag_idx = cur.tag_idx + 1; next.tag_value_acc = cur.tag_value_acc * b + Value::known(F::from(rlp_bytes[cur.byte_idx + 1] as u64)); + next.tag_bytes_rlc = cur.tag_bytes_rlc * keccak_rand + + Value::known(F::from(rlp_bytes[cur.byte_idx + 1] as u64)); } else { // assertions is_output = true; @@ -564,6 +573,7 @@ impl Transaction { next.tag_length = lb_len; next.tag_value_acc = Value::known(F::from(u64::from(rlp_bytes[cur.byte_idx + 1]))); + next.tag_bytes_rlc = next.tag_value_acc; next.state = State::Bytes; } } @@ -648,6 +658,13 @@ impl Transaction { RlpTag::Tag(_) => cur.tag_value_acc, RlpTag::Null => unreachable!("Null is not used"), }; + let (tag_bytes_rlc, tag_length) = match rlp_tag { + // Len | RLC | GasCost are just meta-info extracted from keccak input bytes + RlpTag::Len => (Value::known(F::zero()), cur.tag_length), + RlpTag::RLC | RlpTag::GasCost => (Value::known(F::zero()), 0), + RlpTag::Tag(_) => (cur.tag_bytes_rlc, cur.tag_length), + RlpTag::Null => unreachable!("Null is not used"), + }; witness.push(RlpFsmWitnessRow { rlp_table: RlpTable { @@ -655,6 +672,8 @@ impl Transaction { format, rlp_tag, tag_value, + tag_bytes_rlc, + tag_length, is_output, is_none, }, @@ -667,7 +686,6 @@ impl Transaction { byte_rev_idx: rlp_bytes.len() - cur.byte_idx, byte_value, tag_idx: cur.tag_idx, - tag_length: cur.tag_length, tag_acc_value: cur.tag_value_acc, depth: cur.depth, bytes_rlc, From 97678799c8d6ba60b92f364324415d0c84cd6bb0 Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Wed, 16 Aug 2023 12:45:12 +0800 Subject: [PATCH 13/16] refactor bytes_rlc type --- zkevm-circuits/src/tx_circuit.rs | 80 ++++++++++++++------------------ 1 file changed, 34 insertions(+), 46 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index d6f2a15b0b..f81e6cce59 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -1227,7 +1227,7 @@ impl TxCircuitConfig { tx_id_next: usize, tag: TxFieldTag, value: Value, - be_bytes_rlc: Option>, + be_bytes_rlc: Value, be_bytes_length: Option, rlp_tag: Option, is_none: Option, @@ -1264,7 +1264,7 @@ impl TxCircuitConfig { || "value_be_bytes_rlc", self.tx_value_rlc, *offset, - || be_bytes_rlc.map_or(Value::known(F::zero()), |rlc| rlc), + || be_bytes_rlc, )?; region.assign_advice( || "value_be_bytes_length", @@ -1697,7 +1697,7 @@ impl TxCircuit { !sigs.is_empty() as usize, // tx_id_next TxFieldTag::Null, Value::known(F::zero()), - None, + challenges.keccak_input().map(|_| F::zero()), None, None, None, @@ -1706,6 +1706,7 @@ impl TxCircuit { None, None, )?; + let zero_rlc = challenges.keccak_input().map(|_| F::zero()); // Assign all tx fields except for call data for (i, sign_data) in sigs.iter().enumerate() { @@ -1742,12 +1743,9 @@ impl TxCircuit { // need to be in same order as that tx table load function uses ( Nonce, - Some(Tag::Nonce.into()), + Some(Tag::Nonce.into()), // lookup into RLP table Some(tx.nonce == 0), - Some(rlc_be_bytes( - &tx.nonce.to_be_bytes(), - challenges.keccak_input(), - )), + rlc_be_bytes(&tx.nonce.to_be_bytes(), challenges.keccak_input()), Some(tx.nonce.tag_length()), Value::known(F::from(tx.nonce)), ), @@ -1755,10 +1753,7 @@ impl TxCircuit { Gas, Some(Tag::Gas.into()), Some(tx.gas == 0), - Some(rlc_be_bytes( - &tx.gas.to_be_bytes(), - challenges.keccak_input(), - )), + rlc_be_bytes(&tx.gas.to_be_bytes(), challenges.keccak_input()), Some(tx.gas.tag_length()), Value::known(F::from(tx.gas)), ), @@ -1766,10 +1761,7 @@ impl TxCircuit { GasPrice, Some(Tag::GasPrice.into()), Some(tx.gas_price.is_zero()), - Some(rlc_be_bytes( - &tx.gas_price.to_be_bytes(), - challenges.keccak_input(), - )), + rlc_be_bytes(&tx.gas_price.to_be_bytes(), challenges.keccak_input()), Some(tx.gas_price.tag_length()), challenges .evm_word() @@ -1779,10 +1771,10 @@ impl TxCircuit { CallerAddress, Some(Tag::Sender.into()), None, - Some(rlc_be_bytes( + rlc_be_bytes( &tx.caller_address.to_fixed_bytes(), challenges.keccak_input(), - )), + ), Some(tx.caller_address.tag_length()), Value::known(tx.caller_address.to_scalar().expect("tx.from too big")), ), @@ -1790,9 +1782,11 @@ impl TxCircuit { CalleeAddress, Some(Tag::To.into()), Some(tx.callee_address.is_none()), - Some(tx.callee_address.map_or(Value::known(F::zero()), |callee| { - rlc_be_bytes(&callee.to_fixed_bytes(), challenges.keccak_input()) - })), + rlc_be_bytes( + &tx.callee_address + .map_or(vec![], |callee| callee.to_fixed_bytes().to_vec()), + challenges.keccak_input(), + ), Some(tx.callee_address.tag_length()), Value::known( tx.callee_address @@ -1803,9 +1797,9 @@ impl TxCircuit { ), ( IsCreate, + None, // do not lookup into RLP table None, - None, - None, + zero_rlc, None, Value::known(F::from(tx.is_create as u64)), ), @@ -1813,10 +1807,7 @@ impl TxCircuit { TxFieldTag::Value, Some(Tag::Value.into()), Some(tx.value.is_zero()), - Some(rlc_be_bytes( - &tx.value.to_be_bytes(), - challenges.keccak_input(), - )), + rlc_be_bytes(&tx.value.to_be_bytes(), challenges.keccak_input()), Some(tx.value.tag_length()), challenges .evm_word() @@ -1826,7 +1817,7 @@ impl TxCircuit { CallDataRLC, Some(Tag::Data.into()), Some(tx.call_data.is_empty()), - Some(rlc_be_bytes(&tx.call_data, challenges.keccak_input())), + rlc_be_bytes(&tx.call_data, challenges.keccak_input()), Some(tx.call_data.tag_length()), rlc_be_bytes(&tx.call_data, challenges.keccak_input()), ), @@ -1834,7 +1825,7 @@ impl TxCircuit { CallDataLength, None, None, - None, + zero_rlc, None, Value::known(F::from(tx.call_data.len() as u64)), ), @@ -1842,7 +1833,7 @@ impl TxCircuit { CallDataGasCost, None, None, - None, + zero_rlc, None, Value::known(F::from(tx.call_data_gas_cost)), ), @@ -1850,7 +1841,7 @@ impl TxCircuit { TxDataGasCost, Some(GasCost), None, - None, + zero_rlc, None, Value::known(F::from(tx.tx_data_gas_cost)), ), @@ -1858,10 +1849,7 @@ impl TxCircuit { ChainID, None, None, - Some(rlc_be_bytes( - &tx.chain_id.to_be_bytes(), - challenges.keccak_input(), - )), + rlc_be_bytes(&tx.chain_id.to_be_bytes(), challenges.keccak_input()), Some(tx.chain_id.tag_length()), Value::known(F::from(tx.chain_id)), ), @@ -1869,7 +1857,7 @@ impl TxCircuit { SigV, Some(Tag::SigV.into()), Some(tx.v.is_zero()), - Some(rlc_be_bytes(&tx.v.to_be_bytes(), challenges.keccak_input())), + rlc_be_bytes(&tx.v.to_be_bytes(), challenges.keccak_input()), Some(tx.v.tag_length()), Value::known(F::from(tx.v)), ), @@ -1877,7 +1865,7 @@ impl TxCircuit { SigR, Some(Tag::SigR.into()), Some(tx.r.is_zero()), - Some(rlc_be_bytes(&tx.r.to_be_bytes(), challenges.keccak_input())), + rlc_be_bytes(&tx.r.to_be_bytes(), challenges.keccak_input()), Some(tx.r.tag_length()), challenges .evm_word() @@ -1887,7 +1875,7 @@ impl TxCircuit { SigS, Some(Tag::SigS.into()), Some(tx.s.is_zero()), - Some(rlc_be_bytes(&tx.s.to_be_bytes(), challenges.keccak_input())), + rlc_be_bytes(&tx.s.to_be_bytes(), challenges.keccak_input()), Some(tx.s.tag_length()), challenges .evm_word() @@ -1897,7 +1885,7 @@ impl TxCircuit { TxSignLength, Some(Len), Some(false), - None, + zero_rlc, Some(rlp_unsigned_tx_be_bytes.len().tag_length()), Value::known(F::from(rlp_unsigned_tx_be_bytes.len() as u64)), ), @@ -1905,7 +1893,7 @@ impl TxCircuit { TxSignRLC, Some(RLC), Some(false), - None, + zero_rlc, None, challenges.keccak_input().map(|rand| { rlp_unsigned_tx_be_bytes @@ -1913,12 +1901,12 @@ impl TxCircuit { .fold(F::zero(), |acc, byte| acc * rand + F::from(*byte as u64)) }), ), - (TxSignHash, None, None, None, None, tx_sign_hash), + (TxSignHash, None, None, zero_rlc, None, tx_sign_hash), ( TxHashLength, Some(Len), Some(false), - None, + zero_rlc, Some(rlp_signed_tx_be_bytes.len().tag_length()), Value::known(F::from(rlp_signed_tx_be_bytes.len() as u64)), ), @@ -1926,7 +1914,7 @@ impl TxCircuit { TxHashRLC, Some(RLC), Some(false), - None, + zero_rlc, None, challenges.keccak_input().map(|rand| { rlp_signed_tx_be_bytes @@ -1938,7 +1926,7 @@ impl TxCircuit { TxFieldTag::TxHash, None, None, - None, + zero_rlc, None, challenges.evm_word().map(|challenge| { tx.hash @@ -1953,7 +1941,7 @@ impl TxCircuit { BlockNumber, None, None, - None, + zero_rlc, None, Value::known(F::from(tx.block_number)), ), @@ -2037,7 +2025,7 @@ impl TxCircuit { tx_id_next, // tx_id_next CallData, Value::known(F::from(*byte as u64)), - None, + zero_rlc, None, None, None, From 7cdca82ddd6cfce897fbcb8f3f3aa42ed9848eed Mon Sep 17 00:00:00 2001 From: xkx Date: Wed, 16 Aug 2023 16:36:38 +0800 Subject: [PATCH 14/16] 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 * fix finding 29 (#623) Co-authored-by: Rohit Narurkar * 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 * Fix Zellic / Kalos finding25 (#619) * fix finding 25 * add comment --------- Co-authored-by: Rohit Narurkar * fix conflicts --------- Co-authored-by: Rohit Narurkar Co-authored-by: Zhang Zhuo --- eth-types/src/geth_types.rs | 5 + zkevm-circuits/src/pi_circuit.rs | 23 +- zkevm-circuits/src/tx_circuit.rs | 405 +++++++++++++++++++++++--- zkevm-circuits/src/tx_circuit/test.rs | 2 +- zkevm-circuits/src/witness/tx.rs | 12 +- 5 files changed, 395 insertions(+), 52 deletions(-) diff --git a/eth-types/src/geth_types.rs b/eth-types/src/geth_types.rs index 344d8cd738..bfc3c6e73b 100644 --- a/eth-types/src/geth_types.rs +++ b/eth-types/src/geth_types.rs @@ -52,6 +52,11 @@ impl TxType { matches!(*self, TxType::L1Msg) } + /// If this type is Eip155 or not + pub fn is_eip155_tx(&self) -> bool { + matches!(*self, TxType::Eip155) + } + /// Get the type of transaction pub fn get_tx_type(tx: &crate::Transaction) -> Self { match tx.transaction_type { diff --git a/zkevm-circuits/src/pi_circuit.rs b/zkevm-circuits/src/pi_circuit.rs index 84c2b65153..d2929d82f9 100644 --- a/zkevm-circuits/src/pi_circuit.rs +++ b/zkevm-circuits/src/pi_circuit.rs @@ -1371,6 +1371,11 @@ impl PiCircuitConfig { Coinbase, Timestamp, Number, Difficulty, GasLimit, BaseFee, ChainId, NumTxs, CumNumTxs, ]; + + // index_cells of same block are equal to block_number. + let mut index_cells = vec![]; + let mut block_number_cell = None; + let mut cum_num_txs_field = F::from(cum_num_txs as u64); cum_num_txs += num_txs; for (row, tag) in block_ctx @@ -1384,9 +1389,6 @@ impl PiCircuitConfig { offset, || row[0], )?; - // index_cells of same block are equal to block_number. - let mut index_cells = vec![]; - let mut block_number_cell = None; for (column, value) in block_table_columns.iter().zip_eq(&row[1..]) { let cell = region.assign_advice( || format!("block table row {offset}"), @@ -1404,15 +1406,6 @@ impl PiCircuitConfig { block_value_cells.push(cell); } } - for i in 0..(index_cells.len() - 1) { - region.constrain_equal(index_cells[i].cell(), index_cells[i + 1].cell())?; - } - if *tag == Number { - region.constrain_equal( - block_number_cell.unwrap().cell(), - index_cells[0].cell(), - )?; - } region.assign_fixed( || "is_block_num_txs", @@ -1452,6 +1445,12 @@ impl PiCircuitConfig { } offset += 1; } + // block_num == index[0] + region.constrain_equal(block_number_cell.unwrap().cell(), index_cells[0].cell())?; + // index[i] == index[i+1] + for i in 0..(index_cells.len() - 1) { + region.constrain_equal(index_cells[i].cell(), index_cells[i + 1].cell())?; + } } Ok(block_value_cells) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 6e144db1dd..ee1454a3f9 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -16,7 +16,7 @@ use crate::{ evm_circuit::util::constraint_builder::{BaseConstraintBuilder, ConstrainBuilderCommon}, sig_circuit::SigCircuit, table::{ - BlockContextFieldTag::CumNumTxs, + BlockContextFieldTag::{CumNumTxs, NumTxs}, BlockTable, KeccakTable, LookupTable, RlpFsmRlpTable as RlpTable, SigTable, TxFieldTag, TxFieldTag::{ BlockNumber, CallData, CallDataGasCost, CallDataLength, CallDataRLC, CalleeAddress, @@ -52,6 +52,7 @@ use gadgets::{ binary_number::{BinaryNumberChip, BinaryNumberConfig}, comparator::{ComparatorChip, ComparatorConfig, ComparatorInstruction}, is_equal::{IsEqualChip, IsEqualConfig, IsEqualInstruction}, + less_than::{LtChip, LtConfig, LtInstruction}, util::{and, not, select, sum, Expr}, }; use halo2_proofs::{ @@ -99,9 +100,14 @@ enum LookupCondition { pub struct TxCircuitConfig { minimum_rows: usize, + // This is only true at the first row of calldata part of tx table + q_calldata_first: Column, + q_calldata_last: Column, tx_table: TxTable, tx_tag_bits: BinaryNumberConfig, + // A selector which is enabled at 2nd row + q_second: Column, tx_type: Column, tx_type_bits: BinaryNumberConfig, // The associated rlp tag to lookup in the RLP table @@ -138,13 +144,22 @@ pub struct TxCircuitConfig { /// An accumulator value used to correctly calculate the calldata gas cost /// for a tx. calldata_gas_cost_acc: Column, + /// An accumulator value used to correctly calculate the RLC(calldata) for a tx. + calldata_rlc: Column, + /// 1st phase column which equals to tx_table.value when is_calldata is true + /// We need this because tx_table.value is a 2nd phase column and is used to get calldata_rlc. + /// It's not safe to do RLC on columns of same phase. + calldata_byte: Column, /// Columns for ensuring that BlockNum is correct is_padding_tx: Column, /// Tx id must be no greater than cum_num_txs tx_id_cmp_cum_num_txs: ComparatorConfig, + tx_id_gt_prev_cnt: LtConfig, /// Cumulative number of txs up to a block cum_num_txs: Column, + /// Number of txs in a block + num_txs: Column, /// Address recovered by SignVerifyChip sv_address: Column, @@ -193,11 +208,13 @@ impl SubCircuitConfig for TxCircuitConfig { sig_table, u8_table, u16_table, - challenges: _, + challenges, }: Self::ConfigArgs, ) -> Self { let q_enable = tx_table.q_enable; - + let q_second = meta.fixed_column(); + let q_calldata_first = meta.fixed_column(); + let q_calldata_last = meta.fixed_column(); // tag, rlp_tag, tx_type, is_none let tx_type = meta.advice_column(); let rlp_tag = meta.advice_column(); @@ -209,11 +226,15 @@ impl SubCircuitConfig for TxCircuitConfig { // columns for constraining BlockNum is valid let cum_num_txs = meta.advice_column(); + // num_of_txs that each block contains + let num_txs = meta.advice_column(); let is_padding_tx = meta.advice_column(); // columns for accumulating length and gas_cost of call_data let is_final = meta.advice_column(); let calldata_gas_cost_acc = meta.advice_column(); + let calldata_rlc = meta.advice_column_in(SecondPhase); + let calldata_byte = meta.advice_column(); // booleans to reduce degree let is_l1_msg = meta.advice_column(); @@ -285,6 +306,13 @@ impl SubCircuitConfig for TxCircuitConfig { is_tx_tag!(is_block_num, BlockNumber); is_tx_tag!(is_tx_type, TxType); + let tx_id_unchanged = IsEqualChip::configure( + meta, + |meta| meta.query_fixed(q_enable, Rotation::cur()), + |meta| meta.query_advice(tx_table.tx_id, Rotation::cur()), + |meta| meta.query_advice(tx_table.tx_id, Rotation::next()), + ); + // testing if value is zero for tags let value_is_zero = IsZeroChip::configure( meta, @@ -306,6 +334,17 @@ impl SubCircuitConfig for TxCircuitConfig { ); // tx_id transition + meta.create_gate("tx_id starts with 1", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_equal( + "tx_id == 1", + meta.query_advice(tx_table.tx_id, Rotation::cur()), + 1.expr(), + ); + + cb.gate(meta.query_fixed(q_second, Rotation::cur())) + }); meta.create_gate("tx_id transition", |meta| { let mut cb = BaseConstraintBuilder::default(); @@ -334,6 +373,19 @@ impl SubCircuitConfig for TxCircuitConfig { }, ); + cb.condition(tx_id_unchanged.is_equal_expression.expr(), |cb| { + cb.require_equal( + "sv_address does not change", + meta.query_advice(sv_address, Rotation::next()), + meta.query_advice(sv_address, Rotation::cur()), + ); + cb.require_equal( + "is_padding_tx does not change", + meta.query_advice(is_padding_tx, Rotation::next()), + meta.query_advice(is_padding_tx, Rotation::cur()), + ); + }); + cb.gate(and::expr([ meta.query_fixed(q_enable, Rotation::cur()), not::expr(meta.query_advice(is_calldata, Rotation::next())), @@ -369,7 +421,7 @@ impl SubCircuitConfig for TxCircuitConfig { (is_hash(meta), Null), (is_data(meta), Null), (is_block_num(meta), Null), - (is_chain_id_expr(meta), Null), + (is_chain_id_expr(meta), Tag::ChainId.into()), (is_tx_type(meta), Null), ]; @@ -464,7 +516,7 @@ impl SubCircuitConfig for TxCircuitConfig { cb.require_equal( "is_calldata", - tag_bits.value_equals(CallData, Rotation::cur())(meta), + is_data(meta), meta.query_advice(is_calldata, Rotation::cur()), ); @@ -476,7 +528,7 @@ impl SubCircuitConfig for TxCircuitConfig { cb.require_equal( "is_caller_address", - tag_bits.value_equals(CallerAddress, Rotation::cur())(meta), + is_caller_addr(meta), meta.query_advice(is_caller_address, Rotation::cur()), ); @@ -488,7 +540,7 @@ impl SubCircuitConfig for TxCircuitConfig { cb.require_equal( "is_chain_id", - tag_bits.value_equals(ChainID, Rotation::cur())(meta), + is_chain_id_expr(meta), meta.query_advice(is_chain_id, Rotation::cur()), ); @@ -547,6 +599,10 @@ impl SubCircuitConfig for TxCircuitConfig { is_to(meta), is_value(meta), is_data_rlc(meta), + and::expr([ + meta.query_advice(is_chain_id, Rotation::cur()), + tx_type_bits.value_equals(Eip155, Rotation::cur())(meta), + ]), is_sign_length(meta), is_sign_rlc(meta), ]); @@ -654,13 +710,16 @@ impl SubCircuitConfig for TxCircuitConfig { tx_value_rlc, tx_value_length, tx_type_bits, + tx_id_is_zero.clone(), is_none, &lookup_conditions, is_final, + is_calldata, is_chain_id, is_l1_msg, sv_address, calldata_gas_cost_acc, + calldata_rlc, tx_table.clone(), keccak_table.clone(), rlp_table, @@ -703,6 +762,44 @@ impl SubCircuitConfig for TxCircuitConfig { cb.gate(meta.query_fixed(q_enable, Rotation::cur())) }); + // prev block's cum_num_txs < tx_id + let tx_id_gt_prev_cnt = LtChip::configure( + meta, + |meta| meta.query_fixed(q_enable, Rotation::cur()), + |meta| { + let num_txs = meta.query_advice(num_txs, Rotation::cur()); + let cum_num_txs = meta.query_advice(cum_num_txs, Rotation::cur()); + + cum_num_txs - num_txs + }, + |meta| meta.query_advice(tx_table.tx_id, Rotation::cur()), + u8_table.into(), + ); + + // last non-padding tx must have tx_id == cum_num_txs + meta.create_gate( + "last non-padding tx must have tx_id == cum_num_txs", + |meta| { + let mut cb = BaseConstraintBuilder::default(); + let is_tag_block_num = meta.query_advice(is_tag_block_num, Rotation::cur()); + let is_cur_tx_non_padding = + not::expr(meta.query_advice(is_padding_tx, Rotation::cur())); + let is_next_tx_padding = meta.query_advice(is_padding_tx, Rotation::next()); + let cum_num_txs = meta.query_advice(cum_num_txs, Rotation::cur()); + let tx_id = meta.query_advice(tx_table.tx_id, Rotation::cur()); + + // tag == BlockNum && cur tx is the last non-padding tx + cb.condition( + and::expr([is_tag_block_num, is_cur_tx_non_padding, is_next_tx_padding]), + |cb| { + cb.require_equal("tx_id == cum_num_txs", tx_id, cum_num_txs); + }, + ); + + cb.gate(meta.query_fixed(tx_table.q_enable, Rotation::cur())) + }, + ); + // tx_id <= cum_num_txs let tx_id_cmp_cum_num_txs = ComparatorChip::configure( meta, @@ -726,6 +823,26 @@ impl SubCircuitConfig for TxCircuitConfig { ])) }); + meta.lookup_any("num_txs in block table", |meta| { + let is_tag_block_num = meta.query_advice(is_tag_block_num, Rotation::cur()); + let block_num = meta.query_advice(tx_table.value, Rotation::cur()); + let num_txs = meta.query_advice(num_txs, Rotation::cur()); + + let input_expr = vec![NumTxs.expr(), block_num, num_txs]; + let table_expr = block_table.table_exprs(meta); + let condition = and::expr([ + is_tag_block_num, + not::expr(meta.query_advice(is_padding_tx, Rotation::cur())), + meta.query_fixed(q_enable, Rotation::cur()), + ]); + + input_expr + .into_iter() + .zip(table_expr.into_iter()) + .map(|(input, table)| (input * condition.clone(), table)) + .collect::>() + }); + meta.lookup_any("cum_num_txs in block table", |meta| { let is_tag_block_num = meta.query_advice(is_tag_block_num, Rotation::cur()); let block_num = meta.query_advice(tx_table.value, Rotation::cur()); @@ -749,13 +866,6 @@ impl SubCircuitConfig for TxCircuitConfig { //////////////////////////////////////////////////////////////////////// /////////// CallData length and gas_cost calculation ///////////////// //////////////////////////////////////////////////////////////////////// - let tx_id_unchanged = IsEqualChip::configure( - meta, - |meta| meta.query_fixed(q_enable, Rotation::cur()), - |meta| meta.query_advice(tx_table.tx_id, Rotation::cur()), - |meta| meta.query_advice(tx_table.tx_id, Rotation::next()), - ); - meta.lookup("tx_id_diff must in u16", |meta| { let q_enable = meta.query_fixed(q_enable, Rotation::next()); let is_calldata = meta.query_advice(is_calldata, Rotation::cur()); @@ -769,6 +879,55 @@ impl SubCircuitConfig for TxCircuitConfig { vec![(lookup_condition * (tx_id_next - tx_id), u16_table.into())] }); + meta.create_gate("last row of call data", |meta| { + let q_calldata_last = meta.query_fixed(q_calldata_last, Rotation::cur()); + let is_final = meta.query_advice(is_final, Rotation::cur()); + + vec![(q_calldata_last * (is_final - true.expr()))] + }); + meta.create_gate("calldata_byte == tx_table.value", |meta| { + let mut cb = BaseConstraintBuilder::default(); + let is_calldata = meta.query_advice(is_calldata, Rotation::cur()); + + cb.condition(is_calldata, |cb| { + cb.require_equal( + "calldata_byte == tx_table.value", + meta.query_advice(calldata_byte, Rotation::cur()), + meta.query_advice(tx_table.value, Rotation::cur()), + ); + }); + + cb.gate(meta.query_fixed(tx_table.q_enable, Rotation::cur())) + }); + + meta.create_gate("tx call data init", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + let value_is_zero = value_is_zero.expr(Rotation::cur())(meta); + let gas_cost = select::expr(value_is_zero, 4.expr(), 16.expr()); + + cb.require_equal( + "index == 0", + meta.query_advice(tx_table.index, Rotation::cur()), + 0.expr(), + ); + cb.require_equal( + "calldata_gas_cost_acc == gas_cost", + meta.query_advice(calldata_gas_cost_acc, Rotation::cur()), + gas_cost, + ); + cb.require_equal( + "calldata_rlc == byte", + meta.query_advice(calldata_rlc, Rotation::cur()), + meta.query_advice(tx_table.value, Rotation::cur()), + ); + + cb.gate(and::expr([ + meta.query_fixed(q_calldata_first, Rotation::cur()), + not::expr(tx_id_is_zero.expr(Rotation::cur())(meta)), + ])) + }); + meta.create_gate("tx call data bytes", |meta| { let mut cb = BaseConstraintBuilder::default(); @@ -796,16 +955,49 @@ impl SubCircuitConfig for TxCircuitConfig { meta.query_advice(calldata_gas_cost_acc, Rotation::next()), meta.query_advice(calldata_gas_cost_acc, Rotation::cur()) + gas_cost_next, ); + cb.require_equal( + "calldata_rlc' = calldata_rlc * r + byte'", + meta.query_advice(calldata_rlc, Rotation::next()), + meta.query_advice(calldata_rlc, Rotation::cur()) * challenges.keccak_input() + + meta.query_advice(tx_table.value, Rotation::next()), + ); }); // on the final call data byte, tx_id must change. - cb.condition(is_final_cur, |cb| { + cb.condition(is_final_cur.expr(), |cb| { cb.require_zero( "tx_id changes at is_final == 1", tx_id_unchanged.is_equal_expression.clone(), ); }); + cb.condition( + and::expr([ + is_final_cur, + not::expr(tx_id_is_zero.expr(Rotation::next())(meta)), + ]), + |cb| { + let value_next_is_zero = value_is_zero.expr(Rotation::next())(meta); + let gas_cost_next = select::expr(value_next_is_zero, 4.expr(), 16.expr()); + + cb.require_equal( + "index' == 0", + meta.query_advice(tx_table.index, Rotation::next()), + 0.expr(), + ); + cb.require_equal( + "calldata_gas_cost_acc' == gas_cost_next", + meta.query_advice(calldata_gas_cost_acc, Rotation::next()), + gas_cost_next, + ); + cb.require_equal( + "calldata_rlc' == byte'", + meta.query_advice(calldata_rlc, Rotation::next()), + meta.query_advice(tx_table.value, Rotation::next()), + ); + }, + ); + cb.gate(and::expr(vec![ meta.query_fixed(q_enable, Rotation::cur()), meta.query_advice(is_calldata, Rotation::cur()), @@ -894,6 +1086,8 @@ impl SubCircuitConfig for TxCircuitConfig { Self { minimum_rows: meta.minimum_rows(), + q_calldata_first, + q_calldata_last, tx_tag_bits: tag_bits, tx_type, tx_type_bits, @@ -901,6 +1095,7 @@ impl SubCircuitConfig for TxCircuitConfig { is_none, tx_value_rlc, tx_value_length, + q_second, u8_table, u16_table, tx_id_is_zero, @@ -909,6 +1104,7 @@ impl SubCircuitConfig for TxCircuitConfig { is_calldata, is_caller_address, tx_id_cmp_cum_num_txs, + tx_id_gt_prev_cnt, cum_num_txs, is_padding_tx, lookup_conditions, @@ -916,6 +1112,8 @@ impl SubCircuitConfig for TxCircuitConfig { is_chain_id, is_final, calldata_gas_cost_acc, + calldata_rlc, + calldata_byte, sv_address, sig_table, block_table, @@ -924,6 +1122,7 @@ impl SubCircuitConfig for TxCircuitConfig { rlp_table, is_tag_block_num, _marker: PhantomData, + num_txs, } } } @@ -937,13 +1136,16 @@ impl TxCircuitConfig { tx_value_rlc: Column, tx_value_length: Column, tx_type_bits: BinaryNumberConfig, + tx_id_is_zero: IsZeroConfig, is_none: Column, lookup_conditions: &HashMap>, is_final: Column, + is_calldata: Column, is_chain_id: Column, is_l1_msg_col: Column, sv_address: Column, calldata_gas_cost_acc: Column, + calldata_rlc: Column, tx_table: TxTable, keccak_table: KeccakTable, rlp_table: RlpTable, @@ -1026,6 +1228,33 @@ impl TxCircuitConfig { .map(|(arg, table)| (enable.clone() * arg, table)) .collect() }); + meta.lookup_any("lookup CallDataRLC in the calldata part", |meta| { + let is_call_data = meta.query_advice(is_calldata, Rotation::cur()); + let calldata_rlc = meta.query_advice(calldata_rlc, Rotation::cur()); + let enable = and::expr([ + meta.query_fixed(tx_table.q_enable, Rotation::cur()), + is_call_data, + not::expr(tx_id_is_zero.expr(Rotation::cur())(meta)), + meta.query_advice(is_final, Rotation::cur()), + ]); + + let input_exprs = vec![ + meta.query_advice(tx_table.tx_id, Rotation::cur()), + CallDataRLC.expr(), + calldata_rlc.expr(), + ]; + let table_exprs = vec![ + meta.query_advice(tx_table.tx_id, Rotation::cur()), + meta.query_fixed(tx_table.tag, Rotation::cur()), + meta.query_advice(tx_table.value, Rotation::cur()), + ]; + + input_exprs + .into_iter() + .zip(table_exprs.into_iter()) + .map(|(input, table)| (input * enable.expr(), table)) + .collect() + }); ///////////////////////////////////////////////////////////////// ///////////////// RLP table lookups ////////////////////// @@ -1228,8 +1457,10 @@ impl TxCircuitConfig { is_none: Option, is_padding_tx: Option, cum_num_txs: Option, + num_txs: Option, is_final: Option, calldata_gas_cost_acc: Option, + calldata_rlc: Option>, ) -> Result<(), Error> { // assign to tag, rlp_tag, is_none let tag_chip = BinaryNumberChip::construct(self.tx_tag_bits); @@ -1237,6 +1468,7 @@ impl TxCircuitConfig { let tx_type = tx.map_or(Default::default(), |tx| tx.tx_type); let tx_type_chip = BinaryNumberChip::construct(self.tx_type_bits); tx_type_chip.assign(region, *offset, &tx_type)?; + region.assign_advice( || "tx_type", self.tx_type, @@ -1304,7 +1536,9 @@ impl TxCircuitConfig { TxSignRLC, ]; let is_tag_in_set = sign_set.into_iter().filter(|_tag| tag == *_tag).count() == 1; - Value::known(F::from((is_tag_in_set && !is_l1_msg) as u64)) + let case1 = is_tag_in_set && !is_l1_msg; + let case2 = (tag == ChainID) && tx.map_or(false, |tx| tx.tx_type.is_eip155_tx()); + Value::known(F::from((case1 || case2) as u64)) }); // lookup to RLP table for hashing (non L1 msg) conditions.insert(LookupCondition::RlpHashTag, { @@ -1417,6 +1651,15 @@ impl TxCircuitConfig { *offset, || Value::known(F::from(calldata_gas_cost_acc.unwrap_or_default())), )?; + region.assign_advice( + || "calldata_rlc", + self.calldata_rlc, + *offset, + || calldata_rlc.unwrap_or(Value::known(F::zero())), + )?; + if tag == CallData { + region.assign_advice(|| "calldata_byte", self.calldata_byte, *offset, || value)?; + } // assign to region.assign_advice( @@ -1432,12 +1675,25 @@ impl TxCircuitConfig { F::from(tx_id as u64), F::from(cum_num_txs.unwrap_or_default() as u64), )?; + let tx_id_gt_prev_cnt = LtChip::construct(self.tx_id_gt_prev_cnt); + tx_id_gt_prev_cnt.assign( + region, + *offset, + F::from((cum_num_txs.unwrap_or_default() - num_txs.unwrap_or_default()) as u64), + F::from(tx_id as u64), + )?; region.assign_advice( || "cum_num_txs", self.cum_num_txs, *offset, || Value::known(F::from(cum_num_txs.unwrap_or_default() as u64)), )?; + region.assign_advice( + || "num_txs", + self.num_txs, + *offset, + || Value::known(F::from(num_txs.unwrap_or_default() as u64)), + )?; *offset += 1; @@ -1633,31 +1889,50 @@ impl TxCircuit { layouter.assign_region( || "dev block table", |mut region| { - for (offset, (block_num, cum_num_txs)) in iter::once((0, 0)) + for (offset, (block_num, cum_num_txs, num_txs)) in iter::once((0, 0, 0)) .chain(block_nums.iter().scan(0, |cum_num_txs, block_num| { - *cum_num_txs += num_txs_in_blocks[block_num]; - Some((*block_num, *cum_num_txs)) + let num_txs = num_txs_in_blocks[block_num]; + *cum_num_txs += num_txs; + Some((*block_num, *cum_num_txs, num_txs)) })) .enumerate() { region.assign_fixed( || "block_table.tag", config.block_table.tag, - offset, + 2 * offset, || Value::known(F::from(CumNumTxs as u64)), )?; region.assign_advice( || "block_table.index", config.block_table.index, - offset, + 2 * offset, || Value::known(F::from(block_num)), )?; region.assign_advice( || "block_table.value", config.block_table.value, - offset, + 2 * offset, || Value::known(F::from(cum_num_txs as u64)), )?; + region.assign_fixed( + || "block_table.tag", + config.block_table.tag, + 2 * offset + 1, + || Value::known(F::from(NumTxs as u64)), + )?; + region.assign_advice( + || "block_table.index", + config.block_table.index, + 2 * offset + 1, + || Value::known(F::from(block_num)), + )?; + region.assign_advice( + || "block_table.value", + config.block_table.value, + 2 * offset + 1, + || Value::known(F::from(num_txs as u64)), + )?; } Ok(()) }, @@ -1682,6 +1957,7 @@ impl TxCircuit { debug_assert_eq!(padding_txs.len() + self.txs.len(), sigs.len()); let mut cum_num_txs; + let mut num_txs; let mut is_padding_tx; // Empty entry config.assign_row( @@ -1700,6 +1976,15 @@ impl TxCircuit { None, None, None, + None, + None, + )?; + + region.assign_fixed( + || "q_second", + config.q_second, + 1, + || Value::known(F::one()), )?; let zero_rlc = challenges.keccak_input().map(|_| F::zero()); @@ -1710,17 +1995,25 @@ impl TxCircuit { } else { &padding_txs[i - self.txs.len()] }; + let block_num = tx.block_number; let rlp_unsigned_tx_be_bytes = tx.rlp_unsigned.clone(); let rlp_signed_tx_be_bytes = tx.rlp_signed.clone(); if i < self.txs.len() { cum_num_txs = self .txs .iter() - .filter(|tx| tx.block_number <= self.txs[i].block_number) + .filter(|tx| tx.block_number <= block_num) + .count(); + num_txs = self + .txs + .iter() + .filter(|tx| tx.block_number == block_num) .count(); + log::info!("num_txs: {}", num_txs); is_padding_tx = false; } else { cum_num_txs = 0; + num_txs = 0; is_padding_tx = true; } @@ -1744,14 +2037,6 @@ impl TxCircuit { Some(tx.nonce.tag_length()), Value::known(F::from(tx.nonce)), ), - ( - Gas, - Some(Tag::Gas.into()), - Some(tx.gas == 0), - rlc_be_bytes(&tx.gas.to_be_bytes(), challenges.keccak_input()), - Some(tx.gas.tag_length()), - Value::known(F::from(tx.gas)), - ), ( GasPrice, Some(Tag::GasPrice.into()), @@ -1762,6 +2047,14 @@ impl TxCircuit { .evm_word() .map(|challenge| rlc(tx.gas_price.to_le_bytes(), challenge)), ), + ( + Gas, + Some(Tag::Gas.into()), + Some(tx.gas == 0), + rlc_be_bytes(&tx.gas.to_be_bytes(), challenges.keccak_input()), + Some(tx.gas.tag_length()), + Value::known(F::from(tx.gas)), + ), ( CallerAddress, Some(Tag::Sender.into()), @@ -1842,8 +2135,8 @@ impl TxCircuit { ), ( ChainID, - None, - None, + Some(Tag::ChainId.into()), + Some(tx.chain_id.is_zero()), rlc_be_bytes(&tx.chain_id.to_be_bytes(), challenges.keccak_input()), Some(tx.chain_id.tag_length()), Value::known(F::from(tx.chain_id)), @@ -1978,6 +2271,8 @@ impl TxCircuit { is_none, Some(is_padding_tx), Some(cum_num_txs), + Some(num_txs), + None, None, None, )?; @@ -1992,11 +2287,13 @@ impl TxCircuit { } log::debug!("assigning calldata, offset {}", offset); + assert_eq!(offset, self.max_txs * TX_LEN + 1); // Assign call data let mut calldata_count = 0; for (i, tx) in self.txs.iter().enumerate() { let mut calldata_gas_cost = 0; + let mut calldata_rlc = Value::known(F::zero()); let calldata_length = tx.call_data.len(); calldata_count += calldata_length; for (index, byte) in tx.call_data.iter().enumerate() { @@ -2020,6 +2317,17 @@ impl TxCircuit { (i + 1, false) }; calldata_gas_cost += if byte.is_zero() { 4 } else { 16 }; + if i == 0 && index == 0 { + region.assign_fixed( + || "q_calldata_first", + config.q_calldata_first, + offset, + || Value::known(F::one()), + )?; + } + calldata_rlc = calldata_rlc + .zip(challenges.keccak_input()) + .map(|(rlc, r)| rlc * r + F::from(*byte as u64)); config.assign_row( &mut region, &mut offset, @@ -2034,12 +2342,24 @@ impl TxCircuit { None, None, None, + None, Some(is_final), Some(calldata_gas_cost), + Some(calldata_rlc), )?; } } + assert!(calldata_count <= self.max_calldata); + let q_calldata_last_offset = self.max_txs * TX_LEN + self.max_calldata; + if offset == q_calldata_last_offset + 1 { + region.assign_fixed( + || "q_calldata_last", + config.q_calldata_last, + q_calldata_last_offset, + || Value::known(F::one()), + )?; + } debug_assert_eq!(offset, self.max_txs * TX_LEN + 1 + calldata_count); Ok(offset) @@ -2056,6 +2376,25 @@ impl TxCircuit { layouter.assign_region( || "tx table (calldata zeros and paddings)", |mut region| { + if last_off == self.max_txs * TX_LEN + 1 { + // The txs do not have any call data bytes + // Therefore q_calldata_first is not assigned in prev region. + region.assign_fixed( + || "q_calldata_first", + config.q_calldata_first, + 0, + || Value::known(F::one()), + )?; + } + if last_off < self.max_txs * TX_LEN + 1 + self.max_calldata { + let calldata_count = last_off - self.max_txs * TX_LEN - 1; + region.assign_fixed( + || "q_calldata_last", + config.q_calldata_last, + self.max_calldata - calldata_count - 1, + || Value::known(F::one()), + )?; + } config.assign_calldata_zeros( &mut region, 0, diff --git a/zkevm-circuits/src/tx_circuit/test.rs b/zkevm-circuits/src/tx_circuit/test.rs index 165e3bbb90..8fb6b5595e 100644 --- a/zkevm-circuits/src/tx_circuit/test.rs +++ b/zkevm-circuits/src/tx_circuit/test.rs @@ -137,7 +137,7 @@ fn run( #[cfg(feature = "scroll")] fn tx_circuit_2tx_2max_tx() { const NUM_TXS: usize = 2; - const MAX_TXS: usize = 4; + const MAX_TXS: usize = 2; const MAX_CALLDATA: usize = 32; assert_eq!( diff --git a/zkevm-circuits/src/witness/tx.rs b/zkevm-circuits/src/witness/tx.rs index e071ffed01..5074292945 100644 --- a/zkevm-circuits/src/witness/tx.rs +++ b/zkevm-circuits/src/witness/tx.rs @@ -170,12 +170,6 @@ impl Transaction { Value::known(F::zero()), Value::known(F::from(self.nonce)), ], - [ - Value::known(F::from(self.id as u64)), - Value::known(F::from(TxContextFieldTag::Gas as u64)), - Value::known(F::zero()), - Value::known(F::from(self.gas)), - ], [ Value::known(F::from(self.id as u64)), Value::known(F::from(TxContextFieldTag::GasPrice as u64)), @@ -184,6 +178,12 @@ impl Transaction { .evm_word() .map(|challenge| rlc::value(&self.gas_price.to_le_bytes(), challenge)), ], + [ + Value::known(F::from(self.id as u64)), + Value::known(F::from(TxContextFieldTag::Gas as u64)), + Value::known(F::zero()), + Value::known(F::from(self.gas)), + ], [ Value::known(F::from(self.id as u64)), Value::known(F::from(TxContextFieldTag::CallerAddress as u64)), From 622039cef4babf6d90b0d9733b0605894855b81f Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Wed, 16 Aug 2023 17:44:58 +0800 Subject: [PATCH 15/16] use q_first instead --- zkevm-circuits/src/tx_circuit.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 862be1da9c..31fc2353bd 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -102,8 +102,8 @@ pub struct TxCircuitConfig { // This is only true at the first row of calldata part of tx table q_calldata_first: Column, q_calldata_last: Column, - // A selector which is enabled at 2nd row - q_second: Column, + // A selector which is enabled at 1st row + q_first: Column, tx_table: TxTable, tx_tag_bits: BinaryNumberConfig, @@ -136,7 +136,7 @@ pub struct TxCircuitConfig { is_chain_id: Column, lookup_conditions: HashMap>, - /// Columns for computing num_l1_msgs and num_l2_txs + /// Columns for computing num_all_txs tx_nonce: Column, block_num: Column, block_num_unchanged: IsEqualConfig, @@ -219,7 +219,7 @@ impl SubCircuitConfig for TxCircuitConfig { ) -> Self { let q_enable = tx_table.q_enable; - let q_second = meta.fixed_column(); + let q_first = meta.fixed_column(); let q_calldata_first = meta.fixed_column(); let q_calldata_last = meta.fixed_column(); // Since we allow skipping l1 txs that could cause potential circuit overflow, @@ -366,14 +366,16 @@ impl SubCircuitConfig for TxCircuitConfig { meta.create_gate("tx_id starts with 1", |meta| { let mut cb = BaseConstraintBuilder::default(); + // the first row in tx table are all-zero rows cb.require_equal( "tx_id == 1", - meta.query_advice(tx_table.tx_id, Rotation::cur()), + meta.query_advice(tx_table.tx_id, Rotation::next()), 1.expr(), ); - cb.gate(meta.query_fixed(q_second, Rotation::cur())) + cb.gate(meta.query_fixed(q_first, Rotation::cur())) }); + meta.create_gate("tx_id transition in the fixed part of tx table", |meta| { let mut cb = BaseConstraintBuilder::default(); @@ -845,7 +847,7 @@ impl SubCircuitConfig for TxCircuitConfig { let mut cb = BaseConstraintBuilder::default(); let queue_index = tx_nonce; // first tx in tx table - cb.condition(meta.query_fixed(q_second, Rotation::cur()), |cb| { + cb.condition(meta.query_fixed(q_first, Rotation::next()), |cb| { cb.require_equal( "num_all_txs_acc = is_l1_msg ? queue_index - total_l1_popped_before + 1 : 1", meta.query_advice(num_all_txs_acc, Rotation::cur()), @@ -1311,7 +1313,7 @@ impl SubCircuitConfig for TxCircuitConfig { Self { minimum_rows: meta.minimum_rows(), - q_second, + q_first, q_calldata_first, q_calldata_last, tx_tag_bits: tag_bits, @@ -2281,9 +2283,9 @@ impl TxCircuit { )?; region.assign_fixed( - || "q_second", - config.q_second, - 1, + || "q_first", + config.q_first, + 0, || Value::known(F::one()), )?; let zero_rlc = challenges.keccak_input().map(|_| F::zero()); From b19250899342e7b3910e8f317197d9726cfea70a Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Wed, 16 Aug 2023 17:53:37 +0800 Subject: [PATCH 16/16] fmt --- zkevm-circuits/src/tx_circuit.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 31fc2353bd..b05e1c2bce 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -2282,12 +2282,7 @@ impl TxCircuit { None, )?; - region.assign_fixed( - || "q_first", - config.q_first, - 0, - || Value::known(F::one()), - )?; + region.assign_fixed(|| "q_first", config.q_first, 0, || Value::known(F::one()))?; let zero_rlc = challenges.keccak_input().map(|_| F::zero()); // Assign all tx fields except for call data