refactor(network/capabilities): small improvements

* Add smoke tests for `Compression`.
* Clarify documentation.
* Fix `toggle()` if called multiple times.

elfo-network/src/socket/capabilities/compression.rs

@@ -1,56 +1,54 @@
-// Layouts are specified from highest to lowest bits.
-
 use std::fmt;
 
 use crate::config::Preference;
 
 bitflags::bitflags! {
-    /// Set of algorithms. 24 bits.
+    /// Set of algorithms.
     #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-    pub(crate) struct Algorithms: u32 {
+    pub(crate) struct Algorithms: u8 {
         const LZ4 = 1;
-        // NB: Shift by 2: `const ZSTD = 1 << 2;`.
+
+        #[cfg(test)] // not implemented yet, but useful for tests
+        const ZSTD = 1 << 2;
     }
 }
 
 /// Compression capabilities.
-///
-/// Layout:
-/// ```text
-///       22        2
-/// ┌────────────┬─────┐
-/// │  Reserved  │ Lz4 │
-/// └────────────┴─────┘
-/// ```
-///
-/// Each mentioned algorithm here occupies two bits for a reason, here's the
-/// layout of those bits:
-/// ```text
-///       1           1
-/// ┌───────────┬───────────┐
-/// │ Preferred │ Supported │
-/// └───────────┴───────────┘
-/// ```
-///
-/// 1. Preferred - the compression algorithm is preferred, implies `Supported`.
-/// 2. Supported - the compression algorithm is supported.
+// ~
+// Layout:
+// ```text
+//    6 bits   2 bits
+// ┌──────────┬─────┐
+// │ Reserved │ Lz4 │
+// └──────────┴─────┘
+// ```
+//
+// Each mentioned algorithm here occupies two bits for a reason, here's the
+// layout of those bits:
+// ```text
+//     1 bit       1 bit
+// ┌───────────┬───────────┐
+// │ Preferred │ Supported │
+// └───────────┴───────────┘
+// ```
+//
+// 1. Preferred — the compression algorithm is preferred, implies `Supported`.
+// 2. Supported — the compression algorithm is supported.
 #[derive(Debug, Clone, Copy)]
