Switch to optimistic conf for notifying gossip subscribers (#11289)

Co-authored-by: Carl <carl@solana.com>

core/src/cluster_info_vote_listener.rs

@@ -24,7 +24,6 @@ use solana_perf::packet::{self, Packets};
 use solana_runtime::{
     bank::Bank,
     bank_forks::BankForks,
-    commitment::VOTE_THRESHOLD_SIZE,
     epoch_stakes::{EpochAuthorizedVoters, EpochStakes},
     stakes::Stakes,
 };
@@ -62,7 +61,6 @@ pub struct SlotVoteTracker {
     voted: HashMap<Arc<Pubkey>, bool>,
     optimistic_votes_tracker: HashMap<Hash, VoteStakeTracker>,
     updates: Option<Vec<Arc<Pubkey>>>,
-    total_voted_stake: u64,
     gossip_only_stake: u64,
 }
 
@@ -118,7 +116,6 @@ impl VoteTracker {
                 voted: HashMap::new(),
                 optimistic_votes_tracker: HashMap::default(),
                 updates: None,
-                total_voted_stake: 0,
                 gossip_only_stake: 0,
             }));
             self.slot_vote_trackers
@@ -599,7 +596,8 @@ impl ClusterInfoVoteListener {
 
                 if is_confirmed {
                     new_optimistic_confirmed_slots.push((*slot, last_vote_hash));
-                    // TODO: Notify subscribers about new optimistic confirmation
+                    // Notify subscribers about new optimistic confirmation
+                    subscriptions.notify_gossip_subscribers(*slot);
                 }
 
                 if !is_new && !is_gossip_vote {
@@ -714,25 +712,19 @@ impl ClusterInfoVoteListener {
             if w_slot_tracker.updates.is_none() {
                 w_slot_tracker.updates = Some(vec![]);
             }
-            let mut current_stake = 0;
             let mut gossip_only_stake = 0;
             let epoch = root_bank.epoch_schedule().get_epoch(slot);
             let epoch_stakes = root_bank.epoch_stakes(epoch);
 
             for (pubkey, seen_in_gossip_above) in slot_diff {
-                let is_new = !w_slot_tracker.voted.contains_key(&pubkey);
+                if seen_in_gossip_above {
-                Self::sum_stake(
-                    &mut current_stake,
-                    &mut gossip_only_stake,
-                    epoch_stakes,
-                    &pubkey,
                     // By this point we know if the vote was seen in gossip above,
-                    // it was not seen in gossip at any point in the past, so it's
+                    // it was not seen in gossip at any point in the past (if it was seen
-                    // safe to pass this in here as an overall indicator of whether
+                    // in gossip in the past, `is_new` would be false and it would have
-                    // this vote is new
+                    // been filtered out above), so it's safe to increment the gossip-only
-                    seen_in_gossip_above,
+                    // stake
-                    is_new,
+                    Self::sum_stake(&mut gossip_only_stake, epoch_stakes, &pubkey);
-                );
+                }
 
                 // From the `slot_diff.retain` earlier, we know because there are
                 // no other writers to `slot_vote_tracker` that
@@ -743,14 +735,7 @@ impl ClusterInfoVoteListener {
                     .insert(pubkey.clone(), seen_in_gossip_above);
                 w_slot_tracker.updates.as_mut().unwrap().push(pubkey);
             }
-            Self::notify_for_stake_change(
+
-                current_stake,
-                w_slot_tracker.total_voted_stake,
-                &subscriptions,
-                epoch_stakes,
-                slot,
-            );
-            w_slot_tracker.total_voted_stake += current_stake;
             w_slot_tracker.gossip_only_stake += gossip_only_stake
         }
         new_optimistic_confirmed_slots
@@ -775,43 +760,10 @@ impl ClusterInfoVoteListener {
             .add_vote_pubkey(pubkey, stake, total_epoch_stake)
     }
 
-    fn notify_for_stake_change(
+    fn sum_stake(sum: &mut u64, epoch_stakes: Option<&EpochStakes>, pubkey: &Pubkey) {
-        current_stake: u64,
-        previous_stake: u64,
-        subscriptions: &RpcSubscriptions,
-        epoch_stakes: Option<&EpochStakes>,
-        slot: Slot,
-    ) {
-        if let Some(stakes) = epoch_stakes {
-            let supermajority_stake = (stakes.total_stake() as f64 * VOTE_THRESHOLD_SIZE) as u64;
-            if previous_stake < supermajority_stake
-                && (previous_stake + current_stake) > supermajority_stake
-            {
-                subscriptions.notify_gossip_subscribers(slot);
-            }
-        }
-    }
-
-    fn sum_stake(
-        sum: &mut u64,
-        gossip_only_stake: &mut u64,
-        epoch_stakes: Option<&EpochStakes>,
-        pubkey: &Pubkey,
-        is_new_from_gossip: bool,
-        is_new: bool,
-    ) {
-        if !is_new_from_gossip && !is_new {
-            return;
-        }
-
         if let Some(stakes) = epoch_stakes {
             if let Some(vote_account) = stakes.stakes().vote_accounts().get(pubkey) {
-                if is_new {
+                *sum += vote_account.0;
-                    *sum += vote_account.0;
-                }
-                if is_new_from_gossip {
-                    *gossip_only_stake += vote_account.0;
-                }
             }
         }
     }
@@ -1315,7 +1267,7 @@ mod tests {
         let (replay_votes_sender, replay_votes_receiver) = unbounded();
 
         let vote_slot = 1;
-
+        let vote_bank_hash = Hash::default();
         // Events:
         // 0: Send gossip vote
         // 1: Send replay vote
@@ -1339,7 +1291,7 @@ mod tests {
                     // Create vote transaction
                     let vote_tx = vote_transaction::new_vote_transaction(
                         vec![vote_slot],
-                        Hash::default(),
+                        vote_bank_hash,
                         Hash::default(),
                         node_keypair,
                         vote_keypair,
@@ -1371,12 +1323,24 @@ mod tests {
 
             if events == vec![1] {
                 // Check `gossip_only_stake` is not incremented
-                assert_eq!(r_slot_vote_tracker.total_voted_stake, 100);
+                assert_eq!(
+                    r_slot_vote_tracker
+                        .optimistic_votes_tracker(&vote_bank_hash)
+                        .unwrap()
+                        .stake(),
+                    100
+                );
                 assert_eq!(r_slot_vote_tracker.gossip_only_stake, 0);
             } else {
                 // Check that both the `gossip_only_stake` and `total_voted_stake` both
                 // increased
-                assert_eq!(r_slot_vote_tracker.total_voted_stake, 100);
+                assert_eq!(
+                    r_slot_vote_tracker
+                        .optimistic_votes_tracker(&vote_bank_hash)
+                        .unwrap()
+                        .stake(),
+                    100
+                );
                 assert_eq!(r_slot_vote_tracker.gossip_only_stake, 100);
             }
         }
@@ -1718,72 +1682,13 @@ mod tests {
         let (_, bank, validator_voting_keypairs, _) = setup();
         let vote_keypair = &validator_voting_keypairs[0].vote_keypair;
         let epoch_stakes = bank.epoch_stakes(bank.epoch()).unwrap();
-
-        // If `is_new_from_gossip` and `is_new` are both true, both fields
-        // should increase
-        let mut total_voted_stake = 0;
         let mut gossip_only_stake = 0;
-        let is_new_from_gossip = true;
+
-        let is_new = true;
         ClusterInfoVoteListener::sum_stake(
-            &mut total_voted_stake,
             &mut gossip_only_stake,
             Some(epoch_stakes),
             &vote_keypair.pubkey(),
-            is_new_from_gossip,
-            is_new,
         );
-        assert_eq!(total_voted_stake, 100);
-        assert_eq!(gossip_only_stake, 100);
-
-        // If `is_new_from_gossip` and `is_new` are both false, none should increase
-        let mut total_voted_stake = 0;
-        let mut gossip_only_stake = 0;
-        let is_new_from_gossip = false;
-        let is_new = false;
-        ClusterInfoVoteListener::sum_stake(
-            &mut total_voted_stake,
-            &mut gossip_only_stake,
-            Some(epoch_stakes),
-            &vote_keypair.pubkey(),
-            is_new_from_gossip,
-            is_new,
-        );
-        assert_eq!(total_voted_stake, 0);
-        assert_eq!(gossip_only_stake, 0);
-
-        // If only `is_new`, but not `is_new_from_gossip` then
-        // `total_voted_stake` will increase, but `gossip_only_stake` won't
-        let mut total_voted_stake = 0;
-        let mut gossip_only_stake = 0;
-        let is_new_from_gossip = false;
-        let is_new = true;
-        ClusterInfoVoteListener::sum_stake(
-            &mut total_voted_stake,
-            &mut gossip_only_stake,
-            Some(epoch_stakes),
-            &vote_keypair.pubkey(),
-            is_new_from_gossip,
-            is_new,
-        );
-        assert_eq!(total_voted_stake, 100);
-        assert_eq!(gossip_only_stake, 0);
-
-        // If only `is_new_from_gossip`, but not `is_new` then
-        // `gossip_only_stake` will increase, but `total_voted_stake` won't
-        let mut total_voted_stake = 0;
-        let mut gossip_only_stake = 0;
-        let is_new_from_gossip = true;
-        let is_new = false;
-        ClusterInfoVoteListener::sum_stake(
-            &mut total_voted_stake,
-            &mut gossip_only_stake,
-            Some(epoch_stakes),
-            &vote_keypair.pubkey(),
-            is_new_from_gossip,
-            is_new,
-        );
-        assert_eq!(total_voted_stake, 0);
         assert_eq!(gossip_only_stake, 100);
     }
 

core/src/vote_stake_tracker.rs

@@ -22,12 +22,11 @@ impl VoteStakeTracker {
         let is_new = !self.voted.contains(&vote_pubkey);
         if is_new {
             self.voted.insert(vote_pubkey);
-            let ratio_before = self.stake as f64 / total_epoch_stake as f64;
+            let supermajority_stake = (total_epoch_stake as f64 * VOTE_THRESHOLD_SIZE) as u64;
+            let previous_stake = self.stake;
             self.stake += stake;
-            let ratio_now = self.stake as f64 / total_epoch_stake as f64;
-
             (
-                ratio_before <= VOTE_THRESHOLD_SIZE && ratio_now > VOTE_THRESHOLD_SIZE,
+                previous_stake <= supermajority_stake && self.stake > supermajority_stake,
                 is_new,
             )
         } else {
@@ -66,7 +65,7 @@ mod test {
             assert!(!is_confirmed2);
             assert!(!is_new2);
 
-            // at i == 7, the voted stake is 70%, which is the first time crossing
+            // at i == 6, the voted stake is 70%, which is the first time crossing
             // the supermajority threshold
             if i == 6 {
                 assert!(is_confirmed);