apollo_storage: drop the legacy state commitment infos table and file at startup

crates/apollo_storage/src/db/db_test.rs

@@ -14,6 +14,29 @@ pub(crate) fn get_test_env() -> ((DbReader, DbWriter), TempDir) {
     (open_env(&config.db_config).expect("Failed to open environment."), temp_dir)
 }
 
+#[test]
+fn drop_table_if_exists() {
+    let ((reader, mut writer), _temp_dir) = get_test_env();
+    let table_id =
+        writer.create_simple_table::<[u8; 3], NoVersionValueWrapper<[u8; 5]>>("legacy").unwrap();
+    // Scoped so that no table handle is open when the table is dropped.
+    {
+        let rtxn = reader.begin_ro_txn().unwrap();
+        let table = rtxn.open_table(&table_id).unwrap();
+        let wtxn = writer.begin_persistent_rw_txn().unwrap();
+        table.insert(wtxn.txn(), b"key", b"data0").unwrap();
+        wtxn.commit().unwrap();
+    }
+
+    assert!(writer.drop_table_if_exists("legacy").unwrap());
+    // The table is gone, not merely emptied.
+    assert!(matches!(
+        reader.begin_ro_txn().unwrap().open_table(&table_id),
+        Err(DbError::Inner(libmdbx::Error::NotFound))
+    ));
+    assert!(!writer.drop_table_if_exists("legacy").unwrap());
+}
+
 #[test]
 fn open_env_scenario() {
     get_test_env();

crates/apollo_storage/src/db/mod.rs

@@ -285,6 +285,22 @@ impl DbReader {
 type DbReadTransaction<'env> = DbTransaction<'env, RO>;
 
 impl DbWriter {
+    /// Drops the table if it exists and returns whether it did. Meant for tables removed from the
+    /// schema, which a storage created by an older version still holds.
+    pub(crate) fn drop_table_if_exists(&mut self, name: &str) -> DbResult<bool> {
+        let txn = self.env.begin_rw_txn()?;
+        let table = match txn.open_table(Some(name)) {
+            Ok(table) => table,
+            Err(libmdbx::Error::NotFound) => return Ok(false),
+            Err(err) => return Err(err.into()),
+        };
+        // Safety: the handle was opened in this transaction and is the only one referring to the
+        // table.
+        unsafe { txn.drop_table(table)? };
+        txn.commit()?;
+        Ok(true)
+    }
+
     /// Creates a persistent write transaction that can be stored in structs without lifetime
     /// constraints.
     ///

crates/apollo_storage/src/lib.rs

@@ -242,6 +242,7 @@ fn open_storage_internal(
     }
 
     let (db_reader, mut db_writer) = open_env(&storage_config.db_config)?;
+    remove_legacy_state_commitment_infos(&mut db_writer, &storage_config.db_config)?;
     let tables = Arc::new(Tables {
         block_hash_to_number: db_writer.create_simple_table("block_hash_to_number")?,
         block_signatures: db_writer.create_simple_table("block_signatures")?,
@@ -1295,6 +1296,23 @@ impl<Mode: TransactionKind> FileHandlers<Mode> {
     }
 }
 
+/// Removes the state commitment infos table and file that a storage created before they moved to
+/// the committer still holds.
+fn remove_legacy_state_commitment_infos(
+    db_writer: &mut DbWriter,
+    db_config: &DbConfig,
+) -> StorageResult<()> {
+    if db_writer.drop_table_if_exists("state_commitment_infos")? {
+        info!("Dropped the legacy state_commitment_infos table.");
+    }
+    let file_path = db_config.path().join("state_commitment_infos.dat");
+    if file_path.exists() {
+        fs::remove_file(&file_path)?;
+        info!("Removed the legacy state commitment infos file {}.", file_path.display());
+    }
+    Ok(())
+}
+
 fn open_storage_files(
     db_config: &DbConfig,
     mmap_file_config: MmapFileConfig,

crates/apollo_storage/src/open_storage_test.rs

@@ -2,15 +2,17 @@
 
 use std::num::NonZeroUsize;
 use std::sync::Arc;
-use std::thread;
 use std::time::Duration;
+use std::{fs, thread};
 
 use starknet_api::block::{BlockHash, BlockHeader, BlockNumber};
 use starknet_api::felt;
 use tempfile::tempdir;
 
+use crate::db::open_env;
+use crate::db::serialization::NoVersionValueWrapper;
 use crate::header::{HeaderStorageReader, HeaderStorageWriter};
-use crate::test_utils::get_test_config_with_path;
+use crate::test_utils::{get_test_config, get_test_config_with_path};
 use crate::{open_storage, BatchConfig, StorageConfig, StorageError, StorageReader, StorageScope};
 
 /// Check that storage reader can access storage
@@ -1416,3 +1418,26 @@ fn test_set_batch_size_to_one_flushes_pending_batch() {
         );
     }
 }
+
+/// A storage created before the state commitment infos moved to the committer still holds their
+/// table and file; opening it removes both.
+#[test]
+fn open_storage_removes_legacy_state_commitment_infos() {
+    let (config, _temp_dir) = get_test_config(None);
+    let legacy_file_path = config.db_config.path().join("state_commitment_infos.dat");
+    {
+        let (_reader, mut writer) = open_env(&config.db_config).unwrap();
+        writer
+            .create_simple_table::<[u8; 3], NoVersionValueWrapper<[u8; 5]>>(
+                "state_commitment_infos",
+            )
+            .unwrap();
+    }
+    fs::write(&legacy_file_path, b"legacy").unwrap();
+
+    drop(open_storage(config.clone()).unwrap());
+
+    assert!(!legacy_file_path.exists());
+    let (_reader, mut writer) = open_env(&config.db_config).unwrap();
+    assert!(!writer.drop_table_if_exists("state_commitment_infos").unwrap());
+}