From 661e66d07879bd45e38f68c5540ae1cce55c4b68 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 8 Oct 2026 13:02:35 +0000 Subject: [PATCH 1/2] fix(crypto): deserialize legacy BLS public keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept both legacy hex strings and current raw bytes in binary Serde formats with matching string and byte-buffer layouts, including bincode. Keep current serialization and native bincode codecs unchanged. Cover the old format with a frozen fixture from 40268cc, malformed input, length limits, alternate bincode configuration and nested values. 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent Co-Authored-By: Codex GPT-6 --- CHANGELOG.md | 6 + crypto/Cargo.toml | 1 + crypto/src/bls.rs | 8 +- crypto/src/bls/public_key_bytes.rs | 130 ++++++++++++++++++ crypto/tests/data/legacy-serde/README.md | 21 +++ .../data/legacy-serde/bls-public-key.bin | 1 + crypto/tests/legacy_bls_serde.rs | 96 +++++++++++++ 7 files changed, 258 insertions(+), 5 deletions(-) create mode 100644 crypto/src/bls/public_key_bytes.rs create mode 100644 crypto/tests/data/legacy-serde/README.md create mode 100644 crypto/tests/data/legacy-serde/bls-public-key.bin create mode 100644 crypto/tests/legacy_bls_serde.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index eaf3fbc3a..df7d1bb2e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/crypto/Cargo.toml b/crypto/Cargo.toml index e1868a5a0..09c716b6f 100644 --- a/crypto/Cargo.toml +++ b/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" } diff --git a/crypto/src/bls.rs b/crypto/src/bls.rs index 7255828a6..5c621bd04 100644 --- a/crypto/src/bls.rs +++ b/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")] diff --git a/crypto/src/bls/public_key_bytes.rs b/crypto/src/bls/public_key_bytes.rs new file mode 100644 index 000000000..d58427e92 --- /dev/null +++ b/crypto/src/bls/public_key_bytes.rs @@ -0,0 +1,130 @@ +//! 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 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 { + 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 { + 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(&self, serializer: S) -> Result { + self.0.serialize(serializer) + } +} + +#[cfg(feature = "serde")] +impl<'de> serde::Deserialize<'de> for BlsPkBytes { + fn deserialize>(deserializer: D) -> Result { + 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_bytes(self, bytes: &[u8]) -> Result { + 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_bytes(Visitor) + } +} diff --git a/crypto/tests/data/legacy-serde/README.md b/crypto/tests/data/legacy-serde/README.md new file mode 100644 index 000000000..9a4417b4a --- /dev/null +++ b/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. diff --git a/crypto/tests/data/legacy-serde/bls-public-key.bin b/crypto/tests/data/legacy-serde/bls-public-key.bin new file mode 100644 index 000000000..65ca4202a --- /dev/null +++ b/crypto/tests/data/legacy-serde/bls-public-key.bin @@ -0,0 +1 @@ +`000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f \ No newline at end of file diff --git a/crypto/tests/legacy_bls_serde.rs b/crypto/tests/legacy_bls_serde.rs new file mode 100644 index 000000000..ee099371d --- /dev/null +++ b/crypto/tests/legacy_bls_serde.rs @@ -0,0 +1,96 @@ +#![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!(¤t[1..], expected.as_bytes()); + let (key, consumed): (BlsPkBytes, _) = + bincode::serde::decode_from_slice(¤t, bincode::config::standard()).unwrap(); + assert_eq!(key, expected); + assert_eq!(consumed, current.len()); + assert_eq!( + serde_json::from_str::(&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::( + &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::( + &bytes[..end], + bincode::config::standard() + ) + .is_err()); + } + assert!(bincode::serde::decode_from_slice::( + 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()); +} From 40e7b24c46100496a279f34952b851d40aa93a5f Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 8 Oct 2026 13:03:51 +0000 Subject: [PATCH 2/2] fix(crypto): retain binary Serde string visitor support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preserve string callbacks and the byte-buffer deserializer hint supported by the existing byte type. Cover binary deserializers delivering hex text. 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent Co-Authored-By: Codex GPT-6 --- crypto/src/bls/public_key_bytes.rs | 6 +++++- crypto/tests/legacy_bls_serde.rs | 31 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/crypto/src/bls/public_key_bytes.rs b/crypto/src/bls/public_key_bytes.rs index d58427e92..d67da7044 100644 --- a/crypto/src/bls/public_key_bytes.rs +++ b/crypto/src/bls/public_key_bytes.rs @@ -112,6 +112,10 @@ impl<'de> serde::Deserialize<'de> for BlsPkBytes { f.write_str("48 raw bytes or 96 ASCII hex digits for a BLS public key") } + fn visit_str(self, hex: &str) -> Result { + hex.parse().map_err(E::custom) + } + fn visit_bytes(self, bytes: &[u8]) -> Result { match bytes.len() { BLS_PK_LEN => BlsPkBytes::try_from(bytes).map_err(E::custom), @@ -125,6 +129,6 @@ impl<'de> serde::Deserialize<'de> for BlsPkBytes { } } - deserializer.deserialize_bytes(Visitor) + deserializer.deserialize_byte_buf(Visitor) } } diff --git a/crypto/tests/legacy_bls_serde.rs b/crypto/tests/legacy_bls_serde.rs index ee099371d..71ab6e47b 100644 --- a/crypto/tests/legacy_bls_serde.rs +++ b/crypto/tests/legacy_bls_serde.rs @@ -94,3 +94,34 @@ fn should_decode_legacy_key_between_other_fields() { 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>( + self, + visitor: V, + ) -> Result { + 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 = ::deserialize(BinaryString(&hex)).unwrap(); + assert_eq!(key.as_bytes(), &[0xab; 48]); + assert!(::deserialize(BinaryString("invalid")).is_err()); +}