Skip to content

Commit 8099ab0

Browse files
authored
fix: BWIP race condition (#2405)
## What ❔ Separately insert proof_generation_details and gen data blob URLs. ## Why ❔ Sometimes BWIP generates data before insert_proof_generation_details is called, which results in errors. ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zk fmt` and `zk lint`.
1 parent 948b532 commit 8099ab0

8 files changed

+87
-24
lines changed

core/lib/dal/.sqlx/query-41a2731a3fe6ae441902632dcce15601ff39acd03e3c8a2265c9036b3dc54383.json

Lines changed: 0 additions & 15 deletions
This file was deleted.

core/lib/dal/.sqlx/query-5137159db7d3ff456e368e6246b07554ce738a2d7005472e7e76a64a8fbd57ad.json

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/lib/dal/.sqlx/query-b61b2545ff82bc3e2a198b21546735c1dcccdd6c439827fc4c3ba57e8767076e.json

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE proof_generation_details ALTER COLUMN proof_gen_data_blob_url SET NOT NULL;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE proof_generation_details ALTER COLUMN proof_gen_data_blob_url DROP NOT NULL;

core/lib/dal/src/proof_generation_dal.rs

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -155,26 +155,60 @@ impl ProofGenerationDal<'_, '_> {
155155
Ok(())
156156
}
157157

158+
pub async fn save_merkle_paths_artifacts_metadata(
159+
&mut self,
160+
batch_number: L1BatchNumber,
161+
proof_gen_data_blob_url: &str,
162+
) -> DalResult<()> {
163+
let batch_number = i64::from(batch_number.0);
164+
let query = sqlx::query!(
165+
r#"
166+
UPDATE proof_generation_details
167+
SET
168+
proof_gen_data_blob_url = $1,
169+
updated_at = NOW()
170+
WHERE
171+
l1_batch_number = $2
172+
"#,
173+
proof_gen_data_blob_url,
174+
batch_number
175+
);
176+
let instrumentation = Instrumented::new("save_proof_artifacts_metadata")
177+
.with_arg("proof_gen_data_blob_url", &proof_gen_data_blob_url)
178+
.with_arg("l1_batch_number", &batch_number);
179+
let result = instrumentation
180+
.clone()
181+
.with(query)
182+
.execute(self.storage)
183+
.await?;
184+
if result.rows_affected() == 0 {
185+
let err = instrumentation.constraint_error(anyhow::anyhow!(
186+
"Cannot save proof_gen_data_blob_url for a batch number {} that does not exist",
187+
batch_number
188+
));
189+
return Err(err);
190+
}
191+
192+
Ok(())
193+
}
194+
158195
/// The caller should ensure that `l1_batch_number` exists in the database.
159196
pub async fn insert_proof_generation_details(
160197
&mut self,
161198
l1_batch_number: L1BatchNumber,
162-
proof_gen_data_blob_url: &str,
163199
) -> DalResult<()> {
164200
let result = sqlx::query!(
165201
r#"
166202
INSERT INTO
167-
proof_generation_details (l1_batch_number, status, proof_gen_data_blob_url, created_at, updated_at)
203+
proof_generation_details (l1_batch_number, status, created_at, updated_at)
168204
VALUES
169-
($1, 'unpicked', $2, NOW(), NOW())
205+
($1, 'unpicked', NOW(), NOW())
170206
ON CONFLICT (l1_batch_number) DO NOTHING
171207
"#,
172-
i64::from(l1_batch_number.0),
173-
proof_gen_data_blob_url,
208+
i64::from(l1_batch_number.0),
174209
)
175210
.instrument("insert_proof_generation_details")
176211
.with_arg("l1_batch_number", &l1_batch_number)
177-
.with_arg("proof_gen_data_blob_url", &proof_gen_data_blob_url)
178212
.report_latency()
179213
.execute(self.storage)
180214
.await?;
@@ -303,7 +337,7 @@ mod tests {
303337
assert_eq!(unpicked_l1_batch, None);
304338

305339
conn.proof_generation_dal()
306-
.insert_proof_generation_details(L1BatchNumber(1), "generation_data")
340+
.insert_proof_generation_details(L1BatchNumber(1))
307341
.await
308342
.unwrap();
309343

@@ -316,13 +350,17 @@ mod tests {
316350

317351
// Calling the method multiple times should work fine.
318352
conn.proof_generation_dal()
319-
.insert_proof_generation_details(L1BatchNumber(1), "generation_data")
353+
.insert_proof_generation_details(L1BatchNumber(1))
320354
.await
321355
.unwrap();
322356
conn.proof_generation_dal()
323357
.save_vm_runner_artifacts_metadata(L1BatchNumber(1), "vm_run")
324358
.await
325359
.unwrap();
360+
conn.proof_generation_dal()
361+
.save_merkle_paths_artifacts_metadata(L1BatchNumber(1), "data")
362+
.await
363+
.unwrap();
326364
conn.blocks_dal()
327365
.save_l1_batch_tree_data(
328366
L1BatchNumber(1),

core/node/metadata_calculator/src/updater.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,11 @@ impl TreeUpdater {
159159
// Save the proof generation details to Postgres
160160
storage
161161
.proof_generation_dal()
162-
.insert_proof_generation_details(l1_batch_number, object_key)
162+
.insert_proof_generation_details(l1_batch_number)
163+
.await?;
164+
storage
165+
.proof_generation_dal()
166+
.save_merkle_paths_artifacts_metadata(l1_batch_number, object_key)
163167
.await?;
164168
}
165169
drop(storage);

core/node/vm_runner/src/impls/bwip.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,11 @@ impl StateKeeperOutputHandler for BasicWitnessInputProducerOutputHandler {
172172

173173
tracing::info!(%l1_batch_number, "Saved VM run data");
174174

175+
connection
176+
.proof_generation_dal()
177+
.insert_proof_generation_details(l1_batch_number)
178+
.await?;
179+
175180
connection
176181
.proof_generation_dal()
177182
.save_vm_runner_artifacts_metadata(l1_batch_number, &blob_url)

0 commit comments

Comments
 (0)