From 1aa2f0082a288ada8dba1a95d03188be3cefaaf2 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sat, 21 Jul 2018 16:13:19 +0200 Subject: [PATCH] Refactor out disable/disconnect peer to make API more declarative (#394) * Refactor out disable/disconnect peer to make API more declarative * Minor fixes. * rename `disconnect_peer` to `drop_peer` in low-level --- polkadot/network/src/lib.rs | 22 +++++++++++----------- polkadot/network/src/tests.rs | 13 ++++++------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/polkadot/network/src/lib.rs b/polkadot/network/src/lib.rs index cb3d97bdbb..016a884fe9 100644 --- a/polkadot/network/src/lib.rs +++ b/polkadot/network/src/lib.rs @@ -49,7 +49,7 @@ use parking_lot::Mutex; use polkadot_consensus::{Statement, SignedStatement, GenericStatement}; use polkadot_primitives::{AccountId, Block, SessionKey, Hash, Header}; use polkadot_primitives::parachain::{Id as ParaId, BlockData, Extrinsic, CandidateReceipt, Collation}; -use substrate_network::{PeerId, RequestId, Context}; +use substrate_network::{PeerId, RequestId, Context, Severity}; use substrate_network::consensus_gossip::ConsensusGossip; use substrate_network::{message, generic_message}; use substrate_network::specialization::Specialization; @@ -403,7 +403,7 @@ impl PolkadotProtocol { }; if !info.claimed_validator { - ctx.disable_peer(peer_id, "Session key broadcasted without setting authority role"); + ctx.report_peer(peer_id, Severity::Bad("Session key broadcasted without setting authority role")); return; } @@ -438,7 +438,7 @@ impl PolkadotProtocol { self.pending.push(req); self.dispatch_pending_requests(ctx); } - None => ctx.disable_peer(peer_id, "Unexpected block data response"), + None => ctx.report_peer(peer_id, Severity::Bad("Unexpected block data response")), } } @@ -453,9 +453,9 @@ impl PolkadotProtocol { }; match info.validator_key { - None => ctx.disable_peer( + None => ctx.report_peer( peer_id, - "Sent collator role without registering first as validator", + Severity::Bad("Sent collator role without registering first as validator"), ), Some(key) => for (relay_parent, collation) in self.local_collations.note_validator_role(key, role) { send_polkadot_message( @@ -483,7 +483,7 @@ impl Specialization for PolkadotProtocol { if let Some((ref acc_id, ref para_id)) = local_status.collating_for { if self.collator_peer_id(acc_id.clone()).is_some() { - ctx.disconnect_peer(peer_id); + ctx.report_peer(peer_id, Severity::Useless("Unknown Polkadot-specific reason")); return } @@ -571,7 +571,7 @@ impl Specialization for PolkadotProtocol { Some(msg) => self.on_polkadot_message(ctx, peer_id, raw, msg), None => { trace!(target: "p_net", "Bad message from {}", peer_id); - ctx.disable_peer(peer_id, "Invalid polkadot protocol message format"); + ctx.report_peer(peer_id, Severity::Bad("Invalid polkadot protocol message format")); } } } @@ -616,15 +616,15 @@ impl PolkadotProtocol { let collated_acc = collation.receipt.collator; match self.peers.get(&from) { - None => ctx.disconnect_peer(from), + None => ctx.report_peer(from, Severity::Useless("Unknown Polkadot specific reason")), Some(peer_info) => match peer_info.collating_for { - None => ctx.disable_peer(from, "Sent collation without registering collator intent"), + None => ctx.report_peer(from, Severity::Bad("Sent collation without registering collator intent")), Some((ref acc_id, ref para_id)) => { let structurally_valid = para_id == &collation_para && acc_id == &collated_acc; if structurally_valid && collation.receipt.check_signature().is_ok() { self.collators.on_collation(acc_id.clone(), relay_parent, collation) } else { - ctx.disable_peer(from, "Sent malformed collation") + ctx.report_peer(from, Severity::Bad("Sent malformed collation")) }; } }, @@ -654,7 +654,7 @@ impl PolkadotProtocol { // disconnect a collator by account-id. fn disconnect_bad_collator(&self, ctx: &mut Context, account_id: AccountId) { if let Some(peer_id) = self.collator_peer_id(account_id) { - ctx.disable_peer(peer_id, "Consensus layer determined the given collator misbehaved") + ctx.report_peer(peer_id, Severity::Bad("Consensus layer determined the given collator misbehaved")) } } } diff --git a/polkadot/network/src/tests.rs b/polkadot/network/src/tests.rs index 021ea739b1..6d7fde3fe5 100644 --- a/polkadot/network/src/tests.rs +++ b/polkadot/network/src/tests.rs @@ -24,7 +24,7 @@ use polkadot_primitives::{Block, Hash, SessionKey}; use polkadot_primitives::parachain::{CandidateReceipt, HeadData, BlockData}; use substrate_primitives::H512; use codec::Encode; -use substrate_network::{PeerId, PeerInfo, ClientHandle, Context, Roles, message::Message as SubstrateMessage, specialization::Specialization, generic_message::Message as GenericMessage}; +use substrate_network::{Severity, PeerId, PeerInfo, ClientHandle, Context, Roles, message::Message as SubstrateMessage, specialization::Specialization, generic_message::Message as GenericMessage}; use std::sync::Arc; use futures::Future; @@ -41,12 +41,11 @@ impl Context for TestContext { unimplemented!() } - fn disable_peer(&mut self, peer: PeerId, _reason: &str) { - self.disabled.push(peer); - } - - fn disconnect_peer(&mut self, peer: PeerId) { - self.disconnected.push(peer); + fn report_peer(&mut self, peer: PeerId, reason: Severity) { + match reason { + Severity::Bad(_) => self.disabled.push(peer), + _ => self.disconnected.push(peer), + } } fn peer_info(&self, _peer: PeerId) -> Option> {