-pub(crate) struct Compression(u32);
+#[cfg_attr(test, derive(PartialEq))]
+pub(crate) struct Compression(u8);
 
 impl Compression {
     pub(crate) const fn empty() -> Self {
         Self::new(Algorithms::empty(), Algorithms::empty())
     }
 
-    pub(super) const fn from_bits_unchecked(v: u32) -> Self {
+    pub(super) const fn from_bits(v: u8) -> Self {
         Self(v)
     }
 
-    pub(crate) const fn from_bits_truncate(v: u32) -> Self {
-        let supported = Algorithms::from_bits_truncate(v);
-        let preferred = Algorithms::from_bits_truncate(v >> 1);
-
-        Self::new(supported, preferred)
+    pub(crate) const fn into_bits(self) -> u8 {
+        self.0
     }
 
     pub(crate) const fn new(supported: Algorithms, preferred: Algorithms) -> Self {
@@ -62,20 +60,20 @@ impl Compression {
         // 1 0 1 0 | Preferred
         // -------
         // 1 1 1 1
-        let joined = supported | (preferred << 1);
-
-        Self(joined)
+        Self(supported | (preferred << 1))
     }
 
     pub(crate) fn toggle(&mut self, algos: Algorithms, pref: Preference) {
-        let preferred = self.preferred();
-        let supported = self.supported();
+        let mut preferred = self.preferred() - algos;
+        let mut supported = self.supported() - algos;
 
-        *self = match pref {
-            Preference::Preferred => Self::new(supported, preferred | algos),
-            Preference::Supported => Self::new(supported | algos, preferred),
-            Preference::Disabled => *self,
+        match pref {
+            Preference::Preferred => preferred |= algos,
+            Preference::Supported => supported |= algos,
+            Preference::Disabled => {}
         };
+
+        *self = Self::new(supported, preferred);
     }
 
     pub(crate) fn intersection(self, rhs: Self) -> Self {
@@ -93,20 +91,14 @@ impl Compression {
 
         Self::new(supported, preferred)
     }
-}
-
-impl Compression {
-    pub(crate) const fn bits(self) -> u32 {
-        self.0
-    }
 
     pub(crate) const fn supported(self) -> Algorithms {
-        // `preferred` bits would be discarded.
+        // `Preferred` bits would be discarded.
         Algorithms::from_bits_truncate(self.0)
     }
 
     pub(crate) const fn preferred(self) -> Algorithms {
-        // `supported` bits would be discarded.
+        // `Supported` bits would be discarded.
         Algorithms::from_bits_truncate(self.0 >> 1)
     }
 }
@@ -134,3 +126,73 @@ impl fmt::Display for Compression {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn toggle() {
+        let mut compression = Compression::empty();
+        assert!(!compression.preferred().contains(Algorithms::LZ4));
+        assert!(!compression.supported().contains(Algorithms::LZ4));
+        assert!(!compression.preferred().contains(Algorithms::ZSTD));
+        assert!(!compression.supported().contains(Algorithms::ZSTD));
+
+        compression.toggle(Algorithms::ZSTD, Preference::Preferred);
+        assert!(compression.preferred().contains(Algorithms::ZSTD));
+        assert!(compression.supported().contains(Algorithms::ZSTD));
+
+        for _ in 0..2 {
+            compression.toggle(Algorithms::LZ4, Preference::Preferred);
+            assert!(compression.preferred().contains(Algorithms::LZ4));
+            assert!(compression.supported().contains(Algorithms::LZ4));
+
+            compression.toggle(Algorithms::LZ4, Preference::Supported);
+            assert!(!compression.preferred().contains(Algorithms::LZ4));
+            assert!(compression.supported().contains(Algorithms::LZ4));
+
+            compression.toggle(Algorithms::LZ4, Preference::Disabled);
+            assert!(!compression.preferred().contains(Algorithms::LZ4));
+            assert!(!compression.supported().contains(Algorithms::LZ4));
+        }
+
+        assert!(compression.preferred().contains(Algorithms::ZSTD));
+        assert!(compression.supported().contains(Algorithms::ZSTD));
+    }
+
+    #[test]
+    fn intersection() {
+        let a = Compression::empty();
+        let b = Compression::new(Algorithms::LZ4, Algorithms::LZ4);
+        assert_eq!(a.intersection(b), Compression::empty());
+
+        // Some algorithm is preferred by both nodes.
+        let a = Compression::new(Algorithms::LZ4, Algorithms::ZSTD | Algorithms::LZ4);
+        let b = Compression::new(Algorithms::LZ4, Algorithms::ZSTD);
+        let s = a.intersection(b);
+        assert_eq!(s.preferred(), Algorithms::ZSTD);
+        assert_eq!(s.supported(), Algorithms::LZ4 | Algorithms::ZSTD);
+
+        // A preferred algo is not supported by the other node.
+        let a = Compression::new(Algorithms::LZ4, Algorithms::ZSTD);
+        let b = Compression::new(Algorithms::LZ4, Algorithms::empty());
+        let s = a.intersection(b);
+        assert_eq!(s.preferred(), Algorithms::empty());
+        assert_eq!(s.supported(), Algorithms::LZ4);
+
+        // Some node prefer an algo supported by the other node.
+        let a = Compression::new(Algorithms::LZ4, Algorithms::LZ4);
+        let b = Compression::new(Algorithms::LZ4, Algorithms::empty());
+        let s = a.intersection(b);
+        assert_eq!(s.preferred(), Algorithms::LZ4);
+        assert_eq!(s.supported(), Algorithms::LZ4);
+
+        // No common supported algorithms.
+        let a = Compression::new(Algorithms::LZ4, Algorithms::empty());
+        let b = Compression::new(Algorithms::ZSTD, Algorithms::empty());
+        let s = a.intersection(b);
+        assert_eq!(s.preferred(), Algorithms::empty());
+        assert_eq!(s.supported(), Algorithms::empty());
+    }
+}

elfo-network/src/socket/capabilities/mod.rs

@@ -5,46 +5,40 @@ use self::compression::Compression;
 pub(crate) mod compression;
 
 /// Things supported by the node.
-///
-/// ### Layout
-///
-/// ```text
-///         24 bits              8 bits
-/// ┌─────────────────────┬──────────────────┐
-/// │     Compression     │     Reserved     │
-/// └─────────────────────┴──────────────────┘
-/// ```
-///
-/// 1. [`Compression`] - compression capabilities.
+// ~
+// Layout:
+// ```text
+//      16 bits       8 bits         8 bits
+// ┌──────────────┬─────────────┬──────────────┐
+// │   Reserved   │ Compression │   Reserved   │
+// └──────────────┴─────────────┴──────────────┘
+// ```
+//
+// 1. [`Compression`] - compression capabilities.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct Capabilities(u32);
 
 impl Capabilities {
-    pub(crate) const fn new(compression: Compression) -> Self {
-        let compression = compression.bits();
-        let joined = compression << 8;
+    pub(crate) fn new(compression: Compression) -> Self {
+        let compression = compression.into_bits();
+        Self(u32::from(compression) << 8)
+    }
 
-        Self(joined)
+    pub(crate) const fn from_bits(bits: u32) -> Self {
+        Self(bits)
     }
 
-    pub(crate) const fn from_bits_truncate(bits: u32) -> Self {
-        let compression = Compression::from_bits_truncate(bits >> 8);
-        Self::new(compression)
+    pub(crate) const fn into_bits(self) -> u32 {
+        self.0
     }
 
     pub(crate) fn intersection(self, rhs: Self) -> Self {
         let compr = self.compression().intersection(rhs.compression());
         Self::new(compr)
     }
-}
 
-impl Capabilities {
     pub(crate) const fn compression(self) -> Compression {
-        Compression::from_bits_unchecked(self.0 >> 8)
-    }
-
-    pub(crate) const fn bits(self) -> u32 {
-        self.0
+        Compression::from_bits((self.0 >> 8) as u8)
     }
 }
 
@@ -64,7 +58,7 @@ mod tests {
     #[test]
     fn format_is_compatible_with_020alpha17() {
         let caps = Capabilities::new(Compression::new(Algorithms::LZ4, Algorithms::empty()));
-        let lz4_bit = caps.bits() & (1 << 8);
+        let lz4_bit = caps.into_bits() & (1 << 8);
 
         assert_eq!(lz4_bit, 1 << 8);
     }
@@ -81,8 +75,8 @@ mod tests {
 
             // Just in case we should decode same caps.
 
-            let bits = caps.bits();
-            let same_caps = Capabilities::from_bits_truncate(bits);
+            let bits = caps.into_bits();
+            let same_caps = Capabilities::from_bits(bits);
 
             assert_eq!(caps, same_caps);
         }
@@ -109,8 +103,8 @@ mod tests {
     proptest! {
         #[test]
         fn intersection_is_commutative(lhs in prop::num::u32::ANY, rhs in prop::num::u32::ANY) {
-            let lhs = Capabilities::from_bits_truncate(lhs);
-            let rhs = Capabilities::from_bits_truncate(rhs);
+            let lhs = Capabilities::from_bits(lhs);
+            let rhs = Capabilities::from_bits(rhs);
             prop_assert_eq!(lhs.intersection(rhs), rhs.intersection(lhs));
         }
     }

elfo-network/src/socket/handshake.rs

@@ -46,7 +46,7 @@ impl Handshake {
         buf.write_u8(self.version)?;
         buf.write_u16::<LittleEndian>(self.node_no.into_bits())?;
         buf.write_u64::<LittleEndian>(self.launch_id.into_bits())?;
-        buf.write_u32::<LittleEndian>(self.capabilities.bits())?;
+        buf.write_u32::<LittleEndian>(self.capabilities.into_bits())?;
 
         let result = buf.into_inner();
         debug_assert_eq!(result.len(), HANDSHAKE_LENGTH);
@@ -73,7 +73,7 @@ impl Handshake {
             node_no: NodeNo::from_bits(input.read_u16::<LittleEndian>()?)
                 .ok_or_else(|| eyre!("invalid node no"))?,
             launch_id: NodeLaunchId::from_bits(input.read_u64::<LittleEndian>()?),
-            capabilities: Capabilities::from_bits_truncate(input.read_u32::<LittleEndian>()?),
+            capabilities: Capabilities::from_bits(input.read_u32::<LittleEndian>()?),
         };
 
         Ok(result)

elfo-network/src/socket/mod.rs

@@ -304,7 +304,7 @@ mod tests {
 
     use super::*;
 
-    const EMPTY_CAPS: Capabilities = Capabilities::new(Compression::empty());
+    const EMPTY_CAPS: Capabilities = Capabilities::from_bits(0);
 
     #[message]
     #[derive(PartialEq)]