From b81524ea3d7a1de34541ecf093fab75294c6a603 Mon Sep 17 00:00:00 2001 From: xkx Date: Wed, 11 Jan 2023 09:13:50 +0800 Subject: [PATCH] Fix/rlp data prefix (#243) * bug-fix: add placeholder row for DataPrefix if |tx.data|=1 * add fn min_num_rows_block * byte_value < 0x80 --- zkevm-circuits/src/rlp_circuit.rs | 171 +++++++++++++++--- .../src/witness/rlp_encode/common.rs | 15 +- 2 files changed, 157 insertions(+), 29 deletions(-) diff --git a/zkevm-circuits/src/rlp_circuit.rs b/zkevm-circuits/src/rlp_circuit.rs index 2c6c6aeef..96396ba78 100644 --- a/zkevm-circuits/src/rlp_circuit.rs +++ b/zkevm-circuits/src/rlp_circuit.rs @@ -65,6 +65,9 @@ pub struct RlpCircuitConfig { /// Denotes the index of this row, but reversed. It starts from `n` where /// `n` is the byte length of the RLP-encoded data and ends at `1`. rindex: Column, + /// Placeholder row do not increase the index and mainly used for + /// DataPrefix when |tx.data| = 1 and tx.data < 0x80. + placeholder: Column, /// Denotes the byte value at this row index from the RLP-encoded data. byte_value: Column, /// Denotes the RLC accumulator value used for call data bytes. @@ -130,6 +133,7 @@ impl RlpCircuitConfig { let is_last = meta.advice_column(); let index = meta.advice_column(); let rindex = meta.advice_column(); + let placeholder = meta.advice_column(); let byte_value = meta.advice_column(); let calldata_bytes_rlc_acc = meta.advice_column_in(SecondPhase); let tag_length = meta.advice_column(); @@ -578,6 +582,11 @@ impl RlpCircuitConfig { SigV.expr(), ChainId.expr(), ); + cb.require_equal( + "value_acc == length_acc", + meta.query_advice(rlp_table.value_acc, Rotation::cur()), + meta.query_advice(length_acc, Rotation::cur()), + ); cb.require_equal( "tag::next == RlpTxTag::ChainId", meta.query_advice(rlp_table.tag, Rotation::next()), @@ -610,6 +619,11 @@ impl RlpCircuitConfig { meta.query_advice(tag_length, Rotation::next()), meta.query_advice(length_acc, Rotation::cur()), ); + cb.require_equal( + "value_acc == length_acc", + meta.query_advice(rlp_table.value_acc, Rotation::cur()), + meta.query_advice(length_acc, Rotation::cur()), + ); }, ); @@ -648,12 +662,16 @@ impl RlpCircuitConfig { cb.condition( is_dp_tag.expr() * tindex_eq_tlength * tlength_eq, |cb| { - cb.require_equal("127 < value", value_gt_127.is_lt(meta, None), 1.expr()); cb.require_equal("value < 184", value_lt_184.is_lt(meta, None), 1.expr()); + let real_length_acc = select::expr( + value_gt_127.is_lt(meta, None), + meta.query_advice(byte_value, Rotation::cur()) - 128.expr(), // value > 127 + 1.expr(), + ); cb.require_equal( - "length_acc == value - 0x80", + "length_acc == value > 127 ? value - 0x80 : 1", meta.query_advice(length_acc, Rotation::cur()), - meta.query_advice(byte_value, Rotation::cur()) - 128.expr(), + real_length_acc, ); }, ); @@ -966,16 +984,50 @@ impl RlpCircuitConfig { meta.create_gate("is_last == 0", |meta| { let mut cb = BaseConstraintBuilder::new(9); - cb.require_equal( - "index' == index + 1", - meta.query_advice(index, Rotation::next()), - meta.query_advice(index, Rotation::cur()) + 1.expr(), - ); - cb.require_equal( - "rindex' == rindex - 1", - meta.query_advice(rindex, Rotation::next()), - meta.query_advice(rindex, Rotation::cur()) - 1.expr(), + cb.condition( + not::expr(meta.query_advice(placeholder, Rotation::cur())), + |cb| { + cb.require_equal( + "index' == index + 1", + meta.query_advice(index, Rotation::next()), + meta.query_advice(index, Rotation::cur()) + 1.expr(), + ); + cb.require_equal( + "rindex' == rindex - 1", + meta.query_advice(rindex, Rotation::next()), + meta.query_advice(rindex, Rotation::cur()) - 1.expr(), + ); + cb.require_equal( + "all_bytes_rlc_acc' == (all_bytes_rlc_acc * r) + byte_value'", + meta.query_advice(all_bytes_rlc_acc, Rotation::next()), + meta.query_advice(all_bytes_rlc_acc, Rotation::cur()) * keccak_input_rand + + meta.query_advice(byte_value, Rotation::next()), + ); + }, ); + cb.condition(meta.query_advice(placeholder, Rotation::cur()), |cb| { + cb.require_equal( + "index' == index", + meta.query_advice(index, Rotation::next()), + meta.query_advice(index, Rotation::cur()), + ); + cb.require_equal( + "rindex' == rindex", + meta.query_advice(rindex, Rotation::next()), + meta.query_advice(rindex, Rotation::cur()), + ); + cb.require_equal( + "all_bytes_rlc_acc' == all_bytes_rlc_acc", + meta.query_advice(all_bytes_rlc_acc, Rotation::next()), + meta.query_advice(all_bytes_rlc_acc, Rotation::cur()), + ); + cb.require_equal( + "byte_value' == byte_value", + meta.query_advice(byte_value, Rotation::next()), + meta.query_advice(byte_value, Rotation::cur()), + ); + }); + cb.require_equal( "tx_id' == tx_id", meta.query_advice(rlp_table.tx_id, Rotation::next()), @@ -986,12 +1038,6 @@ impl RlpCircuitConfig { meta.query_advice(rlp_table.data_type, Rotation::next()), meta.query_advice(rlp_table.data_type, Rotation::cur()), ); - cb.require_equal( - "all_bytes_rlc_acc' == (all_bytes_rlc_acc * r) + byte_value'", - meta.query_advice(all_bytes_rlc_acc, Rotation::next()), - meta.query_advice(all_bytes_rlc_acc, Rotation::cur()) * keccak_input_rand - + meta.query_advice(byte_value, Rotation::next()), - ); cb.gate(and::expr(vec![ meta.query_fixed(q_usable, Rotation::cur()), @@ -1000,6 +1046,28 @@ impl RlpCircuitConfig { ])) }); + meta.create_gate("placeholder row only happens on DataPrefix", |meta| { + let mut cb = BaseConstraintBuilder::default(); + let (_, tag_length_eq_one) = tag_length_cmp_1.expr(meta, Some(Rotation::cur())); + + cb.require_equal("tag == DataPrefix", is_data_prefix(meta), 1.expr()); + cb.require_equal("tag_length == 1", tag_length_eq_one, 1.expr()); + cb.require_equal( + "byte_value <= 0x80", + value_lt_129.is_lt(meta, None), + 1.expr(), + ); + cb.require_zero( + "byte_value != 0x80", + value_eq_128.is_equal_expression.expr(), + ); + + cb.gate(and::expr(vec![ + meta.query_fixed(q_usable, Rotation::cur()), + meta.query_advice(placeholder, Rotation::cur()), + ])) + }); + // Constraints for the last row, i.e. RLP summary row. meta.create_gate("is_last == 1", |meta| { let mut cb = BaseConstraintBuilder::new(9); @@ -1119,6 +1187,7 @@ impl RlpCircuitConfig { rlp_table: *rlp_table, index, rindex, + placeholder, byte_value, calldata_bytes_rlc_acc, tag_bits, @@ -1245,16 +1314,27 @@ impl RlpCircuitConfig { // tx hash (signed tx) let mut all_bytes_rlc_acc = Value::known(F::zero()); let tx_hash_rows = signed_tx.gen_witness(challenges); - let n_rows = tx_hash_rows.len(); + let has_placeholder = + signed_tx.tx.call_data.len() == 1 && signed_tx.tx.call_data[0] < 0x80; + let n_rows = if has_placeholder { + tx_hash_rows.len() - 1 + } else { + tx_hash_rows.len() + }; for (idx, row) in tx_hash_rows .iter() .chain(signed_tx.rlp_rows(keccak_input_rand).iter()) .enumerate() { + let prev_row_placeholder = row.tag == RlpTxTag::Data && has_placeholder; + let cur_row_placeholder = row.tag == DataPrefix && has_placeholder; // update value accumulator over the entire RLP encoding. - all_bytes_rlc_acc = all_bytes_rlc_acc - .zip(keccak_input_rand) - .map(|(acc, rand)| acc * rand + F::from(row.value as u64)); + if !prev_row_placeholder { + // prev row has already accumulate the byte_value + all_bytes_rlc_acc = all_bytes_rlc_acc + .zip(keccak_input_rand) + .map(|(acc, rand)| acc * rand + F::from(row.value as u64)); + } // q_usable region.assign_fixed( @@ -1289,6 +1369,7 @@ impl RlpCircuitConfig { ("data_type", rlp_table.data_type, (row.data_type as u64)), ("index", self.index, (row.index as u64)), ("rindex", self.rindex, (rindex)), + ("placeholder", self.placeholder, cur_row_placeholder as u64), ("value", self.byte_value, (row.value as u64)), ("tag_length", self.tag_length, (row.tag_length as u64)), ("length_acc", self.length_acc, (row.length_acc)), @@ -1382,16 +1463,27 @@ impl RlpCircuitConfig { // tx sign (unsigned tx) let mut all_bytes_rlc_acc = Value::known(F::zero()); let tx_sign_rows = signed_tx.tx.gen_witness(challenges); - let n_rows = tx_sign_rows.len(); + let has_placeholder = + signed_tx.tx.call_data.len() == 1 && signed_tx.tx.call_data[0] < 0x80; + let n_rows = if has_placeholder { + tx_sign_rows.len() - 1 + } else { + tx_sign_rows.len() + }; for (idx, row) in tx_sign_rows .iter() .chain(signed_tx.tx.rlp_rows(challenges.keccak_input()).iter()) .enumerate() { + let prev_row_placeholder = row.tag == RlpTxTag::Data && has_placeholder; + let cur_row_placeholder = row.tag == DataPrefix && has_placeholder; // update value accumulator over the entire RLP encoding. - all_bytes_rlc_acc = all_bytes_rlc_acc - .zip(keccak_input_rand) - .map(|(acc, rand)| acc * rand + F::from(row.value as u64)); + if !prev_row_placeholder { + // prev row has already accumulate the byte_value + all_bytes_rlc_acc = all_bytes_rlc_acc + .zip(keccak_input_rand) + .map(|(acc, rand)| acc * rand + F::from(row.value as u64)); + } // q_usable region.assign_fixed( @@ -1425,6 +1517,7 @@ impl RlpCircuitConfig { ("data_type", rlp_table.data_type, row.data_type as u64), ("index", self.index, row.index as u64), ("rindex", self.rindex, rindex), + ("placeholder", self.placeholder, cur_row_placeholder as u64), ("byte value", self.byte_value, row.value as u64), ("tag_length", self.tag_length, row.tag_length as u64), ("length_acc", self.length_acc, row.length_acc), @@ -1629,8 +1722,24 @@ impl SubCircuit for RlpCircuit { config.assign(layouter, &self.inputs, self.size, challenges) } - fn min_num_rows_block(_block: &crate::witness::Block) -> usize { - todo!() + fn min_num_rows_block(block: &crate::witness::Block) -> usize { + std::cmp::max( + 1 << 18, + block + .txs + .iter() + .zip(block.sigs.iter()) + .map(|(tx, sig)| { + let mut len = rlp::encode(tx).len() + 1; // 1 for DataPrefix placeholder + let signed_tx = SignedTransaction { + tx: tx.clone(), + signature: *sig, + }; + len += rlp::encode(&signed_tx).len() + 1; // 1 for DataPrefix placeholder + len + }) + .count(), + ) } } @@ -1705,6 +1814,12 @@ mod tests { fn rlp_circuit_tx_1() { verify_txs::(8, vec![CORRECT_MOCK_TXS[0].clone().into()], true); verify_txs::(8, vec![CORRECT_MOCK_TXS[4].clone().into()], true); + + // test against the case in which tx.data has only one byte and is less than + // 0x80 + let mut mock_tx = CORRECT_MOCK_TXS[0].clone(); + mock_tx.input(vec![0x3f].into()); + verify_txs::(8, vec![mock_tx.into()], true); } #[test] diff --git a/zkevm-circuits/src/witness/rlp_encode/common.rs b/zkevm-circuits/src/witness/rlp_encode/common.rs index 5a41bf4ea..fd3c74e69 100644 --- a/zkevm-circuits/src/witness/rlp_encode/common.rs +++ b/zkevm-circuits/src/witness/rlp_encode/common.rs @@ -374,6 +374,19 @@ pub fn handle_bytes( if length == 1 && call_data[0] < 0x80 { assert_eq!(rlp_data[idx], call_data[0]); + // add a placeholder row for this case + rows.push(RlpWitnessRow { + tx_id, + index: idx + 1, + data_type, + value: call_data[0], + value_acc: Value::known(F::from(1)), + value_rlc_acc: Value::known(F::zero()), + tag: prefix_tag, + tag_length: 1, + tag_rindex: 1, + length_acc: 1, + }); rows.push(RlpWitnessRow { tx_id, index: idx + 1, @@ -399,7 +412,7 @@ pub fn handle_bytes( index: idx + 1, data_type, value: (0x80 + length) as u8, - value_acc: Value::known(F::from((128 + length) as u64)), + value_acc: Value::known(F::from((length) as u64)), value_rlc_acc: Value::known(F::zero()), tag: prefix_tag, tag_length: 1,