fix(node): use array for handshake commit hashes

Author: kaimast

node/bft/events/src/challenge_request.rs

@@ -16,25 +16,31 @@
 use super::*;
 
 use snarkos_node_network::built_info;
+use snarkvm::prelude::{FromBytes, ToBytes, io_error};
+
+use std::borrow::Cow;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ChallengeRequest<N: Network> {
     pub version: u32,
     pub listener_port: u16,
     pub address: Address<N>,
     pub nonce: u64,
-    pub snarkos_sha: String,
+    pub snarkos_sha: [u8; 40],
 }
 
 impl<N: Network> ChallengeRequest<N> {
-    /// Creates a new `ChallengeRequest` event.
+    pub const UNKNOWN_SHA: [u8; 40] = snarkos_node_network::UNKNOWN_SHA;
+
     pub fn new(listener_port: u16, address: Address<N>, nonce: u64) -> Self {
         Self {
             version: Event::<N>::VERSION,
             listener_port,
             address,
             nonce,
-            snarkos_sha: built_info::GIT_COMMIT_HASH.unwrap_or_default().into(),
+            snarkos_sha: built_info::GIT_COMMIT_HASH
+                .and_then(|s| s.as_bytes().try_into().ok())
+                .unwrap_or(Self::UNKNOWN_SHA),
         }
     }
 }
