diff --git a/core-primitives/settings/src/lib.rs b/core-primitives/settings/src/lib.rs index f498a37904..d607e329d5 100644 --- a/core-primitives/settings/src/lib.rs +++ b/core-primitives/settings/src/lib.rs @@ -53,7 +53,7 @@ pub mod files { pub static RA_API_KEY_FILE: &str = "key.txt"; pub const SPID_MIN_LENGTH: usize = 32; - pub const STATE_SNAPSHOTS_CACHE_SIZE: usize = 40; + pub const STATE_SNAPSHOTS_CACHE_SIZE: usize = 120; } /// Settings concerning the worker diff --git a/core-primitives/stf-executor/src/executor_tests.rs b/core-primitives/stf-executor/src/executor_tests.rs index 06ad8d60d2..30fec56908 100644 --- a/core-primitives/stf-executor/src/executor_tests.rs +++ b/core-primitives/stf-executor/src/executor_tests.rs @@ -133,7 +133,7 @@ pub fn propose_state_update_always_executes_preprocessing_step() { // given let shard = ShardIdentifier::default(); let (stf_executor, _, state_handler) = stf_executor(); - let _init_hash = state_handler.initialize_shard(&shard).unwrap(); + let _init_hash = state_handler.initialize_shard(shard).unwrap(); let key = "my_key".encode(); let value = "my_value".encode(); let old_state_hash = state_hash(&state_handler.load(&shard).unwrap()); @@ -239,7 +239,7 @@ pub fn execute_update_works() { // given let shard = ShardIdentifier::default(); let (stf_executor, _ocall_api, state_handler) = stf_executor(); - let _init_hash = state_handler.initialize_shard(&shard).unwrap(); + let _init_hash = state_handler.initialize_shard(shard).unwrap(); let key = "my_key".encode(); let value = "my_value".encode(); let old_state_hash = state_hash(&state_handler.load(&shard).unwrap()); @@ -303,7 +303,7 @@ fn init_state_and_shard_with_state_handler>( state_handler: &S, ) -> (State, ShardIdentifier) { let shard = ShardIdentifier::default(); - let _hash = state_handler.initialize_shard(&shard).unwrap(); + let _hash = state_handler.initialize_shard(shard).unwrap(); let (lock, mut state) = state_handler.load_for_mutation(&shard).unwrap(); test_genesis_setup(&mut state); diff --git a/core-primitives/stf-state-handler/src/file_io.rs b/core-primitives/stf-state-handler/src/file_io.rs index d4eebcaa8d..27e98ee709 100644 --- a/core-primitives/stf-state-handler/src/file_io.rs +++ b/core-primitives/stf-state-handler/src/file_io.rs @@ -245,12 +245,11 @@ pub mod sgx { Ok(directory_items .iter() .flat_map(|item| { - extract_state_id_from_file_name(item.as_str()) - // Maybe there is a better way? Need to call a function in case of `None`. - .ok_or_else(|| { - warn!("Found item ({}) that does not match state snapshot naming pattern, ignoring it", item) - }) - .ok() + let maybe_state_id = extract_state_id_from_file_name(item.as_str()); + if maybe_state_id.is_none() { + warn!("Found item ({}) that does not match state snapshot naming pattern, ignoring it", item) + } + maybe_state_id }) .collect()) } @@ -291,6 +290,7 @@ fn file_for_state_exists(shard: &ShardIdentifier, state_id: StateId) -> bool { } #[allow(unused)] +/// Returns true if a shard directory for a given identifier exists AND contains at least one state file. pub(crate) fn shard_exists(shard: &ShardIdentifier) -> bool { let shard_path = shard_path(shard); if !shard_path.exists() { @@ -351,6 +351,12 @@ mod tests { assert!(to_file_name(generate_current_timestamp_state_id()) .strip_suffix(format!("_{}", ENCRYPTED_STATE_FILE).as_str()) .is_some()); + + let now_time_stamp = generate_current_timestamp_state_id(); + assert_eq!( + extract_state_id_from_file_name(to_file_name(now_time_stamp).as_str()).unwrap(), + now_time_stamp + ); } #[test] diff --git a/core-primitives/stf-state-handler/src/handle_state.rs b/core-primitives/stf-state-handler/src/handle_state.rs index 9090a70acf..7d8443abcb 100644 --- a/core-primitives/stf-state-handler/src/handle_state.rs +++ b/core-primitives/stf-state-handler/src/handle_state.rs @@ -33,7 +33,7 @@ pub trait HandleState { /// Initialize a new shard. /// /// Initializes a default state for the shard and returns its hash. - fn initialize_shard(&self, shard: &ShardIdentifier) -> Result; + fn initialize_shard(&self, shard: ShardIdentifier) -> Result; /// Load the state for a given shard. /// diff --git a/core-primitives/stf-state-handler/src/in_memory_state_file_io.rs b/core-primitives/stf-state-handler/src/in_memory_state_file_io.rs index 6d0142f40c..d637b293d4 100644 --- a/core-primitives/stf-state-handler/src/in_memory_state_file_io.rs +++ b/core-primitives/stf-state-handler/src/in_memory_state_file_io.rs @@ -120,7 +120,7 @@ where states_for_shard .get(&state_id) .map(|(_, s)| -> State { s.clone() }) - .ok_or_else(|| Error::InvalidStateId(state_id)) + .ok_or(Error::InvalidStateId(state_id)) } fn compute_hash( @@ -179,7 +179,7 @@ where states_for_shard .remove(&state_id) - .ok_or_else(|| Error::InvalidStateId(state_id)) + .ok_or(Error::InvalidStateId(state_id)) .map(|_| {}) } diff --git a/core-primitives/stf-state-handler/src/state_handler.rs b/core-primitives/stf-state-handler/src/state_handler.rs index 75a17a8df3..032fa788b7 100644 --- a/core-primitives/stf-state-handler/src/state_handler.rs +++ b/core-primitives/stf-state-handler/src/state_handler.rs @@ -53,7 +53,7 @@ where type StateT = Repository::StateType; type HashType = Repository::HashType; - fn initialize_shard(&self, shard: &ShardIdentifier) -> Result { + fn initialize_shard(&self, shard: ShardIdentifier) -> Result { let mut state_write_lock = self.state_snapshot_repository.write().map_err(|_| Error::LockPoisoning)?; state_write_lock.initialize_new_shard(shard) diff --git a/core-primitives/stf-state-handler/src/state_snapshot_repository.rs b/core-primitives/stf-state-handler/src/state_snapshot_repository.rs index 1057524a22..6239aed50d 100644 --- a/core-primitives/stf-state-handler/src/state_snapshot_repository.rs +++ b/core-primitives/stf-state-handler/src/state_snapshot_repository.rs @@ -51,10 +51,8 @@ pub trait VersionedStateAccess { ) -> Result; /// Initialize a new shard. - fn initialize_new_shard( - &mut self, - shard_identifier: &ShardIdentifier, - ) -> Result; + fn initialize_new_shard(&mut self, shard_identifier: ShardIdentifier) + -> Result; /// Checks if a shard for a given identifier exists. fn shard_exists(&self, shard_identifier: &ShardIdentifier) -> bool; @@ -248,19 +246,19 @@ where fn initialize_new_shard( &mut self, - shard_identifier: &ShardIdentifier, + shard_identifier: ShardIdentifier, ) -> Result { - if let Some(state_snapshots) = self.snapshot_history.get(shard_identifier) { + if let Some(state_snapshots) = self.snapshot_history.get(&shard_identifier) { warn!("Shard ({:?}) already exists, will not initialize again", shard_identifier); return state_snapshots.front().map(|s| s.state_hash).ok_or(Error::EmptyRepository) } let snapshot_metadata = - initialize_shard_with_snapshot(shard_identifier, self.file_io.as_ref())?; + initialize_shard_with_snapshot(&shard_identifier, self.file_io.as_ref())?; let state_hash = snapshot_metadata.state_hash; self.snapshot_history - .insert(*shard_identifier, VecDeque::from([snapshot_metadata])); + .insert(shard_identifier, VecDeque::from([snapshot_metadata])); Ok(state_hash) } @@ -413,7 +411,7 @@ mod tests { assert!(state_snapshot_repository.load_latest(&shard_id).is_err()); assert!(state_snapshot_repository.list_shards().unwrap().is_empty()); - let _hash = state_snapshot_repository.initialize_new_shard(&shard_id).unwrap(); + let _hash = state_snapshot_repository.initialize_new_shard(shard_id).unwrap(); assert!(state_snapshot_repository.load_latest(&shard_id).is_ok()); assert_eq!(1, state_snapshot_repository.list_shards().unwrap().len()); @@ -424,7 +422,7 @@ mod tests { let shard_id = ShardIdentifier::random(); let (_, mut state_snapshot_repository) = create_state_snapshot_repository(&[shard_id], 2); - let _hash = state_snapshot_repository.initialize_new_shard(&shard_id).unwrap(); + let _hash = state_snapshot_repository.initialize_new_shard(shard_id).unwrap(); assert!(state_snapshot_repository.load_latest(&shard_id).is_ok()); assert_eq!(1, state_snapshot_repository.list_shards().unwrap().len()); diff --git a/core-primitives/stf-state-handler/src/state_snapshot_repository_loader.rs b/core-primitives/stf-state-handler/src/state_snapshot_repository_loader.rs index ef882059c3..c67fa649c6 100644 --- a/core-primitives/stf-state-handler/src/state_snapshot_repository_loader.rs +++ b/core-primitives/stf-state-handler/src/state_snapshot_repository_loader.rs @@ -174,22 +174,6 @@ mod tests { assert_latest_state_id(&snapshot_history, &shards[2], 14_000_000); } - #[test] - fn ignore_invalid_file_names() { - let shard = ShardIdentifier::random(); - let (file_io, loader) = create_test_fixtures(&[shard]); - - // Only 2 of these file names are valid - file_io.create_initialized(&shard, 12).unwrap(); - file_io.create_initialized(&shard, 13).unwrap(); - file_io.create_initialized(&shard, 8_000_000).unwrap(); - file_io.create_initialized(&shard, 4_000_000).unwrap(); - - let snapshot_history = loader.load_state_snapshot_history().unwrap(); - assert_eq!(2, snapshot_history.get(&shard).unwrap().len()); - assert_latest_state_id(&snapshot_history, &shard, 8_000_000); - } - fn add_state_snapshots(file_io: &TestFileIo, shard: &ShardIdentifier, state_ids: &[StateId]) { for state_id in state_ids { add_snapshot_with_state_ids(file_io, shard, *state_id); diff --git a/core-primitives/stf-state-handler/src/test/mocks/versioned_state_access_mock.rs b/core-primitives/stf-state-handler/src/test/mocks/versioned_state_access_mock.rs index 81052124a9..fae320f054 100644 --- a/core-primitives/stf-state-handler/src/test/mocks/versioned_state_access_mock.rs +++ b/core-primitives/stf-state-handler/src/test/mocks/versioned_state_access_mock.rs @@ -81,9 +81,9 @@ where fn initialize_new_shard( &mut self, - shard_identifier: &ShardIdentifier, + shard_identifier: ShardIdentifier, ) -> Result { - self.state_history.insert(*shard_identifier, VecDeque::from([State::default()])); + self.state_history.insert(shard_identifier, VecDeque::from([State::default()])); Ok(Hash::default()) } diff --git a/core-primitives/stf-state-handler/src/test/sgx_tests.rs b/core-primitives/stf-state-handler/src/test/sgx_tests.rs index 019df4f9b3..82048438c4 100644 --- a/core-primitives/stf-state-handler/src/test/sgx_tests.rs +++ b/core-primitives/stf-state-handler/src/test/sgx_tests.rs @@ -29,7 +29,7 @@ use crate::{ use codec::{Decode, Encode}; use ita_stf::{State as StfState, StateType as StfStateType}; use itp_sgx_crypto::{Aes, AesSeal, StateCrypto}; -use itp_sgx_io::SealedIO; +use itp_sgx_io::{write, SealedIO}; use itp_types::{ShardIdentifier, H256}; use sgx_externalities::SgxExternalitiesTrait; use sp_core::hashing::blake2_256; @@ -268,6 +268,21 @@ pub fn test_state_files_from_handler_can_be_loaded_again() { ); } +pub fn test_list_state_ids_ignores_files_not_matching_the_pattern() { + let shard: ShardIdentifier = [21u8; 32].into(); + let _shard_dir_handle = ShardDirectoryHandle::new(shard).unwrap(); + + let file_io = TestStateFileIo::new(AesSeal::unseal().unwrap()); + + let mut invalid_state_file_path = shard_path(&shard); + invalid_state_file_path.push("invalid-state.bin"); + write(&[0, 1, 2, 3, 4, 5], invalid_state_file_path).unwrap(); + + file_io.create_initialized(&shard, 1234).unwrap(); + + assert_eq!(1, file_io.list_state_ids_for_shard(&shard).unwrap().len()); +} + fn initialize_state_handler_with_directory_handle( shard: &ShardIdentifier, ) -> (Arc, ShardDirectoryHandle) { diff --git a/core-primitives/test/src/mock/handle_state_mock.rs b/core-primitives/test/src/mock/handle_state_mock.rs index 0cbcb26b5f..c23fa4354b 100644 --- a/core-primitives/test/src/mock/handle_state_mock.rs +++ b/core-primitives/test/src/mock/handle_state_mock.rs @@ -41,7 +41,7 @@ pub struct HandleStateMock { } impl HandleStateMock { - pub fn from_shard(shard: &ShardIdentifier) -> Result { + pub fn from_shard(shard: ShardIdentifier) -> Result { let state_handler = HandleStateMock { state_map: Default::default() }; state_handler.initialize_shard(shard)?; Ok(state_handler) @@ -53,15 +53,15 @@ impl HandleState for HandleStateMock { type StateT = StfState; type HashType = H256; - fn initialize_shard(&self, shard: &ShardIdentifier) -> Result { - let maybe_state = self.state_map.read().unwrap().get(shard).cloned(); + fn initialize_shard(&self, shard: ShardIdentifier) -> Result { + let maybe_state = self.state_map.read().unwrap().get(&shard).cloned(); return match maybe_state { // Initialize with default state, if it doesn't exist yet. None => { let state = StfState::default(); let state_hash = state.using_encoded(blake2_256).into(); - self.state_map.write().unwrap().insert(*shard, state); + self.state_map.write().unwrap().insert(shard, state); Ok(state_hash) }, Some(s) => Ok(s.using_encoded(blake2_256).into()), @@ -127,7 +127,7 @@ pub mod tests { pub fn shard_exists_after_inserting() { let state_handler = HandleStateMock::default(); let shard = ShardIdentifier::default(); - state_handler.initialize_shard(&shard).unwrap(); + state_handler.initialize_shard(shard).unwrap(); assert!(state_handler.load(&shard).is_ok()); assert!(state_handler.shard_exists(&shard).unwrap()); @@ -136,7 +136,7 @@ pub mod tests { pub fn initialize_creates_default_state() { let state_handler = HandleStateMock::default(); let shard = ShardIdentifier::default(); - state_handler.initialize_shard(&shard).unwrap(); + state_handler.initialize_shard(shard).unwrap(); let loaded_state_result = state_handler.load(&shard); @@ -146,7 +146,7 @@ pub mod tests { pub fn load_mutate_and_write_works() { let state_handler = HandleStateMock::default(); let shard = ShardIdentifier::default(); - state_handler.initialize_shard(&shard).unwrap(); + state_handler.initialize_shard(shard).unwrap(); let (lock, mut state) = state_handler.load_for_mutation(&shard).unwrap(); @@ -165,7 +165,7 @@ pub mod tests { pub fn ensure_subsequent_state_loads_have_same_hash() { let state_handler = HandleStateMock::default(); let shard = ShardIdentifier::default(); - state_handler.initialize_shard(&shard).unwrap(); + state_handler.initialize_shard(shard).unwrap(); let (lock, _) = state_handler.load_for_mutation(&shard).unwrap(); let initial_state = Stf::init_state(); diff --git a/enclave-runtime/src/initialization.rs b/enclave-runtime/src/initialization.rs index 0d2e1b6833..46a9c4192b 100644 --- a/enclave-runtime/src/initialization.rs +++ b/enclave-runtime/src/initialization.rs @@ -246,7 +246,7 @@ pub(crate) fn init_direct_invocation_server(server_addr: String) -> EnclaveResul Ok(()) } -pub(crate) fn init_shard(shard: &ShardIdentifier) -> EnclaveResult<()> { +pub(crate) fn init_shard(shard: ShardIdentifier) -> EnclaveResult<()> { let state_handler = GLOBAL_STATE_HANDLER_COMPONENT.get()?; let _ = state_handler.initialize_shard(shard)?; Ok(()) diff --git a/enclave-runtime/src/lib.rs b/enclave-runtime/src/lib.rs index 78fb5229fa..0751d38ab1 100644 --- a/enclave-runtime/src/lib.rs +++ b/enclave-runtime/src/lib.rs @@ -433,7 +433,7 @@ pub unsafe extern "C" fn init_shard(shard: *const u8, shard_size: u32) -> sgx_st let shard_identifier = ShardIdentifier::from_slice(slice::from_raw_parts(shard, shard_size as usize)); - if let Err(e) = initialization::init_shard(&shard_identifier) { + if let Err(e) = initialization::init_shard(shard_identifier) { error!("Failed to initialize shard ({:?}): {:?}", shard_identifier, e); return sgx_status_t::SGX_ERROR_UNEXPECTED } diff --git a/enclave-runtime/src/test/fixtures/initialize_test_state.rs b/enclave-runtime/src/test/fixtures/initialize_test_state.rs index 8983a600f4..e95e0ee936 100644 --- a/enclave-runtime/src/test/fixtures/initialize_test_state.rs +++ b/enclave-runtime/src/test/fixtures/initialize_test_state.rs @@ -27,7 +27,7 @@ pub fn init_state>( ) -> (State, ShardIdentifier) { let shard = ShardIdentifier::default(); - let _hash = state_handler.initialize_shard(&shard).unwrap(); + let _hash = state_handler.initialize_shard(shard).unwrap(); let (lock, _) = state_handler.load_for_mutation(&shard).unwrap(); let mut state = Stf::init_state(); diff --git a/enclave-runtime/src/tests.rs b/enclave-runtime/src/tests.rs index 75d7bd86d5..55ed4c3eb8 100644 --- a/enclave-runtime/src/tests.rs +++ b/enclave-runtime/src/tests.rs @@ -94,6 +94,7 @@ pub extern "C" fn test_main_entrance() -> size_t { itp_stf_state_handler::test::sgx_tests::test_multiple_state_updates_create_snapshots_up_to_cache_size, itp_stf_state_handler::test::sgx_tests::test_state_files_from_handler_can_be_loaded_again, itp_stf_state_handler::test::sgx_tests::test_file_io_get_state_hash_works, + itp_stf_state_handler::test::sgx_tests::test_list_state_ids_ignores_files_not_matching_the_pattern, test_compose_block_and_confirmation, test_submit_trusted_call_to_top_pool, test_submit_trusted_getter_to_top_pool, diff --git a/enclave-runtime/src/tls_ra/seal_handler.rs b/enclave-runtime/src/tls_ra/seal_handler.rs index 6e3d854fe5..4818b88c15 100644 --- a/enclave-runtime/src/tls_ra/seal_handler.rs +++ b/enclave-runtime/src/tls_ra/seal_handler.rs @@ -192,7 +192,7 @@ pub mod test { let seal_handler = SealHandlerMock::default(); let state = ::StateT::default(); let shard = ShardIdentifier::default(); - let _init_hash = seal_handler.state_handler.initialize_shard(&shard).unwrap(); + let _init_hash = seal_handler.state_handler.initialize_shard(shard).unwrap(); let result = seal_handler.seal_state(&state.encode(), &shard); @@ -211,7 +211,7 @@ pub mod test { pub fn unseal_seal_state_works() { let seal_handler = SealHandlerMock::default(); let shard = ShardIdentifier::default(); - seal_handler.state_handler.initialize_shard(&shard).unwrap(); + seal_handler.state_handler.initialize_shard(shard).unwrap(); // Fill our mock state: let (lock, mut state) = seal_handler.state_handler.load_for_mutation(&shard).unwrap(); let (key, value) = ("my_key", "my_value"); diff --git a/sidechain/consensus/aura/src/test/block_importer_tests.rs b/sidechain/consensus/aura/src/test/block_importer_tests.rs index 5c82817285..4fb95f17eb 100644 --- a/sidechain/consensus/aura/src/test/block_importer_tests.rs +++ b/sidechain/consensus/aura/src/test/block_importer_tests.rs @@ -75,7 +75,7 @@ fn default_authority() -> Pair { fn test_fixtures( parentchain_block_import_trigger: Arc, ) -> (TestBlockImporter, Arc, Arc) { - let state_handler = Arc::new(HandleStateMock::from_shard(&shard()).unwrap()); + let state_handler = Arc::new(HandleStateMock::from_shard(shard()).unwrap()); let top_pool_call_operator = Arc::new(TestTopPoolCallOperator::default()); let ocall_api = Arc::new( OnchainMock::default() diff --git a/sidechain/top-pool-rpc-author/src/author_tests.rs b/sidechain/top-pool-rpc-author/src/author_tests.rs index 2bad9270ef..6be39ddae3 100644 --- a/sidechain/top-pool-rpc-author/src/author_tests.rs +++ b/sidechain/top-pool-rpc-author/src/author_tests.rs @@ -104,7 +104,7 @@ fn create_author_with_filter>( let top_pool = Arc::new(TrustedOperationPoolMock::default()); let shard_id = shard_id(); - let state_facade = HandleStateMock::from_shard(&shard_id).unwrap(); + let state_facade = HandleStateMock::from_shard(shard_id).unwrap(); let _ = state_facade.load(&shard_id).unwrap(); let encryption_key = ShieldingCryptoMock::default();