refactor get_blob_data_bytes to avoid inconsistency

aggregator/src/aggregation/blob_data.rs

@@ -1,5 +1,3 @@
-use std::io::Write;
-
 use gadgets::util::Expr;
 use halo2_ecc::bigint::CRTInteger;
 use halo2_proofs::{
@@ -12,7 +10,7 @@ use itertools::Itertools;
 use zkevm_circuits::{table::U8Table, util::Challenges};
 
 use crate::{
-    aggregation::{decoder::witgen::init_zstd_encoder, rlc::POWS_OF_256},
+    aggregation::rlc::POWS_OF_256,
     blob::{BatchData, BLOB_WIDTH, N_BLOB_BYTES, N_DATA_BYTES_PER_COEFFICIENT},
     RlcConfig,
 };
@@ -201,23 +199,8 @@ impl<const N_SNARKS: usize> BlobDataConfig<N_SNARKS> {
         ),
         Error,
     > {
-        let batch_bytes = batch_data.get_batch_data_bytes();
-        let mut blob_bytes = {
-            let mut encoder = init_zstd_encoder(None);
-            encoder
-                .set_pledged_src_size(Some(batch_bytes.len() as u64))
-                .map_err(|_| Error::Synthesis)?;
-            encoder
-                .write_all(&batch_bytes)
-                .map_err(|_| Error::Synthesis)?;
-            encoder.finish().map_err(|_| Error::Synthesis)?
-        };
-
-        let enable_encoding = blob_bytes.len() < batch_bytes.len();
-        if !enable_encoding {
-            blob_bytes = batch_bytes.clone();
-        }
-        blob_bytes.insert(0, enable_encoding as u8);
+        let blob_bytes = batch_data.get_blob_data_bytes();
+        let enable_encoding = blob_bytes[0].eq(&1);
 
         assert!(blob_bytes.len() <= N_BLOB_BYTES, "too many blob bytes");
 

aggregator/src/aggregation/circuit.rs

@@ -1,7 +1,7 @@
 use crate::{
     aggregation::decoder::WORKED_EXAMPLE,
     blob::BatchData,
-    witgen::{init_zstd_encoder, MultiBlockProcessResult},
+    witgen::{zstd_encode, MultiBlockProcessResult},
     LOG_DEGREE, PI_CHAIN_ID, PI_CURRENT_BATCH_HASH, PI_CURRENT_STATE_ROOT,
     PI_CURRENT_WITHDRAW_ROOT, PI_PARENT_BATCH_HASH, PI_PARENT_STATE_ROOT,
 };
@@ -21,7 +21,7 @@ use itertools::Itertools;
 use rand::Rng;
 #[cfg(not(feature = "disable_proof_aggregation"))]
 use std::rc::Rc;
-use std::{env, fs::File, io::Write};
+use std::{env, fs::File};
 
 #[cfg(not(feature = "disable_proof_aggregation"))]
 use snark_verifier::loader::halo2::{halo2_ecc::halo2_base::AssignedValue, Halo2Loader};
@@ -482,22 +482,12 @@ impl<const N_SNARKS: usize> Circuit<Fr> for BatchCircuit<N_SNARKS> {
             )?;
 
             // conditionally encode those bytes. By default we use a worked example.
-            let (batch_bytes, encoded_bytes) = if blob_data_exports.enable_encoding_bool {
-                (
-                    batch_data.get_batch_data_bytes(),
-                    batch_data.get_encoded_batch_data_bytes(),
-                )
+            let raw_bytes = if blob_data_exports.enable_encoding_bool {
+                batch_data.get_batch_data_bytes()
             } else {
-                let dummy_bytes = WORKED_EXAMPLE.as_bytes().to_vec();
-                let mut encoder = init_zstd_encoder(None);
-                encoder
-                    .set_pledged_src_size(Some(dummy_bytes.len() as u64))
-                    .map_err(|_| Error::Synthesis)?;
-                encoder
-                    .write_all(&dummy_bytes)
-                    .map_err(|_| Error::Synthesis)?;
-                (dummy_bytes, encoder.finish().map_err(|_| Error::Synthesis)?)
+                WORKED_EXAMPLE.as_bytes().to_vec()
             };
+            let encoded_bytes = zstd_encode(&raw_bytes);
 
             let MultiBlockProcessResult {
                 witness_rows,
@@ -520,14 +510,14 @@ impl<const N_SNARKS: usize> Circuit<Fr> for BatchCircuit<N_SNARKS> {
             );
             if blob_data_exports.enable_encoding_bool {
                 assert_eq!(
-                    batch_bytes, recovered_bytes,
+                    raw_bytes, recovered_bytes,
                     "original and recovered bytes mismatch"
                 );
             }
 
             let decoder_exports = config.decoder_config.assign(
                 &mut layouter,
-                &batch_bytes,
+                &raw_bytes,
                 &encoded_bytes,
                 witness_rows,
                 decoded_literals,

aggregator/src/aggregation/decoder/witgen/params.rs

@@ -13,6 +13,8 @@ pub const N_BITS_ZSTD_TAG: usize = 4;
 /// Number of bits in the repeat bits that follow value=1 in reconstructing FSE table.
 pub const N_BITS_REPEAT_FLAG: usize = 2;
 
+use std::io::Write;
+
 /// re-export constants in zstd-encoder
 pub use zstd_encoder::{N_BLOCK_SIZE_TARGET, N_MAX_BLOCKS};
 
@@ -24,3 +26,13 @@ pub fn init_zstd_encoder(
 ) -> zstd::stream::Encoder<'static, Vec<u8>> {
     init_zstd_encoder_n(target_block_size.unwrap_or(N_BLOCK_SIZE_TARGET))
 }
+
+/// Encode input bytes by using the default encoder.
+pub fn zstd_encode(bytes: &[u8]) -> Vec<u8> {
+    let mut encoder = init_zstd_encoder(None);
+    encoder
+        .set_pledged_src_size(Some(bytes.len() as u64))
+        .expect("infallible");
+    encoder.write_all(bytes).expect("infallible");
+    encoder.finish().expect("infallible")
+}

aggregator/src/blob.rs

@@ -1,5 +1,6 @@
 use crate::{
-    aggregation::{interpolate, witgen::init_zstd_encoder, BLS_MODULUS},
+    aggregation::{interpolate, BLS_MODULUS},
+    witgen::zstd_encode,
     BatchHash, ChunkInfo,
 };
 
@@ -16,7 +17,6 @@ use itertools::Itertools;
 use once_cell::sync::Lazy;
 use revm_primitives::VERSIONED_HASH_VERSION_KZG;
 use std::{
-    io::Write,
     iter::{once, repeat},
     sync::Arc,
 };
@@ -276,36 +276,26 @@ impl<const N_SNARKS: usize> BatchData<N_SNARKS> {
             .collect()
     }
 
-    /// Get the zstd encoded batch data bytes.
-    pub fn get_encoded_batch_data_bytes(&self) -> Vec<u8> {
+    /// Get the blob data bytes that will be populated in BlobDataConfig.
+    pub(crate) fn get_blob_data_bytes(&self) -> Vec<u8> {
         let batch_data_bytes = self.get_batch_data_bytes();
-        let mut encoder = init_zstd_encoder(None);
-        encoder
-            .set_pledged_src_size(Some(batch_data_bytes.len() as u64))
-            .expect("infallible");
-        encoder.write_all(&batch_data_bytes).expect("infallible");
-        let encoded_bytes = encoder.finish().expect("infallible");
-        log::info!(
-            "compress batch data from {} to {}, compression ratio {:.2}, blob usage {:.3}",
-            batch_data_bytes.len(),
-            encoded_bytes.len(),
-            batch_data_bytes.len() as f32 / encoded_bytes.len() as f32,
-            encoded_bytes.len() as f32 / N_BLOB_BYTES as f32
-        );
-        encoded_bytes
+        let mut blob_data_bytes = zstd_encode(&batch_data_bytes);
+
+        // Whether we encode batch -> blob or not.
+        let enable_encoding = blob_data_bytes.len() < batch_data_bytes.len();
+        if !enable_encoding {
+            blob_data_bytes = batch_data_bytes;
+        }
+        blob_data_bytes.insert(0, enable_encoding as u8);
+
+        blob_data_bytes
     }
 
     /// Get the BLOB_WIDTH number of scalar field elements, as 32-bytes unsigned integers.
     pub(crate) fn get_coefficients(&self) -> [U256; BLOB_WIDTH] {
         let mut coefficients = [[0u8; N_BYTES_U256]; BLOB_WIDTH];
 
-        // We only consider the data from `valid` chunks and ignore the padded chunks.
-        let batch_bytes = self.get_batch_data_bytes();
-        let mut blob_bytes = self.get_encoded_batch_data_bytes();
-
-        // Whether we encode batch -> blob or not.
-        let enable_encoding = blob_bytes.len() < batch_bytes.len();
-        blob_bytes.insert(0, enable_encoding as u8);
+        let blob_bytes = self.get_blob_data_bytes();
 
         assert!(
             blob_bytes.len() <= N_BLOB_BYTES,

aggregator/src/param.rs

@@ -21,7 +21,7 @@ impl ConfigParams {
         Self {
             strategy: FpStrategy::Simple,
             degree: 21,
-            num_advice: vec![64],
+            num_advice: vec![100],
             num_lookup_advice: vec![8],
             num_fixed: 2,
             lookup_bits: 20,

aggregator/src/tests/blob.rs

@@ -258,7 +258,6 @@ fn check_circuit(circuit: &BlobCircuit) -> Result<(), Vec<VerifyFailure>> {
 #[test]
 fn blob_circuit_completeness() {
     // TODO: enable this once we have another deterministic case of batch -> blob (fully packed).
-    let full_blob = vec![vec![123; BatchData::<MAX_AGG_SNARKS>::n_rows_data()]];
     // let full_blob = hex::decode(
     //     fs::read_to_string("./data/test_batches/batch274.hex")
     //         .expect("file path exists")
@@ -301,10 +300,10 @@ fn blob_circuit_completeness() {
         // TODO: enable this once we have another deterministic case of batch -> blob (fully
         // packed).
         // if idx == 0 {
-        //     let encoded_len = batch_data.get_encoded_batch_data_bytes().len();
+        //     let blob_data_bytes_len = batch_data.get_blob_data_bytes().len();
         //     assert_eq!(
-        //         encoded_len, N_BLOB_BYTES,
-        //         "should be full blob: expected={N_BLOB_BYTES}, got={encoded_len}",
+        //         blob_data_bytes_len, N_BLOB_BYTES,
+        //         "should be full blob: expected={N_BLOB_BYTES}, got={blob_data_bytes_len}",
         //     );
         // }
     }