diff --git a/crates/apollo_storage/src/db/db_test.rs b/crates/apollo_storage/src/db/db_test.rs index b8f72f46a17..7d65e3464ff 100644 --- a/crates/apollo_storage/src/db/db_test.rs +++ b/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(); diff --git a/crates/apollo_storage/src/db/mod.rs b/crates/apollo_storage/src/db/mod.rs index 4ad236e9720..87bd2f2985b 100644 --- a/crates/apollo_storage/src/db/mod.rs +++ b/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 { + 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. /// diff --git a/crates/apollo_storage/src/lib.rs b/crates/apollo_storage/src/lib.rs index 5ebb5d6ecc6..554440e5400 100644 --- a/crates/apollo_storage/src/lib.rs +++ b/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 FileHandlers { } } +/// 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, diff --git a/crates/apollo_storage/src/open_storage_test.rs b/crates/apollo_storage/src/open_storage_test.rs index 7d999de60ee..c5370a9d858 100644 --- a/crates/apollo_storage/src/open_storage_test.rs +++ b/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()); +}