Skip to content

Commit 3e360e1

Browse files
fix(prover): Fixing sequence_number assigning (#4319)
## What ❔ 1. `sequence_number` is now not continuous across all the BWG output artifacts but instead reflects a sequence number within the same circuit type. 2. Also it is guaranteed that urls match (sequence_number, circuit_id) pair in DB. <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ Circuit url includes a sequence_number which is a continuous numbering across all the circuits. It turned out that the ordering between different circuit types is not strict. e.g. we have circuit_10 A, B, C and circuit_2 X, Y, Z. They can be pushed to vec in different order like [A,B,C,X,Y,Z], [A,X,B,Y,C,Z], [A,X,Y,B,C,Z] etc. And then sequence_number is assigned like [0,1,2,3,4,5]. That can cause creating two identical jobs with different url and following proving failure. <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- The `Why` has to be clear to non-Matter Labs entities running their own ZK Chain --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Is this a breaking change? - [ ] Yes - [ ] No ## Operational changes <!-- Any config changes? Any new flags? Any changes to any scripts? --> <!-- Please add anything that non-Matter Labs entities running their own ZK Chain may need to know --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`.
1 parent 91843a2 commit 3e360e1

File tree

10 files changed

+72
-60
lines changed

10 files changed

+72
-60
lines changed

prover/crates/lib/prover_dal/src/fri_prover_dal.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,21 +70,18 @@ impl FriProverDal<'_, '_> {
7070
pub async fn insert_prover_jobs(
7171
&mut self,
7272
batch_id: L1BatchId,
73-
circuit_ids_and_urls: Vec<(u8, String)>,
73+
circuit_ids_sequence_numbers_and_urls: Vec<(u8, usize, String)>,
7474
aggregation_round: AggregationRound,
7575
depth: u16,
7676
protocol_version_id: ProtocolSemanticVersion,
7777
batch_sealed_at: DateTime<Utc>,
7878
) {
7979
let _latency = MethodLatency::new("save_fri_prover_jobs");
80-
if circuit_ids_and_urls.is_empty() {
80+
if circuit_ids_sequence_numbers_and_urls.is_empty() {
8181
return;
8282
}
8383

84-
for (chunk_index, chunk) in circuit_ids_and_urls
85-
.chunks(Self::INSERT_JOBS_CHUNK_SIZE)
86-
.enumerate()
87-
{
84+
for chunk in circuit_ids_sequence_numbers_and_urls.chunks(Self::INSERT_JOBS_CHUNK_SIZE) {
8885
// Build multi-row INSERT for the current chunk
8986
let mut query_builder = QueryBuilder::new(
9087
r#"
@@ -108,14 +105,14 @@ impl FriProverDal<'_, '_> {
108105
);
109106

110107
query_builder.push_values(
111-
chunk.iter().enumerate(),
112-
|mut row, (i, (circuit_id, circuit_blob_url))| {
108+
chunk.iter(),
109+
|mut row, (circuit_id, sequence_number, circuit_blob_url)| {
113110
row.push_bind(batch_id.batch_number().0 as i64)
114111
.push_bind(batch_id.chain_id().inner() as i64)
115112
.push_bind(*circuit_id as i16)
116113
.push_bind(circuit_blob_url)
117114
.push_bind(aggregation_round as i64)
118-
.push_bind((chunk_index * Self::INSERT_JOBS_CHUNK_SIZE + i) as i64) // sequence_number
115+
.push_bind(*sequence_number as i64) // sequence_number
119116
.push_bind(depth as i32)
120117
.push_bind(false) // is_node_final_proof
121118
.push_bind(protocol_version_id.minor as i32)
@@ -973,9 +970,9 @@ mod tests {
973970
use super::*;
974971
use crate::ProverDal;
975972

976-
fn mock_circuit_ids_and_urls(num_circuits: usize) -> Vec<(u8, String)> {
973+
fn mock_circuit_ids_and_urls(num_circuits: usize) -> Vec<(u8, usize, String)> {
977974
(0..num_circuits)
978-
.map(|i| (i as u8, format!("circuit{}", i)))
975+
.map(|i| (i as u8, i, format!("circuit{}", i)))
979976
.collect()
980977
}
981978

prover/crates/lib/witness_generator_service/src/artifact_manager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use zksync_types::{L1BatchId, L1BatchNumber, L2ChainId};
88
#[derive(Debug)]
99
pub struct AggregationBlobUrls {
1010
pub aggregation_urls: String,
11-
pub circuit_ids_and_urls: Vec<(u8, String)>,
11+
pub circuit_ids_sequence_numbers_and_urls: Vec<(u8, usize, String)>,
1212
}
1313

1414
#[derive(Debug, Clone, Copy)]

prover/crates/lib/witness_generator_service/src/rounds/basic_circuits/artifacts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ impl ArtifactsManager for BasicCircuits {
8787
.fri_prover_jobs_dal()
8888
.insert_prover_jobs(
8989
job_id.into(),
90-
artifacts.circuit_urls,
90+
artifacts.circuit_ids_sequence_numbers_and_urls,
9191
AggregationRound::BasicCircuits,
9292
0,
9393
protocol_version_id,

prover/crates/lib/witness_generator_service/src/rounds/basic_circuits/mod.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ mod utils;
2828

2929
#[derive(Clone)]
3030
pub struct BasicCircuitArtifacts {
31-
pub(super) circuit_urls: Vec<(u8, String)>,
31+
pub(super) circuit_ids_sequence_numbers_and_urls: Vec<(u8, usize, String)>,
3232
pub(super) queue_urls: Vec<(u8, String, usize)>,
3333
pub(super) scheduler_witness: SchedulerCircuitInstanceWitness<
3434
GoldilocksField,
@@ -45,7 +45,7 @@ pub struct BasicWitnessGeneratorJob {
4545
}
4646

4747
type Witness = (
48-
Vec<(u8, String)>,
48+
Vec<(u8, usize, String)>,
4949
Vec<(u8, String, usize)>,
5050
SchedulerCircuitInstanceWitness<
5151
GoldilocksField,
@@ -75,11 +75,15 @@ impl JobManager for BasicCircuits {
7575
data: job,
7676
} = job;
7777

78-
let (circuit_urls, queue_urls, scheduler_witness, aux_output_witness) =
79-
generate_witness(batch_id, object_store, job, max_circuits_in_flight).await;
78+
let (
79+
circuit_ids_sequence_numbers_and_urls,
80+
queue_urls,
81+
scheduler_witness,
82+
aux_output_witness,
83+
) = generate_witness(batch_id, object_store, job, max_circuits_in_flight).await;
8084

8185
Ok(BasicCircuitArtifacts {
82-
circuit_urls,
86+
circuit_ids_sequence_numbers_and_urls,
8387
queue_urls,
8488
scheduler_witness,
8589
aux_output_witness,

prover/crates/lib/witness_generator_service/src/rounds/basic_circuits/utils.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use std::{
2-
collections::HashSet,
2+
collections::{HashMap, HashSet},
33
hash::{DefaultHasher, Hash, Hasher},
44
sync::Arc,
55
};
@@ -155,15 +155,19 @@ pub(super) async fn generate_witness(
155155
// Ordering determines how we compose the circuit proofs in Leaf Aggregation Round.
156156
// Sequence is used to determine circuit ordering (the sequencing of instructions) .
157157
// If the order is tampered with, proving will fail (as the proof would be computed for a different sequence of instruction).
158-
let mut circuit_sequence = 0;
158+
let mut circuit_sequence_numbers = HashMap::new();
159159

160160
while let Some(circuit) = circuit_receiver
161161
.recv()
162162
.instrument(tracing::info_span!("wait_for_circuit"))
163163
.await
164164
{
165-
let sequence = circuit_sequence;
166-
circuit_sequence += 1;
165+
let circuit_id = circuit.numeric_circuit_type();
166+
let sequence = circuit_sequence_numbers
167+
.get(&circuit_id)
168+
.cloned()
169+
.unwrap_or(0);
170+
circuit_sequence_numbers.insert(circuit_id, sequence + 1);
167171
let object_store = object_store.clone();
168172
let semaphore = semaphore.clone();
169173
let permit = semaphore
@@ -175,7 +179,7 @@ pub(super) async fn generate_witness(
175179
let (circuit_id, circuit_url) =
176180
save_circuit(batch_id, circuit, sequence, object_store).await;
177181
drop(permit);
178-
(circuit_id, circuit_url)
182+
(circuit_id, sequence, circuit_url)
179183
}));
180184
}
181185
}
@@ -217,13 +221,14 @@ pub(super) async fn generate_witness(
217221
// `circuits_present` stores which circuits exist and is used to filter queues in `recursion_urls` later.
218222
let mut circuits_present = HashSet::<u8>::new();
219223

220-
let circuit_urls = futures::future::join_all(save_circuit_handles)
224+
let circuit_ids_sequence_numbers_and_urls = futures::future::join_all(save_circuit_handles)
221225
.await
222226
.into_iter()
223227
.map(|result| {
224-
let (circuit_id, circuit_url) = result.expect("failed to save circuit");
228+
let (circuit_id, sequence_number, circuit_url) =
229+
result.expect("failed to save circuit");
225230
circuits_present.insert(circuit_id);
226-
(circuit_id, circuit_url)
231+
(circuit_id, sequence_number, circuit_url)
227232
})
228233
.collect();
229234

@@ -240,7 +245,7 @@ pub(super) async fn generate_witness(
240245
scheduler_witness.previous_block_aux_hash = input.previous_batch_metadata.aux_hash.0;
241246

242247
(
243-
circuit_urls,
248+
circuit_ids_sequence_numbers_and_urls,
244249
recursion_urls,
245250
scheduler_witness,
246251
block_aux_witness,

prover/crates/lib/witness_generator_service/src/rounds/leaf_aggregation/artifacts.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl ArtifactsManager for LeafAggregation {
5858

5959
AggregationBlobUrls {
6060
aggregation_urls,
61-
circuit_ids_and_urls: artifacts.circuit_ids_and_urls,
61+
circuit_ids_sequence_numbers_and_urls: artifacts.circuit_ids_sequence_numbers_and_urls,
6262
}
6363
}
6464

@@ -82,7 +82,7 @@ impl ArtifactsManager for LeafAggregation {
8282

8383
let mut prover_connection = connection_pool.connection().await.unwrap();
8484
let mut transaction = prover_connection.start_transaction().await.unwrap();
85-
let number_of_dependent_jobs = blob_urls.circuit_ids_and_urls.len();
85+
let number_of_dependent_jobs = blob_urls.circuit_ids_sequence_numbers_and_urls.len();
8686
let protocol_version_id = transaction
8787
.fri_basic_witness_generator_dal()
8888
.protocol_version_for_l1_batch(artifacts.batch_id)
@@ -95,7 +95,7 @@ impl ArtifactsManager for LeafAggregation {
9595

9696
tracing::info!(
9797
"Inserting {} prover jobs for job_id {}, block {} with circuit id {}",
98-
blob_urls.circuit_ids_and_urls.len(),
98+
blob_urls.circuit_ids_sequence_numbers_and_urls.len(),
9999
job_id,
100100
artifacts.batch_id,
101101
artifacts.circuit_id,
@@ -104,7 +104,7 @@ impl ArtifactsManager for LeafAggregation {
104104
.fri_prover_jobs_dal()
105105
.insert_prover_jobs(
106106
artifacts.batch_id,
107-
blob_urls.circuit_ids_and_urls,
107+
blob_urls.circuit_ids_sequence_numbers_and_urls,
108108
AggregationRound::LeafAggregation,
109109
0,
110110
protocol_version_id,

prover/crates/lib/witness_generator_service/src/rounds/leaf_aggregation/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub struct LeafAggregationArtifacts {
5555
circuit_id: u8,
5656
batch_id: L1BatchId,
5757
pub aggregations: Vec<(u64, RecursionQueueSimulator<GoldilocksField>)>,
58-
pub circuit_ids_and_urls: Vec<(u8, String)>,
58+
pub circuit_ids_sequence_numbers_and_urls: Vec<(u8, usize, String)>,
5959
#[allow(dead_code)]
6060
closed_form_inputs: Vec<ZkSyncBaseLayerClosedFormInput<GoldilocksField>>,
6161
}
@@ -167,8 +167,9 @@ impl JobManager for LeafAggregation {
167167
handles.push(handle);
168168
}
169169

170-
let circuit_ids_and_urls_results = futures::future::join_all(handles).await;
171-
let circuit_ids_and_urls = circuit_ids_and_urls_results
170+
let circuit_ids_sequence_numbers_and_urls_results =
171+
futures::future::join_all(handles).await;
172+
let circuit_ids_sequence_numbers_and_urls = circuit_ids_sequence_numbers_and_urls_results
172173
.into_iter()
173174
.flat_map(|x| x.unwrap())
174175
.collect();
@@ -184,7 +185,7 @@ impl JobManager for LeafAggregation {
184185
circuit_id,
185186
batch_id: job.batch_id,
186187
aggregations,
187-
circuit_ids_and_urls,
188+
circuit_ids_sequence_numbers_and_urls,
188189
closed_form_inputs: job.closed_form_inputs.0,
189190
})
190191
}

prover/crates/lib/witness_generator_service/src/rounds/node_aggregation/artifacts.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ impl ArtifactsManager for NodeAggregation {
6363

6464
AggregationBlobUrls {
6565
aggregation_urls,
66-
circuit_ids_and_urls: artifacts.recursive_circuit_ids_and_urls,
66+
circuit_ids_sequence_numbers_and_urls: artifacts
67+
.recursive_circuit_ids_sequence_numbers_and_urls,
6768
}
6869
}
6970

@@ -80,7 +81,7 @@ impl ArtifactsManager for NodeAggregation {
8081
) -> anyhow::Result<()> {
8182
let mut prover_connection = connection_pool.connection().await.unwrap();
8283
let mut transaction = prover_connection.start_transaction().await.unwrap();
83-
let dependent_jobs = blob_urls.circuit_ids_and_urls.len();
84+
let dependent_jobs = blob_urls.circuit_ids_sequence_numbers_and_urls.len();
8485
let protocol_version_id = transaction
8586
.fri_basic_witness_generator_dal()
8687
.protocol_version_for_l1_batch(artifacts.batch_id)
@@ -97,7 +98,7 @@ impl ArtifactsManager for NodeAggregation {
9798
.fri_prover_jobs_dal()
9899
.insert_prover_jobs(
99100
artifacts.batch_id,
100-
blob_urls.circuit_ids_and_urls,
101+
blob_urls.circuit_ids_sequence_numbers_and_urls,
101102
AggregationRound::NodeAggregation,
102103
artifacts.depth,
103104
protocol_version_id,
@@ -118,7 +119,7 @@ impl ArtifactsManager for NodeAggregation {
118119
.await;
119120
}
120121
false => {
121-
let (_, blob_url) = blob_urls.circuit_ids_and_urls[0].clone();
122+
let (_, _, blob_url) = blob_urls.circuit_ids_sequence_numbers_and_urls[0].clone();
122123
transaction
123124
.fri_prover_jobs_dal()
124125
.insert_prover_job(

prover/crates/lib/witness_generator_service/src/rounds/node_aggregation/mod.rs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ pub struct NodeAggregationArtifacts {
4040
batch_id: L1BatchId,
4141
depth: u16,
4242
pub next_aggregations: Vec<(u64, RecursionQueueSimulator<GoldilocksField>)>,
43-
pub recursive_circuit_ids_and_urls: Vec<(u8, String)>,
43+
pub recursive_circuit_ids_sequence_numbers_and_urls: Vec<(u8, usize, String)>,
4444
}
4545

4646
#[derive(Clone)]
@@ -150,33 +150,36 @@ impl JobManager for NodeAggregation {
150150
&all_leafs_layer_params,
151151
);
152152

153-
let recursive_circuit_id_and_url = save_recursive_layer_prover_input_artifacts(
154-
job.batch_id,
155-
circuit_idx,
156-
vec![recursive_circuit],
157-
AggregationRound::NodeAggregation,
158-
job.depth + 1,
159-
&*object_store,
160-
Some(job.circuit_id),
161-
)
162-
.await;
153+
let recursive_circuit_id_sequence_number_and_url =
154+
save_recursive_layer_prover_input_artifacts(
155+
job.batch_id,
156+
circuit_idx,
157+
vec![recursive_circuit],
158+
AggregationRound::NodeAggregation,
159+
job.depth + 1,
160+
&*object_store,
161+
Some(job.circuit_id),
162+
)
163+
.await;
163164

164165
(
165166
(result_circuit_id, input_queue),
166-
recursive_circuit_id_and_url,
167+
recursive_circuit_id_sequence_number_and_url,
167168
)
168169
});
169170

170171
handles.push(handle);
171172
}
172173

173174
let mut next_aggregations = vec![];
174-
let mut recursive_circuit_ids_and_urls = vec![];
175+
let mut recursive_circuit_ids_sequence_numbers_and_urls = vec![];
175176
for handle in handles {
176-
let (next_aggregation, recursive_circuit_id_and_url) = handle.await.unwrap();
177+
let (next_aggregation, recursive_circuit_id_sequence_number_and_url) =
178+
handle.await.unwrap();
177179

178180
next_aggregations.push(next_aggregation);
179-
recursive_circuit_ids_and_urls.extend(recursive_circuit_id_and_url);
181+
recursive_circuit_ids_sequence_numbers_and_urls
182+
.extend(recursive_circuit_id_sequence_number_and_url);
180183
}
181184

182185
tracing::info!(
@@ -193,7 +196,7 @@ impl JobManager for NodeAggregation {
193196
batch_id: job.batch_id,
194197
depth: job.depth + 1,
195198
next_aggregations,
196-
recursive_circuit_ids_and_urls,
199+
recursive_circuit_ids_sequence_numbers_and_urls,
197200
})
198201
}
199202

prover/crates/lib/witness_generator_service/src/utils.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,14 @@ pub async fn save_recursive_layer_prover_input_artifacts(
192192
depth: u16,
193193
object_store: &dyn ObjectStore,
194194
base_layer_circuit_id: Option<u8>,
195-
) -> Vec<(u8, String)> {
196-
let mut ids_and_urls = Vec::with_capacity(recursive_circuits.len());
195+
) -> Vec<(u8, usize, String)> {
196+
let mut ids_sequence_numbers_and_urls = Vec::with_capacity(recursive_circuits.len());
197197
for (sequence_number, circuit) in recursive_circuits.into_iter().enumerate() {
198198
let circuit_id = base_layer_circuit_id.unwrap_or_else(|| circuit.numeric_circuit_type());
199+
let sequence_number = sequence_number_offset + sequence_number;
199200
let circuit_key = FriCircuitKey {
200201
batch_id,
201-
sequence_number: sequence_number_offset + sequence_number,
202+
sequence_number,
202203
circuit_id,
203204
aggregation_round,
204205
depth,
@@ -207,9 +208,9 @@ pub async fn save_recursive_layer_prover_input_artifacts(
207208
.put(circuit_key, &CircuitWrapper::Recursive(circuit))
208209
.await
209210
.unwrap();
210-
ids_and_urls.push((circuit_id, blob_url));
211+
ids_sequence_numbers_and_urls.push((circuit_id, sequence_number, blob_url));
211212
}
212-
ids_and_urls
213+
ids_sequence_numbers_and_urls
213214
}
214215

215216
#[tracing::instrument(skip_all)]

0 commit comments

Comments
 (0)