From d811cb811a683cdcfaa70f44e9daa1527cfd2252 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 5 Aug 2024 11:08:27 +0200 Subject: [PATCH 1/8] Fix proofs repacking for node witnesses creation --- .../crates/bin/witness_generator/src/node_aggregation.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/prover/crates/bin/witness_generator/src/node_aggregation.rs b/prover/crates/bin/witness_generator/src/node_aggregation.rs index ec6d011f8644..254f256cc0a2 100644 --- a/prover/crates/bin/witness_generator/src/node_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/node_aggregation.rs @@ -113,14 +113,7 @@ impl NodeAggregationWitnessGenerator { let mut proof_ids_iter = job.proofs_ids.into_iter(); let mut proofs_ids = vec![]; for queues in job.aggregations.chunks(RECURSION_ARITY) { - let mut proofs_for_chunk = vec![]; - for (_, queue) in queues { - let proofs_ids_for_queue: Vec<_> = (&mut proof_ids_iter) - .take(queue.num_items as usize) - .collect(); - assert_eq!(queue.num_items as usize, proofs_ids_for_queue.len()); - proofs_for_chunk.extend(proofs_ids_for_queue); - } + let proofs_for_chunk: Vec<_> = (&mut proof_ids_iter).take(queues.len()).collect(); proofs_ids.push(proofs_for_chunk); } From 057986e6e4251e057bb8543a3bb54552be00eed1 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 5 Aug 2024 12:22:37 +0200 Subject: [PATCH 2/8] Remove invalid assert --- prover/crates/bin/witness_generator/src/node_aggregation.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/prover/crates/bin/witness_generator/src/node_aggregation.rs b/prover/crates/bin/witness_generator/src/node_aggregation.rs index 254f256cc0a2..8acf864276b3 100644 --- a/prover/crates/bin/witness_generator/src/node_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/node_aggregation.rs @@ -168,8 +168,6 @@ impl NodeAggregationWitnessGenerator { &all_leafs_layer_params, ); - assert_eq!(job.circuit_id as u64, result_circuit_id); - let recursive_circuit_id_and_url = save_recursive_layer_prover_input_artifacts( job.block_number, circuit_idx, From 0978fc87493537068f1d9b80de4088bd03e526c6 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 5 Aug 2024 14:20:35 +0200 Subject: [PATCH 3/8] Handle legacy AggregationWrapper struct in storage --- .../witness_generator/src/node_aggregation.rs | 34 +++++++++++++++---- .../crates/bin/witness_generator/src/utils.rs | 26 ++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/prover/crates/bin/witness_generator/src/node_aggregation.rs b/prover/crates/bin/witness_generator/src/node_aggregation.rs index 8acf864276b3..d6d12e263149 100644 --- a/prover/crates/bin/witness_generator/src/node_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/node_aggregation.rs @@ -8,7 +8,7 @@ use zkevm_test_harness::witness::recursive_aggregation::{ compute_node_vk_commitment, create_node_witness, }; use zksync_config::configs::FriWitnessGeneratorConfig; -use zksync_object_store::ObjectStore; +use zksync_object_store::{ObjectStore, ObjectStoreError}; use zksync_prover_dal::{ConnectionPool, Prover, ProverDal}; use zksync_prover_fri_types::{ circuit_definitions::{ @@ -34,7 +34,7 @@ use crate::{ metrics::WITNESS_GENERATOR_METRICS, utils::{ load_proofs_for_job_ids, save_node_aggregations_artifacts, - save_recursive_layer_prover_input_artifacts, AggregationWrapper, + save_recursive_layer_prover_input_artifacts, AggregationWrapper, AggregationWrapperLegacy, }, }; @@ -444,10 +444,32 @@ async fn get_artifacts( circuit_id: metadata.circuit_id, depth: metadata.depth, }; - object_store - .get(key) - .await - .unwrap_or_else(|_| panic!("node aggregation job artifacts missing: {:?}", key)) + let result = object_store.get(key).await; + + // TODO: remove after transition + match result { + Ok(aggregation_wrapper) => return aggregation_wrapper, + Err(error) => { + // probably legacy struct is saved in GCS + if let ObjectStoreError::Serialization(_) = error { + let legacy_wrapper: AggregationWrapperLegacy = + object_store.get(key).await.unwrap_or_else(|inner_error| { + panic!( + "node aggregation job artifacts getting error. Key: {:?}, error: {:?}", + key, inner_error + ) + }); + + let result = AggregationWrapper { + 0: legacy_wrapper.0.into_iter().map(|x| (x.0, x.1)).collect(), + }; + + return result; + } else { + panic!("node aggregation job artifacts missing: {:?}", key) + } + } + } } #[tracing::instrument( diff --git a/prover/crates/bin/witness_generator/src/utils.rs b/prover/crates/bin/witness_generator/src/utils.rs index c1fd5dd3b407..530978b0bc7b 100644 --- a/prover/crates/bin/witness_generator/src/utils.rs +++ b/prover/crates/bin/witness_generator/src/utils.rs @@ -97,6 +97,32 @@ impl StoredObject for AggregationWrapper { serialize_using_bincode!(); } +#[derive(serde::Serialize, serde::Deserialize)] +/// TODO: remove after transition +pub struct AggregationWrapperLegacy( + pub Vec<( + u64, + RecursionQueueSimulator, + ZkSyncRecursiveLayerCircuit, + )>, +); + +impl StoredObject for AggregationWrapperLegacy { + const BUCKET: Bucket = Bucket::NodeAggregationWitnessJobsFri; + type Key<'a> = AggregationsKey; + + fn encode_key(key: Self::Key<'_>) -> String { + let AggregationsKey { + block_number, + circuit_id, + depth, + } = key; + format!("aggregations_{block_number}_{circuit_id}_{depth}.bin") + } + + serialize_using_bincode!(); +} + #[derive(serde::Serialize, serde::Deserialize)] pub struct SchedulerPartialInputWrapper( pub SchedulerCircuitInstanceWitness< From ef8bbb2afa1ec4f4b8f8222a53a5cda253a7fe4e Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 5 Aug 2024 14:30:43 +0200 Subject: [PATCH 4/8] Fix lint issues --- .../bin/witness_generator/src/node_aggregation.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/prover/crates/bin/witness_generator/src/node_aggregation.rs b/prover/crates/bin/witness_generator/src/node_aggregation.rs index d6d12e263149..551d861638de 100644 --- a/prover/crates/bin/witness_generator/src/node_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/node_aggregation.rs @@ -447,8 +447,8 @@ async fn get_artifacts( let result = object_store.get(key).await; // TODO: remove after transition - match result { - Ok(aggregation_wrapper) => return aggregation_wrapper, + return match result { + Ok(aggregation_wrapper) => aggregation_wrapper, Err(error) => { // probably legacy struct is saved in GCS if let ObjectStoreError::Serialization(_) = error { @@ -459,17 +459,12 @@ async fn get_artifacts( key, inner_error ) }); - - let result = AggregationWrapper { - 0: legacy_wrapper.0.into_iter().map(|x| (x.0, x.1)).collect(), - }; - - return result; + AggregationWrapper(legacy_wrapper.0.into_iter().map(|x| (x.0, x.1)).collect()) } else { panic!("node aggregation job artifacts missing: {:?}", key) } } - } + }; } #[tracing::instrument( From 6f0d2614787045c8d4b53a8a84d7935bacf800a0 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 5 Aug 2024 14:39:53 +0200 Subject: [PATCH 5/8] Print serialization error as well --- prover/crates/bin/witness_generator/src/node_aggregation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/prover/crates/bin/witness_generator/src/node_aggregation.rs b/prover/crates/bin/witness_generator/src/node_aggregation.rs index 551d861638de..68ab2e97bf0c 100644 --- a/prover/crates/bin/witness_generator/src/node_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/node_aggregation.rs @@ -451,12 +451,12 @@ async fn get_artifacts( Ok(aggregation_wrapper) => aggregation_wrapper, Err(error) => { // probably legacy struct is saved in GCS - if let ObjectStoreError::Serialization(_) = error { + if let ObjectStoreError::Serialization(serialization_error) = error { let legacy_wrapper: AggregationWrapperLegacy = object_store.get(key).await.unwrap_or_else(|inner_error| { panic!( - "node aggregation job artifacts getting error. Key: {:?}, error: {:?}", - key, inner_error + "node aggregation job artifacts getting error. Key: {:?}, errors: {:?} {:?}", + key,serialization_error, inner_error ) }); AggregationWrapper(legacy_wrapper.0.into_iter().map(|x| (x.0, x.1)).collect()) From 69deb1b52b598d608e1e3a471e60a1b31b020887 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 5 Aug 2024 15:58:54 +0200 Subject: [PATCH 6/8] Fix fmt issues --- prover/crates/bin/witness_generator/src/node_aggregation.rs | 2 +- prover/crates/bin/witness_generator/src/utils.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/prover/crates/bin/witness_generator/src/node_aggregation.rs b/prover/crates/bin/witness_generator/src/node_aggregation.rs index 68ab2e97bf0c..12f6026bdfd5 100644 --- a/prover/crates/bin/witness_generator/src/node_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/node_aggregation.rs @@ -456,7 +456,7 @@ async fn get_artifacts( object_store.get(key).await.unwrap_or_else(|inner_error| { panic!( "node aggregation job artifacts getting error. Key: {:?}, errors: {:?} {:?}", - key,serialization_error, inner_error + key, serialization_error, inner_error ) }); AggregationWrapper(legacy_wrapper.0.into_iter().map(|x| (x.0, x.1)).collect()) diff --git a/prover/crates/bin/witness_generator/src/utils.rs b/prover/crates/bin/witness_generator/src/utils.rs index 530978b0bc7b..a21aabc5d6d1 100644 --- a/prover/crates/bin/witness_generator/src/utils.rs +++ b/prover/crates/bin/witness_generator/src/utils.rs @@ -97,8 +97,8 @@ impl StoredObject for AggregationWrapper { serialize_using_bincode!(); } -#[derive(serde::Serialize, serde::Deserialize)] /// TODO: remove after transition +#[derive(serde::Serialize, serde::Deserialize)] pub struct AggregationWrapperLegacy( pub Vec<( u64, From 38f99fe1dab8600e412e7b8eed3e2e469f3883e3 Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Mon, 5 Aug 2024 16:44:45 +0200 Subject: [PATCH 7/8] Add new assert --- prover/crates/bin/witness_generator/src/leaf_aggregation.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prover/crates/bin/witness_generator/src/leaf_aggregation.rs b/prover/crates/bin/witness_generator/src/leaf_aggregation.rs index 00546a11407e..d8cad84e777d 100644 --- a/prover/crates/bin/witness_generator/src/leaf_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/leaf_aggregation.rs @@ -267,6 +267,8 @@ pub async fn process_leaf_aggregation_job( let circuit_id = job.circuit_id; let queues = split_recursion_queue(job.closed_form_inputs.1); + assert_eq!(circuit_id, job.base_vk.numeric_circuit_type()); + let aggregations = queues .iter() .cloned() From e05bfe752cff87f39ada5b170b84c77641443e2d Mon Sep 17 00:00:00 2001 From: Vladislav Volosnikov Date: Tue, 6 Aug 2024 10:46:00 +0200 Subject: [PATCH 8/8] Fix node aggregation depth values in saved inputs --- prover/crates/bin/witness_generator/src/node_aggregation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prover/crates/bin/witness_generator/src/node_aggregation.rs b/prover/crates/bin/witness_generator/src/node_aggregation.rs index 12f6026bdfd5..c9d5ab32bc5f 100644 --- a/prover/crates/bin/witness_generator/src/node_aggregation.rs +++ b/prover/crates/bin/witness_generator/src/node_aggregation.rs @@ -173,7 +173,7 @@ impl NodeAggregationWitnessGenerator { circuit_idx, vec![recursive_circuit], AggregationRound::NodeAggregation, - job.depth, + job.depth + 1, &*object_store, Some(job.circuit_id), )