diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 3d08b90c4..f412f0328 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -20,6 +20,7 @@ use { accounts_index::AccountSecondaryIndexes, bank::{Bank, BankSlotDelta}, bank_forks::BankForks, + epoch_accounts_hash::EpochAccountsHash, genesis_utils::{create_genesis_config_with_leader, GenesisConfigInfo}, runtime_config::RuntimeConfig, snapshot_archive_info::FullSnapshotArchiveInfo, @@ -616,13 +617,21 @@ fn test_slots_to_snapshot(snapshot_version: SnapshotVersion, cluster_type: Clust // Since the accounts background services are not runnning, EpochAccountsHash // calculation requests will not be handled. To prevent banks from hanging during - // Bank::freeze() due to waiting for EAH to complete, just set the EAH to Invalid. - current_bank + // Bank::freeze() due to waiting for EAH to complete, just set the EAH to Valid. + let epoch_accounts_hash_manager = ¤t_bank .rc .accounts .accounts_db - .epoch_accounts_hash_manager - .set_invalid_for_tests(); + .epoch_accounts_hash_manager; + if epoch_accounts_hash_manager + .try_get_epoch_accounts_hash() + .is_none() + { + epoch_accounts_hash_manager.set_valid( + EpochAccountsHash::new(Hash::new_unique()), + current_bank.slot(), + ) + } } let num_old_slots = num_set_roots * *add_root_interval - MAX_CACHE_ENTRIES + 1; diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 142dc38b8..ebc5b9c71 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -31,6 +31,7 @@ use { block_cost_limits::*, commitment::VOTE_THRESHOLD_SIZE, cost_model::CostModel, + epoch_accounts_hash::EpochAccountsHash, prioritization_fee_cache::PrioritizationFeeCache, runtime_config::RuntimeConfig, transaction_batch::TransactionBatch, @@ -730,9 +731,9 @@ pub fn test_process_blockstore( opts: &ProcessOptions, exit: &Arc, ) -> (Arc>, LeaderScheduleCache) { - // Spin up a thread to be a fake Accounts Background Service. Need to intercept and handle - // (i.e. skip/make invalid) all EpochAccountsHash requests so future rooted banks do not hang - // in Bank::freeze() waiting for an in-flight EAH calculation to complete. + // Spin up a thread to be a fake Accounts Background Service. Need to intercept and handle all + // EpochAccountsHash requests so future rooted banks do not hang in Bank::freeze() waiting for + // an in-flight EAH calculation to complete. let (snapshot_request_sender, snapshot_request_receiver) = crossbeam_channel::unbounded(); let abs_request_sender = AbsRequestSender::new(snapshot_request_sender); let bg_exit = Arc::new(AtomicBool::new(false)); @@ -752,7 +753,10 @@ pub fn test_process_blockstore( .accounts .accounts_db .epoch_accounts_hash_manager - .set_invalid_for_tests(); + .set_valid( + EpochAccountsHash::new(Hash::new_unique()), + snapshot_request.snapshot_root_bank.slot(), + ) }); std::thread::sleep(Duration::from_millis(100)); } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 7dd0a9870..d5b7e604e 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -20,6 +20,7 @@ use { bank_forks::BankForks, builtins::Builtin, commitment::BlockCommitmentCache, + epoch_accounts_hash::EpochAccountsHash, genesis_utils::{create_genesis_config_with_leader_ex, GenesisConfigInfo}, runtime_config::RuntimeConfig, }, @@ -1138,8 +1139,8 @@ impl ProgramTestContext { bank_forks.set_root(pre_warp_slot, &abs_request_sender, Some(pre_warp_slot)); // The call to `set_root()` above will send an EAH request. Need to intercept and handle - // (i.e. skip/make invalid) all EpochAccountsHash requests so future rooted banks do not - // hang in Bank::freeze() waiting for an in-flight EAH calculation to complete. + // all EpochAccountsHash requests so future rooted banks do not hang in Bank::freeze() + // waiting for an in-flight EAH calculation to complete. snapshot_request_receiver .try_iter() .filter(|snapshot_request| { @@ -1152,7 +1153,10 @@ impl ProgramTestContext { .accounts .accounts_db .epoch_accounts_hash_manager - .set_invalid_for_tests(); + .set_valid( + EpochAccountsHash::new(Hash::new_unique()), + snapshot_request.snapshot_root_bank.slot(), + ) }); // warp_bank is frozen so go forward to get unfrozen bank at warp_slot diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 6a17a4279..1b6c3a6c9 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -781,7 +781,10 @@ fn cmp_requests_by_priority( mod test { use { super::*, - crate::{epoch_accounts_hash, genesis_utils::create_genesis_config}, + crate::{ + epoch_accounts_hash::{self, EpochAccountsHash}, + genesis_utils::create_genesis_config, + }, crossbeam_channel::unbounded, solana_sdk::{account::AccountSharedData, epoch_schedule::EpochSchedule, pubkey::Pubkey}, }; @@ -859,6 +862,13 @@ mod test { EpochSchedule::custom(SLOTS_PER_EPOCH, SLOTS_PER_EPOCH, false); let bank = Arc::new(Bank::new_for_tests(&genesis_config_info.genesis_config)); bank.set_startup_verification_complete(); + // Need to set the EAH to Valid so that `Bank::new_from_parent()` doesn't panic during + // freeze when parent is in the EAH calculation window. + bank.rc + .accounts + .accounts_db + .epoch_accounts_hash_manager + .set_valid(EpochAccountsHash::new(Hash::new_unique()), 0); // Create new banks and send snapshot requests so that the following requests will be in // the channel before handling the requests: diff --git a/runtime/src/bank_forks.rs b/runtime/src/bank_forks.rs index 4ed60cc11..ba3224dbb 100644 --- a/runtime/src/bank_forks.rs +++ b/runtime/src/bank_forks.rs @@ -635,6 +635,7 @@ mod tests { super::*, crate::{ bank::tests::update_vote_account_timestamp, + epoch_accounts_hash::EpochAccountsHash, genesis_utils::{ create_genesis_config, create_genesis_config_with_leader, GenesisConfigInfo, }, @@ -746,8 +747,8 @@ mod tests { genesis_config.epoch_schedule = EpochSchedule::new(slots_in_epoch); // Spin up a thread to be a fake Accounts Background Service. Need to intercept and handle - // (i.e. skip/make invalid) all EpochAccountsHash requests so future rooted banks do not hang - // in Bank::freeze() waiting for an in-flight EAH calculation to complete. + // all EpochAccountsHash requests so future rooted banks do not hang in Bank::freeze() + // waiting for an in-flight EAH calculation to complete. let (snapshot_request_sender, snapshot_request_receiver) = crossbeam_channel::unbounded(); let abs_request_sender = AbsRequestSender::new(snapshot_request_sender); let bg_exit = Arc::new(AtomicBool::new(false)); @@ -767,7 +768,10 @@ mod tests { .accounts .accounts_db .epoch_accounts_hash_manager - .set_invalid_for_tests(); + .set_valid( + EpochAccountsHash::new(Hash::new_unique()), + snapshot_request.snapshot_root_bank.slot(), + ) }); std::thread::sleep(Duration::from_millis(100)); } diff --git a/runtime/src/epoch_accounts_hash/manager.rs b/runtime/src/epoch_accounts_hash/manager.rs index 0cfa34f92..90a51c6f5 100644 --- a/runtime/src/epoch_accounts_hash/manager.rs +++ b/runtime/src/epoch_accounts_hash/manager.rs @@ -1,6 +1,6 @@ use { super::EpochAccountsHash, - solana_sdk::{clock::Slot, hash::Hash}, + solana_sdk::clock::Slot, std::sync::{Condvar, Mutex}, }; @@ -68,8 +68,8 @@ impl Manager { loop { match &*state { State::Valid(epoch_accounts_hash, _slot) => break *epoch_accounts_hash, - State::Invalid => break SENTINEL_EPOCH_ACCOUNTS_HASH, State::InFlight(_slot) => state = self.cvar.wait(state).unwrap(), + State::Invalid => panic!("The epoch accounts hash cannot be awaited when Invalid!"), } } } @@ -84,23 +84,15 @@ impl Manager { _ => None, } } - - /// **FOR TESTS ONLY** - /// Set the state to Invalid - /// This is needed by tests that do not fully startup all the accounts background services. - /// **FOR TESTS ONLY** - pub fn set_invalid_for_tests(&self) { - *self.state.lock().unwrap() = State::Invalid; - } } /// The EpochAccountsHash is calculated in the background via AccountsBackgroundService. This enum -/// is used to track the state of that calculation, and queried when saving the EAH into a Bank. +/// is used to track the state of that calculation, and queried when saving the EAH into a Bank or +/// Snapshot. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum State { - /// On startup from genesis/slot0, the initial state of the EAH is invalid since one has not - /// yet been requested. This state should only really occur for tests and new clusters; not - /// for established running clusters. + /// The initial state of the EAH. This can occur when loading from a snapshot that does not + /// include an EAH, or when starting from genesis (before an EAH calculation is requested). Invalid, /// An EAH calculation has been requested (for `Slot`) and is in flight. The Bank that should /// save the EAH must wait until the calculation has completed. @@ -109,19 +101,9 @@ enum State { Valid(EpochAccountsHash, Slot), } -/// Sentinel epoch accounts hash value; used when getting an Invalid EAH -/// -/// Displays as "Sentine1EpochAccountsHash111111111111111111" -const SENTINEL_EPOCH_ACCOUNTS_HASH: EpochAccountsHash = - EpochAccountsHash::new(Hash::new_from_array([ - 0x06, 0x92, 0x40, 0x3b, 0xee, 0xea, 0x7e, 0xe2, 0x7d, 0xf4, 0x90, 0x7f, 0xbd, 0x9e, 0xd0, - 0xd2, 0x1c, 0x2b, 0x66, 0x9a, 0xc4, 0xda, 0xce, 0xd7, 0x23, 0x41, 0x69, 0xab, 0xb7, 0x80, - 0x00, 0x00, - ])); - #[cfg(test)] mod tests { - use {super::*, std::time::Duration}; + use {super::*, solana_sdk::hash::Hash, std::time::Duration}; #[test] fn test_new_valid() { @@ -138,10 +120,6 @@ mod tests { fn test_new_invalid() { let manager = Manager::new_invalid(); assert!(manager.try_get_epoch_accounts_hash().is_none()); - assert_eq!( - manager.wait_get_epoch_accounts_hash(), - SENTINEL_EPOCH_ACCOUNTS_HASH, - ); } #[test] @@ -163,15 +141,6 @@ mod tests { #[test] fn test_wait_epoch_accounts_hash() { - // Test: State is Invalid, no need to wait - { - let manager = Manager::new_invalid(); - assert_eq!( - manager.wait_get_epoch_accounts_hash(), - SENTINEL_EPOCH_ACCOUNTS_HASH, - ); - } - // Test: State is Valid, no need to wait { let epoch_accounts_hash = EpochAccountsHash::new(Hash::new_unique()); @@ -196,4 +165,12 @@ mod tests { }); } } + + #[test] + #[should_panic] + fn test_wait_epoch_accounts_hash_invalid() { + // Test: State is Invalid, should panic + let manager = Manager::new_invalid(); + let _epoch_accounts_hash = manager.wait_get_epoch_accounts_hash(); + } } diff --git a/runtime/tests/stake.rs b/runtime/tests/stake.rs index 4d8219018..7db9e1c34 100644 --- a/runtime/tests/stake.rs +++ b/runtime/tests/stake.rs @@ -3,12 +3,14 @@ use { solana_runtime::{ bank::Bank, bank_client::BankClient, + epoch_accounts_hash::EpochAccountsHash, genesis_utils::{create_genesis_config_with_leader, GenesisConfigInfo}, }, solana_sdk::{ account::from_account, account_utils::StateMut, client::SyncClient, + hash::Hash, message::Message, pubkey::Pubkey, rent::Rent, @@ -282,6 +284,13 @@ fn test_stake_account_lifetime() { let bank = Bank::new_for_tests(&genesis_config); let mint_pubkey = mint_keypair.pubkey(); let mut bank = Arc::new(bank); + // Need to set the EAH to Valid so that `Bank::new_from_parent()` doesn't panic during freeze + // when parent is in the EAH calculation window. + bank.rc + .accounts + .accounts_db + .epoch_accounts_hash_manager + .set_valid(EpochAccountsHash::new(Hash::new_unique()), bank.slot()); let bank_client = BankClient::new_shared(&bank); let (vote_balance, stake_rent_exempt_reserve, stake_minimum_delegation) = {