@@ -48,27 +54,25 @@ impl<N: Network> EventTrait for ChallengeRequest<N> {
 }
 
 impl<N: Network> ToBytes for ChallengeRequest<N> {
-    fn write_le<W: Write>(&self, mut writer: W) -> IoResult<()> {
+    fn write_le<W: Write>(&self, mut writer: W) -> io::Result<()> {
         self.version.write_le(&mut writer)?;
         self.listener_port.write_le(&mut writer)?;
         self.address.write_le(&mut writer)?;
         self.nonce.write_le(&mut writer)?;
-        self.snarkos_sha.as_bytes().write_le(&mut writer)?;
+        self.snarkos_sha.write_le(&mut writer)?;
 
         Ok(())
     }
 }
 
 impl<N: Network> FromBytes for ChallengeRequest<N> {
-    fn read_le<R: Read>(mut reader: R) -> IoResult<Self> {
+    fn read_le<R: Read>(mut reader: R) -> io::Result<Self> {
         let version = u32::read_le(&mut reader)?;
         let listener_port = u16::read_le(&mut reader)?;
         let address = Address::<N>::read_le(&mut reader)?;
         let nonce = u64::read_le(&mut reader)?;
-        let snarkos_sha = str::from_utf8(&<[u8; 40]>::read_le(&mut reader).unwrap_or([b'?'; 40])[..])
-            .map(|str| if str.starts_with('?') { "unknown" } else { str })
-            .map_err(|_| error("Invalid snarkOS SHA"))?
-            .to_owned();
+        let snarkos_sha =
+            <[u8; 40]>::read_le(&mut reader).map_err(|err| io_error(format!("Invalid snarkOS SHA - {err}")))?;
 
         Ok(Self { version, listener_port, address, nonce, snarkos_sha })
     }
@@ -102,21 +106,21 @@ pub mod prop_tests {
                 nonce,
                 version,
                 listener_port,
-                snarkos_sha: sha.into_iter().map(|b| b as char).collect(),
+                snarkos_sha: sha.try_into().unwrap(),
             })
             .boxed()
     }
 
     #[proptest]
-    fn serialize_deserialize(#[strategy(any_challenge_request())] original: ChallengeRequest<CurrentNetwork>) {
+    fn challenge_request_roundtrip(#[strategy(any_challenge_request())] original: ChallengeRequest<CurrentNetwork>) {
         let mut buf = BytesMut::default().writer();
         ChallengeRequest::write_le(&original, &mut buf).unwrap();
 
         let mut deserialized: ChallengeRequest<CurrentNetwork> =
             ChallengeRequest::read_le(buf.into_inner().reader()).unwrap();
         // Upon deserialization, unsupplied SHA is registered as "unknown".
-        if deserialized.snarkos_sha == "unknown" {
-            deserialized.snarkos_sha = original.snarkos_sha.clone();
+        if deserialized.snarkos_sha == snarkos_node_network::UNKNOWN_SHA {
+            deserialized.snarkos_sha = original.snarkos_sha;
         }
         assert_eq!(original, deserialized);
     }

node/bft/tests/gateway_e2e.rs

@@ -110,7 +110,7 @@ async fn handshake_responder_side_invalid_challenge_request() {
     let listener_port = test_peer.listening_addr().port();
     let address = accounts.get(1).unwrap().address();
     let nonce = rng.r#gen();
-    let snarkos_sha = String::new();
+    let snarkos_sha = ChallengeRequest::<CurrentNetwork>::UNKNOWN_SHA;
     // Set the wrong version so the challenge request is invalid.
     let challenge_request = ChallengeRequest { version: 0, listener_port, address, nonce, snarkos_sha };
 
@@ -150,7 +150,7 @@ async fn handshake_responder_side_invalid_challenge_response() {
     let listener_port = test_peer.listening_addr().port();
     let address = accounts.get(1).unwrap().address();
     let our_nonce = rng.r#gen();
-    let snarkos_sha = String::new();
+    let snarkos_sha = ChallengeRequest::<CurrentNetwork>::UNKNOWN_SHA;
     let version = Event::<CurrentNetwork>::VERSION;
     let challenge_request = ChallengeRequest { version, listener_port, address, nonce: our_nonce, snarkos_sha };
 

node/network/src/lib.rs

@@ -37,6 +37,9 @@ pub mod built_info {
     include!(concat!(env!("OUT_DIR"), "/built.rs"));
 }
 
+/// Constant for an unknown commit hash.
+pub const UNKNOWN_SHA: [u8; 40] = [b'?'; 40];
+
 /// Returns the list of bootstrap peers.
 #[allow(clippy::if_same_then_else)]
 pub fn bootstrap_peers<N: Network>(is_dev: bool) -> Vec<SocketAddr> {
@@ -80,16 +83,23 @@ pub fn bootstrap_peers<N: Network>(is_dev: bool) -> Vec<SocketAddr> {
 }
 
 /// Logs the peer's snarkOS repo SHA and how it compares to ours.
-pub fn log_repo_sha_comparison(peer_addr: SocketAddr, peer_sha: &str, ctx: &str) {
-    let our_sha = built_info::GIT_COMMIT_HASH.unwrap_or_default();
-    let sha_cmp = if peer_sha == "unknown" {
+pub fn log_repo_sha_comparison(peer_addr: SocketAddr, peer_sha: &[u8; 40], ctx: &str) {
+    // Get our SHA from the build information (or None if it is not set or does not 40 bytes long).
+    let our_sha: Option<[u8; 40]> = built_info::GIT_COMMIT_HASH.and_then(|sha| sha.as_bytes().try_into().ok());
+    let peer_sha_str = str::from_utf8(peer_sha).unwrap_or("");
+
+    let sha_cmp = if peer_sha == &UNKNOWN_SHA || peer_sha_str.is_empty() {
+        // Handle the case where the peer send no or an invalid SHA.
         " with an unknown repo SHA".to_owned()
-    } else if peer_sha == our_sha {
-        format!("@{peer_sha} (same as us)")
-    } else if our_sha.is_empty() {
-        format!("@{peer_sha} (potentially different than us)")
+    } else if let Some(our_sha) = &our_sha {
+        if peer_sha == our_sha {
+            format!("@{peer_sha_str} (same as us)")
+        } else {
+            format!("@{peer_sha_str} (different than us)")
+        }
     } else {
-        format!("@{peer_sha} (different than us)")
+        // Handle the case where our SHA is unknown.
+        format!("@{peer_sha_str} (potentially different than us)")
     };
 
     debug!("{ctx} Peer '{peer_addr}' uses snarkOS{sha_cmp}");

node/router/messages/src/challenge_request.rs

@@ -16,7 +16,7 @@
 use super::*;
 
 use snarkos_node_network::{NodeType, built_info};
-use snarkvm::prelude::{FromBytes, ToBytes};
+use snarkvm::prelude::{FromBytes, ToBytes, io_error};
 
 use std::borrow::Cow;
 
@@ -27,7 +27,7 @@ pub struct ChallengeRequest<N: Network> {
     pub node_type: NodeType,
     pub address: Address<N>,
     pub nonce: u64,
-    pub snarkos_sha: String,
+    pub snarkos_sha: [u8; 40],
 }
 
 impl<N: Network> MessageTrait for ChallengeRequest<N> {
@@ -45,7 +45,7 @@ impl<N: Network> ToBytes for ChallengeRequest<N> {
         self.node_type.write_le(&mut writer)?;
         self.address.write_le(&mut writer)?;
         self.nonce.write_le(&mut writer)?;
-        self.snarkos_sha.as_bytes().write_le(&mut writer)?;
+        self.snarkos_sha.write_le(&mut writer)?;
 
         Ok(())
     }
@@ -58,24 +58,26 @@ impl<N: Network> FromBytes for ChallengeRequest<N> {
         let node_type = NodeType::read_le(&mut reader)?;
         let address = Address::<N>::read_le(&mut reader)?;
         let nonce = u64::read_le(&mut reader)?;
-        let snarkos_sha = str::from_utf8(&<[u8; 40]>::read_le(&mut reader).unwrap_or([b'?'; 40])[..])
-            .map(|str| if str.starts_with('?') { "unknown" } else { str })
-            .map_err(|_| error("Invalid snarkOS SHA"))?
-            .to_owned();
+        let snarkos_sha =
+            <[u8; 40]>::read_le(&mut reader).map_err(|err| io_error(format!("Invalid snarkOS SHA - {err}")))?;
 
         Ok(Self { version, listener_port, node_type, address, nonce, snarkos_sha })
     }
 }
 
 impl<N: Network> ChallengeRequest<N> {
+    pub const UNKNOWN_SHA: [u8; 40] = snarkos_node_network::UNKNOWN_SHA;
+
     pub fn new(listener_port: u16, node_type: NodeType, address: Address<N>, nonce: u64) -> Self {
         Self {
             version: Message::<N>::latest_message_version(),
             listener_port,
             node_type,
             address,
             nonce,
-            snarkos_sha: built_info::GIT_COMMIT_HASH.unwrap_or_default().into(),
+            snarkos_sha: built_info::GIT_COMMIT_HASH
+                .and_then(|s| s.as_bytes().try_into().ok())
+                .unwrap_or(Self::UNKNOWN_SHA),
         }
     }
 }
@@ -121,7 +123,7 @@ pub mod prop_tests {
                 version,
                 listener_port,
                 node_type,
-                snarkos_sha: sha.into_iter().map(|b| b as char).collect(),
+                snarkos_sha: sha.try_into().unwrap(),
             })
             .boxed()
     }
@@ -134,8 +136,8 @@ pub mod prop_tests {
         let mut deserialized: ChallengeRequest<CurrentNetwork> =
             ChallengeRequest::read_le(buf.into_inner().reader()).unwrap();
         // Upon deserialization, unsupplied SHA is registered as "unknown".
-        if deserialized.snarkos_sha == "unknown" {
-            deserialized.snarkos_sha = original.snarkos_sha.clone();
+        if deserialized.snarkos_sha == snarkos_node_network::UNKNOWN_SHA {
+            deserialized.snarkos_sha = original.snarkos_sha;
         }
         assert_eq!(original, deserialized);
     }