fix(crypto): deserialize legacy BLS public keys

CHANGELOG.md

@@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## Unreleased
 
+### Fixed
+
+- BLS public keys accept legacy hex-string encodings when deserialized through
+  binary Serde formats with compatible string and byte-buffer layouts, such as
+  bincode. Serialization continues to use the current raw-byte encoding.
+
 ### Changed
 
 - **Breaking:** the `bincode` feature and binary serialization dependencies now use

crypto/Cargo.toml

@@ -38,4 +38,5 @@ tracing = { version = "0.1", optional = true }
 unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)', 'cfg(fuzzing)', 'cfg(kani)'] }
 
 [dev-dependencies]
+bincode = { workspace = true, features = ["serde"] }
 serde_json = { version = "1.0" }

crypto/src/bls.rs

@@ -22,6 +22,9 @@ use thiserror::Error as ThisError;
 #[cfg(feature = "bls")]
 use tracing::error;
 
+mod public_key_bytes;
+pub use public_key_bytes::BlsPkBytes;
+
 /// Raw BLS public key length (G1 compressed).
 pub const BLS_PK_LEN: usize = 48;
 
@@ -79,11 +82,6 @@ fn reduce(bytes: &[u8; BLS_SK_LEN]) -> Result<[u8; BLS_SK_LEN], BlsError> {
     Ok(out)
 }
 
-make_bytes! {
-    /// BLS public key (48 bytes, unvalidated).
-    BlsPkBytes, BLS_PK_LEN
-}
-
 impl BlsPkBytes {
     /// Pairs these bytes with `scheme`.
     #[cfg(feature = "bls")]

crypto/src/bls/public_key_bytes.rs

@@ -0,0 +1,134 @@
+//! BLS public key bytes with compatibility for persisted binary Serde keys.
+
+use core::{array::TryFromSliceError, fmt, str::FromStr};
+
+use dash_types::{impl_bytes, type_cvrt, type_id::TypeId, ParseHexError};
+
+use super::BLS_PK_LEN;
+
+// Keep dash-types' byte API, formatting and codecs; only Serde decoding differs.
+mod raw {
+    use super::BLS_PK_LEN;
+
+    dash_types::make_bytes! {
+        /// BLS public key bytes.
+        BlsPkBytes, BLS_PK_LEN
+    }
+}
+
+/// BLS public key (48 bytes, unvalidated).
+#[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash, TypeId)]
+pub struct BlsPkBytes(raw::BlsPkBytes);
+
+impl BlsPkBytes {
+    /// Wraps raw bytes without validation.
+    pub const fn from_bytes(bytes: [u8; BLS_PK_LEN]) -> Self {
+        Self(raw::BlsPkBytes::from_bytes(bytes))
+    }
+
+    /// Copies out the inner byte array.
+    pub const fn to_bytes(&self) -> [u8; BLS_PK_LEN] {
+        self.0.to_bytes()
+    }
+
+    /// Borrows the inner byte array.
+    pub const fn as_bytes(&self) -> &[u8; BLS_PK_LEN] {
+        self.0.as_bytes()
+    }
+
+    /// Returns `true` when every byte is zero.
+    pub fn is_null(&self) -> bool {
+        self.0.is_null()
+    }
+}
+
+impl_bytes!(BlsPkBytes, BLS_PK_LEN);
+type_cvrt!(From<[u8; BLS_PK_LEN]> for BlsPkBytes, |bytes| Self::from_bytes(*bytes));
+
+impl From<BlsPkBytes> for [u8; BLS_PK_LEN] {
+    fn from(key: BlsPkBytes) -> Self {
+        key.to_bytes()
+    }
+}
+
+impl TryFrom<&[u8]> for BlsPkBytes {
+    type Error = TryFromSliceError;
+
+    fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
+        raw::BlsPkBytes::try_from(bytes).map(Self)
+    }
+}
+
+impl AsRef<[u8]> for BlsPkBytes {
+    fn as_ref(&self) -> &[u8] {
+        self.as_bytes()
+    }
+}
+
+impl AsRef<[u8; BLS_PK_LEN]> for BlsPkBytes {
+    fn as_ref(&self) -> &[u8; BLS_PK_LEN] {
+        self.as_bytes()
+    }
+}
+
+impl FromStr for BlsPkBytes {
+    type Err = ParseHexError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        s.parse().map(Self)
+    }
+}
+
+macro_rules! delegate_format {
+    ($($trait:ident),+ $(,)?) => {$(
+        impl fmt::$trait for BlsPkBytes {
+            fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+                fmt::$trait::fmt(&self.0, f)
+            }
+        }
+    )+};
+}
+delegate_format!(Display, Debug, LowerHex, UpperHex);
+
+#[cfg(feature = "serde")]
+impl serde::Serialize for BlsPkBytes {
+    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        self.0.serialize(serializer)
+    }
+}
+
+#[cfg(feature = "serde")]
+impl<'de> serde::Deserialize<'de> for BlsPkBytes {
+    fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        if deserializer.is_human_readable() {
+            return raw::BlsPkBytes::deserialize(deserializer).map(Self);
+        }
+
+        struct Visitor;
+        impl serde::de::Visitor<'_> for Visitor {
+            type Value = BlsPkBytes;
+
+            fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+                f.write_str("48 raw bytes or 96 ASCII hex digits for a BLS public key")
+            }
+
+            fn visit_str<E: serde::de::Error>(self, hex: &str) -> Result<Self::Value, E> {
+                hex.parse().map_err(E::custom)
+            }
+
+            fn visit_bytes<E: serde::de::Error>(self, bytes: &[u8]) -> Result<Self::Value, E> {
+                match bytes.len() {
+                    BLS_PK_LEN => BlsPkBytes::try_from(bytes).map_err(E::custom),
+                    // Binary Serde strings and byte buffers share a length prefix in bincode.
+                    len if len == 2 * BLS_PK_LEN => {
+                        let hex = core::str::from_utf8(bytes).map_err(E::custom)?;
+                        BlsPkBytes::from_hex(hex).map_err(E::custom)
+                    }
+                    len => Err(E::invalid_length(len, &self)),
+                }
+            }
+        }
+
+        deserializer.deserialize_byte_buf(Visitor)
+    }
+}

