Add switch threshold local cluster tests + fix check_switch_threshold

core/src/consensus.rs

@@ -383,6 +383,7 @@ impl Tower {
         total_stake: u64,
         epoch_vote_accounts: &HashMap<Pubkey, (u64, Account)>,
     ) -> SwitchForkDecision {
+        let root = self.lockouts.root_slot.unwrap_or(0);
         self.last_vote()
             .slots
             .last()
@@ -407,12 +408,18 @@ impl Tower {
                 let mut locked_out_stake = 0;
                 let mut locked_out_vote_accounts = HashSet::new();
                 for (candidate_slot, descendants) in descendants.iter() {
-                    // 1) Only consider lockouts a tips of forks as that
-                    //    includes all ancestors of that fork.
-                    // 2) Don't consider lockouts on the `last_vote` itself
-                    // 3) Don't consider lockouts on any descendants of
+                    // 1) Don't consider any banks that haven't been frozen yet
+                    //    because the neededd stats are unavailable
+                    // 2) Only consider lockouts at the latest `frozen` bank
+                    //    on each fork, as that bank will contain all the
+                    //    lockout intervals for ancestors on that fork as well.
+                    // 3) Don't consider lockouts on the `last_vote` itself
+                    // 4) Don't consider lockouts on any descendants of
                     //    `last_vote`
-                    if !descendants.is_empty()
+                    // 5) Don't consider any banks before the root because
+                    //    all lockouts must be ancestors of `last_vote`
+                    if !progress.get_fork_stats(*candidate_slot).map(|stats| stats.computed).unwrap_or(false)
+                        || descendants.iter().any(|d| progress.get_fork_stats(*d).map(|stats| stats.computed).unwrap_or(false))
                         || candidate_slot == last_vote
                         || ancestors
                             .get(&candidate_slot)
@@ -421,6 +428,7 @@ impl Tower {
                                 exist in the ancestors map",
                             )
                             .contains(last_vote)
+                        || *candidate_slot <= root
                     {
                         continue;
                     }
@@ -448,10 +456,11 @@ impl Tower {
                             // 2) Not from before the current root as we can't determine if
                             // anything before the root was an ancestor of `last_vote` or not
                             if !last_vote_ancestors.contains(lockout_interval_start)
-                                // The check if the key exists in the ancestors map
-                                // is equivalent to checking if the key is above the
-                                // current root.
-                                && ancestors.contains_key(lockout_interval_start)
+                                // Given a `lockout_interval_start` < root that appears in a
+                                // bank for a `candidate_slot`, it must be that `lockout_interval_start`
+                                // is an ancestor of the current root, because `candidate_slot` is a
+                                // descendant of the current root
+                                && *lockout_interval_start > root
                                 && !locked_out_vote_accounts.contains(vote_account_pubkey)
                             {
                                 let stake = epoch_vote_accounts
@@ -1044,8 +1053,11 @@ pub mod test {
 
         // Fill the BankForks according to the above fork structure
         vote_simulator.fill_bank_forks(forks, &HashMap::new());
+        for (_, fork_progress) in vote_simulator.progress.iter_mut() {
+            fork_progress.fork_stats.computed = true;
+        }
         let ancestors = vote_simulator.bank_forks.read().unwrap().ancestors();
-        let descendants = vote_simulator.bank_forks.read().unwrap().descendants();
+        let mut descendants = vote_simulator.bank_forks.read().unwrap().descendants();
         let mut tower = Tower::new_with_key(&my_pubkey);
 
         // Last vote is 47
@@ -1122,6 +1134,23 @@ pub mod test {
             SwitchForkDecision::FailedSwitchThreshold
         );
 
+        // Adding another validator lockout on a different fork, and the lockout
+        // covers the last vote would count towards the switch threshold,
+        // unless the bank is not the most recent frozen bank on the fork (14 is a
+        // frozen/computed bank > 13 on the same fork in this case)
+        vote_simulator.simulate_lockout_interval(13, (12, 47), &other_vote_account);
+        assert_eq!(
+            tower.check_switch_threshold(
+                110,
+                &ancestors,
+                &descendants,
+                &vote_simulator.progress,
+                total_stake,
+                bank0.epoch_vote_accounts(0).unwrap(),
+            ),
+            SwitchForkDecision::FailedSwitchThreshold
+        );
+
         // Adding another validator lockout on a different fork, and the lockout
         // covers the last vote, should satisfy the switch threshold
         vote_simulator.simulate_lockout_interval(14, (12, 47), &other_vote_account);
@@ -1137,10 +1166,26 @@ pub mod test {
             SwitchForkDecision::SwitchProof(Hash::default())
         );
 
+        // Adding another unfrozen descendant of the tip of 14 should not remove
+        // slot 14 from consideration because it is still the most recent frozen
+        // bank on its fork
+        descendants.get_mut(&14).unwrap().insert(10000);
+        assert_eq!(
+            tower.check_switch_threshold(
+                110,
+                &ancestors,
+                &descendants,
+                &vote_simulator.progress,
+                total_stake,
+                bank0.epoch_vote_accounts(0).unwrap(),
+            ),
+            SwitchForkDecision::SwitchProof(Hash::default())
+        );
+
         // If we set a root, then any lockout intervals below the root shouldn't
         // count toward the switch threshold. This means the other validator's
         // vote lockout no longer counts
-        vote_simulator.set_root(43);
+        tower.lockouts.root_slot = Some(43);
         assert_eq!(
             tower.check_switch_threshold(
                 110,

local-cluster/src/cluster_tests.rs

@@ -284,7 +284,7 @@ pub fn kill_entry_and_spend_and_verify_rest(
     }
 }
 
-pub fn check_for_new_roots(num_new_roots: usize, contact_infos: &[ContactInfo]) {
+pub fn check_for_new_roots(num_new_roots: usize, contact_infos: &[ContactInfo], test_name: &str) {
     let mut roots = vec![HashSet::new(); contact_infos.len()];
     let mut done = false;
     let mut last_print = Instant::now();
@@ -295,7 +295,7 @@ pub fn check_for_new_roots(num_new_roots: usize, contact_infos: &[ContactInfo])
             roots[i].insert(slot);
             let min_node = roots.iter().map(|r| r.len()).min().unwrap_or(0);
             if last_print.elapsed().as_secs() > 3 {
-                info!("PARTITION_TEST min observed roots {}/16", min_node);
+                info!("{} min observed roots {}/16", test_name, min_node);
                 last_print = Instant::now();
             }
             done = min_node >= num_new_roots;
@@ -304,6 +304,60 @@ pub fn check_for_new_roots(num_new_roots: usize, contact_infos: &[ContactInfo])
     }
 }
 
+pub fn check_no_new_roots(
+    num_slots_to_wait: usize,
+    contact_infos: &[ContactInfo],
+    test_name: &str,
+) {
+    assert!(!contact_infos.is_empty());
+    let mut roots = vec![0; contact_infos.len()];
+    let max_slot = contact_infos
+        .iter()
+        .enumerate()
+        .map(|(i, ingress_node)| {
+            let client = create_client(ingress_node.client_facing_addr(), VALIDATOR_PORT_RANGE);
+            let initial_root = client
+                .get_slot()
+                .unwrap_or_else(|_| panic!("get_slot for {} failed", ingress_node.id));
+            roots[i] = initial_root;
+            client
+                .get_slot_with_commitment(CommitmentConfig::recent())
+                .unwrap_or_else(|_| panic!("get_slot for {} failed", ingress_node.id))
+        })
+        .max()
+        .unwrap();
+
+    let end_slot = max_slot + num_slots_to_wait as u64;
+    let mut current_slot;
+    let mut last_print = Instant::now();
+    let client = create_client(contact_infos[0].client_facing_addr(), VALIDATOR_PORT_RANGE);
+    loop {
+        current_slot = client
+            .get_slot_with_commitment(CommitmentConfig::recent())
+            .unwrap_or_else(|_| panic!("get_slot for {} failed", contact_infos[0].id));
+        if current_slot > end_slot {
+            break;
+        }
+        if last_print.elapsed().as_secs() > 3 {
+            info!(
+                "{} current slot: {}, waiting for slot: {}",
+                test_name, current_slot, end_slot
+            );
+            last_print = Instant::now();
+        }
+    }
+
+    for (i, ingress_node) in contact_infos.iter().enumerate() {
+        let client = create_client(ingress_node.client_facing_addr(), VALIDATOR_PORT_RANGE);
+        assert_eq!(
+            client
+                .get_slot()
+                .unwrap_or_else(|_| panic!("get_slot for {} failed", ingress_node.id)),
+            roots[i]
+        );
+    }
+}
+
 fn poll_all_nodes_for_signature(
     entry_point_info: &ContactInfo,
     cluster_nodes: &[ContactInfo],

local-cluster/src/local_cluster.rs

@@ -1,4 +1,7 @@
-use crate::cluster::{Cluster, ClusterValidatorInfo, ValidatorInfo};
+use crate::{
+    cluster::{Cluster, ClusterValidatorInfo, ValidatorInfo},
+    cluster_tests,
+};
 use itertools::izip;
 use log::*;
 use solana_client::thin_client::{create_client, ThinClient};
@@ -336,6 +339,44 @@ impl LocalCluster {
         Self::transfer_with_client(&client, source_keypair, dest_pubkey, lamports)
     }
 
+    pub fn check_for_new_roots(&self, num_new_roots: usize, test_name: &str) {
+        let alive_node_contact_infos: Vec<_> = self
+            .validators
+            .values()
+            .map(|v| v.info.contact_info.clone())
+            .collect();
+        assert!(!alive_node_contact_infos.is_empty());
+        info!("{} discovering nodes", test_name);
+        let cluster_nodes = discover_cluster(
+            &alive_node_contact_infos[0].gossip,
+            alive_node_contact_infos.len(),
+        )
+        .unwrap();
+        info!("{} discovered {} nodes", test_name, cluster_nodes.len());
+        info!("{} looking for new roots on all nodes", test_name);
+        cluster_tests::check_for_new_roots(num_new_roots, &alive_node_contact_infos, test_name);
+        info!("{} done waiting for roots", test_name);
+    }
+
+    pub fn check_no_new_roots(&self, num_slots_to_wait: usize, test_name: &str) {
+        let alive_node_contact_infos: Vec<_> = self
+            .validators
+            .values()
+            .map(|v| v.info.contact_info.clone())
+            .collect();
+        assert!(!alive_node_contact_infos.is_empty());
+        info!("{} discovering nodes", test_name);
+        let cluster_nodes = discover_cluster(
+            &alive_node_contact_infos[0].gossip,
+            alive_node_contact_infos.len(),
+        )
+        .unwrap();
+        info!("{} discovered {} nodes", test_name, cluster_nodes.len());
+        info!("{} making sure no new roots on any nodes", test_name);
+        cluster_tests::check_no_new_roots(num_slots_to_wait, &alive_node_contact_infos, test_name);
+        info!("{} done waiting for roots", test_name);
+    }
+
     fn transfer_with_client(
         client: &ThinClient,
         source_keypair: &Keypair,