diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 94cef77b5f..4730de90f7 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -2558,10 +2558,10 @@ pub(crate) mod tests { #[test] fn test_replay_commitment_cache() { - fn leader_vote(bank: &Arc, pubkey: &Pubkey) { + fn leader_vote(vote_slot: Slot, bank: &Arc, pubkey: &Pubkey) { let mut leader_vote_account = bank.get_account(&pubkey).unwrap(); let mut vote_state = VoteState::from(&leader_vote_account).unwrap(); - vote_state.process_slot_vote_unchecked(bank.slot()); + vote_state.process_slot_vote_unchecked(vote_slot); let versioned = VoteStateVersions::Current(Box::new(vote_state)); VoteState::to(&versioned, &mut leader_vote_account).unwrap(); bank.store_account(&pubkey, &leader_vote_account); @@ -2581,10 +2581,7 @@ pub(crate) mod tests { } bank0.freeze(); let arc_bank0 = Arc::new(bank0); - let bank_forks = Arc::new(RwLock::new(BankForks::new_from_banks( - &[arc_bank0.clone()], - 0, - ))); + let bank_forks = Arc::new(RwLock::new(BankForks::new_from_banks(&[arc_bank0], 0))); let exit = Arc::new(AtomicBool::new(false)); let block_commitment_cache = Arc::new(RwLock::new(BlockCommitmentCache::default())); @@ -2608,44 +2605,33 @@ pub(crate) mod tests { .get_block_commitment(1) .is_none()); - let bank1 = Bank::new_from_parent(&arc_bank0, &Pubkey::default(), arc_bank0.slot() + 1); - let _res = bank1.transfer( - 10, - &genesis_config_info.mint_keypair, - &solana_sdk::pubkey::new_rand(), - ); - for _ in 0..genesis_config.ticks_per_slot { - bank1.register_tick(&Hash::default()); + for i in 1..=3 { + let prev_bank = bank_forks.read().unwrap().get(i - 1).unwrap().clone(); + let bank = Bank::new_from_parent(&prev_bank, &Pubkey::default(), prev_bank.slot() + 1); + let _res = bank.transfer( + 10, + &genesis_config_info.mint_keypair, + &solana_sdk::pubkey::new_rand(), + ); + for _ in 0..genesis_config.ticks_per_slot { + bank.register_tick(&Hash::default()); + } + bank_forks.write().unwrap().insert(bank); + let arc_bank = bank_forks.read().unwrap().get(i).unwrap().clone(); + leader_vote(i - 1, &arc_bank, &leader_voting_pubkey); + ReplayStage::update_commitment_cache( + arc_bank.clone(), + 0, + leader_lamports, + &lockouts_sender, + ); + arc_bank.freeze(); } - bank1.freeze(); - bank_forks.write().unwrap().insert(bank1); - let arc_bank1 = bank_forks.read().unwrap().get(1).unwrap().clone(); - leader_vote(&arc_bank1, &leader_voting_pubkey); - ReplayStage::update_commitment_cache( - arc_bank1.clone(), - 0, - leader_lamports, - &lockouts_sender, - ); - let bank2 = Bank::new_from_parent(&arc_bank1, &Pubkey::default(), arc_bank1.slot() + 1); - let _res = bank2.transfer( - 10, - &genesis_config_info.mint_keypair, - &solana_sdk::pubkey::new_rand(), - ); - for _ in 0..genesis_config.ticks_per_slot { - bank2.register_tick(&Hash::default()); - } - bank2.freeze(); - bank_forks.write().unwrap().insert(bank2); - let arc_bank2 = bank_forks.read().unwrap().get(2).unwrap().clone(); - leader_vote(&arc_bank2, &leader_voting_pubkey); - ReplayStage::update_commitment_cache(arc_bank2, 0, leader_lamports, &lockouts_sender); thread::sleep(Duration::from_millis(200)); let mut expected0 = BlockCommitment::default(); - expected0.increase_confirmation_stake(2, leader_lamports); + expected0.increase_confirmation_stake(3, leader_lamports); assert_eq!( block_commitment_cache .read() diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 84894f7716..de3ebdafcb 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -867,6 +867,8 @@ pub struct Bank { pub feature_set: Arc, pub drop_callback: RwLock, + + pub freeze_started: AtomicBool, } impl Default for BlockhashQueue { @@ -1022,6 +1024,7 @@ impl Bank { .as_ref() .map(|drop_callback| drop_callback.clone_box()), )), + freeze_started: AtomicBool::new(false), }; datapoint_info!( @@ -1145,6 +1148,7 @@ impl Bank { transaction_log_collector: new(), feature_set: new(), drop_callback: RwLock::new(OptionalDropCallback(None)), + freeze_started: AtomicBool::new(fields.hash != Hash::default()), }; bank.finish_init(genesis_config, additional_builtins); @@ -1248,6 +1252,10 @@ impl Bank { *self.hash.read().unwrap() != Hash::default() } + pub fn freeze_started(&self) -> bool { + self.freeze_started.load(Relaxed) + } + pub fn status_cache_ancestors(&self) -> Vec { let mut roots = self.src.status_cache.read().unwrap().roots().clone(); let min = roots.iter().min().cloned().unwrap_or(0); @@ -1941,6 +1949,17 @@ impl Bank { } pub fn freeze(&self) { + // This lock prevents any new commits from BankingStage + // `process_and_record_transactions_locked()` from coming + // in after the last tick is observed. This is because in + // BankingStage, any transaction successfully recorded in + // `record_transactions()` is recorded after this `hash` lock + // is grabbed. At the time of the successful record, + // this means the PoH has not yet reached the last tick, + // so this means freeze() hasn't been called yet. And because + // BankingStage doesn't release this hash lock until both + // record and commit are finished, those transactions will be + // committed before this write lock can be obtained here. let mut hash = self.hash.write().unwrap(); if *hash == Hash::default() { @@ -1952,6 +1971,7 @@ impl Bank { self.run_incinerator(); // freeze is a one-way trip, idempotent + self.freeze_started.store(true, Relaxed); *hash = self.hash_internal_state(); } } @@ -2145,7 +2165,7 @@ impl Bank { } assert!( - !self.is_frozen(), + !self.freeze_started(), "Can't change frozen bank by adding not-existing new native program ({}, {}). \ Maybe, inconsistent program activation is detected on snapshot restore?", name, @@ -2272,22 +2292,24 @@ impl Bank { /// bank will reject transactions using that `hash`. pub fn register_tick(&self, hash: &Hash) { assert!( - !self.is_frozen(), - "register_tick() working on a frozen bank!" + !self.freeze_started(), + "register_tick() working on a bank that is already frozen or is undergoing freezing!" ); inc_new_counter_debug!("bank-register_tick-registered", 1); - // Grab blockhash lock before incrementing tick height so that replay stage does - // not attempt to freeze after observing the last tick and before blockhash is - // updated let mut w_blockhash_queue = self.blockhash_queue.write().unwrap(); - let current_tick_height = self.tick_height.fetch_add(1, Relaxed) as u64; - if self.is_block_boundary(current_tick_height + 1) { + if self.is_block_boundary(self.tick_height.load(Relaxed) + 1) { w_blockhash_queue.register_hash(hash, &self.fee_calculator); if self.fix_recent_blockhashes_sysvar_delay() { self.update_recent_blockhashes_locked(&w_blockhash_queue); } } + // ReplayStage will start computing the accounts delta hash when it + // detects the tick height has reached the boundary, so the system + // needs to guarantee all account updates for the slot have been + // committed before this tick height is incremented (like the blockhash + // sysvar above) + self.tick_height.fetch_add(1, Relaxed); } pub fn is_complete(&self) -> bool { @@ -3053,8 +3075,8 @@ impl Bank { signature_count: u64, ) -> TransactionResults { assert!( - !self.is_frozen(), - "commit_transactions() working on a frozen bank!" + !self.freeze_started(), + "commit_transactions() working on a bank that is already frozen or is undergoing freezing!" ); self.increment_transaction_count(tx_count); @@ -3768,6 +3790,7 @@ impl Bank { } pub fn store_account(&self, pubkey: &Pubkey, account: &Account) { + assert!(!self.freeze_started()); self.rc.accounts.store_slow(self.slot(), pubkey, account); if Stakes::is_stake(account) {