Validator History: Fix gossip version

programs/validator-history/src/crds_value.rs

@@ -46,20 +46,110 @@ enum ClientId {
     Unknown(u16),
 }
 
-#[derive(Default, Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
-pub struct Version {
-    #[serde(with = "serde_varint")]
-    pub major: u16,
+// Mirrors the agave 4.x PackedMinor wire format from solana-version/src/v4.rs.
+// bits 14-15 of the packed u16 hold a prerelease tag (0=stable,1=rc,2=beta,3=alpha);
+// bits 0-13 hold the actual minor version.  For prerelease builds the wire
+// `patch` field carries the prerelease number; actual patch is always 0.
+#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
+#[serde(transparent)]
+struct PackedMinor(#[serde(with = "serde_varint")] u16);
+
+impl PackedMinor {
+    const PRERELEASE_BITS_OFFSET: u32 = 14;
+    const PRERELEASE_MASK: u16 = 3; // 2-bit tag occupying bits 14-15
+
+    // Returns (minor, patch) with prerelease encoding stripped.
+    fn try_unpack(self, patch: u16) -> Option<(u16, u16)> {
+        let Self(packed) = self;
+        let shifted = packed >> Self::PRERELEASE_BITS_OFFSET;
+        // Bits above the 2-bit tag are reserved and must be zero.
+        if shifted & !Self::PRERELEASE_MASK != 0 {
+            return None;
+        }
+        let prerelease_tag = shifted & Self::PRERELEASE_MASK;
+        let minor = packed & !(Self::PRERELEASE_MASK << Self::PRERELEASE_BITS_OFFSET);
+        // For any prerelease variant the wire `patch` is the prerelease number;
+        // the real patch is 0.
+        let patch = if prerelease_tag == 0 { patch } else { 0 };
+        Some((minor, patch))
+    }
+}
+
+// Internal struct that matches the on-the-wire layout used by agave 4.x.
+#[derive(Deserialize, Serialize)]
+struct SerializedVersion {
     #[serde(with = "serde_varint")]
-    pub minor: u16,
+    major: u16,
+
+    #[serde(rename = "minor")]
+    packed_minor: PackedMinor,
+
     #[serde(with = "serde_varint")]
-    pub patch: u16,
-    pub commit: u32,      // first 4 bytes of the sha1 commit hash
-    pub feature_set: u32, // first 4 bytes of the FeatureSet identifier
+    patch: u16,
+
+    commit: u32,
+
+    feature_set: u32,
+
     #[serde(with = "serde_varint")]
+    client: u16,
+}
+
+#[derive(Default, Debug, Clone, PartialEq, Eq)]
+pub struct Version {
+    pub major: u16,
+
+    pub minor: u16, // decoded actual minor
+
+    pub patch: u16, // decoded actual patch (0 for any prerelease build)
+
+    pub commit: u32,
+
+    pub feature_set: u32,
+
     pub client: u16,
 }
 
+impl Serialize for Version {
+    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        SerializedVersion {
+            major: self.major,
+            packed_minor: PackedMinor(self.minor),
+            patch: self.patch,
+            commit: self.commit,
+            feature_set: self.feature_set,
+            client: self.client,
+        }
+        .serialize(serializer)
+    }
+}
+
+impl<'de> Deserialize<'de> for Version {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let SerializedVersion {
+            major,
+            packed_minor,
+            patch,
+            commit,
+            feature_set,
+            client,
+        } = SerializedVersion::deserialize(deserializer)?;
+
+        let (minor, patch) = packed_minor
+            .try_unpack(patch)
+            .ok_or_else(|| serde::de::Error::custom("invalid PackedMinor: reserved bits set"))?;
+
+        Ok(Version {
+            major,
+            minor,
+            patch,
+            commit,
+            feature_set,
+            client,
+        })
+    }
+}
+
 impl Version {
     pub fn as_semver_version(&self) -> semver::Version {
         semver::Version::new(self.major as u64, self.minor as u64, self.patch as u64)
@@ -309,3 +399,148 @@ impl From<LegacyVersion1> for LegacyVersion2 {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // -----------------------------------------------------------------------
+    // PackedMinor::try_unpack
+    // -----------------------------------------------------------------------
+
+    #[test]
+    fn test_packed_minor_stable() {
+        // tag=0 → minor and patch pass through unchanged
+        assert_eq!(PackedMinor(0x0000).try_unpack(0), Some((0, 0)));
+        assert_eq!(PackedMinor(0x0000).try_unpack(5), Some((0, 5)));
+        assert_eq!(PackedMinor(0x3FFF).try_unpack(100), Some((0x3FFF, 100)));
+    }
+
+    #[test]
+    fn test_packed_minor_release_candidate() {
+        // tag=1 (bits 14-15 = 0b01 → 0x4000) → actual patch = 0
+        assert_eq!(PackedMinor(0x4000).try_unpack(1), Some((0, 0))); // minor=0, rc.1
+        assert_eq!(PackedMinor(0x4001).try_unpack(2), Some((1, 0))); // minor=1, rc.2
+        assert_eq!(PackedMinor(0x7FFF).try_unpack(42), Some((0x3FFF, 0)));
+    }
+
+    #[test]
+    fn test_packed_minor_beta() {
+        // tag=2 (bits 14-15 = 0b10 → 0x8000) → actual patch = 0
+        assert_eq!(PackedMinor(0x8000).try_unpack(3), Some((0, 0)));
+        assert_eq!(PackedMinor(0xBFFF).try_unpack(u16::MAX), Some((0x3FFF, 0)));
+    }
+
+    #[test]
+    fn test_packed_minor_alpha() {
+        // tag=3 (bits 14-15 = 0b11 → 0xC000) → actual patch = 0
+        assert_eq!(PackedMinor(0xC000).try_unpack(7), Some((0, 0)));
+        assert_eq!(PackedMinor(0xFFFF).try_unpack(u16::MAX), Some((0x3FFF, 0)));
+    }
+
+    // -----------------------------------------------------------------------
+    // Version deserialization — wire bytes
+    //
+    // Wire layout (bincode / LEB128 varint):
+    //   major       varint u16
+    //   minor       varint u16  (PackedMinor: bits 14-15 = prerelease tag)
+    //   patch       varint u16  (prerelease number for non-stable, real patch for stable)
+    //   commit      u32 LE
+    //   feature_set u32 LE
+    //   client      varint u16
+    // -----------------------------------------------------------------------
+
+    fn zero_suffix() -> [u8; 9] {
+        // commit(4) + feature_set(4) + client(1 varint zero) = 9 zero bytes
+        [0u8; 9]
+    }
+
+    #[test]
+    fn test_version_bytes_stable_zero() {
+        // 0.0.0 stable → 12 zero bytes
+        let bytes = [0u8; 12];
+        let v: Version = bincode::deserialize(&bytes).unwrap();
+        assert_eq!(v.major, 0);
+        assert_eq!(v.minor, 0);
+        assert_eq!(v.patch, 0);
+
+        let roundtrip = bincode::serialize(&v).unwrap();
+        assert_eq!(roundtrip, bytes);
+    }
+
+    #[test]
+    fn test_version_bytes_4_0_0_rc_1() {
+        // 4.0.0-rc.1 as broadcast by agave 4.x nodes:
+        //   major       = 4        → [0x04]
+        //   packed_minor= 16384    → LEB128 [0x80, 0x80, 0x01]  (0x4000, rc tag)
+        //   patch       = 1        → [0x01]                      (rc number)
+        //   commit/feat/client = 0 → [0x00 × 9]
+        let bytes: &[u8] = &[
+            0x04, // major = 4
+            0x80, 0x80, 0x01, // packed_minor = 16384 (0x4000, rc tag in bits 14-15)
+            0x01, // wire patch = 1 (rc.1 number)
+            0x00, 0x00, 0x00, 0x00, // commit = 0
+            0x00, 0x00, 0x00, 0x00, // feature_set = 0
+            0x00, // client = 0
+        ];
+        let v: Version = bincode::deserialize(bytes).unwrap();
+
+        // After PackedMinor decode: minor=0 (low 14 bits of 0x4000), patch=0 (prerelease)
+        assert_eq!(v.major, 4);
+        assert_eq!(v.minor, 0);
+        assert_eq!(v.patch, 0);
+    }
+
+    #[test]
+    fn test_version_bytes_4_0_1_stable() {
+        // 4.0.1 stable (to contrast with 4.0.0-rc.1 above):
+        //   major       = 4  → [0x04]
+        //   packed_minor= 0  → [0x00]   (no prerelease tag)
+        //   patch       = 1  → [0x01]   (real patch version)
+        let mut bytes = vec![0x04u8, 0x00, 0x01];
+        bytes.extend_from_slice(&zero_suffix());
+
+        let v: Version = bincode::deserialize(&bytes).unwrap();
+        assert_eq!(v.major, 4);
+        assert_eq!(v.minor, 0);
+        assert_eq!(v.patch, 1);
+    }
+
+    #[test]
+    fn test_version_bytes_4_0_0_stable() {
+        // 4.0.0 stable: same byte length as 4.0.1, just patch=0
+        let mut bytes = vec![0x04u8, 0x00, 0x00];
+        bytes.extend_from_slice(&zero_suffix());
+
+        let v: Version = bincode::deserialize(&bytes).unwrap();
+        assert_eq!(v.major, 4);
+        assert_eq!(v.minor, 0);
+        assert_eq!(v.patch, 0);
+    }
+
+    #[test]
+    fn test_version_rc_vs_stable_different_byte_lengths() {
+        // 4.0.0-rc.1 uses 14 bytes; 4.0.1 stable uses 12 bytes.
+        // The old code (raw u16 for minor) would store both as version 4.0.1,
+        // even though their wire representations are completely different.
+        let rc_bytes: &[u8] = &[
+            0x04, 0x80, 0x80, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        ];
+        let stable_bytes: &[u8] = &[
+            0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        ];
+
+        assert_eq!(rc_bytes.len(), 14);
+        assert_eq!(stable_bytes.len(), 12);
+
+        let rc: Version = bincode::deserialize(rc_bytes).unwrap();
+        let stable: Version = bincode::deserialize(stable_bytes).unwrap();
+
+        // rc.1 correctly decodes to 4.0.0, NOT 4.0.1
+        assert_eq!((rc.major, rc.minor, rc.patch), (4, 0, 0));
+        // stable 4.0.1 decodes to 4.0.1
+        assert_eq!((stable.major, stable.minor, stable.patch), (4, 0, 1));
+
+        assert_ne!(rc, stable);
+    }
+}