From 907d9903c5a470809c0038543d7ddb790fb89510 Mon Sep 17 00:00:00 2001 From: anatoly yakovenko Date: Mon, 18 Mar 2019 12:12:33 -0700 Subject: [PATCH 1/8] Implement locktower voting (#3251) * locktower components and tests * integrate locktower into replay stage * track locktower duration * make sure threshold is checked after simulating the vote * check vote lockouts using the VoteState program * duplicate vote test * epoch stakes * disable impossible to verify tests --- core/src/bank_forks.rs | 78 +++- core/src/lib.rs | 1 + core/src/locktower.rs | 575 ++++++++++++++++++++++++++++ core/src/replay_stage.rs | 87 ++--- core/src/staking_utils.rs | 2 +- programs/vote_api/src/vote_state.rs | 46 ++- tests/local_cluster.rs | 2 + 7 files changed, 736 insertions(+), 55 deletions(-) create mode 100644 core/src/locktower.rs diff --git a/core/src/bank_forks.rs b/core/src/bank_forks.rs index 1e3f1e2763ace2..fbbbd5d9cdfea9 100644 --- a/core/src/bank_forks.rs +++ b/core/src/bank_forks.rs @@ -1,7 +1,7 @@ //! The `bank_forks` module implments BankForks a DAG of checkpointed Banks +use hashbrown::{HashMap, HashSet}; use solana_runtime::bank::Bank; -use std::collections::HashMap; use std::ops::Index; use std::sync::Arc; @@ -27,6 +27,47 @@ impl BankForks { working_bank, } } + + /// Create a map of bank slot id to the set of ancestors for the bank slot. + pub fn ancestors(&self) -> HashMap> { + let mut ancestors = HashMap::new(); + let mut pending: Vec> = self.banks.values().cloned().collect(); + while !pending.is_empty() { + let bank = pending.pop().unwrap(); + if ancestors.get(&bank.slot()).is_some() { + continue; + } + let set = bank.parents().into_iter().map(|b| b.slot()).collect(); + ancestors.insert(bank.slot(), set); + pending.extend(bank.parents().into_iter()); + } + ancestors + } + + /// Create a map of bank slot id to the set of all of its descendants + pub fn decendants(&self) -> HashMap> { + let mut decendants = HashMap::new(); + let mut pending: Vec> = self.banks.values().cloned().collect(); + let mut done = HashSet::new(); + assert!(!pending.is_empty()); + while !pending.is_empty() { + let bank = pending.pop().unwrap(); + if done.contains(&bank.slot()) { + continue; + } + done.insert(bank.slot()); + let _ = decendants.entry(bank.slot()).or_insert(HashSet::new()); + for parent in bank.parents() { + decendants + .entry(parent.slot()) + .or_insert(HashSet::new()) + .insert(bank.slot()); + } + pending.extend(bank.parents().into_iter()); + } + decendants + } + pub fn frozen_banks(&self) -> HashMap> { let mut frozen_banks: Vec> = vec![]; frozen_banks.extend(self.banks.values().filter(|v| v.is_frozen()).cloned()); @@ -106,6 +147,41 @@ mod tests { assert_eq!(bank_forks.working_bank().tick_height(), 1); } + #[test] + fn test_bank_forks_decendants() { + let (genesis_block, _) = GenesisBlock::new(10_000); + let bank = Bank::new(&genesis_block); + let mut bank_forks = BankForks::new(0, bank); + let bank0 = bank_forks[0].clone(); + let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 1); + bank_forks.insert(1, bank); + let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 2); + bank_forks.insert(2, bank); + let decendants = bank_forks.decendants(); + let children: Vec = decendants[&0].iter().cloned().collect(); + assert_eq!(children, vec![1, 2]); + assert!(decendants[&1].is_empty()); + assert!(decendants[&2].is_empty()); + } + + #[test] + fn test_bank_forks_ancestors() { + let (genesis_block, _) = GenesisBlock::new(10_000); + let bank = Bank::new(&genesis_block); + let mut bank_forks = BankForks::new(0, bank); + let bank0 = bank_forks[0].clone(); + let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 1); + bank_forks.insert(1, bank); + let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 2); + bank_forks.insert(2, bank); + let ancestors = bank_forks.ancestors(); + assert!(ancestors[&0].is_empty()); + let parents: Vec = ancestors[&1].iter().cloned().collect(); + assert_eq!(parents, vec![0]); + let parents: Vec = ancestors[&2].iter().cloned().collect(); + assert_eq!(parents, vec![0]); + } + #[test] fn test_bank_forks_frozen_banks() { let (genesis_block, _) = GenesisBlock::new(10_000); diff --git a/core/src/lib.rs b/core/src/lib.rs index 0c53b506faa1e1..065a0ae8ae1c03 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -44,6 +44,7 @@ pub mod leader_schedule; pub mod leader_schedule_utils; pub mod local_cluster; pub mod local_vote_signer_service; +pub mod locktower; pub mod packet; pub mod poh; pub mod poh_recorder; diff --git a/core/src/locktower.rs b/core/src/locktower.rs new file mode 100644 index 00000000000000..39634ffee563b2 --- /dev/null +++ b/core/src/locktower.rs @@ -0,0 +1,575 @@ +use crate::bank_forks::BankForks; +use crate::staking_utils; +use hashbrown::{HashMap, HashSet}; +use solana_runtime::bank::Bank; +use solana_sdk::account::Account; +use solana_sdk::pubkey::Pubkey; +use solana_vote_api::vote_instruction::Vote; +use solana_vote_api::vote_state::{Lockout, VoteState, MAX_LOCKOUT_HISTORY}; + +const VOTE_THRESHOLD_DEPTH: usize = 8; +const VOTE_THRESHOLD_SIZE: f64 = 2f64 / 3f64; + +#[derive(Default)] +pub struct EpochStakes { + slot: u64, + stakes: HashMap, + total_staked: u64, +} + +#[derive(Default)] +pub struct StakeLockout { + lockout: u64, + stake: u64, +} + +#[derive(Default)] +pub struct Locktower { + epoch_stakes: EpochStakes, + threshold_depth: usize, + threshold_size: f64, + lockouts: VoteState, +} + +impl EpochStakes { + pub fn new(slot: u64, stakes: HashMap) -> Self { + let total_staked = stakes.values().sum(); + Self { + slot, + stakes, + total_staked, + } + } + pub fn new_for_tests(lamports: u64) -> Self { + Self::new(0, vec![(Pubkey::default(), lamports)].into_iter().collect()) + } + pub fn new_from_stake_accounts(slot: u64, accounts: &[(Pubkey, Account)]) -> Self { + let stakes = accounts.iter().map(|(k, v)| (*k, v.lamports)).collect(); + Self::new(slot, stakes) + } + pub fn new_from_bank(bank: &Bank) -> Self { + let bank_epoch = bank.get_epoch_and_slot_index(bank.slot()).0; + let stakes = staking_utils::vote_account_balances_at_epoch(bank, bank_epoch) + .expect("voting require a bank with stakes"); + Self::new(bank_epoch, stakes) + } +} + +impl Locktower { + pub fn new_from_forks(bank_forks: &BankForks) -> Self { + //TODO: which bank to start with? + let mut frozen_banks: Vec<_> = bank_forks.frozen_banks().values().cloned().collect(); + frozen_banks.sort_by_key(|b| (b.parents().len(), b.slot())); + if let Some(bank) = frozen_banks.last() { + Self::new_from_bank(bank) + } else { + Self::default() + } + } + + pub fn new_from_bank(bank: &Bank) -> Self { + let current_epoch = bank.get_epoch_and_slot_index(bank.slot()).0; + let mut lockouts = VoteState::default(); + if let Some(iter) = staking_utils::node_staked_accounts_at_epoch(bank, current_epoch) { + for (delegate_id, _, account) in iter { + if *delegate_id == bank.collector_id() { + let state = VoteState::deserialize(&account.data).expect("votes"); + if lockouts.votes.len() < state.votes.len() { + //TODO: which state to init with? + lockouts = state; + } + } + } + } + let epoch_stakes = EpochStakes::new_from_bank(bank); + Self { + epoch_stakes, + threshold_depth: VOTE_THRESHOLD_DEPTH, + threshold_size: VOTE_THRESHOLD_SIZE, + lockouts, + } + } + pub fn new(epoch_stakes: EpochStakes, threshold_depth: usize, threshold_size: f64) -> Self { + Self { + epoch_stakes, + threshold_depth, + threshold_size, + lockouts: VoteState::default(), + } + } + pub fn collect_vote_lockouts( + &self, + bank_slot: u64, + vote_accounts: F, + ancestors: &HashMap>, + ) -> HashMap + where + F: Iterator, + { + let mut stake_lockouts = HashMap::new(); + for (key, account) in vote_accounts { + let lamports: u64 = *self.epoch_stakes.stakes.get(&key).unwrap_or(&0); + if lamports == 0 { + continue; + } + let mut vote_state: VoteState = VoteState::deserialize(&account.data) + .expect("bank should always have valid VoteState data"); + let start_root = vote_state.root_slot; + vote_state.process_vote(Vote { slot: bank_slot }); + for vote in &vote_state.votes { + Self::update_ancestor_lockouts(&mut stake_lockouts, &vote, ancestors); + } + if start_root != vote_state.root_slot { + if let Some(root) = start_root { + let vote = Lockout { + confirmation_count: MAX_LOCKOUT_HISTORY as u32, + slot: root, + }; + Self::update_ancestor_lockouts(&mut stake_lockouts, &vote, ancestors); + } + } + if let Some(root) = vote_state.root_slot { + let vote = Lockout { + confirmation_count: MAX_LOCKOUT_HISTORY as u32, + slot: root, + }; + Self::update_ancestor_lockouts(&mut stake_lockouts, &vote, ancestors); + } + // each account hash a stake for all the forks in the active tree for this bank + Self::update_ancestor_stakes(&mut stake_lockouts, bank_slot, lamports, ancestors); + } + stake_lockouts + } + + pub fn update_epoch(&mut self, bank: &Bank) { + let bank_epoch = bank.get_epoch_and_slot_index(bank.slot()).0; + if bank_epoch != self.epoch_stakes.slot { + assert!( + bank_epoch > self.epoch_stakes.slot, + "epoch_stakes cannot move backwards" + ); + self.epoch_stakes = EpochStakes::new_from_bank(bank); + } + } + + pub fn record_vote(&mut self, slot: u64) { + self.lockouts.process_vote(Vote { slot }); + } + + pub fn calculate_weight(&self, stake_lockouts: &HashMap) -> u128 { + let mut sum = 0u128; + let root_slot = self.lockouts.root_slot.unwrap_or(0); + for (slot, stake_lockout) in stake_lockouts { + if self.lockouts.root_slot.is_some() && *slot <= root_slot { + continue; + } + sum += u128::from(stake_lockout.lockout) * u128::from(stake_lockout.stake) + } + sum + } + + pub fn has_voted(&self, slot: u64) -> bool { + for vote in &self.lockouts.votes { + if vote.slot == slot { + return true; + } + } + false + } + + pub fn is_locked_out(&self, slot: u64, decendants: &HashMap>) -> bool { + let mut lockouts = self.lockouts.clone(); + lockouts.process_vote(Vote { slot }); + for vote in &lockouts.votes { + if vote.slot == slot { + continue; + } + if !decendants[&vote.slot].contains(&slot) { + return false; + } + } + if let Some(root) = lockouts.root_slot { + decendants[&root].contains(&slot) + } else { + true + } + } + + pub fn check_vote_stake_threshold( + &self, + slot: u64, + stake_lockouts: &HashMap, + ) -> bool { + let mut lockouts = self.lockouts.clone(); + lockouts.process_vote(Vote { slot }); + let vote = lockouts.nth_recent_vote(self.threshold_depth); + if let Some(vote) = vote { + if let Some(fork_stake) = stake_lockouts.get(&vote.slot) { + (fork_stake.stake as f64 / self.epoch_stakes.total_staked as f64) + > self.threshold_size + } else { + false + } + } else { + true + } + } + + /// Update lockouts for all the ancestors + fn update_ancestor_lockouts( + stake_lockouts: &mut HashMap, + vote: &Lockout, + ancestors: &HashMap>, + ) { + let mut slot_with_ancestors = vec![vote.slot]; + slot_with_ancestors.extend(&ancestors[&vote.slot]); + for slot in slot_with_ancestors { + let entry = &mut stake_lockouts.entry(slot).or_default(); + entry.lockout += vote.lockout(); + } + } + + /// Update stake for all the ancestors. + /// Note, stake is the same for all the ancestor. + fn update_ancestor_stakes( + stake_lockouts: &mut HashMap, + slot: u64, + lamports: u64, + ancestors: &HashMap>, + ) { + let mut slot_with_ancestors = vec![slot]; + slot_with_ancestors.extend(&ancestors[&slot]); + for slot in slot_with_ancestors { + let entry = &mut stake_lockouts.entry(slot).or_default(); + entry.stake += lamports; + } + } +} + +#[cfg(test)] +mod test { + use super::*; + use solana_sdk::signature::{Keypair, KeypairUtil}; + + fn gen_accounts(stake_votes: &[(u64, &[u64])]) -> Vec<(Pubkey, Account)> { + let mut accounts = vec![]; + for (lamports, votes) in stake_votes { + let mut account = Account::default(); + account.data = vec![0; 1024]; + account.lamports = *lamports; + let mut vote_state = VoteState::default(); + for slot in *votes { + vote_state.process_vote(Vote { slot: *slot }); + } + vote_state + .serialize(&mut account.data) + .expect("serialize state"); + accounts.push((Keypair::new().pubkey(), account)); + } + accounts + } + + #[test] + fn test_collect_vote_lockouts_no_epoch_stakes() { + let accounts = gen_accounts(&[(1, &[0])]); + let epoch_stakes = EpochStakes::new_for_tests(2); + let locktower = Locktower::new(epoch_stakes, 0, 0.67); + let ancestors = vec![(1, vec![0].into_iter().collect()), (0, HashSet::new())] + .into_iter() + .collect(); + let staked_lockouts = locktower.collect_vote_lockouts(1, accounts.into_iter(), &ancestors); + assert!(staked_lockouts.is_empty()); + } + + #[test] + fn test_collect_vote_lockouts_sums() { + //two accounts voting for slot 0 with 1 token staked + let accounts = gen_accounts(&[(1, &[0]), (1, &[0])]); + let epoch_stakes = EpochStakes::new_from_stake_accounts(0, &accounts); + let locktower = Locktower::new(epoch_stakes, 0, 0.67); + let ancestors = vec![(1, vec![0].into_iter().collect()), (0, HashSet::new())] + .into_iter() + .collect(); + let staked_lockouts = locktower.collect_vote_lockouts(1, accounts.into_iter(), &ancestors); + assert_eq!(staked_lockouts[&0].stake, 2); + assert_eq!(staked_lockouts[&0].lockout, 2 + 2 + 4 + 4); + } + + #[test] + fn test_collect_vote_lockouts_root() { + let votes: Vec = (0..MAX_LOCKOUT_HISTORY as u64).into_iter().collect(); + //two accounts voting for slot 0 with 1 token staked + let accounts = gen_accounts(&[(1, &votes), (1, &votes)]); + let epoch_stakes = EpochStakes::new_from_stake_accounts(0, &accounts); + let mut locktower = Locktower::new(epoch_stakes, 0, 0.67); + let mut ancestors = HashMap::new(); + for i in 0..(MAX_LOCKOUT_HISTORY + 1) { + locktower.record_vote(i as u64); + ancestors.insert(i as u64, (0..i as u64).into_iter().collect()); + } + assert_eq!(locktower.lockouts.root_slot, Some(0)); + let staked_lockouts = locktower.collect_vote_lockouts( + MAX_LOCKOUT_HISTORY as u64, + accounts.into_iter(), + &ancestors, + ); + for i in 0..MAX_LOCKOUT_HISTORY { + assert_eq!(staked_lockouts[&(i as u64)].stake, 2); + } + // should be the sum of all the weights for root + assert!(staked_lockouts[&0].lockout > (2 * (1 << MAX_LOCKOUT_HISTORY))); + } + + #[test] + fn test_calculate_weight_skips_root() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + locktower.lockouts.root_slot = Some(1); + let stakes = vec![ + ( + 0, + StakeLockout { + stake: 1, + lockout: 8, + }, + ), + ( + 1, + StakeLockout { + stake: 1, + lockout: 8, + }, + ), + ] + .into_iter() + .collect(); + assert_eq!(locktower.calculate_weight(&stakes), 0u128); + } + + #[test] + fn test_calculate_weight() { + let locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let stakes = vec![( + 0, + StakeLockout { + stake: 1, + lockout: 8, + }, + )] + .into_iter() + .collect(); + assert_eq!(locktower.calculate_weight(&stakes), 8u128); + } + + #[test] + fn test_check_vote_threshold_without_votes() { + let locktower = Locktower::new(EpochStakes::new_for_tests(2), 1, 0.67); + let stakes = vec![( + 0, + StakeLockout { + stake: 1, + lockout: 8, + }, + )] + .into_iter() + .collect(); + assert!(locktower.check_vote_stake_threshold(0, &stakes)); + } + + #[test] + fn test_is_locked_out_empty() { + let locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let decendants = HashMap::new(); + assert!(locktower.is_locked_out(0, &decendants)); + } + + #[test] + fn test_is_locked_out_root_slot_child() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let decendants = vec![(0, vec![1].into_iter().collect())] + .into_iter() + .collect(); + locktower.lockouts.root_slot = Some(0); + assert!(locktower.is_locked_out(1, &decendants)); + } + + #[test] + fn test_is_locked_out_root_slot_sibling() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let decendants = vec![(0, vec![1].into_iter().collect())] + .into_iter() + .collect(); + locktower.lockouts.root_slot = Some(0); + assert!(!locktower.is_locked_out(2, &decendants)); + } + + #[test] + fn test_check_already_voted() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + locktower.record_vote(0); + assert!(locktower.has_voted(0)); + assert!(!locktower.has_voted(1)); + } + + #[test] + fn test_is_locked_out_double_vote() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let decendants = vec![(0, vec![1].into_iter().collect()), (1, HashSet::new())] + .into_iter() + .collect(); + locktower.record_vote(0); + locktower.record_vote(1); + assert!(!locktower.is_locked_out(0, &decendants)); + } + + #[test] + fn test_is_locked_out_child() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let decendants = vec![(0, vec![1].into_iter().collect())] + .into_iter() + .collect(); + locktower.record_vote(0); + assert!(locktower.is_locked_out(1, &decendants)); + } + + #[test] + fn test_is_locked_out_sibling() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let decendants = vec![ + (0, vec![1, 2].into_iter().collect()), + (1, HashSet::new()), + (2, HashSet::new()), + ] + .into_iter() + .collect(); + locktower.record_vote(0); + locktower.record_vote(1); + assert!(!locktower.is_locked_out(2, &decendants)); + } + + #[test] + fn test_is_locked_out_last_vote_expired() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); + let decendants = vec![(0, vec![1, 4].into_iter().collect()), (1, HashSet::new())] + .into_iter() + .collect(); + locktower.record_vote(0); + locktower.record_vote(1); + assert!(locktower.is_locked_out(4, &decendants)); + locktower.record_vote(4); + assert_eq!(locktower.lockouts.votes[0].slot, 0); + assert_eq!(locktower.lockouts.votes[0].confirmation_count, 2); + assert_eq!(locktower.lockouts.votes[1].slot, 4); + assert_eq!(locktower.lockouts.votes[1].confirmation_count, 1); + } + + #[test] + fn test_check_vote_threshold_below_threshold() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 1, 0.67); + let stakes = vec![( + 0, + StakeLockout { + stake: 1, + lockout: 8, + }, + )] + .into_iter() + .collect(); + locktower.record_vote(0); + assert!(!locktower.check_vote_stake_threshold(1, &stakes)); + } + #[test] + fn test_check_vote_threshold_above_threshold() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 1, 0.67); + let stakes = vec![( + 0, + StakeLockout { + stake: 2, + lockout: 8, + }, + )] + .into_iter() + .collect(); + locktower.record_vote(0); + assert!(locktower.check_vote_stake_threshold(1, &stakes)); + } + + #[test] + fn test_check_vote_threshold_above_threshold_after_pop() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 1, 0.67); + let stakes = vec![( + 0, + StakeLockout { + stake: 2, + lockout: 8, + }, + )] + .into_iter() + .collect(); + locktower.record_vote(0); + locktower.record_vote(1); + locktower.record_vote(2); + assert!(locktower.check_vote_stake_threshold(6, &stakes)); + } + + #[test] + fn test_check_vote_threshold_above_threshold_no_stake() { + let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 1, 0.67); + let stakes = HashMap::new(); + locktower.record_vote(0); + assert!(!locktower.check_vote_stake_threshold(1, &stakes)); + } + + #[test] + fn test_lockout_is_updated_for_entire_branch() { + let mut stake_lockouts = HashMap::new(); + let vote = Lockout { + slot: 2, + confirmation_count: 1, + }; + let set: HashSet = vec![0u64, 1u64].into_iter().collect(); + let mut ancestors = HashMap::new(); + ancestors.insert(2, set); + let set: HashSet = vec![0u64].into_iter().collect(); + ancestors.insert(1, set); + Locktower::update_ancestor_lockouts(&mut stake_lockouts, &vote, &ancestors); + assert_eq!(stake_lockouts[&0].lockout, 2); + assert_eq!(stake_lockouts[&1].lockout, 2); + assert_eq!(stake_lockouts[&2].lockout, 2); + } + + #[test] + fn test_lockout_is_updated_for_slot_or_lower() { + let mut stake_lockouts = HashMap::new(); + let set: HashSet = vec![0u64, 1u64].into_iter().collect(); + let mut ancestors = HashMap::new(); + ancestors.insert(2, set); + let set: HashSet = vec![0u64].into_iter().collect(); + ancestors.insert(1, set); + let vote = Lockout { + slot: 2, + confirmation_count: 1, + }; + Locktower::update_ancestor_lockouts(&mut stake_lockouts, &vote, &ancestors); + let vote = Lockout { + slot: 1, + confirmation_count: 2, + }; + Locktower::update_ancestor_lockouts(&mut stake_lockouts, &vote, &ancestors); + assert_eq!(stake_lockouts[&0].lockout, 2 + 4); + assert_eq!(stake_lockouts[&1].lockout, 2 + 4); + assert_eq!(stake_lockouts[&2].lockout, 2); + } + + #[test] + fn test_stake_is_updated_for_entire_branch() { + let mut stake_lockouts = HashMap::new(); + let mut account = Account::default(); + account.lamports = 1; + let set: HashSet = vec![0u64, 1u64].into_iter().collect(); + let ancestors: HashMap> = [(2u64, set)].into_iter().cloned().collect(); + Locktower::update_ancestor_stakes(&mut stake_lockouts, 2, account.lamports, &ancestors); + assert_eq!(stake_lockouts[&0].stake, 1); + assert_eq!(stake_lockouts[&1].stake, 1); + assert_eq!(stake_lockouts[&2].stake, 1); + } +} diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index a2ceb023402300..e2d3c8d254b84a 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -6,6 +6,7 @@ use crate::blocktree_processor; use crate::cluster_info::ClusterInfo; use crate::entry::{Entry, EntryReceiver, EntrySender, EntrySlice}; use crate::leader_schedule_utils; +use crate::locktower::Locktower; use crate::packet::BlobError; use crate::poh_recorder::PohRecorder; use crate::result; @@ -17,7 +18,7 @@ use solana_runtime::bank::Bank; use solana_sdk::hash::Hash; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::KeypairUtil; -use solana_sdk::timing::duration_as_ms; +use solana_sdk::timing::{self, duration_as_ms}; use solana_vote_api::vote_transaction::VoteTransaction; use std::collections::HashMap; use std::sync::atomic::{AtomicBool, Ordering}; @@ -78,6 +79,7 @@ impl ReplayStage { let my_id = *my_id; let vote_account = *vote_account; let mut ticks_per_slot = 0; + let mut locktower = Locktower::new_from_forks(&bank_forks.read().unwrap()); // Start the replay stage loop let t_replay = Builder::new() @@ -94,7 +96,6 @@ impl ReplayStage { Self::generate_new_bank_forks(&blocktree, &mut bank_forks.write().unwrap()); let active_banks = bank_forks.read().unwrap().active_banks(); trace!("active banks {:?}", active_banks); - let mut votable: Vec> = vec![]; let mut is_tpu_bank_active = poh_recorder.lock().unwrap().bank().is_some(); for bank_slot in &active_banks { let bank = bank_forks.read().unwrap().get(*bank_slot).unwrap().clone(); @@ -113,7 +114,6 @@ impl ReplayStage { &my_id, bank, &mut progress, - &mut votable, &slot_full_sender, ); } @@ -125,11 +125,40 @@ impl ReplayStage { ticks_per_slot = bank.ticks_per_slot(); } - // TODO: fork selection - // vote on the latest one for now - votable.sort_by(|b1, b2| b1.slot().cmp(&b2.slot())); - - if let Some(bank) = votable.last() { + let locktower_start = Instant::now(); + // Locktower voting + let decendants = bank_forks.read().unwrap().decendants(); + let ancestors = bank_forks.read().unwrap().ancestors(); + let frozen_banks = bank_forks.read().unwrap().frozen_banks(); + let mut votable: Vec<(u128, Arc)> = frozen_banks + .values() + .filter(|b| b.is_votable()) + .filter(|b| !locktower.has_voted(b.slot())) + .filter(|b| !locktower.is_locked_out(b.slot(), &decendants)) + .map(|bank| { + ( + bank, + locktower.collect_vote_lockouts( + bank.slot(), + bank.vote_accounts(), + &ancestors, + ), + ) + }) + .filter(|(b, stake_lockouts)| { + locktower.check_vote_stake_threshold(b.slot(), &stake_lockouts) + }) + .map(|(b, stake_lockouts)| { + (locktower.calculate_weight(&stake_lockouts), b.clone()) + }) + .collect(); + + votable.sort_by_key(|b| b.0); + let ms = timing::duration_as_ms(&locktower_start.elapsed()); + info!("@{:?} locktower duration: {:?}", timing::timestamp(), ms,); + inc_new_counter_info!("replay_stage-locktower_duration", ms as usize); + + if let Some((_, bank)) = votable.last() { subscriptions.notify_subscribers(&bank); if let Some(ref voting_keypair) = voting_keypair { @@ -141,6 +170,8 @@ impl ReplayStage { bank.last_blockhash(), 0, ); + locktower.record_vote(bank.slot()); + locktower.update_epoch(&bank); cluster_info.write().unwrap().push_vote(vote); } let next_leader_slot = @@ -350,7 +381,6 @@ impl ReplayStage { my_id: &Pubkey, bank: Arc, progress: &mut HashMap, - votable: &mut Vec>, slot_full_sender: &Sender<(u64, Pubkey)>, ) { bank.freeze(); @@ -359,9 +389,6 @@ impl ReplayStage { if let Err(e) = slot_full_sender.send((bank.slot(), bank.collector_id())) { info!("{} slot_full alert failed: {:?}", my_id, e); } - if bank.is_votable() { - votable.push(bank); - } } fn generate_new_bank_forks(blocktree: &Blocktree, forks: &mut BankForks) { @@ -488,42 +515,6 @@ mod test { let _ignored = remove_dir_all(&my_ledger_path); } - #[test] - fn test_no_vote_empty_transmission() { - let genesis_block = GenesisBlock::new(10_000).0; - let bank = Arc::new(Bank::new(&genesis_block)); - let mut blockhash = bank.last_blockhash(); - let mut entries = Vec::new(); - for _ in 0..genesis_block.ticks_per_slot { - let entry = next_entry_mut(&mut blockhash, 1, vec![]); //just ticks - entries.push(entry); - } - let (sender, _receiver) = channel(); - - let mut progress = HashMap::new(); - let (forward_entry_sender, _forward_entry_receiver) = channel(); - ReplayStage::replay_entries_into_bank( - &bank, - entries.clone(), - &mut progress, - &forward_entry_sender, - 0, - ) - .unwrap(); - - let mut votable = vec![]; - ReplayStage::process_completed_bank( - &Pubkey::default(), - bank, - &mut progress, - &mut votable, - &sender, - ); - assert!(progress.is_empty()); - // Don't vote on slot that only contained ticks - assert!(votable.is_empty()); - } - #[test] fn test_replay_stage_poh_ok_entry_receiver() { let (forward_entry_sender, forward_entry_receiver) = channel(); diff --git a/core/src/staking_utils.rs b/core/src/staking_utils.rs index e8b01586e4ef77..8124c14340fa62 100644 --- a/core/src/staking_utils.rs +++ b/core/src/staking_utils.rs @@ -58,7 +58,7 @@ fn node_staked_accounts(bank: &Bank) -> impl Iterator Option> { diff --git a/programs/vote_api/src/vote_state.rs b/programs/vote_api/src/vote_state.rs index ad99a880cd5a2a..fd13356b893b6e 100644 --- a/programs/vote_api/src/vote_state.rs +++ b/programs/vote_api/src/vote_state.rs @@ -39,6 +39,9 @@ impl Lockout { pub fn expiration_slot(&self) -> u64 { self.slot + self.lockout() } + pub fn is_expired(&self, slot: u64) -> bool { + self.expiration_slot() < slot + } } #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone)] @@ -110,6 +113,15 @@ impl VoteState { self.double_lockouts(); } + pub fn nth_recent_vote(&self, position: usize) -> Option<&Lockout> { + if position < self.votes.len() { + let pos = self.votes.len() - 1 - position; + self.votes.get(pos) + } else { + None + } + } + /// Number of "credits" owed to this account from the mining pool. Submit this /// VoteState to the Rewards program to trade credits for lamports. pub fn credits(&self) -> u64 { @@ -123,11 +135,7 @@ impl VoteState { fn pop_expired_votes(&mut self, slot: u64) { loop { - if self - .votes - .back() - .map_or(false, |v| v.expiration_slot() < slot) - { + if self.votes.back().map_or(false, |v| v.is_expired(slot)) { self.votes.pop_back(); } else { break; @@ -459,6 +467,34 @@ mod tests { assert_eq!(vote_state.credits(), 0); } + #[test] + fn test_duplicate_vote() { + let voter_id = Keypair::new().pubkey(); + let mut vote_state = VoteState::new(&voter_id); + vote_state.process_vote(Vote::new(0)); + vote_state.process_vote(Vote::new(1)); + vote_state.process_vote(Vote::new(0)); + assert_eq!(vote_state.nth_recent_vote(0).unwrap().slot, 1); + assert_eq!(vote_state.nth_recent_vote(1).unwrap().slot, 0); + assert!(vote_state.nth_recent_vote(2).is_none()); + } + + #[test] + fn test_nth_recent_vote() { + let voter_id = Keypair::new().pubkey(); + let mut vote_state = VoteState::new(&voter_id); + for i in 0..MAX_LOCKOUT_HISTORY { + vote_state.process_vote(Vote::new(i as u64)); + } + for i in 0..(MAX_LOCKOUT_HISTORY - 1) { + assert_eq!( + vote_state.nth_recent_vote(i).unwrap().slot as usize, + MAX_LOCKOUT_HISTORY - i - 1, + ); + } + assert!(vote_state.nth_recent_vote(MAX_LOCKOUT_HISTORY).is_none()); + } + fn check_lockouts(vote_state: &VoteState) { for (i, vote) in vote_state.votes.iter().enumerate() { let num_lockouts = vote_state.votes.len() - i; diff --git a/tests/local_cluster.rs b/tests/local_cluster.rs index 6fcd1400b5d1e6..8efe60bf085ead 100644 --- a/tests/local_cluster.rs +++ b/tests/local_cluster.rs @@ -21,6 +21,7 @@ fn test_spend_and_verify_all_nodes_1() { } #[test] +#[ignore] //TODO: confirmations are not useful: #3346 fn test_spend_and_verify_all_nodes_2() { solana_logger::setup(); let num_nodes = 2; @@ -33,6 +34,7 @@ fn test_spend_and_verify_all_nodes_2() { } #[test] +#[ignore] //TODO: confirmations are not useful: #3346 fn test_spend_and_verify_all_nodes_3() { solana_logger::setup(); let num_nodes = 3; From e79dfdf3068decf7c6c8a05b936128554da9ce3b Mon Sep 17 00:00:00 2001 From: Stephen Akridge Date: Mon, 18 Mar 2019 15:20:04 -0700 Subject: [PATCH 2/8] Decendent is not a word --- core/src/bank_forks.rs | 20 ++++++++++---------- core/src/locktower.rs | 34 +++++++++++++++++----------------- core/src/replay_stage.rs | 4 ++-- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/core/src/bank_forks.rs b/core/src/bank_forks.rs index fbbbd5d9cdfea9..507af0bffa2748 100644 --- a/core/src/bank_forks.rs +++ b/core/src/bank_forks.rs @@ -45,8 +45,8 @@ impl BankForks { } /// Create a map of bank slot id to the set of all of its descendants - pub fn decendants(&self) -> HashMap> { - let mut decendants = HashMap::new(); + pub fn descendants(&self) -> HashMap> { + let mut descendants = HashMap::new(); let mut pending: Vec> = self.banks.values().cloned().collect(); let mut done = HashSet::new(); assert!(!pending.is_empty()); @@ -56,16 +56,16 @@ impl BankForks { continue; } done.insert(bank.slot()); - let _ = decendants.entry(bank.slot()).or_insert(HashSet::new()); + let _ = descendants.entry(bank.slot()).or_insert(HashSet::new()); for parent in bank.parents() { - decendants + descendants .entry(parent.slot()) .or_insert(HashSet::new()) .insert(bank.slot()); } pending.extend(bank.parents().into_iter()); } - decendants + descendants } pub fn frozen_banks(&self) -> HashMap> { @@ -148,7 +148,7 @@ mod tests { } #[test] - fn test_bank_forks_decendants() { + fn test_bank_forks_descendants() { let (genesis_block, _) = GenesisBlock::new(10_000); let bank = Bank::new(&genesis_block); let mut bank_forks = BankForks::new(0, bank); @@ -157,11 +157,11 @@ mod tests { bank_forks.insert(1, bank); let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 2); bank_forks.insert(2, bank); - let decendants = bank_forks.decendants(); - let children: Vec = decendants[&0].iter().cloned().collect(); + let descendants = bank_forks.descendants(); + let children: Vec = descendants[&0].iter().cloned().collect(); assert_eq!(children, vec![1, 2]); - assert!(decendants[&1].is_empty()); - assert!(decendants[&2].is_empty()); + assert!(descendants[&1].is_empty()); + assert!(descendants[&2].is_empty()); } #[test] diff --git a/core/src/locktower.rs b/core/src/locktower.rs index 39634ffee563b2..c5c4be8ecb28f8 100644 --- a/core/src/locktower.rs +++ b/core/src/locktower.rs @@ -177,19 +177,19 @@ impl Locktower { false } - pub fn is_locked_out(&self, slot: u64, decendants: &HashMap>) -> bool { + pub fn is_locked_out(&self, slot: u64, descendants: &HashMap>) -> bool { let mut lockouts = self.lockouts.clone(); lockouts.process_vote(Vote { slot }); for vote in &lockouts.votes { if vote.slot == slot { continue; } - if !decendants[&vote.slot].contains(&slot) { + if !descendants[&vote.slot].contains(&slot) { return false; } } if let Some(root) = lockouts.root_slot { - decendants[&root].contains(&slot) + descendants[&root].contains(&slot) } else { true } @@ -378,28 +378,28 @@ mod test { #[test] fn test_is_locked_out_empty() { let locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); - let decendants = HashMap::new(); - assert!(locktower.is_locked_out(0, &decendants)); + let descendants = HashMap::new(); + assert!(locktower.is_locked_out(0, &descendants)); } #[test] fn test_is_locked_out_root_slot_child() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); - let decendants = vec![(0, vec![1].into_iter().collect())] + let descendants = vec![(0, vec![1].into_iter().collect())] .into_iter() .collect(); locktower.lockouts.root_slot = Some(0); - assert!(locktower.is_locked_out(1, &decendants)); + assert!(locktower.is_locked_out(1, &descendants)); } #[test] fn test_is_locked_out_root_slot_sibling() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); - let decendants = vec![(0, vec![1].into_iter().collect())] + let descendants = vec![(0, vec![1].into_iter().collect())] .into_iter() .collect(); locktower.lockouts.root_slot = Some(0); - assert!(!locktower.is_locked_out(2, &decendants)); + assert!(!locktower.is_locked_out(2, &descendants)); } #[test] @@ -413,28 +413,28 @@ mod test { #[test] fn test_is_locked_out_double_vote() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); - let decendants = vec![(0, vec![1].into_iter().collect()), (1, HashSet::new())] + let descendants = vec![(0, vec![1].into_iter().collect()), (1, HashSet::new())] .into_iter() .collect(); locktower.record_vote(0); locktower.record_vote(1); - assert!(!locktower.is_locked_out(0, &decendants)); + assert!(!locktower.is_locked_out(0, &descendants)); } #[test] fn test_is_locked_out_child() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); - let decendants = vec![(0, vec![1].into_iter().collect())] + let descendants = vec![(0, vec![1].into_iter().collect())] .into_iter() .collect(); locktower.record_vote(0); - assert!(locktower.is_locked_out(1, &decendants)); + assert!(locktower.is_locked_out(1, &descendants)); } #[test] fn test_is_locked_out_sibling() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); - let decendants = vec![ + let descendants = vec![ (0, vec![1, 2].into_iter().collect()), (1, HashSet::new()), (2, HashSet::new()), @@ -443,18 +443,18 @@ mod test { .collect(); locktower.record_vote(0); locktower.record_vote(1); - assert!(!locktower.is_locked_out(2, &decendants)); + assert!(!locktower.is_locked_out(2, &descendants)); } #[test] fn test_is_locked_out_last_vote_expired() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); - let decendants = vec![(0, vec![1, 4].into_iter().collect()), (1, HashSet::new())] + let descendants = vec![(0, vec![1, 4].into_iter().collect()), (1, HashSet::new())] .into_iter() .collect(); locktower.record_vote(0); locktower.record_vote(1); - assert!(locktower.is_locked_out(4, &decendants)); + assert!(locktower.is_locked_out(4, &descendants)); locktower.record_vote(4); assert_eq!(locktower.lockouts.votes[0].slot, 0); assert_eq!(locktower.lockouts.votes[0].confirmation_count, 2); diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index e2d3c8d254b84a..a74b02113a2bae 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -127,14 +127,14 @@ impl ReplayStage { let locktower_start = Instant::now(); // Locktower voting - let decendants = bank_forks.read().unwrap().decendants(); + let descendants = bank_forks.read().unwrap().descendants(); let ancestors = bank_forks.read().unwrap().ancestors(); let frozen_banks = bank_forks.read().unwrap().frozen_banks(); let mut votable: Vec<(u128, Arc)> = frozen_banks .values() .filter(|b| b.is_votable()) .filter(|b| !locktower.has_voted(b.slot())) - .filter(|b| !locktower.is_locked_out(b.slot(), &decendants)) + .filter(|b| !locktower.is_locked_out(b.slot(), &descendants)) .map(|bank| { ( bank, From 6fcc9d82d9709452602c937b3a8d3cd03094c798 Mon Sep 17 00:00:00 2001 From: Carl Date: Mon, 18 Mar 2019 16:04:36 -0700 Subject: [PATCH 3/8] Modify bank_forks to support squashing/filtering new root and also don't remove parents from bank_forks when inserting, otherwise we lose potential fork points when querying blocktree for child slots --- core/src/bank_forks.rs | 65 +++++++++++++++++----------------------- core/src/locktower.rs | 8 ++++- core/src/replay_stage.rs | 50 ++++++++++++++++++++++++++----- runtime/src/bank.rs | 42 ++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 45 deletions(-) diff --git a/core/src/bank_forks.rs b/core/src/bank_forks.rs index 507af0bffa2748..6872e37e9d9208 100644 --- a/core/src/bank_forks.rs +++ b/core/src/bank_forks.rs @@ -31,15 +31,9 @@ impl BankForks { /// Create a map of bank slot id to the set of ancestors for the bank slot. pub fn ancestors(&self) -> HashMap> { let mut ancestors = HashMap::new(); - let mut pending: Vec> = self.banks.values().cloned().collect(); - while !pending.is_empty() { - let bank = pending.pop().unwrap(); - if ancestors.get(&bank.slot()).is_some() { - continue; - } + for bank in self.banks.values() { let set = bank.parents().into_iter().map(|b| b.slot()).collect(); ancestors.insert(bank.slot(), set); - pending.extend(bank.parents().into_iter()); } ancestors } @@ -47,15 +41,7 @@ impl BankForks { /// Create a map of bank slot id to the set of all of its descendants pub fn descendants(&self) -> HashMap> { let mut descendants = HashMap::new(); - let mut pending: Vec> = self.banks.values().cloned().collect(); - let mut done = HashSet::new(); - assert!(!pending.is_empty()); - while !pending.is_empty() { - let bank = pending.pop().unwrap(); - if done.contains(&bank.slot()) { - continue; - } - done.insert(bank.slot()); + for bank in self.banks.values() { let _ = descendants.entry(bank.slot()).or_insert(HashSet::new()); for parent in bank.parents() { descendants @@ -63,22 +49,18 @@ impl BankForks { .or_insert(HashSet::new()) .insert(bank.slot()); } - pending.extend(bank.parents().into_iter()); } descendants } pub fn frozen_banks(&self) -> HashMap> { - let mut frozen_banks: Vec> = vec![]; - frozen_banks.extend(self.banks.values().filter(|v| v.is_frozen()).cloned()); - frozen_banks.extend( - self.banks - .iter() - .flat_map(|(_, v)| v.parents()) - .filter(|v| v.is_frozen()), - ); - frozen_banks.into_iter().map(|b| (b.slot(), b)).collect() + self.banks + .iter() + .filter(|(_, b)| b.is_frozen()) + .map(|(k, b)| (*k, b.clone())) + .collect() } + pub fn active_banks(&self) -> Vec { self.banks .iter() @@ -104,28 +86,37 @@ impl BankForks { // TODO: use the bank's own ID instead of receiving a parameter? pub fn insert(&mut self, bank_slot: u64, bank: Bank) { - let mut bank = Arc::new(bank); + let bank = Arc::new(bank); assert_eq!(bank_slot, bank.slot()); let prev = self.banks.insert(bank_slot, bank.clone()); assert!(prev.is_none()); self.working_bank = bank.clone(); - - // TODO: this really only needs to look at the first - // parent if we're always calling insert() - // when we construct a child bank - while let Some(parent) = bank.parent() { - if let Some(prev) = self.banks.remove(&parent.slot()) { - assert!(Arc::ptr_eq(&prev, &parent)); - } - bank = parent; - } } // TODO: really want to kill this... pub fn working_bank(&self) -> Arc { self.working_bank.clone() } + + pub fn set_root(&mut self, root: u64) { + let root_bank = self + .banks + .get(&root) + .expect("root bank didn't exist in bank_forks"); + root_bank.squash(); + self.prune_non_root(root); + } + + fn prune_non_root(&mut self, root: u64) { + self.banks.retain(|slot, bank| { + if *slot < root { + false + } else { + bank.is_descendant_of(root) + } + }) + } } #[cfg(test)] diff --git a/core/src/locktower.rs b/core/src/locktower.rs index c5c4be8ecb28f8..45bff8eb276a71 100644 --- a/core/src/locktower.rs +++ b/core/src/locktower.rs @@ -152,8 +152,14 @@ impl Locktower { } } - pub fn record_vote(&mut self, slot: u64) { + pub fn record_vote(&mut self, slot: u64) -> Option { + let root_slot = self.lockouts.root_slot; self.lockouts.process_vote(Vote { slot }); + if root_slot != self.lockouts.root_slot { + Some(self.lockouts.root_slot.unwrap()) + } else { + None + } } pub fn calculate_weight(&self, stake_lockouts: &HashMap) -> u128 { diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index a74b02113a2bae..737f8a86383a41 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -170,7 +170,9 @@ impl ReplayStage { bank.last_blockhash(), 0, ); - locktower.record_vote(bank.slot()); + if let Some(new_root) = locktower.record_vote(bank.slot()) { + bank_forks.write().unwrap().set_root(new_root); + } locktower.update_epoch(&bank); cluster_info.write().unwrap().push_vote(vote); } @@ -399,6 +401,7 @@ impl ReplayStage { let next_slots = blocktree .get_slots_since(&frozen_bank_slots) .expect("Db error"); + // Filter out what we've already seen trace!("generate new forks {:?}", next_slots); for (parent_id, children) in next_slots { let parent_bank = frozen_banks @@ -406,12 +409,8 @@ impl ReplayStage { .expect("missing parent in bank forks") .clone(); for child_id in children { - if frozen_banks.get(&child_id).is_some() { - trace!("child already frozen {}", child_id); - continue; - } if forks.get(child_id).is_some() { - trace!("child already active {}", child_id); + trace!("child already active or frozen {}", child_id); continue; } let leader = leader_schedule_utils::slot_leader_at(child_id, &parent_bank).unwrap(); @@ -437,11 +436,12 @@ impl Service for ReplayStage { mod test { use super::*; use crate::banking_stage::create_test_recorder; - use crate::blocktree::create_new_tmp_ledger; + use crate::blocktree::{create_new_tmp_ledger, get_tmp_ledger_path}; use crate::cluster_info::{ClusterInfo, Node}; use crate::entry::create_ticks; use crate::entry::{next_entry_mut, Entry}; use crate::fullnode::new_banks_from_blocktree; + use crate::packet::Blob; use crate::replay_stage::ReplayStage; use crate::result::Error; use solana_sdk::genesis_block::GenesisBlock; @@ -574,4 +574,40 @@ mod test { } assert!(forward_entry_receiver.try_recv().is_err()); } + + #[test] + fn test_child_slots_of_same_parent() { + let ledger_path = get_tmp_ledger_path!(); + { + let blocktree = Arc::new( + Blocktree::open(&ledger_path).expect("Expected to be able to open database ledger"), + ); + + let genesis_block = GenesisBlock::new(10_000).0; + let bank0 = Bank::new(&genesis_block); + let mut bank_forks = BankForks::new(0, bank0); + bank_forks.working_bank().freeze(); + + // Insert blob for slot 1, generate new forks, check result + let mut blob_slot_1 = Blob::default(); + blob_slot_1.set_slot(1); + blob_slot_1.set_parent(0); + blocktree.insert_data_blobs(&vec![blob_slot_1]).unwrap(); + assert!(bank_forks.get(1).is_none()); + ReplayStage::generate_new_bank_forks(&blocktree, &mut bank_forks); + assert!(bank_forks.get(1).is_some()); + + // Inset blob for slot 3, generate new forks, check result + let mut blob_slot_2 = Blob::default(); + blob_slot_2.set_slot(2); + blob_slot_2.set_parent(0); + blocktree.insert_data_blobs(&vec![blob_slot_2]).unwrap(); + assert!(bank_forks.get(2).is_none()); + ReplayStage::generate_new_bank_forks(&blocktree, &mut bank_forks); + assert!(bank_forks.get(1).is_some()); + assert!(bank_forks.get(2).is_some()); + } + + let _ignored = remove_dir_all(&ledger_path); + } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 9d766eb78f2948..20e70a6f8f7ae5 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -894,6 +894,24 @@ impl Bank { let max_tick_height = (self.slot + 1) * self.ticks_per_slot - 1; self.is_delta.load(Ordering::Relaxed) && self.tick_height() == max_tick_height } + + pub fn is_descendant_of(&self, parent: u64) -> bool { + if self.slot() == parent { + return true; + } + let mut next_parent = self.parent(); + + while let Some(p) = next_parent { + if p.slot() == parent { + return true; + } else if p.slot() < parent { + return false; + } + next_parent = p.parent(); + } + + false + } } #[cfg(test)] @@ -1709,4 +1727,28 @@ mod tests { assert_eq!(bank.is_votable(), true); } + + #[test] + fn test_is_descendant_of() { + let (genesis_block, _) = GenesisBlock::new(1); + let parent = Arc::new(Bank::new(&genesis_block)); + // Bank 1 + let bank = Arc::new(new_from_parent(&parent)); + // Bank 2 + let bank2 = new_from_parent(&bank); + // Bank 5 + let bank5 = Bank::new_from_parent(&bank, &Pubkey::default(), 5); + + // Parents of bank 2: 0 -> 1 -> 2 + assert!(bank2.is_descendant_of(0)); + assert!(bank2.is_descendant_of(1)); + assert!(bank2.is_descendant_of(2)); + assert!(!bank2.is_descendant_of(3)); + + // Parents of bank 3: 0 -> 1 -> 5 + assert!(bank5.is_descendant_of(0)); + assert!(bank5.is_descendant_of(1)); + assert!(!bank5.is_descendant_of(2)); + assert!(!bank5.is_descendant_of(4)); + } } From 99e03429cd064dd6efbd72f46abfa2afba77b8f4 Mon Sep 17 00:00:00 2001 From: Carl Date: Mon, 18 Mar 2019 20:23:34 -0700 Subject: [PATCH 4/8] PR comments --- core/src/bank_forks.rs | 29 +++++++++++------------------ core/src/replay_stage.rs | 9 +++------ runtime/src/bank.rs | 24 ++++++++++++------------ 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/core/src/bank_forks.rs b/core/src/bank_forks.rs index 6872e37e9d9208..0174b0e2d04d87 100644 --- a/core/src/bank_forks.rs +++ b/core/src/bank_forks.rs @@ -84,11 +84,9 @@ impl BankForks { } } - // TODO: use the bank's own ID instead of receiving a parameter? - pub fn insert(&mut self, bank_slot: u64, bank: Bank) { + pub fn insert(&mut self, bank: Bank) { let bank = Arc::new(bank); - assert_eq!(bank_slot, bank.slot()); - let prev = self.banks.insert(bank_slot, bank.clone()); + let prev = self.banks.insert(bank.slot(), bank.clone()); assert!(prev.is_none()); self.working_bank = bank.clone(); @@ -109,13 +107,8 @@ impl BankForks { } fn prune_non_root(&mut self, root: u64) { - self.banks.retain(|slot, bank| { - if *slot < root { - false - } else { - bank.is_descendant_of(root) - } - }) + self.banks + .retain(|slot, bank| *slot >= root || bank.is_in_subtree_of(root)) } } @@ -133,7 +126,7 @@ mod tests { let mut bank_forks = BankForks::new(0, bank); let child_bank = Bank::new_from_parent(&bank_forks[0u64], &Pubkey::default(), 1); child_bank.register_tick(&Hash::default()); - bank_forks.insert(1, child_bank); + bank_forks.insert(child_bank); assert_eq!(bank_forks[1u64].tick_height(), 1); assert_eq!(bank_forks.working_bank().tick_height(), 1); } @@ -145,9 +138,9 @@ mod tests { let mut bank_forks = BankForks::new(0, bank); let bank0 = bank_forks[0].clone(); let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 1); - bank_forks.insert(1, bank); + bank_forks.insert(bank); let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 2); - bank_forks.insert(2, bank); + bank_forks.insert(bank); let descendants = bank_forks.descendants(); let children: Vec = descendants[&0].iter().cloned().collect(); assert_eq!(children, vec![1, 2]); @@ -162,9 +155,9 @@ mod tests { let mut bank_forks = BankForks::new(0, bank); let bank0 = bank_forks[0].clone(); let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 1); - bank_forks.insert(1, bank); + bank_forks.insert(bank); let bank = Bank::new_from_parent(&bank0, &Pubkey::default(), 2); - bank_forks.insert(2, bank); + bank_forks.insert(bank); let ancestors = bank_forks.ancestors(); assert!(ancestors[&0].is_empty()); let parents: Vec = ancestors[&1].iter().cloned().collect(); @@ -179,7 +172,7 @@ mod tests { let bank = Bank::new(&genesis_block); let mut bank_forks = BankForks::new(0, bank); let child_bank = Bank::new_from_parent(&bank_forks[0u64], &Pubkey::default(), 1); - bank_forks.insert(1, child_bank); + bank_forks.insert(child_bank); assert!(bank_forks.frozen_banks().get(&0).is_some()); assert!(bank_forks.frozen_banks().get(&1).is_none()); } @@ -190,7 +183,7 @@ mod tests { let bank = Bank::new(&genesis_block); let mut bank_forks = BankForks::new(0, bank); let child_bank = Bank::new_from_parent(&bank_forks[0u64], &Pubkey::default(), 1); - bank_forks.insert(1, child_bank); + bank_forks.insert(child_bank); assert_eq!(bank_forks.active_banks(), vec![1]); } diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 737f8a86383a41..4d204e6fcd99f4 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -283,7 +283,7 @@ impl ReplayStage { ) .to_owned(),); let tpu_bank = Bank::new_from_parent(parent, my_id, poh_slot); - bank_forks.write().unwrap().insert(poh_slot, tpu_bank); + bank_forks.write().unwrap().insert(tpu_bank); if let Some(tpu_bank) = bank_forks.read().unwrap().get(poh_slot).cloned() { assert_eq!( bank_forks.read().unwrap().working_bank().slot(), @@ -415,10 +415,7 @@ impl ReplayStage { } let leader = leader_schedule_utils::slot_leader_at(child_id, &parent_bank).unwrap(); info!("new fork:{} parent:{}", child_id, parent_id); - forks.insert( - child_id, - Bank::new_from_parent(&parent_bank, &leader, child_id), - ); + forks.insert(Bank::new_from_parent(&parent_bank, &leader, child_id)); } } } @@ -597,7 +594,7 @@ mod test { ReplayStage::generate_new_bank_forks(&blocktree, &mut bank_forks); assert!(bank_forks.get(1).is_some()); - // Inset blob for slot 3, generate new forks, check result + // Insert blob for slot 3, generate new forks, check result let mut blob_slot_2 = Blob::default(); blob_slot_2.set_slot(2); blob_slot_2.set_parent(0); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 20e70a6f8f7ae5..22fd70101850f1 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -895,7 +895,7 @@ impl Bank { self.is_delta.load(Ordering::Relaxed) && self.tick_height() == max_tick_height } - pub fn is_descendant_of(&self, parent: u64) -> bool { + pub fn is_in_subtree_of(&self, parent: u64) -> bool { if self.slot() == parent { return true; } @@ -1729,7 +1729,7 @@ mod tests { } #[test] - fn test_is_descendant_of() { + fn test_is_in_subtree_of() { let (genesis_block, _) = GenesisBlock::new(1); let parent = Arc::new(Bank::new(&genesis_block)); // Bank 1 @@ -1740,15 +1740,15 @@ mod tests { let bank5 = Bank::new_from_parent(&bank, &Pubkey::default(), 5); // Parents of bank 2: 0 -> 1 -> 2 - assert!(bank2.is_descendant_of(0)); - assert!(bank2.is_descendant_of(1)); - assert!(bank2.is_descendant_of(2)); - assert!(!bank2.is_descendant_of(3)); - - // Parents of bank 3: 0 -> 1 -> 5 - assert!(bank5.is_descendant_of(0)); - assert!(bank5.is_descendant_of(1)); - assert!(!bank5.is_descendant_of(2)); - assert!(!bank5.is_descendant_of(4)); + assert!(bank2.is_in_subtree_of(0)); + assert!(bank2.is_in_subtree_of(1)); + assert!(bank2.is_in_subtree_of(2)); + assert!(!bank2.is_in_subtree_of(3)); + + // Parents of bank 5: 0 -> 1 -> 5 + assert!(bank5.is_in_subtree_of(0)); + assert!(bank5.is_in_subtree_of(1)); + assert!(!bank5.is_in_subtree_of(2)); + assert!(!bank5.is_in_subtree_of(4)); } } From 8a81649591771d5952082a9960f11804e494d718 Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 19 Mar 2019 16:00:52 -0700 Subject: [PATCH 5/8] fix is_locked_out logic --- core/src/locktower.rs | 28 ++++++++++++++-------------- core/src/replay_stage.rs | 37 ++++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/core/src/locktower.rs b/core/src/locktower.rs index 45bff8eb276a71..31df4474cea7c6 100644 --- a/core/src/locktower.rs +++ b/core/src/locktower.rs @@ -7,8 +7,8 @@ use solana_sdk::pubkey::Pubkey; use solana_vote_api::vote_instruction::Vote; use solana_vote_api::vote_state::{Lockout, VoteState, MAX_LOCKOUT_HISTORY}; -const VOTE_THRESHOLD_DEPTH: usize = 8; -const VOTE_THRESHOLD_SIZE: f64 = 2f64 / 3f64; +pub const VOTE_THRESHOLD_DEPTH: usize = 8; +pub const VOTE_THRESHOLD_SIZE: f64 = 2f64 / 3f64; #[derive(Default)] pub struct EpochStakes { @@ -191,13 +191,13 @@ impl Locktower { continue; } if !descendants[&vote.slot].contains(&slot) { - return false; + return true; } } if let Some(root) = lockouts.root_slot { - descendants[&root].contains(&slot) + !descendants[&root].contains(&slot) } else { - true + false } } @@ -385,27 +385,27 @@ mod test { fn test_is_locked_out_empty() { let locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); let descendants = HashMap::new(); - assert!(locktower.is_locked_out(0, &descendants)); + assert!(!locktower.is_locked_out(0, &descendants)); } #[test] - fn test_is_locked_out_root_slot_child() { + fn test_is_locked_out_root_slot_child_pass() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); let descendants = vec![(0, vec![1].into_iter().collect())] .into_iter() .collect(); locktower.lockouts.root_slot = Some(0); - assert!(locktower.is_locked_out(1, &descendants)); + assert!(!locktower.is_locked_out(1, &descendants)); } #[test] - fn test_is_locked_out_root_slot_sibling() { + fn test_is_locked_out_root_slot_sibling_fail() { let mut locktower = Locktower::new(EpochStakes::new_for_tests(2), 0, 0.67); let descendants = vec![(0, vec![1].into_iter().collect())] .into_iter() .collect(); locktower.lockouts.root_slot = Some(0); - assert!(!locktower.is_locked_out(2, &descendants)); + assert!(locktower.is_locked_out(2, &descendants)); } #[test] @@ -424,7 +424,7 @@ mod test { .collect(); locktower.record_vote(0); locktower.record_vote(1); - assert!(!locktower.is_locked_out(0, &descendants)); + assert!(locktower.is_locked_out(0, &descendants)); } #[test] @@ -434,7 +434,7 @@ mod test { .into_iter() .collect(); locktower.record_vote(0); - assert!(locktower.is_locked_out(1, &descendants)); + assert!(!locktower.is_locked_out(1, &descendants)); } #[test] @@ -449,7 +449,7 @@ mod test { .collect(); locktower.record_vote(0); locktower.record_vote(1); - assert!(!locktower.is_locked_out(2, &descendants)); + assert!(locktower.is_locked_out(2, &descendants)); } #[test] @@ -460,7 +460,7 @@ mod test { .collect(); locktower.record_vote(0); locktower.record_vote(1); - assert!(locktower.is_locked_out(4, &descendants)); + assert!(!locktower.is_locked_out(4, &descendants)); locktower.record_vote(4); assert_eq!(locktower.lockouts.votes[0].slot, 0); assert_eq!(locktower.lockouts.votes[0].confirmation_count, 2); diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 4d204e6fcd99f4..51fe542b9767c5 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -130,11 +130,25 @@ impl ReplayStage { let descendants = bank_forks.read().unwrap().descendants(); let ancestors = bank_forks.read().unwrap().ancestors(); let frozen_banks = bank_forks.read().unwrap().frozen_banks(); + + trace!("frozen_banks {}", frozen_banks.len()); let mut votable: Vec<(u128, Arc)> = frozen_banks .values() - .filter(|b| b.is_votable()) - .filter(|b| !locktower.has_voted(b.slot())) - .filter(|b| !locktower.is_locked_out(b.slot(), &descendants)) + .filter(|b| { + let is_votable = b.is_votable(); + trace!("bank is votable: {} {}", b.slot(), is_votable); + is_votable + }) + .filter(|b| { + let has_voted = locktower.has_voted(b.slot()); + trace!("bank is has_voted: {} {}", b.slot(), has_voted); + !has_voted + }) + .filter(|b| { + let is_locked_out = locktower.is_locked_out(b.slot(), &descendants); + trace!("bank is is_locked_out: {} {}", b.slot(), is_locked_out); + !is_locked_out + }) .map(|bank| { ( bank, @@ -146,7 +160,10 @@ impl ReplayStage { ) }) .filter(|(b, stake_lockouts)| { - locktower.check_vote_stake_threshold(b.slot(), &stake_lockouts) + let vote_threshold = + locktower.check_vote_stake_threshold(b.slot(), &stake_lockouts); + trace!("bank vote_threshold: {} {}", b.slot(), vote_threshold); + vote_threshold }) .map(|(b, stake_lockouts)| { (locktower.calculate_weight(&stake_lockouts), b.clone()) @@ -154,8 +171,18 @@ impl ReplayStage { .collect(); votable.sort_by_key(|b| b.0); + trace!("votable_banks {}", votable.len()); let ms = timing::duration_as_ms(&locktower_start.elapsed()); - info!("@{:?} locktower duration: {:?}", timing::timestamp(), ms,); + if !votable.is_empty() { + let weights: Vec = votable.iter().map(|x| x.0).collect(); + info!( + "@{:?} locktower duration: {:?} len: {} weights: {:?}", + timing::timestamp(), + ms, + votable.len(), + weights + ); + } inc_new_counter_info!("replay_stage-locktower_duration", ms as usize); if let Some((_, bank)) = votable.last() { From 5e29f6ca0ac340a3401600eadefd4ca17b7f0482 Mon Sep 17 00:00:00 2001 From: carllin Date: Tue, 19 Mar 2019 17:30:36 -0700 Subject: [PATCH 6/8] Clear progress map on squash (#3377) --- core/src/bank_forks.rs | 1 + core/src/replay_stage.rs | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/core/src/bank_forks.rs b/core/src/bank_forks.rs index 0174b0e2d04d87..4590d39f57fd8d 100644 --- a/core/src/bank_forks.rs +++ b/core/src/bank_forks.rs @@ -68,6 +68,7 @@ impl BankForks { .map(|(k, _v)| *k) .collect() } + pub fn get(&self, bank_slot: u64) -> Option<&Arc> { self.banks.get(&bank_slot) } diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 51fe542b9767c5..0075525ca8205a 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -199,6 +199,7 @@ impl ReplayStage { ); if let Some(new_root) = locktower.record_vote(bank.slot()) { bank_forks.write().unwrap().set_root(new_root); + Self::handle_new_root(&bank_forks, &mut progress); } locktower.update_epoch(&bank); cluster_info.write().unwrap().push_vote(vote); @@ -406,6 +407,14 @@ impl ReplayStage { Ok(()) } + fn handle_new_root( + bank_forks: &Arc>, + progress: &mut HashMap, + ) { + let r_bank_forks = bank_forks.read().unwrap(); + progress.retain(|k, _| r_bank_forks.get(*k).is_some()); + } + fn process_completed_bank( my_id: &Pubkey, bank: Arc, @@ -634,4 +643,14 @@ mod test { let _ignored = remove_dir_all(&ledger_path); } + + #[test] + fn test_handle_new_root() { + let bank0 = Bank::default(); + let bank_forks = Arc::new(RwLock::new(BankForks::new(0, bank0))); + let mut progress = HashMap::new(); + progress.insert(5, (Hash::default(), 0)); + ReplayStage::handle_new_root(&bank_forks, &mut progress); + assert!(progress.is_empty()); + } } From b0d851472e0c40e7976cd0db601ffe63073fe43a Mon Sep 17 00:00:00 2001 From: Anatoly Yakovenko Date: Tue, 19 Mar 2019 16:54:58 -0700 Subject: [PATCH 7/8] allow empty ancestors --- core/src/locktower.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/locktower.rs b/core/src/locktower.rs index 31df4474cea7c6..880cef59ff10a0 100644 --- a/core/src/locktower.rs +++ b/core/src/locktower.rs @@ -125,6 +125,7 @@ impl Locktower { confirmation_count: MAX_LOCKOUT_HISTORY as u32, slot: root, }; + trace!("ROOT: {}", vote.slot); Self::update_ancestor_lockouts(&mut stake_lockouts, &vote, ancestors); } } @@ -228,7 +229,7 @@ impl Locktower { ancestors: &HashMap>, ) { let mut slot_with_ancestors = vec![vote.slot]; - slot_with_ancestors.extend(&ancestors[&vote.slot]); + slot_with_ancestors.extend(ancestors.get(&vote.slot).unwrap_or(&HashSet::new())); for slot in slot_with_ancestors { let entry = &mut stake_lockouts.entry(slot).or_default(); entry.lockout += vote.lockout(); @@ -244,7 +245,7 @@ impl Locktower { ancestors: &HashMap>, ) { let mut slot_with_ancestors = vec![slot]; - slot_with_ancestors.extend(&ancestors[&slot]); + slot_with_ancestors.extend(ancestors.get(&slot).unwrap_or(&HashSet::new())); for slot in slot_with_ancestors { let entry = &mut stake_lockouts.entry(slot).or_default(); entry.stake += lamports; From 0aebf8cb7b3697a6467863ee9579f8354565a371 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 21 Mar 2019 22:41:39 +0000 Subject: [PATCH 8/8] Disable accounts squash call from bank - It's asserting and killing testnet - temporary solution for beacons --- runtime/src/accounts.rs | 4 ++++ runtime/src/bank.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 7cd688eeb4b3b3..64a631b7e1ce5c 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -727,6 +727,7 @@ impl AccountsDB { .map_or(0, |fork_info| fork_info.transaction_count) } + #[allow(dead_code)] fn remove_parents(&self, fork: Fork) -> Vec { let mut info = self.fork_infos.write().unwrap(); let fork_info = info.get_mut(&fork).unwrap(); @@ -743,6 +744,7 @@ impl AccountsDB { .is_empty() } + #[allow(dead_code)] fn get_merged_account_map( &self, fork: Fork, @@ -763,6 +765,7 @@ impl AccountsDB { } /// make fork a root, i.e. forget its heritage + #[allow(dead_code)] fn squash(&self, fork: Fork) { let parents = self.remove_parents(fork); @@ -991,6 +994,7 @@ impl Accounts { /// accounts starts with an empty data structure for every child/fork /// this function squashes all the parents into this instance + #[allow(dead_code)] pub fn squash(&self, fork: Fork) { assert!(!self.account_locks.lock().unwrap().contains_key(&fork)); self.accounts_db.squash(fork); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 22fd70101850f1..ab8f9e280dd48f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -300,7 +300,7 @@ impl Bank { let parents = self.parents(); *self.parent.write().unwrap() = None; - self.accounts().squash(self.accounts_id); + // self.accounts().squash(self.accounts_id); let parent_caches: Vec<_> = parents .iter()