Protocol version type safety (#2811)

* add type safety for protocol_version

* cleanup tui for protocol version

* cleanup

api/src/types.rs

@@ -74,7 +74,7 @@ pub struct Status {
 impl Status {
 	pub fn from_tip_and_peers(current_tip: chain::Tip, connections: u32) -> Status {
 		Status {
-			protocol_version: p2p::msg::PROTOCOL_VERSION,
+			protocol_version: p2p::msg::ProtocolVersion::default().into(),
 			user_agent: p2p::msg::USER_AGENT.to_string(),
 			connections: connections,
 			tip: Tip::from_tip(current_tip),

p2p/src/handshake.rs

@@ -14,7 +14,7 @@
 
 use crate::core::core::hash::Hash;
 use crate::core::pow::Difficulty;
-use crate::msg::{read_message, write_message, Hand, Shake, Type, PROTOCOL_VERSION, USER_AGENT};
+use crate::msg::{read_message, write_message, Hand, ProtocolVersion, Shake, Type, USER_AGENT};
 use crate::peer::Peer;
 use crate::types::{Capabilities, Direction, Error, P2PConfig, PeerAddr, PeerInfo, PeerLiveInfo};
 use crate::util::RwLock;
@@ -73,7 +73,7 @@ impl Handshake {
 		};
 
 		let hand = Hand {
-			version: PROTOCOL_VERSION,
+			version: ProtocolVersion::default(),
 			capabilities: capab,
 			nonce: nonce,
 			genesis: self.genesis,
@@ -167,7 +167,7 @@ impl Handshake {
 
 		// send our reply with our info
 		let shake = Shake {
-			version: PROTOCOL_VERSION,
+			version: ProtocolVersion::default(),
 			capabilities: capab,
 			genesis: self.genesis,
 			total_difficulty: total_difficulty,

p2p/src/msg.rs

@@ -15,6 +15,7 @@
 //! Message types that transit over the network and related serialization code.
 
 use num::FromPrimitive;
+use std::fmt;
 use std::io::{Read, Write};
 use std::time;
 
@@ -36,7 +37,7 @@ use crate::util::read_write::read_exact;
 /// Note: A peer may disconnect and reconnect with an updated protocol version. Normally
 /// the protocol version will increase but we need to handle decreasing values also
 /// as a peer may rollback to previous version of the code.
-pub const PROTOCOL_VERSION: u32 = 1;
+const PROTOCOL_VERSION: u32 = 1;
 
 /// Grin's user agent with current version
 pub const USER_AGENT: &'static str = concat!("MW/Grin ", env!("CARGO_PKG_VERSION"));
@@ -245,11 +246,45 @@ impl Readable for MsgHeader {
 	}
 }
 
+#[derive(Clone, Copy, Debug, Deserialize, Eq, Ord, PartialOrd, PartialEq, Serialize)]
+pub struct ProtocolVersion(pub u32);
+
+impl Default for ProtocolVersion {
+	fn default() -> ProtocolVersion {
+		ProtocolVersion(PROTOCOL_VERSION)
+	}
+}
+
+impl From<ProtocolVersion> for u32 {
+	fn from(v: ProtocolVersion) -> u32 {
+		v.0
+	}
+}
+
+impl fmt::Display for ProtocolVersion {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		write!(f, "{}", self.0)
+	}
+}
+
+impl Writeable for ProtocolVersion {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		writer.write_u32(self.0)
+	}
+}
+
+impl Readable for ProtocolVersion {
+	fn read(reader: &mut dyn Reader) -> Result<ProtocolVersion, ser::Error> {
+		let version = reader.read_u32()?;
+		Ok(ProtocolVersion(version))
+	}
+}
+
 /// First part of a handshake, sender advertises its version and
 /// characteristics.
 pub struct Hand {
 	/// protocol version of the sender
-	pub version: u32,
+	pub version: ProtocolVersion,
 	/// capabilities of the sender
 	pub capabilities: Capabilities,
 	/// randomly generated for each handshake, helps detect self
@@ -269,9 +304,9 @@ pub struct Hand {
 
 impl Writeable for Hand {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		self.version.write(writer)?;
 		ser_multiwrite!(
 			writer,
-			[write_u32, self.version],
 			[write_u32, self.capabilities.bits()],
 			[write_u64, self.nonce]
 		);
@@ -286,23 +321,24 @@ impl Writeable for Hand {
 
 impl Readable for Hand {
 	fn read(reader: &mut dyn Reader) -> Result<Hand, ser::Error> {
-		let (version, capab, nonce) = ser_multiread!(reader, read_u32, read_u32, read_u64);
+		let version = ProtocolVersion::read(reader)?;
+		let (capab, nonce) = ser_multiread!(reader, read_u32, read_u64);
 		let capabilities = Capabilities::from_bits_truncate(capab);
-		let total_diff = Difficulty::read(reader)?;
+		let total_difficulty = Difficulty::read(reader)?;
 		let sender_addr = PeerAddr::read(reader)?;
 		let receiver_addr = PeerAddr::read(reader)?;
 		let ua = reader.read_bytes_len_prefix()?;
 		let user_agent = String::from_utf8(ua).map_err(|_| ser::Error::CorruptedData)?;
 		let genesis = Hash::read(reader)?;
 		Ok(Hand {
-			version: version,
+			version,
-			capabilities: capabilities,
+			capabilities,
-			nonce: nonce,
+			nonce,
-			genesis: genesis,
+			genesis,
-			total_difficulty: total_diff,
+			total_difficulty,
-			sender_addr: sender_addr,
+			sender_addr,
-			receiver_addr: receiver_addr,
+			receiver_addr,
-			user_agent: user_agent,
+			user_agent,
 		})
 	}
 }
@@ -311,7 +347,7 @@ impl Readable for Hand {
 /// version and characteristics.
 pub struct Shake {
 	/// sender version
-	pub version: u32,
+	pub version: ProtocolVersion,
 	/// sender capabilities
 	pub capabilities: Capabilities,
 	/// genesis block of our chain, only connect to peers on the same chain
@@ -325,11 +361,8 @@ pub struct Shake {
 
 impl Writeable for Shake {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
-		ser_multiwrite!(
+		self.version.write(writer)?;
-			writer,
+		writer.write_u32(self.capabilities.bits())?;
-			[write_u32, self.version],
-			[write_u32, self.capabilities.bits()]
-		);
 		self.total_difficulty.write(writer)?;
 		writer.write_bytes(&self.user_agent)?;
 		self.genesis.write(writer)?;
@@ -339,18 +372,21 @@ impl Writeable for Shake {
 
 impl Readable for Shake {
 	fn read(reader: &mut dyn Reader) -> Result<Shake, ser::Error> {
-		let (version, capab) = ser_multiread!(reader, read_u32, read_u32);
+		let version = ProtocolVersion::read(reader)?;
+
+		let capab = reader.read_u32()?;
 		let capabilities = Capabilities::from_bits_truncate(capab);
-		let total_diff = Difficulty::read(reader)?;
+
+		let total_difficulty = Difficulty::read(reader)?;
 		let ua = reader.read_bytes_len_prefix()?;
 		let user_agent = String::from_utf8(ua).map_err(|_| ser::Error::CorruptedData)?;
 		let genesis = Hash::read(reader)?;
 		Ok(Shake {
-			version: version,
+			version,
-			capabilities: capabilities,
+			capabilities,
-			genesis: genesis,
+			genesis,
-			total_difficulty: total_diff,
+			total_difficulty,
-			user_agent: user_agent,
+			user_agent,
 		})
 	}
 }

p2p/src/types.rs

@@ -30,6 +30,7 @@ use crate::core::core::hash::Hash;
 use crate::core::global;
 use crate::core::pow::Difficulty;
 use crate::core::ser::{self, Readable, Reader, Writeable, Writer};
+use crate::msg::ProtocolVersion;
 use grin_store;
 
 /// Maximum number of block headers a peer should ever send
@@ -370,7 +371,7 @@ pub struct PeerLiveInfo {
 pub struct PeerInfo {
 	pub capabilities: Capabilities,
 	pub user_agent: String,
-	pub version: u32,
+	pub version: ProtocolVersion,
 	pub addr: PeerAddr,
 	pub direction: Direction,
 	pub live_info: Arc<RwLock<PeerLiveInfo>>,
@@ -432,7 +433,7 @@ impl PeerInfo {
 pub struct PeerInfoDisplay {
 	pub capabilities: Capabilities,
 	pub user_agent: String,
-	pub version: u32,
+	pub version: ProtocolVersion,
 	pub addr: PeerAddr,
 	pub direction: Direction,
 	pub total_difficulty: Difficulty,
@@ -444,7 +445,7 @@ impl From<PeerInfo> for PeerInfoDisplay {
 		PeerInfoDisplay {
 			capabilities: info.capabilities.clone(),
 			user_agent: info.user_agent.clone(),
-			version: info.version.clone(),
+			version: info.version,
 			addr: info.addr.clone(),
 			direction: info.direction.clone(),
 			total_difficulty: info.total_difficulty(),

servers/src/common/stats.rs

@@ -147,7 +147,7 @@ pub struct PeerStats {
 	/// Address
 	pub addr: String,
 	/// version running
-	pub version: u32,
+	pub version: p2p::msg::ProtocolVersion,
 	/// Peer user agent string.
 	pub user_agent: String,
 	/// difficulty reported by peer
@@ -191,7 +191,7 @@ impl PeerStats {
 		PeerStats {
 			state: state.to_string(),
 			addr: addr,
-			version: peer.info.version.clone(),
+			version: peer.info.version,
 			user_agent: peer.info.user_agent.clone(),
 			total_difficulty: peer.info.total_difficulty().to_num(),
 			height: peer.info.height(),

servers/src/grin/server.rs

@@ -398,8 +398,8 @@ impl Server {
 	}
 
 	/// Current p2p layer protocol version.
-	pub fn protocol_version() -> u32 {
+	pub fn protocol_version() -> p2p::msg::ProtocolVersion {
-		p2p::msg::PROTOCOL_VERSION
+		p2p::msg::ProtocolVersion::default()
 	}
 
 	/// Returns a set of stats about this server. This and the ServerStats

src/bin/cmd/client.rs

@@ -74,7 +74,7 @@ pub fn show_status(config: &ServerConfig, api_secret: Option<String>) {
 	t.reset().unwrap();
 	match get_status_from_node(config, api_secret) {
 		Ok(status) => {
-			writeln!(e, "Protocol version: {}", status.protocol_version).unwrap();
+			writeln!(e, "Protocol version: {:?}", status.protocol_version).unwrap();
 			writeln!(e, "User agent: {}", status.user_agent).unwrap();
 			writeln!(e, "Connections: {}", status.connections).unwrap();
 			writeln!(e, "Chain height: {}", status.tip.height).unwrap();
@@ -139,7 +139,7 @@ pub fn list_connected_peers(config: &ServerConfig, api_secret: Option<String>) {
 				writeln!(e, "Peer {}:", index).unwrap();
 				writeln!(e, "Capabilities: {:?}", connected_peer.capabilities).unwrap();
 				writeln!(e, "User agent: {}", connected_peer.user_agent).unwrap();
-				writeln!(e, "Version: {}", connected_peer.version).unwrap();
+				writeln!(e, "Version: {:?}", connected_peer.version).unwrap();
 				writeln!(e, "Peer address: {}", connected_peer.addr).unwrap();
 				writeln!(e, "Height: {}", connected_peer.height).unwrap();
 				writeln!(e, "Total difficulty: {}", connected_peer.total_difficulty).unwrap();

src/bin/tui/peers.rs

@@ -81,7 +81,7 @@ impl TableViewItem<PeerColumn> for PeerStats {
 			)
 			.to_string(),
 			PeerColumn::Direction => self.direction.clone(),
-			PeerColumn::Version => self.version.to_string(),
+			PeerColumn::Version => format!("{}", self.version),
 			PeerColumn::UserAgent => self.user_agent.clone(),
 		}
 	}
@@ -129,7 +129,7 @@ impl TUIStatusListener for TUIPeerView {
 			.column(PeerColumn::TotalDifficulty, "Total Difficulty", |c| {
 				c.width_percent(24)
 			})
-			.column(PeerColumn::Version, "Ver", |c| c.width_percent(6))
+			.column(PeerColumn::Version, "Proto", |c| c.width_percent(6))
 			.column(PeerColumn::UserAgent, "User Agent", |c| c.width_percent(18));
 		let peer_status_view = BoxView::with_full_screen(
 			LinearLayout::new(Orientation::Vertical)

src/bin/tui/ui.rs

@@ -85,7 +85,7 @@ impl UI {
 		let mut title_string = StyledString::new();
 		title_string.append(StyledString::styled(
 			format!(
-				"Grin Version {} (protocol version: {})",
+				"Grin Version {} (proto: {})",
 				built_info::PKG_VERSION,
 				Server::protocol_version()
 			),