Store and communicate ban reason (#1085)

* Add read_i32 and write_i32
* Store and send reason for ban
* Client corruption ban reason
* Forgotten max message size

api/src/handlers.rs

@@ -29,6 +29,7 @@ use core::core::hash::{Hash, Hashed};
 use core::core::{OutputFeatures, OutputIdentifier, Transaction};
 use core::ser;
 use p2p;
+use p2p::types::ReasonForBan;
 use pool;
 use regex::Regex;
 use rest::*;
@@ -403,7 +404,7 @@ impl Handler for PeerPostHandler {
 			"ban" => {
 				path_elems.pop();
 				if let Ok(addr) = path_elems.last().unwrap().parse() {
-					w(&self.peers).ban_peer(&addr);
+					w(&self.peers).ban_peer(&addr, ReasonForBan::ManualBan);
 					Ok(Response::with((status::Ok, "")))
 				} else {
 					Ok(Response::with((status::BadRequest, "")))
@@ -530,8 +531,8 @@ impl Handler for ChainCompactHandler {
 /// GET /v1/blocks/<hash>
 /// GET /v1/blocks/<height>
 ///
-/// Optionally return results as "compact blocks" by passing "?compact" query param
+/// Optionally return results as "compact blocks" by passing "?compact" query
-/// GET /v1/blocks/<hash>?compact
+/// param GET /v1/blocks/<hash>?compact
 pub struct BlockHandler {
 	pub chain: Weak<chain::Chain>,
 }

core/src/ser.rs

@@ -19,18 +19,18 @@
 //! To use it simply implement `Writeable` or `Readable` and then use the
 //! `serialize` or `deserialize` functions on them as appropriate.
 
-use std::{cmp, error, fmt, mem};
-use std::io::{self, Read, Write};
 use byteorder::{BigEndian, ByteOrder, ReadBytesExt};
-use keychain::{BlindingFactor, Identifier, IDENTIFIER_SIZE};
 use consensus;
 use consensus::VerifySortOrder;
 use core::hash::{Hash, Hashed};
-use util::secp::pedersen::Commitment;
+use keychain::{BlindingFactor, Identifier, IDENTIFIER_SIZE};
-use util::secp::pedersen::RangeProof;
+use std::io::{self, Read, Write};
+use std::{cmp, error, fmt, mem};
 use util::secp::Signature;
 use util::secp::constants::{AGG_SIGNATURE_SIZE, MAX_PROOF_SIZE, PEDERSEN_COMMITMENT_SIZE,
                             SECRET_KEY_SIZE};
+use util::secp::pedersen::Commitment;
+use util::secp::pedersen::RangeProof;
 
 /// Possible errors deriving from serializing or deserializing.
 #[derive(Debug)]
@@ -139,6 +139,13 @@ pub trait Writer {
 		self.write_fixed_bytes(&bytes)
 	}
 
+	/// Writes a u32 as bytes
+	fn write_i32(&mut self, n: i32) -> Result<(), Error> {
+		let mut bytes = [0; 4];
+		BigEndian::write_i32(&mut bytes, n);
+		self.write_fixed_bytes(&bytes)
+	}
+
 	/// Writes a u64 as bytes
 	fn write_u64(&mut self, n: u64) -> Result<(), Error> {
 		let mut bytes = [0; 8];
@@ -177,6 +184,8 @@ pub trait Reader {
 	/// Read a u64 from the underlying Read
 	fn read_u64(&mut self) -> Result<u64, Error>;
 	/// Read a i32 from the underlying Read
+	fn read_i32(&mut self) -> Result<i32, Error>;
+	/// Read a i64 from the underlying Read
 	fn read_i64(&mut self) -> Result<i64, Error>;
 	/// first before the data bytes.
 	fn read_vec(&mut self) -> Result<Vec<u8>, Error>;
@@ -270,6 +279,9 @@ impl<'a> Reader for BinReader<'a> {
 	fn read_u32(&mut self) -> Result<u32, Error> {
 		self.source.read_u32::<BigEndian>().map_err(Error::IOErr)
 	}
+	fn read_i32(&mut self) -> Result<i32, Error> {
+		self.source.read_i32::<BigEndian>().map_err(Error::IOErr)
+	}
 	fn read_u64(&mut self) -> Result<u64, Error> {
 		self.source.read_u64::<BigEndian>().map_err(Error::IOErr)
 	}
@@ -429,12 +441,13 @@ macro_rules! impl_int {
 				reader.$r_fn()
 			}
 		}
-    }
+	};
 }
 
 impl_int!(u8, write_u8, read_u8);
 impl_int!(u16, write_u16, read_u16);
 impl_int!(u32, write_u32, read_u32);
+impl_int!(i32, write_i32, read_i32);
 impl_int!(u64, write_u64, read_u64);
 impl_int!(i64, write_i64, read_i64);
 

p2p/src/lib.rs

@@ -49,9 +49,9 @@ mod serv;
 mod store;
 pub mod types;
 
-pub use serv::{DummyAdapter, Server};
-pub use peers::Peers;
 pub use peer::Peer;
-pub use types::{Capabilities, ChainAdapter, Error, P2PConfig, PeerInfo, TxHashSetRead,
+pub use peers::Peers;
-                MAX_BLOCK_HEADERS, MAX_LOCATORS, MAX_PEER_ADDRS};
+pub use serv::{DummyAdapter, Server};
 pub use store::{PeerData, State};
+pub use types::{Capabilities, ChainAdapter, Error, P2PConfig, PeerInfo, ReasonForBan,
+                TxHashSetRead, MAX_BLOCK_HEADERS, MAX_LOCATORS, MAX_PEER_ADDRS};

p2p/src/msg.rs

@@ -14,11 +14,11 @@
 
 //! Message types that transit over the network and related serialization code.
 
+use num::FromPrimitive;
 use std::io::{self, Read, Write};
 use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6, TcpStream};
 use std::thread;
 use std::time;
-use num::FromPrimitive;
 
 use core::consensus::{MAX_MSG_LEN, MAX_TX_INPUTS, MAX_TX_KERNELS, MAX_TX_OUTPUTS};
 use core::core::BlockHeader;
@@ -56,6 +56,7 @@ enum_from_primitive! {
 		Shake,
 		Ping,
 		Pong,
+		BanReason,
 		GetPeerAddrs,
 		PeerAddrs,
 		GetHeaders,
@@ -72,12 +73,13 @@ enum_from_primitive! {
 	}
 }
 
-const MAX_MSG_SIZES: [u64; 18] = [
+const MAX_MSG_SIZES: [u64; 19] = [
 	0,                                                                  // Error
 	128,                                                                // Hand
 	88,                                                                 // Shake
 	16,                                                                 // Ping
 	16,                                                                 // Pong
+	64,                                                                 // BanReason
 	4,                                                                  // GetPeerAddrs
 	4 + (1 + 16 + 2) * MAX_PEER_ADDRS as u64,                           // PeerAddrs, with all IPv6
 	1 + 32 * MAX_LOCATORS as u64,                                       // GetHeaders locators
@@ -676,6 +678,33 @@ impl Readable for Pong {
 	}
 }
 
+#[derive(Debug)]
+pub struct BanReason {
+	/// the reason for the ban
+	pub ban_reason: ReasonForBan,
+}
+
+impl Writeable for BanReason {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		let ban_reason_i32 = self.ban_reason as i32;
+		ban_reason_i32.write(writer).unwrap();
+		Ok(())
+	}
+}
+
+impl Readable for BanReason {
+	fn read(reader: &mut Reader) -> Result<BanReason, ser::Error> {
+		let ban_reason_i32 = match reader.read_i32() {
+			Ok(h) => h,
+			Err(_) => 0,
+		};
+
+		let ban_reason = ReasonForBan::from_i32(ban_reason_i32).ok_or(ser::Error::CorruptedData)?;
+
+		Ok(BanReason { ban_reason })
+	}
+}
+
 /// Request to get an archive of the full txhashset store, required to sync
 /// a new node.
 pub struct TxHashSetRequest {

p2p/src/peer.rs

@@ -22,8 +22,8 @@ use core::core::hash::{Hash, Hashed};
 use core::core::target::Difficulty;
 use handshake::Handshake;
 use msg;
-use protocol::Protocol;
 use msg::*;
+use protocol::Protocol;
 use types::*;
 use util::LOGGER;
 
@@ -157,6 +157,25 @@ impl Peer {
 			.send(ping_msg, msg::Type::Ping)
 	}
 
+	/// Send the ban reason before banning
+	pub fn send_ban_reason(&self, ban_reason: ReasonForBan) {
+		let ban_reason_msg = BanReason { ban_reason };
+		match self.connection
+			.as_ref()
+			.unwrap()
+			.send(ban_reason_msg, msg::Type::BanReason)
+		{
+			Ok(_) => debug!(
+				LOGGER,
+				"Sent ban reason {:?} to {}", ban_reason, self.info.addr
+			),
+			Err(e) => error!(
+				LOGGER,
+				"Could not send ban reason {:?} to {}: {:?}", ban_reason, self.info.addr, e
+			),
+		};
+	}
+
 	/// Sends the provided block to the remote peer. The request may be dropped
 	/// if the remote peer is known to already have the block.
 	pub fn send_block(&self, b: &core::Block) -> Result<(), Error> {
@@ -235,8 +254,9 @@ impl Peer {
 		}
 	}
 
-	/// Sends the provided stem transaction to the remote peer. The request may be
+	/// Sends the provided stem transaction to the remote peer. The request may
-	/// dropped if the remote peer is known to already have the stem transaction.
+	/// be dropped if the remote peer is known to already have the stem
+	/// transaction.
 	pub fn send_stem_transaction(&self, tx: &core::Transaction) -> Result<(), Error> {
 		if !self.tracking_adapter.has(tx.hash()) {
 			debug!(LOGGER, "Send tx {} to {}", tx.hash(), self.info.addr);

p2p/src/peers.rs

@@ -22,8 +22,8 @@ use rand::{thread_rng, Rng};
 use core::core;
 use core::core::hash::{Hash, Hashed};
 use core::core::target::Difficulty;
-use util::LOGGER;
 use time;
+use util::LOGGER;
 
 use peer::Peer;
 use store::{PeerData, PeerStore, State};
@@ -59,6 +59,7 @@ impl Peers {
 			user_agent: p.info.user_agent.clone(),
 			flags: State::Healthy,
 			last_banned: 0,
+			ban_reason: ReasonForBan::None,
 		};
 		if let Err(e) = self.save_peer(&peer_data) {
 			error!(LOGGER, "Could not save connected peer: {:?}", e);
@@ -205,8 +206,8 @@ impl Peers {
 		}
 	}
 
-	/// Return vec of connected peers that currently have the most worked branch,
+	/// Return vec of connected peers that currently have the most worked
-	/// showing the highest total difficulty.
+	/// branch, showing the highest total difficulty.
 	pub fn most_work_peers(&self) -> Vec<Arc<RwLock<Peer>>> {
 		let peers = self.connected_peers();
 		if peers.len() == 0 {
@@ -235,8 +236,8 @@ impl Peers {
 		max_peers
 	}
 
-	/// Returns single random peer with the most worked branch, showing the highest total
+	/// Returns single random peer with the most worked branch, showing the
-	/// difficulty.
+	/// highest total difficulty.
 	pub fn most_work_peer(&self) -> Option<Arc<RwLock<Peer>>> {
 		match self.most_work_peers().first() {
 			Some(x) => Some(x.clone()),
@@ -253,8 +254,8 @@ impl Peers {
 		false
 	}
 
-	/// Bans a peer, disconnecting it if we're currently connected
+	/// Ban a peer, disconnecting it if we're currently connected
-	pub fn ban_peer(&self, peer_addr: &SocketAddr) {
+	pub fn ban_peer(&self, peer_addr: &SocketAddr, ban_reason: ReasonForBan) {
 		if let Err(e) = self.update_state(peer_addr.clone(), State::Banned) {
 			error!(LOGGER, "Couldn't ban {}: {:?}", peer_addr, e);
 		}
@@ -272,6 +273,7 @@ impl Peers {
 			debug!(LOGGER, "Banning peer {}", peer_addr);
 			// setting peer status will get it removed at the next clean_peer
 			let peer = peer.write().unwrap();
+			peer.send_ban_reason(ban_reason);
 			peer.set_banned();
 			peer.stop();
 		}
@@ -399,8 +401,8 @@ impl Peers {
 		}
 	}
 
-	/// Broadcasts the provided transaction to PEER_PREFERRED_COUNT of our peers.
+	/// Broadcasts the provided transaction to PEER_PREFERRED_COUNT of our
-	/// We may be connected to PEER_MAX_COUNT peers so we only
+	/// peers. We may be connected to PEER_MAX_COUNT peers so we only
 	/// want to broadcast to a random subset of peers.
 	/// A peer implementation may drop the broadcast request
 	/// if it knows the remote peer already has the transaction.
@@ -416,8 +418,8 @@ impl Peers {
 		}
 	}
 
-	/// Ping all our connected peers. Always automatically expects a pong back or
+	/// Ping all our connected peers. Always automatically expects a pong back
-	/// disconnects. This acts as a liveness test.
+	/// or disconnects. This acts as a liveness test.
 	pub fn check_all(&self, total_difficulty: Difficulty, height: u64) {
 		let peers_map = self.peers.read().unwrap();
 		for p in peers_map.values() {
@@ -554,7 +556,7 @@ impl ChainAdapter for Peers {
 				LOGGER,
 				"Received a bad block {} from  {}, the peer will be banned", hash, peer_addr
 			);
-			self.ban_peer(&peer_addr);
+			self.ban_peer(&peer_addr, ReasonForBan::BadBlock);
 			false
 		} else {
 			true
@@ -572,7 +574,7 @@ impl ChainAdapter for Peers {
 				hash,
 				&peer_addr
 			);
-			self.ban_peer(&peer_addr);
+			self.ban_peer(&peer_addr, ReasonForBan::BadCompactBlock);
 			false
 		} else {
 			true
@@ -583,7 +585,7 @@ impl ChainAdapter for Peers {
 		if !self.adapter.header_received(bh, peer_addr) {
 			// if the peer sent us a block header that's intrinsically bad
 			// they are either mistaken or manevolent, both of which require a ban
-			self.ban_peer(&peer_addr);
+			self.ban_peer(&peer_addr, ReasonForBan::BadBlockHeader);
 			false
 		} else {
 			true
@@ -625,7 +627,7 @@ impl ChainAdapter for Peers {
 				LOGGER,
 				"Received a bad txhashset data from {}, the peer will be banned", &peer_addr
 			);
-			self.ban_peer(&peer_addr);
+			self.ban_peer(&peer_addr, ReasonForBan::BadTxHashSet);
 			false
 		} else {
 			true
@@ -661,6 +663,7 @@ impl NetAdapter for Peers {
 				user_agent: "".to_string(),
 				flags: State::Healthy,
 				last_banned: 0,
+				ban_reason: ReasonForBan::None,
 			};
 			if let Err(e) = self.save_peer(&peer) {
 				error!(LOGGER, "Could not save received peer address: {:?}", e);

p2p/src/protocol.rs

@@ -17,9 +17,9 @@ use std::fs::File;
 use std::net::SocketAddr;
 use std::sync::Arc;
 
+use conn::*;
 use core::core;
 use core::core::hash::{Hash, Hashed};
-use conn::*;
 use msg::*;
 use rand;
 use rand::Rng;
@@ -74,6 +74,12 @@ impl MessageHandler for Protocol {
 				Ok(None)
 			}
 
+			Type::BanReason => {
+				let ban_reason: BanReason = msg.body()?;
+				error!(LOGGER, "handle_payload: BanReason {:?}", ban_reason);
+				Ok(None)
+			}
+
 			Type::Transaction => {
 				let tx: core::Transaction = msg.body()?;
 				adapter.transaction_received(tx, false);

p2p/src/store.rs

@@ -14,14 +14,14 @@
 
 //! Storage implementation for peer data.
 
-use std::net::SocketAddr;
 use num::FromPrimitive;
 use rand::{thread_rng, Rng};
+use std::net::SocketAddr;
 
 use core::ser::{self, Readable, Reader, Writeable, Writer};
 use grin_store::{self, option_to_not_found, to_key, Error};
 use msg::SockAddr;
-use types::Capabilities;
+use types::{Capabilities, ReasonForBan};
 use util::LOGGER;
 
 const STORE_SUBPATH: &'static str = "peers";
@@ -52,6 +52,8 @@ pub struct PeerData {
 	pub flags: State,
 	/// The time the peer was last banned
 	pub last_banned: i64,
+	/// The reason for the ban
+	pub ban_reason: ReasonForBan,
 }
 
 impl Writeable for PeerData {
@@ -62,7 +64,8 @@ impl Writeable for PeerData {
 			[write_u32, self.capabilities.bits()],
 			[write_bytes, &self.user_agent],
 			[write_u8, self.flags as u8],
-			[write_i64, self.last_banned]
+			[write_i64, self.last_banned],
+			[write_i32, self.ban_reason as i32]
 		);
 		Ok(())
 	}
@@ -71,10 +74,12 @@ impl Writeable for PeerData {
 impl Readable for PeerData {
 	fn read(reader: &mut Reader) -> Result<PeerData, ser::Error> {
 		let addr = SockAddr::read(reader)?;
-		let (capab, ua, fl, lb) = ser_multiread!(reader, read_u32, read_vec, read_u8, read_i64);
+		let (capab, ua, fl, lb, br) =
+			ser_multiread!(reader, read_u32, read_vec, read_u8, read_i64, read_i32);
 		let user_agent = String::from_utf8(ua).map_err(|_| ser::Error::CorruptedData)?;
 		let capabilities = Capabilities::from_bits(capab).ok_or(ser::Error::CorruptedData)?;
 		let last_banned = lb;
+		let ban_reason = ReasonForBan::from_i32(br).ok_or(ser::Error::CorruptedData)?;
 		match State::from_u8(fl) {
 			Some(flags) => Ok(PeerData {
 				addr: addr.0,
@@ -82,6 +87,7 @@ impl Readable for PeerData {
 				user_agent: user_agent,
 				flags: flags,
 				last_banned: last_banned,
+				ban_reason: ban_reason,
 			}),
 			None => Err(ser::Error::CorruptedData),
 		}

p2p/src/types.rs

@@ -198,6 +198,19 @@ enum_from_primitive! {
 	}
 }
 
+/// Ban reason
+enum_from_primitive! {
+	#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
+	pub enum ReasonForBan {
+		None = 0,
+		BadBlock = 1,
+		BadCompactBlock = 2,
+		BadBlockHeader = 3,
+		BadTxHashSet = 4,
+		ManualBan = 5,
+	}
+}
+
 /// General information about a connected peer that's useful to other modules.
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct PeerInfo {