From 5b0bb7e607feaa399ff8257eb68e5fb9e03af165 Mon Sep 17 00:00:00 2001 From: anatoly yakovenko Date: Sat, 25 Aug 2018 23:12:41 -0700 Subject: [PATCH] Skip invalid nodes for finality (#1068) * skip invalid nodes for finality * check valid last_ids only * fixup! * fixup! --- src/crdt.rs | 40 +++++++++++++++++++++++++++++++++++++++- src/vote_stage.rs | 8 +------- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/crdt.rs b/src/crdt.rs index fdb7d37161..42372c5681 100644 --- a/src/crdt.rs +++ b/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 { + 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)> { 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. diff --git a/src/vote_stage.rs b/src/vote_stage.rs index ec0e63fb1e..204c7550c2 100755 --- a/src/vote_stage.rs +++ b/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 - .read() - .unwrap() - .table - .values() - .map(|x| x.ledger_state.last_id) - .collect(); + let ids: Vec<_> = crdt.read().unwrap().valid_last_ids(); if let Ok((last_id, super_majority_timestamp)) = get_last_id_to_vote_on( debug_id, &ids,