cleanup errors in message and p2p libs

message/src/common/command.rs

@@ -2,7 +2,7 @@ use std::{str, fmt};
 use std::ascii::AsciiExt;
 use hash::H96;
 use ser::{Serializable, Stream, Deserializable, Reader, Error as ReaderError};
-use common::Error;
+use Error;
 
 #[derive(Debug, PartialEq, Clone)]
 pub struct Command(H96);

message/src/common/error.rs

@@ -1,5 +0,0 @@
-#[derive(Debug, PartialEq)]
-pub enum Error {
-	InvalidCommand,
-	InvalidMagic,
-}

message/src/common/magic.rs

@@ -1,8 +1,8 @@
 //! Bitcoin network magic number
 //! https://www.anintegratedworld.com/unravelling-the-mysterious-block-chain-magic-number/
 
-use ser::{Stream, Serializable, Reader, Deserializable, Error as ReaderError};
-use common::Error;
+use ser::{Stream, Serializable};
+use Error;
 
 const MAGIC_MAINNET: u32 = 0xD9B4BEF9;
 const MAGIC_TESTNET: u32 = 0x0709110B;
@@ -55,16 +55,9 @@ impl Serializable for Magic {
 	}
 }
 
-impl Deserializable for Magic {
-	fn deserialize(reader: &mut Reader) -> Result<Self, ReaderError> where Self: Sized {
-		let magic: u32 = try!(reader.read());
-		Magic::from_u32(magic).map_err(|_| ReaderError::MalformedData)
-	}
-}
-
 #[cfg(test)]
 mod tests {
-	use common::Error;
+	use Error;
 	use super::{Magic, MAGIC_MAINNET, MAGIC_TESTNET};
 
 	#[test]

message/src/common/mod.rs

@@ -3,7 +3,6 @@ mod block_header_and_ids;
 mod block_transactions;
 mod block_transactions_request;
 mod command;
-mod error;
 mod inventory;
 mod ip;
 mod magic;
@@ -16,7 +15,6 @@ pub use self::block_header_and_ids::BlockHeaderAndIDs;
 pub use self::block_transactions::BlockTransactions;
 pub use self::block_transactions_request::BlockTransactionsRequest;
 pub use self::command::Command;
-pub use self::error::Error;
 pub use self::inventory::{InventoryVector, InventoryType};
 pub use self::ip::IpAddress;
 pub use self::magic::Magic;

message/src/error.rs

@@ -0,0 +1,21 @@
+use ser::Error as ReaderError;
+
+#[derive(Debug, PartialEq)]
+pub enum Error {
+	/// Deserialization failed.
+	Deserialize,
+	/// Command has wrong format or is unsupported.
+	InvalidCommand,
+	/// Network magic is not supported.
+	InvalidMagic,
+	/// Network magic comes from different network.
+	WrongMagic,
+	/// Invalid checksum.
+	InvalidChecksum,
+}
+
+impl From<ReaderError> for Error {
+	fn from(_: ReaderError) -> Self {
+		Error::Deserialize
+	}
+}

message/src/lib.rs

@@ -7,7 +7,9 @@ extern crate serialization as ser;
 pub mod common;
 mod message;
 pub mod types;
+mod error;
 
 pub use primitives::{hash, bytes};
 
-pub use message::{Message, MessageHeader, Payload, deserialize_payload};
+pub use message::{Message, MessageHeader, Payload};
+pub use error::Error;

message/src/message/message_header.rs

@@ -1,9 +1,7 @@
 use hash::H32;
-use ser::{
-	Serializable, Stream,
-	Deserializable, Reader, Error as ReaderError
-};
+use ser::{Serializable, Stream, Reader};
 use common::{Command, Magic};
+use Error;
 
 #[derive(Debug, PartialEq)]
 pub struct MessageHeader {
@@ -13,10 +11,21 @@ pub struct MessageHeader {
 	pub checksum: H32,
 }
 
-impl Deserializable for MessageHeader {
-	fn deserialize(reader: &mut Reader) -> Result<Self, ReaderError> where Self: Sized {
+impl MessageHeader {
+	pub fn deserialize(data: &[u8], expected: Magic) -> Result<Self, Error> {
+		if data.len() != 24 {
+			return Err(Error::Deserialize);
+		}
+
+		let mut reader = Reader::new(data);
+		let magic: u32 = try!(reader.read());
+		let magic = try!(Magic::from_u32(magic));
+		if expected != magic {
+			return Err(Error::WrongMagic);
+		}
+
 		let header = MessageHeader {
-			magic: try!(reader.read()),
+			magic: magic,
 			command: try!(reader.read()),
 			len: try!(reader.read()),
 			checksum: try!(reader.read()),
@@ -39,7 +48,7 @@ impl Serializable for MessageHeader {
 #[cfg(test)]
 mod tests {
 	use bytes::Bytes;
-	use ser::{serialize, deserialize};
+	use ser::serialize;
 	use common::Magic;
 	use super::MessageHeader;
 
@@ -66,6 +75,6 @@ mod tests {
 			checksum: "ed52399b".into(),
 		};
 
-		assert_eq!(expected, deserialize(&raw).unwrap());
+		assert_eq!(expected, MessageHeader::deserialize(&raw, Magic::Mainnet).unwrap());
 	}
 }

message/src/message/mod.rs

@@ -4,4 +4,4 @@ mod payload;
 
 pub use self::message::Message;
 pub use self::message_header::MessageHeader;
-pub use self::payload::{Payload, deserialize_payload};
+pub use self::payload::Payload;

message/src/message/payload.rs

@@ -1,8 +1,7 @@
-use ser::{
-	Serializable, Stream,
-	Error as ReaderError, deserialize
-};
+use hash::H32;
+use ser::{Serializable, Stream, deserialize};
 use chain::{Transaction, Block};
+use crypto::checksum;
 use common::Command;
 use types::{
 	Version, Addr, AddrBelow31402, Inv,
@@ -10,41 +9,7 @@ use types::{
 	Ping, Pong, Reject, FilterLoad, FilterAdd, FeeFilter,
 	MerkleBlock, SendCompact, CompactBlock, GetBlockTxn, BlockTxn,
 };
-
-pub fn deserialize_payload(data: &[u8], version: u32, command: &Command) -> Result<Payload, ReaderError> {
-	match &command.to_string() as &str {
-		"version" => deserialize(data).map(Payload::Version),
-		"verack" if data.is_empty() => Ok(Payload::Verack),
-		"addr" => match version >= 31402 {
-			true => deserialize(data).map(Payload::Addr),
-			false => deserialize(data).map(Payload::AddrBelow31402),
-		},
-		"inv" => deserialize(data).map(Payload::Inv),
-		"getdata" => deserialize(data).map(Payload::GetData),
-		"notfound" => deserialize(data).map(Payload::NotFound),
-		"getblocks" => deserialize(data).map(Payload::GetBlocks),
-		"getheaders" => deserialize(data).map(Payload::GetHeaders),
-		"tx" => deserialize(data).map(Payload::Tx),
-		"block" => deserialize(data).map(Payload::Block),
-		"headers" => deserialize(data).map(Payload::Headers),
-		"getaddr" if data.is_empty() => Ok(Payload::GetAddr),
-		"mempool" if data.is_empty() => Ok(Payload::MemPool),
-		"ping" => deserialize(data).map(Payload::Ping),
-		"pong" => deserialize(data).map(Payload::Pong),
-		"reject" => deserialize(data).map(Payload::Reject),
-		"filterload" => deserialize(data).map(Payload::FilterLoad),
-		"filteradd" => deserialize(data).map(Payload::FilterAdd),
-		"filterclear" if data.is_empty() => Ok(Payload::FilterClear),
-		"merkleblock" => deserialize(data).map(Payload::MerkleBlock),
-		"sendheaders" if data.is_empty() => Ok(Payload::SendHeaders),
-		"feefilter" => deserialize(data).map(Payload::FeeFilter),
-		"sendcmpct" => deserialize(data).map(Payload::SendCompact),
-		"cmpctblock" => deserialize(data).map(Payload::CompactBlock),
-		"getblocktxn" => deserialize(data).map(Payload::GetBlockTxn),
-		"blocktxn" => deserialize(data).map(Payload::BlockTxn),
-		_ => Err(ReaderError::MalformedData),
-	}
-}
+use Error;
 
 #[derive(Debug, PartialEq)]
 pub enum Payload {
@@ -111,6 +76,46 @@ impl Payload {
 		cmd.into()
 	}
 
+	pub fn deserialize(data: &[u8], check: &H32, version: u32, command: &Command) -> Result<Self, Error> {
+		if &checksum(data) != check {
+			return Err(Error::InvalidChecksum);
+		}
+
+		let result = match &command.to_string() as &str {
+			"version" => deserialize(data).map(Payload::Version),
+			"verack" if data.is_empty() => Ok(Payload::Verack),
+			"addr" => match version >= 31402 {
+				true => deserialize(data).map(Payload::Addr),
+				false => deserialize(data).map(Payload::AddrBelow31402),
+			},
+			"inv" => deserialize(data).map(Payload::Inv),
+			"getdata" => deserialize(data).map(Payload::GetData),
+			"notfound" => deserialize(data).map(Payload::NotFound),
+			"getblocks" => deserialize(data).map(Payload::GetBlocks),
+			"getheaders" => deserialize(data).map(Payload::GetHeaders),
+			"tx" => deserialize(data).map(Payload::Tx),
+			"block" => deserialize(data).map(Payload::Block),
+			"headers" => deserialize(data).map(Payload::Headers),
+			"getaddr" if data.is_empty() => Ok(Payload::GetAddr),
+			"mempool" if data.is_empty() => Ok(Payload::MemPool),
+			"ping" => deserialize(data).map(Payload::Ping),
+			"pong" => deserialize(data).map(Payload::Pong),
+			"reject" => deserialize(data).map(Payload::Reject),
+			"filterload" => deserialize(data).map(Payload::FilterLoad),
+			"filteradd" => deserialize(data).map(Payload::FilterAdd),
+			"filterclear" if data.is_empty() => Ok(Payload::FilterClear),
+			"merkleblock" => deserialize(data).map(Payload::MerkleBlock),
+			"sendheaders" if data.is_empty() => Ok(Payload::SendHeaders),
+			"feefilter" => deserialize(data).map(Payload::FeeFilter),
+			"sendcmpct" => deserialize(data).map(Payload::SendCompact),
+			"cmpctblock" => deserialize(data).map(Payload::CompactBlock),
+			"getblocktxn" => deserialize(data).map(Payload::GetBlockTxn),
+			"blocktxn" => deserialize(data).map(Payload::BlockTxn),
+			_ => return Err(Error::InvalidCommand),
+		};
+
+		result.map_err(Into::into)
+	}
 }
 
 impl Serializable for Payload {

p2p/src/error.rs

@@ -1,25 +1,11 @@
 use std::io;
-use ser::{Error as ReaderError};
+use message::Error as MessageError;
 
 #[derive(Debug)]
 pub enum Error {
 	Io(io::Error),
-	Data(ReaderError),
-	InvalidNetwork,
-	InvalidChecksum,
-	HandshakeFailed,
-}
-
-impl Error {
-	pub fn kind(&self) -> io::ErrorKind {
-		match *self {
-			Error::Io(ref e) => e.kind(),
-			Error::Data(_) |
-			Error::HandshakeFailed |
-			Error::InvalidChecksum |
-			Error::InvalidNetwork => io::ErrorKind::Other,
-		}
-	}
+	Message(MessageError),
+	Handshake,
 }
 
 impl From<io::Error> for Error {
@@ -28,9 +14,8 @@ impl From<io::Error> for Error {
 	}
 }
 
-impl From<ReaderError> for Error {
-	fn from(e: ReaderError) -> Self {
-		Error::Data(e)
+impl From<MessageError> for Error {
+	fn from(e: MessageError) -> Self {
+		Error::Message(e)
 	}
 }
-

p2p/src/io/handshake.rs

@@ -96,7 +96,7 @@ impl<A> Future for Handshake<A> where A: io::Read + io::Write {
 				let (stream, message) = try_ready!(future.poll());
 				let version = match message.payload {
 					Payload::Version(version) => version,
-					_ => return Err(Error::HandshakeFailed),
+					_ => return Err(Error::Handshake),
 				};
 
 				let next = HandshakeState::ReceiveVerack {
@@ -109,7 +109,7 @@ impl<A> Future for Handshake<A> where A: io::Read + io::Write {
 			HandshakeState::ReceiveVerack { ref mut version, ref mut future } => {
 				let (stream, message) = try_ready!(future.poll());
 				if message.payload != Payload::Verack {
-					return Err(Error::HandshakeFailed);
+					return Err(Error::Handshake);
 				}
 
 				let version = version.take().expect("verack must be preceded by version");
@@ -143,7 +143,7 @@ impl<A> Future for AcceptHandshake<A> where A: io::Read + io::Write {
 				let (stream, message) = try_ready!(future.poll());
 				let version = match message.payload {
 					Payload::Version(version) => version,
-					_ => return Err(Error::HandshakeFailed),
+					_ => return Err(Error::Handshake),
 				};
 
 				let local_version = local_version.take().expect("local version must be set");

p2p/src/io/read_header.rs

@@ -1,7 +1,6 @@
 use std::io;
 use futures::{Future, Poll, Async};
 use tokio_core::io::{ReadExact, read_exact};
-use ser::deserialize;
 use message::MessageHeader;
 use message::common::Magic;
 use Error;
@@ -24,10 +23,7 @@ impl<A> Future for ReadHeader<A> where A: io::Read {
 
 	fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
 		let (read, data) = try_ready!(self.reader.poll());
-		let header: MessageHeader = try!(deserialize(&data));
-		if header.magic != self.magic {
-			return Err(Error::InvalidNetwork);
-		}
+		let header= try!(MessageHeader::deserialize(&data, self.magic));
 		Ok(Async::Ready((read, header)))
 	}
 }

p2p/src/io/read_payload.rs

@@ -3,9 +3,8 @@ use futures::{Future, Poll, Async};
 use tokio_core::io::{ReadExact, read_exact};
 use bytes::Bytes;
 use hash::H32;
-use message::{Payload, deserialize_payload};
+use message::Payload;
 use message::common::Command;
-use crypto::checksum;
 use Error;
 
 pub fn read_payload<A>(a: A, version: u32, len: usize, command: Command, checksum: H32) -> ReadPayload<A> where A: io::Read {
@@ -30,10 +29,7 @@ impl<A> Future for ReadPayload<A> where A: io::Read {
 
 	fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
 		let (read, data) = try_ready!(self.reader.poll());
-		if self.checksum != checksum(&data) {
-			return Err(Error::InvalidChecksum);
-		}
-		let payload = try!(deserialize_payload(&data, self.version, &self.command));
+		let payload = try!(Payload::deserialize(&data, &self.checksum, self.version, &self.command));
 		Ok(Async::Ready((read, payload)))
 	}
 }