From 81afbf4b63c3bff5bc52d42dce7501f29116f34a Mon Sep 17 00:00:00 2001 From: Lech <88630083+Artemka374@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:27:24 +0300 Subject: [PATCH 1/3] fix bwip race condition --- core/lib/dal/src/proof_generation_dal.rs | 50 ++++++++++++++++---- core/node/metadata_calculator/src/updater.rs | 6 ++- core/node/vm_runner/src/impls/bwip.rs | 5 ++ 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/core/lib/dal/src/proof_generation_dal.rs b/core/lib/dal/src/proof_generation_dal.rs index d64df3a752f8..83c77cce1058 100644 --- a/core/lib/dal/src/proof_generation_dal.rs +++ b/core/lib/dal/src/proof_generation_dal.rs @@ -155,26 +155,60 @@ impl ProofGenerationDal<'_, '_> { Ok(()) } + pub async fn save_merkle_paths_artifacts_metadata( + &mut self, + batch_number: L1BatchNumber, + proof_gen_data_blob_url: &str, + ) -> DalResult<()> { + let batch_number = i64::from(batch_number.0); + let query = sqlx::query!( + r#" + UPDATE proof_generation_details + SET + proof_gen_data_blob_url = $1, + updated_at = NOW() + WHERE + l1_batch_number = $2 + "#, + proof_gen_data_blob_url, + batch_number + ); + let instrumentation = Instrumented::new("save_proof_artifacts_metadata") + .with_arg("proof_gen_data_blob_url", &proof_gen_data_blob_url) + .with_arg("l1_batch_number", &batch_number); + let result = instrumentation + .clone() + .with(query) + .execute(self.storage) + .await?; + if result.rows_affected() == 0 { + let err = instrumentation.constraint_error(anyhow::anyhow!( + "Cannot save proof_gen_data_blob_url for a batch number {} that does not exist", + batch_number + )); + return Err(err); + } + + Ok(()) + } + /// The caller should ensure that `l1_batch_number` exists in the database. pub async fn insert_proof_generation_details( &mut self, l1_batch_number: L1BatchNumber, - proof_gen_data_blob_url: &str, ) -> DalResult<()> { let result = sqlx::query!( r#" INSERT INTO - proof_generation_details (l1_batch_number, status, proof_gen_data_blob_url, created_at, updated_at) + proof_generation_details (l1_batch_number, status, created_at, updated_at) VALUES - ($1, 'unpicked', $2, NOW(), NOW()) + ($1, 'unpicked', NOW(), NOW()) ON CONFLICT (l1_batch_number) DO NOTHING "#, - i64::from(l1_batch_number.0), - proof_gen_data_blob_url, + i64::from(l1_batch_number.0), ) .instrument("insert_proof_generation_details") .with_arg("l1_batch_number", &l1_batch_number) - .with_arg("proof_gen_data_blob_url", &proof_gen_data_blob_url) .report_latency() .execute(self.storage) .await?; @@ -303,7 +337,7 @@ mod tests { assert_eq!(unpicked_l1_batch, None); conn.proof_generation_dal() - .insert_proof_generation_details(L1BatchNumber(1), "generation_data") + .insert_proof_generation_details(L1BatchNumber(1)) .await .unwrap(); @@ -316,7 +350,7 @@ mod tests { // Calling the method multiple times should work fine. conn.proof_generation_dal() - .insert_proof_generation_details(L1BatchNumber(1), "generation_data") + .insert_proof_generation_details(L1BatchNumber(1)) .await .unwrap(); conn.proof_generation_dal() diff --git a/core/node/metadata_calculator/src/updater.rs b/core/node/metadata_calculator/src/updater.rs index 4568ab193e3d..d0bd2f2b82c0 100644 --- a/core/node/metadata_calculator/src/updater.rs +++ b/core/node/metadata_calculator/src/updater.rs @@ -159,7 +159,11 @@ impl TreeUpdater { // Save the proof generation details to Postgres storage .proof_generation_dal() - .insert_proof_generation_details(l1_batch_number, object_key) + .insert_proof_generation_details(l1_batch_number) + .await?; + storage + .proof_generation_dal() + .save_merkle_paths_artifacts_metadata(l1_batch_number, object_key) .await?; } drop(storage); diff --git a/core/node/vm_runner/src/impls/bwip.rs b/core/node/vm_runner/src/impls/bwip.rs index 6cb0d6655e6f..7ab18397353d 100644 --- a/core/node/vm_runner/src/impls/bwip.rs +++ b/core/node/vm_runner/src/impls/bwip.rs @@ -172,6 +172,11 @@ impl StateKeeperOutputHandler for BasicWitnessInputProducerOutputHandler { tracing::info!(%l1_batch_number, "Saved VM run data"); + connection + .proof_generation_dal() + .insert_proof_generation_details(l1_batch_number) + .await?; + connection .proof_generation_dal() .save_vm_runner_artifacts_metadata(l1_batch_number, &blob_url) From d5eb54f85d993ada6847924f79d21c245cb03fea Mon Sep 17 00:00:00 2001 From: Lech <88630083+Artemka374@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:41:18 +0300 Subject: [PATCH 2/3] update sqlx data --- ...dcce15601ff39acd03e3c8a2265c9036b3dc54383.json | 15 --------------- ...246b07554ce738a2d7005472e7e76a64a8fbd57ad.json | 14 ++++++++++++++ ...1546735c1dcccdd6c439827fc4c3ba57e8767076e.json | 15 +++++++++++++++ 3 files changed, 29 insertions(+), 15 deletions(-) delete mode 100644 core/lib/dal/.sqlx/query-41a2731a3fe6ae441902632dcce15601ff39acd03e3c8a2265c9036b3dc54383.json create mode 100644 core/lib/dal/.sqlx/query-5137159db7d3ff456e368e6246b07554ce738a2d7005472e7e76a64a8fbd57ad.json create mode 100644 core/lib/dal/.sqlx/query-b61b2545ff82bc3e2a198b21546735c1dcccdd6c439827fc4c3ba57e8767076e.json diff --git a/core/lib/dal/.sqlx/query-41a2731a3fe6ae441902632dcce15601ff39acd03e3c8a2265c9036b3dc54383.json b/core/lib/dal/.sqlx/query-41a2731a3fe6ae441902632dcce15601ff39acd03e3c8a2265c9036b3dc54383.json deleted file mode 100644 index 9ec433e52acb..000000000000 --- a/core/lib/dal/.sqlx/query-41a2731a3fe6ae441902632dcce15601ff39acd03e3c8a2265c9036b3dc54383.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n INSERT INTO\n proof_generation_details (l1_batch_number, status, proof_gen_data_blob_url, created_at, updated_at)\n VALUES\n ($1, 'unpicked', $2, NOW(), NOW())\n ON CONFLICT (l1_batch_number) DO NOTHING\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Int8", - "Text" - ] - }, - "nullable": [] - }, - "hash": "41a2731a3fe6ae441902632dcce15601ff39acd03e3c8a2265c9036b3dc54383" -} diff --git a/core/lib/dal/.sqlx/query-5137159db7d3ff456e368e6246b07554ce738a2d7005472e7e76a64a8fbd57ad.json b/core/lib/dal/.sqlx/query-5137159db7d3ff456e368e6246b07554ce738a2d7005472e7e76a64a8fbd57ad.json new file mode 100644 index 000000000000..07ef0aba074d --- /dev/null +++ b/core/lib/dal/.sqlx/query-5137159db7d3ff456e368e6246b07554ce738a2d7005472e7e76a64a8fbd57ad.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "\n INSERT INTO\n proof_generation_details (l1_batch_number, status, created_at, updated_at)\n VALUES\n ($1, 'unpicked', NOW(), NOW())\n ON CONFLICT (l1_batch_number) DO NOTHING\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [] + }, + "hash": "5137159db7d3ff456e368e6246b07554ce738a2d7005472e7e76a64a8fbd57ad" +} diff --git a/core/lib/dal/.sqlx/query-b61b2545ff82bc3e2a198b21546735c1dcccdd6c439827fc4c3ba57e8767076e.json b/core/lib/dal/.sqlx/query-b61b2545ff82bc3e2a198b21546735c1dcccdd6c439827fc4c3ba57e8767076e.json new file mode 100644 index 000000000000..4f7101ed45e4 --- /dev/null +++ b/core/lib/dal/.sqlx/query-b61b2545ff82bc3e2a198b21546735c1dcccdd6c439827fc4c3ba57e8767076e.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE proof_generation_details\n SET\n proof_gen_data_blob_url = $1,\n updated_at = NOW()\n WHERE\n l1_batch_number = $2\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Int8" + ] + }, + "nullable": [] + }, + "hash": "b61b2545ff82bc3e2a198b21546735c1dcccdd6c439827fc4c3ba57e8767076e" +} From d8a17f4585bd9cf73b1106f5c2dd889b93147763 Mon Sep 17 00:00:00 2001 From: Lech <88630083+Artemka374@users.noreply.github.com> Date: Mon, 8 Jul 2024 19:17:28 +0300 Subject: [PATCH 3/3] drop not null constraint --- ...8161016_remove-not-null-proof-gen-data-constraint.down.sql | 1 + ...708161016_remove-not-null-proof-gen-data-constraint.up.sql | 1 + core/lib/dal/src/proof_generation_dal.rs | 4 ++++ 3 files changed, 6 insertions(+) create mode 100644 core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.down.sql create mode 100644 core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.up.sql diff --git a/core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.down.sql b/core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.down.sql new file mode 100644 index 000000000000..c92ecac92618 --- /dev/null +++ b/core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.down.sql @@ -0,0 +1 @@ +ALTER TABLE proof_generation_details ALTER COLUMN proof_gen_data_blob_url SET NOT NULL; diff --git a/core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.up.sql b/core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.up.sql new file mode 100644 index 000000000000..8604cec1b689 --- /dev/null +++ b/core/lib/dal/migrations/20240708161016_remove-not-null-proof-gen-data-constraint.up.sql @@ -0,0 +1 @@ +ALTER TABLE proof_generation_details ALTER COLUMN proof_gen_data_blob_url DROP NOT NULL; diff --git a/core/lib/dal/src/proof_generation_dal.rs b/core/lib/dal/src/proof_generation_dal.rs index 83c77cce1058..cf1437ff411c 100644 --- a/core/lib/dal/src/proof_generation_dal.rs +++ b/core/lib/dal/src/proof_generation_dal.rs @@ -357,6 +357,10 @@ mod tests { .save_vm_runner_artifacts_metadata(L1BatchNumber(1), "vm_run") .await .unwrap(); + conn.proof_generation_dal() + .save_merkle_paths_artifacts_metadata(L1BatchNumber(1), "data") + .await + .unwrap(); conn.blocks_dal() .save_l1_batch_tree_data( L1BatchNumber(1),