crypto/tests/data/legacy-serde/README.md

@@ -0,0 +1,21 @@
+# Legacy BLS binary Serde fixture
+
+`bls-public-key.bin` was generated using rust-dashcore revision
+`40268cc0402a8933ec539f16b2d634c4e25876ad`, before the BLS migration in #1036.
+It encodes the synthetic public-key bytes `0x00` through `0x2f` as a
+length-prefixed, 96-character hex string. It is a serialization fixture,
+not a validated cryptographic point.
+
+To reproduce, use a standalone Cargo package with `dashcore` pointing at that
+exact revision with its `serde` feature enabled, and
+`bincode = { package = "grovedb-bincode", version = "=2.1.0", features = ["serde"] }`:
+
+```rust
+let key = dashcore::bls_sig_utils::BLSPublicKey::from(
+    std::array::from_fn::<_, 48, _>(|i| i as u8),
+);
+let bytes = bincode::serde::encode_to_vec(key, bincode::config::standard()).unwrap();
+std::fs::write("bls-public-key.bin", bytes).unwrap();
+```
+
+Do not regenerate this fixture with current types.

crypto/tests/data/legacy-serde/bls-public-key.bin

@@ -0,0 +1 @@
+`000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f

crypto/tests/legacy_bls_serde.rs

@@ -0,0 +1,127 @@
+#![cfg(all(feature = "serde", feature = "bincode"))]
+
+use dashcore_crypto::bls::BlsPkBytes;
+
+#[test]
+fn should_decode_both_bls_public_key_formats() {
+    let legacy = include_bytes!("data/legacy-serde/bls-public-key.bin");
+    let expected = BlsPkBytes::from_bytes(std::array::from_fn(|i| i as u8));
+    let (key, consumed): (BlsPkBytes, _) =
+        bincode::serde::decode_from_slice(legacy, bincode::config::standard()).unwrap();
+    assert_eq!(key, expected);
+    assert_eq!(consumed, legacy.len());
+    let current = bincode::serde::encode_to_vec(key, bincode::config::standard()).unwrap();
+    assert_eq!(current[0], 48);
+    assert_eq!(&current[1..], expected.as_bytes());
+    let (key, consumed): (BlsPkBytes, _) =
+        bincode::serde::decode_from_slice(&current, bincode::config::standard()).unwrap();
+    assert_eq!(key, expected);
+    assert_eq!(consumed, current.len());
+    assert_eq!(
+        serde_json::from_str::<BlsPkBytes>(&serde_json::to_string(&key).unwrap()).unwrap(),
+        key
+    );
+}
+
+#[test]
+fn should_reject_malformed_legacy_bls_public_keys() {
+    for bytes in [vec![b'g'; 96], vec![0xff; 96], vec![b'0'; 95], vec![b'0'; 97], vec![]] {
+        let encoded = bincode::serde::encode_to_vec(bytes, bincode::config::standard()).unwrap();
+        assert!(bincode::serde::decode_from_slice::<BlsPkBytes, _>(
+            &encoded,
+            bincode::config::standard()
+        )
+        .is_err());
+    }
+}
+
+#[test]
+fn should_not_interpret_raw_bls_public_key_as_hex() {
+    let bytes = [b'a'; 48];
+    let encoded =
+        bincode::serde::encode_to_vec(bytes.as_slice(), bincode::config::standard()).unwrap();
+    let (key, _): (BlsPkBytes, _) =
+        bincode::serde::decode_from_slice(&encoded, bincode::config::standard()).unwrap();
+    assert_eq!(key.as_bytes(), &bytes);
+}
+
+#[test]
+fn should_decode_legacy_bls_with_fixed_big_endian_lengths() {
+    let config = bincode::config::standard().with_fixed_int_encoding().with_big_endian();
+    let key = BlsPkBytes::from_bytes([0xab; 48]);
+    let bytes = bincode::serde::encode_to_vec(key.to_string().to_uppercase(), config).unwrap();
+    let (decoded, consumed): (BlsPkBytes, _) =
+        bincode::serde::decode_from_slice(&bytes, config).unwrap();
+    assert_eq!(decoded, key);
+    assert_eq!(consumed, bytes.len());
+}
+
+#[test]
+fn should_preserve_native_bincode_public_key_encoding() {
+    let key = BlsPkBytes::from_bytes(std::array::from_fn(|i| i as u8));
+    let bytes = bincode::encode_to_vec(key, bincode::config::standard()).unwrap();
+    assert_eq!(bytes, key.as_bytes());
+    let (decoded, consumed): (BlsPkBytes, _) =
+        bincode::decode_from_slice(&bytes, bincode::config::standard()).unwrap();
+    assert_eq!(decoded, key);
+    assert_eq!(consumed, 48);
+}
+
+#[test]
+fn should_reject_truncated_keys_and_respect_limits() {
+    let bytes = include_bytes!("data/legacy-serde/bls-public-key.bin");
+    for end in 0..bytes.len() {
+        assert!(bincode::serde::decode_from_slice::<BlsPkBytes, _>(
+            &bytes[..end],
+            bincode::config::standard()
+        )
+        .is_err());
+    }
+    assert!(bincode::serde::decode_from_slice::<BlsPkBytes, _>(
+        bytes,
+        bincode::config::standard().with_limit::<16>()
+    )
+    .is_err());
+}
+
+#[test]
+fn should_decode_legacy_key_between_other_fields() {
+    let key = BlsPkBytes::from_bytes([0xab; 48]);
+    let config = bincode::config::standard();
+    let bytes = bincode::serde::encode_to_vec((7_u32, key.to_string(), 1234_u64), config).unwrap();
+    let (decoded, consumed): ((u32, BlsPkBytes, u64), _) =
+        bincode::serde::decode_from_slice(&bytes, config).unwrap();
+    assert_eq!(decoded, (7, key, 1234));
+    assert_eq!(consumed, bytes.len());
+}
+
+#[test]
+fn should_accept_binary_deserializers_that_deliver_strings() {
+    struct BinaryString<'a>(&'a str);
+
+    impl<'de> serde::Deserializer<'de> for BinaryString<'de> {
+        type Error = serde::de::value::Error;
+
+        fn is_human_readable(&self) -> bool {
+            false
+        }
+
+        fn deserialize_any<V: serde::de::Visitor<'de>>(
+            self,
+            visitor: V,
+        ) -> Result<V::Value, Self::Error> {
+            visitor.visit_borrowed_str(self.0)
+        }
+
+        serde::forward_to_deserialize_any! {
+            bool i8 i16 i32 i64 u8 u16 u32 u64 f32 f64 char str string
+            bytes byte_buf option unit unit_struct newtype_struct seq tuple
+            tuple_struct map struct enum identifier ignored_any
+        }
+    }
+
+    let hex = "ab".repeat(48);
+    let key = <BlsPkBytes as serde::Deserialize>::deserialize(BinaryString(&hex)).unwrap();
+    assert_eq!(key.as_bytes(), &[0xab; 48]);
+    assert!(<BlsPkBytes as serde::Deserialize>::deserialize(BinaryString("invalid")).is_err());
+}