From 580f1f1e913ac15cf663eb026171f6f682e48a9e Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 19 Dec 2025 12:20:37 -0500 Subject: [PATCH 1/4] chore(crypto): CRP-1031 Remove manual implementations of Eq/PartialEq This was required in old versions of Rust that didn't yet support const generics, but these days we can just derive Eq/PartialEq and things work as we'd like. --- .../crypto_lib/basic_sig/ed25519/src/types.rs | 2 +- .../ed25519/src/types/generic_traits.rs | 8 --- .../crypto_lib/bls12_381/type/src/poly.rs | 4 +- .../multi_sig/bls12_381/src/types.rs | 16 +++-- .../bls12_381/src/types/generic_traits.rs | 72 ------------------- .../src/types/{generic_traits => }/tests.rs | 0 .../threshold_sig/bls12_381/src/types.rs | 4 +- .../bls12_381/src/types/generic_traits.rs | 14 ---- .../crypto_lib/types/src/sign/eddsa.rs | 8 +-- .../src/sign/threshold_sig/public_key.rs | 10 +-- 10 files changed, 17 insertions(+), 121 deletions(-) delete mode 100644 rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/generic_traits.rs rename rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/{generic_traits => }/tests.rs (100%) diff --git a/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types.rs b/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types.rs index 69dd5fa3496c..8fcb19cfcc85 100644 --- a/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types.rs +++ b/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types.rs @@ -24,7 +24,7 @@ impl PublicKeyBytes { } /// A wrapper for Ed25519 signature bytes. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct SignatureBytes(pub [u8; SignatureBytes::SIZE]); ic_crypto_internal_types::derive_serde!(SignatureBytes, SignatureBytes::SIZE); impl SignatureBytes { diff --git a/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types/generic_traits.rs b/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types/generic_traits.rs index 11c53a43a69d..6a94b4feb580 100644 --- a/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types/generic_traits.rs +++ b/rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types/generic_traits.rs @@ -11,7 +11,6 @@ use std::fmt; #[cfg(test)] mod tests; -// Note: This is needed because Rust doesn't support const generics yet. impl fmt::Debug for SignatureBytes { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "SignatureBytes({:?})", base64::encode(&self.0[..])) @@ -22,10 +21,3 @@ impl fmt::Debug for PublicKeyBytes { write!(f, "PublicKeyBytes({:?})", base64::encode(&self.0[..])) } } - -impl PartialEq for SignatureBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } -} -impl Eq for SignatureBytes {} diff --git a/rs/crypto/internal/crypto_lib/bls12_381/type/src/poly.rs b/rs/crypto/internal/crypto_lib/bls12_381/type/src/poly.rs index 3dd74bfbb80b..27128bb69236 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/type/src/poly.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/type/src/poly.rs @@ -5,13 +5,11 @@ use rand::{CryptoRng, RngCore}; /// /// The coefficients are stored in little-endian ordering, ie a_0 is /// self.coefficients\[0\] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq)] pub struct Polynomial { coefficients: Vec, } -impl Eq for Polynomial {} - impl PartialEq for Polynomial { fn eq(&self, other: &Self) -> bool { // Accept leading zero elements diff --git a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs index fe62c00fe0dd..a286a3fb6fb0 100644 --- a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs +++ b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs @@ -9,7 +9,6 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; pub mod arbitrary; pub mod conversions; -mod generic_traits; /// A BLS secret key is a field element. pub type SecretKey = Scalar; @@ -41,8 +40,15 @@ impl SecretKeyBytes { } } +// Note: This is needed to keep sensitive material from getting Debug logged. +impl std::fmt::Debug for SecretKeyBytes { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "REDACTED") + } +} + /// Wrapper for a serialized individual signature. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub struct IndividualSignatureBytes(pub [u8; IndividualSignatureBytes::SIZE]); ic_crypto_internal_types::derive_serde!(IndividualSignatureBytes, IndividualSignatureBytes::SIZE); impl IndividualSignatureBytes { @@ -50,7 +56,7 @@ impl IndividualSignatureBytes { } /// Wrapper for a serialized proof of possession. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub struct PopBytes(pub [u8; PopBytes::SIZE]); ic_crypto_internal_types::derive_serde!(PopBytes, PopBytes::SIZE); impl PopBytes { @@ -58,7 +64,7 @@ impl PopBytes { } /// Wrapper for a serialized combined signature. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub struct CombinedSignatureBytes(pub [u8; CombinedSignatureBytes::SIZE]); ic_crypto_internal_types::derive_serde!(CombinedSignatureBytes, CombinedSignatureBytes::SIZE); impl CombinedSignatureBytes { @@ -66,7 +72,7 @@ impl CombinedSignatureBytes { } /// Wrapper for a serialized public key. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub struct PublicKeyBytes(pub [u8; PublicKeyBytes::SIZE]); ic_crypto_internal_types::derive_serde!(PublicKeyBytes, PublicKeyBytes::SIZE); impl PublicKeyBytes { diff --git a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/generic_traits.rs b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/generic_traits.rs deleted file mode 100644 index 53386cc4516a..000000000000 --- a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/generic_traits.rs +++ /dev/null @@ -1,72 +0,0 @@ -//! These are boilerplate implementations of standard traits that either: -//! - cannot be auto-generated in the normal way because Rust doesn't have const -//! generics yet. -//! - keep sensitive information from being logged via Debug. -//! -//! This code is in a separate file to avoid cluttering the types file with -//! implementation details. - -use super::*; -use std::fmt; - -#[cfg(test)] -mod tests; - -// Note: This is needed to keep sensitive material from getting Debug logged. -impl fmt::Debug for SecretKeyBytes { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "REDACTED") - } -} - -// Note: This is needed because Rust doesn't support const generics yet. -impl fmt::Debug for PublicKeyBytes { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", &self.0[..]) - } -} -impl PartialEq for PublicKeyBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } -} -impl Eq for PublicKeyBytes {} - -// Note: This is needed because Rust doesn't support const generics yet. -impl fmt::Debug for IndividualSignatureBytes { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", base64::encode(&self.0[..])) - } -} -impl PartialEq for IndividualSignatureBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } -} -impl Eq for IndividualSignatureBytes {} - -// Note: This is needed because Rust doesn't support const generics yet. -impl fmt::Debug for PopBytes { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", &self.0[..]) - } -} -impl PartialEq for PopBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } -} -impl Eq for PopBytes {} - -// Note: This is needed because Rust doesn't support const generics yet. -impl fmt::Debug for CombinedSignatureBytes { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", base64::encode(&self.0[..])) - } -} -impl PartialEq for CombinedSignatureBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } -} -impl Eq for CombinedSignatureBytes {} diff --git a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/generic_traits/tests.rs b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/tests.rs similarity index 100% rename from rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/generic_traits/tests.rs rename to rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/tests.rs diff --git a/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types.rs b/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types.rs index 5026d00f82b8..46eb5fcc08aa 100644 --- a/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types.rs +++ b/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types.rs @@ -27,7 +27,7 @@ pub(super) type Signature = G1Affine; pub(super) type IndividualSignature = Signature; /// A serialized individual BLS signature. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct IndividualSignatureBytes(pub [u8; G1Affine::BYTES]); ic_crypto_internal_types::derive_serde!(IndividualSignatureBytes, IndividualSignatureBytes::SIZE); impl IndividualSignatureBytes { @@ -37,7 +37,7 @@ impl IndividualSignatureBytes { pub(super) type CombinedSignature = Signature; /// A serialized combined (threshold-signed) BLS signature. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct CombinedSignatureBytes(pub [u8; G1Affine::BYTES]); ic_crypto_internal_types::derive_serde!(CombinedSignatureBytes, CombinedSignatureBytes::SIZE); impl CombinedSignatureBytes { diff --git a/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types/generic_traits.rs b/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types/generic_traits.rs index 125598fa5ab9..17e19581b16e 100644 --- a/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types/generic_traits.rs +++ b/rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types/generic_traits.rs @@ -12,31 +12,17 @@ use std::fmt; #[cfg(test)] mod tests; -// Note: This is needed because Rust doesn't support const generics yet. impl fmt::Debug for IndividualSignatureBytes { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{:?}", base64::encode(&self.0[..])) } } -impl PartialEq for IndividualSignatureBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } -} -impl Eq for IndividualSignatureBytes {} -// Note: This is needed because Rust doesn't support const generics yet. impl fmt::Debug for CombinedSignatureBytes { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{:?}", base64::encode(&self.0[..])) } } -impl PartialEq for CombinedSignatureBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } -} -impl Eq for CombinedSignatureBytes {} // Note: This is needed to keep sensitive material from getting Debug logged. impl fmt::Debug for SecretKeyBytes { diff --git a/rs/crypto/internal/crypto_lib/types/src/sign/eddsa.rs b/rs/crypto/internal/crypto_lib/types/src/sign/eddsa.rs index 3c9c0fefc46b..af3dbbf9e261 100644 --- a/rs/crypto/internal/crypto_lib/types/src/sign/eddsa.rs +++ b/rs/crypto/internal/crypto_lib/types/src/sign/eddsa.rs @@ -79,7 +79,7 @@ pub mod ed25519 { } /// An Ed25519 signature. - #[derive(Copy, Clone)] + #[derive(Copy, Clone, Eq, PartialEq)] pub struct Signature(pub [u8; Signature::SIZE]); crate::derive_serde!(Signature, Signature::SIZE); @@ -92,12 +92,6 @@ pub mod ed25519 { write!(f, "Signature(0x{hex_sig})") } } - impl PartialEq for Signature { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } - } - impl Eq for Signature {} impl Hash for Signature { fn hash(&self, state: &mut H) { self.0[..].hash(state); diff --git a/rs/crypto/internal/crypto_lib/types/src/sign/threshold_sig/public_key.rs b/rs/crypto/internal/crypto_lib/types/src/sign/threshold_sig/public_key.rs index fbdc01a6c568..23e8f3d4fd36 100644 --- a/rs/crypto/internal/crypto_lib/types/src/sign/threshold_sig/public_key.rs +++ b/rs/crypto/internal/crypto_lib/types/src/sign/threshold_sig/public_key.rs @@ -65,7 +65,7 @@ pub mod bls12_381 { use thiserror::Error; /// A BLS12-381 public key as bytes. - #[derive(Copy, Clone)] + #[derive(Copy, Clone, Eq, PartialEq)] pub struct PublicKeyBytes(pub [u8; PublicKeyBytes::SIZE]); crate::derive_serde!(PublicKeyBytes, PublicKeyBytes::SIZE); @@ -100,14 +100,6 @@ pub mod bls12_381 { } } - impl PartialEq for PublicKeyBytes { - fn eq(&self, other: &Self) -> bool { - self.0[..] == other.0[..] - } - } - - impl Eq for PublicKeyBytes {} - impl Ord for PublicKeyBytes { fn cmp(&self, other: &Self) -> Ordering { self.0.cmp(&other.0) From 978a49ce081989b6082eefc22b0177434cbc1d3b Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 19 Dec 2025 14:47:02 -0500 Subject: [PATCH 2/4] Bring back base64 Debug impls --- .../multi_sig/bls12_381/src/types.rs | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs index a286a3fb6fb0..728593bb5980 100644 --- a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs +++ b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs @@ -4,6 +4,7 @@ use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine, Scalar}; use ic_crypto_secrets_containers::SecretArray; use serde::{Deserialize, Serialize}; use zeroize::{Zeroize, ZeroizeOnDrop}; +use std::fmt; #[cfg(test)] pub mod arbitrary; @@ -41,40 +42,64 @@ impl SecretKeyBytes { } // Note: This is needed to keep sensitive material from getting Debug logged. -impl std::fmt::Debug for SecretKeyBytes { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Debug for SecretKeyBytes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "REDACTED") } } /// Wrapper for a serialized individual signature. -#[derive(Copy, Clone, Eq, PartialEq, Debug)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct IndividualSignatureBytes(pub [u8; IndividualSignatureBytes::SIZE]); ic_crypto_internal_types::derive_serde!(IndividualSignatureBytes, IndividualSignatureBytes::SIZE); impl IndividualSignatureBytes { pub const SIZE: usize = G1Affine::BYTES; } +impl fmt::Debug for IndividualSignatureBytes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", base64::encode(&self.0[..])) + } +} + /// Wrapper for a serialized proof of possession. -#[derive(Copy, Clone, Eq, PartialEq, Debug)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct PopBytes(pub [u8; PopBytes::SIZE]); ic_crypto_internal_types::derive_serde!(PopBytes, PopBytes::SIZE); impl PopBytes { pub const SIZE: usize = G1Affine::BYTES; } +impl fmt::Debug for PopBytes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", &self.0[..]) + } +} + /// Wrapper for a serialized combined signature. -#[derive(Copy, Clone, Eq, PartialEq, Debug)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct CombinedSignatureBytes(pub [u8; CombinedSignatureBytes::SIZE]); ic_crypto_internal_types::derive_serde!(CombinedSignatureBytes, CombinedSignatureBytes::SIZE); impl CombinedSignatureBytes { pub const SIZE: usize = G1Affine::BYTES; } +impl fmt::Debug for CombinedSignatureBytes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", base64::encode(&self.0[..])) + } +} + /// Wrapper for a serialized public key. -#[derive(Copy, Clone, Eq, PartialEq, Debug)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct PublicKeyBytes(pub [u8; PublicKeyBytes::SIZE]); ic_crypto_internal_types::derive_serde!(PublicKeyBytes, PublicKeyBytes::SIZE); impl PublicKeyBytes { pub const SIZE: usize = G2Affine::BYTES; } + +impl fmt::Debug for PublicKeyBytes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", &self.0[..]) + } +} From 520defae44201e571b686dffa8de7152db7f5e36 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 19 Dec 2025 14:47:11 -0500 Subject: [PATCH 3/4] fmt --- rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs index 728593bb5980..9d2be6e7decc 100644 --- a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs +++ b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs @@ -3,8 +3,8 @@ use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine, Scalar}; use ic_crypto_secrets_containers::SecretArray; use serde::{Deserialize, Serialize}; -use zeroize::{Zeroize, ZeroizeOnDrop}; use std::fmt; +use zeroize::{Zeroize, ZeroizeOnDrop}; #[cfg(test)] pub mod arbitrary; From 3c93b64f32b98c186f43090657ef7ddd1e489f84 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Mon, 5 Jan 2026 12:19:16 -0500 Subject: [PATCH 4/4] Add missing mod tests --- rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs index 9d2be6e7decc..cd935c9dce92 100644 --- a/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs +++ b/rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types.rs @@ -9,6 +9,9 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; #[cfg(test)] pub mod arbitrary; +#[cfg(test)] +pub mod tests; + pub mod conversions; /// A BLS secret key is a field element.