Skip invalid nodes for finality (#1068)

* skip invalid nodes for finality

* check valid last_ids only

* fixup!

* fixup!

src/crdt.rs

@@ -200,7 +200,6 @@ impl NodeInfo {
             addr.clone(),
         )
     }
-
     pub fn debug_id(&self) -> u64 {
         make_debug_id(&self.id)
     }
@@ -751,6 +750,18 @@ impl Crdt {
         (id, ups, data)
     }
 
+    pub fn valid_last_ids(&self) -> Vec<Hash> {
+        self.table
+            .values()
+            .filter(|r| {
+                r.id != Pubkey::default()
+                    && (Self::is_valid_address(r.contact_info.tpu)
+                        || Self::is_valid_address(r.contact_info.tvu))
+            })
+            .map(|x| x.ledger_state.last_id)
+            .collect()
+    }
+
     pub fn window_index_request(&self, ix: u64) -> Result<(SocketAddr, Vec<u8>)> {
         let valid: Vec<_> = self
             .table
@@ -2085,6 +2096,33 @@ mod tests {
         assert_eq!(crdt.my_data().leader_id, leader1.id);
     }
 
+    #[test]
+    fn test_valid_last_ids() {
+        logger::setup();
+        let mut leader0 = NodeInfo::new_leader(&"127.0.0.2:1234".parse().unwrap());
+        leader0.ledger_state.last_id = hash(b"0");
+        let mut leader1 = NodeInfo::new_multicast();
+        leader1.ledger_state.last_id = hash(b"1");
+        let mut leader2 =
+            NodeInfo::new_leader_with_pubkey(Pubkey::default(), &"127.0.0.2:1234".parse().unwrap());
+        leader2.ledger_state.last_id = hash(b"2");
+        // test that only valid tvu or tpu are retured as nodes
+        let mut leader3 = NodeInfo::new(
+            Keypair::new().pubkey(),
+            "127.0.0.1:1234".parse().unwrap(),
+            "0.0.0.0:0".parse().unwrap(),
+            "127.0.0.1:1236".parse().unwrap(),
+            "0.0.0.0:0".parse().unwrap(),
+            "127.0.0.1:1238".parse().unwrap(),
+        );
+        leader3.ledger_state.last_id = hash(b"3");
+        let mut crdt = Crdt::new(leader0.clone()).expect("Crdt::new");
+        crdt.insert(&leader1);
+        crdt.insert(&leader2);
+        crdt.insert(&leader3);
+        assert_eq!(crdt.valid_last_ids(), vec![leader0.ledger_state.last_id]);
+    }
+
     /// Validates the node that sent Protocol::ReceiveUpdates gets its
     /// liveness updated, but not if the node sends Protocol::ReceiveUpdates
     /// to itself.

src/vote_stage.rs

@@ -124,13 +124,7 @@ pub fn send_leader_vote(
 ) -> Result<()> {
     let now = timing::timestamp();
     if now - *last_vote > VOTE_TIMEOUT_MS {
-        let ids: Vec<_> = crdt
+        let ids: Vec<_> = crdt.read().unwrap().valid_last_ids();
-            .read()
-            .unwrap()
-            .table
-            .values()
-            .map(|x| x.ledger_state.last_id)
-            .collect();
         if let Ok((last_id, super_majority_timestamp)) = get_last_id_to_vote_on(
             debug_id,
             &ids,