From db19b9304da8ccf83b6f11c690f0a38731f44db5 Mon Sep 17 00:00:00 2001 From: Quang Le Date: Fri, 1 May 2026 15:36:00 +0700 Subject: [PATCH 1/7] fix: address comments and reviews in FROST impl --- Cargo.lock | 3 + Cargo.toml | 1 + crates/frost/Cargo.toml | 3 + crates/frost/src/curve.rs | 25 ++++ crates/frost/src/frost_core.rs | 53 ++++--- crates/frost/src/kryptology.rs | 121 ++++++++++------ .../kryptology_interop_tests.rs} | 14 +- crates/frost/src/lib.rs | 5 +- crates/frost/src/tests.rs | 133 +++++++++++++++++- crates/frost/tests/kryptology_round_trip.rs | 4 +- 10 files changed, 279 insertions(+), 83 deletions(-) rename crates/frost/{tests/kryptology_interop.rs => src/kryptology_interop_tests.rs} (94%) diff --git a/Cargo.lock b/Cargo.lock index fd33866e..5ddb53ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5700,6 +5700,9 @@ dependencies = [ "serde", "serde_json", "sha2", + "subtle", + "thiserror 2.0.18", + "zeroize", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 23d7af85..9eec693d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,7 @@ cipher = "0.4.4" pbkdf2 = "0.12.2" sha2 = "0.10.9" scrypt = "0.11.0" +subtle = "2.6" unicode-normalization = "0.1.25" zeroize = "1.8.2" uuid = { version = "1.19", features = ["serde", "v4"] } diff --git a/crates/frost/Cargo.toml b/crates/frost/Cargo.toml index 9a5192aa..554c30f7 100644 --- a/crates/frost/Cargo.toml +++ b/crates/frost/Cargo.toml @@ -10,6 +10,9 @@ publish.workspace = true blst.workspace = true rand_core.workspace = true sha2.workspace = true +subtle.workspace = true +thiserror.workspace = true +zeroize = { workspace = true, features = ["derive"] } [dev-dependencies] hex.workspace = true diff --git a/crates/frost/src/curve.rs b/crates/frost/src/curve.rs index 70ebd413..6e8f2da9 100644 --- a/crates/frost/src/curve.rs +++ b/crates/frost/src/curve.rs @@ -12,6 +12,8 @@ use std::{ use blst::*; use rand_core::{CryptoRng, RngCore}; +use subtle::ConstantTimeEq; +use zeroize::Zeroize; /// BLS12-381 scalar field element. Wrapper around `blst_fr` in Montgomery form. #[derive(Copy, Clone, Default, PartialEq, Eq)] @@ -78,6 +80,17 @@ impl Scalar { Scalar(fr) } + /// Reduce big-endian bytes modulo the scalar field order. + pub(crate) fn from_be_bytes_wide(bytes: &[u8]) -> Self { + let mut scalar = blst_scalar::default(); + let mut fr = blst_fr::default(); + unsafe { + blst_scalar_from_be_bytes(&mut scalar, bytes.as_ptr(), bytes.len()); + blst_fr_from_scalar(&mut fr, &scalar); + } + Scalar(fr) + } + /// Generate a uniformly random scalar. pub fn random(rng: &mut R) -> Self { let mut wide = [0u8; 64]; @@ -94,6 +107,17 @@ impl Scalar { unsafe { blst_fr_eucl_inverse(&mut out, &self.0) }; Some(Scalar(out)) } + + /// Compare scalar limbs without early-exit equality. + pub(crate) fn constant_time_eq(&self, other: &Self) -> bool { + self.0.l.ct_eq(&other.0.l).into() + } +} + +impl Zeroize for Scalar { + fn zeroize(&mut self) { + self.0.l.zeroize(); + } } impl From for Scalar { @@ -214,6 +238,7 @@ impl Mul for G1Projective { let mut out = blst_p1::default(); unsafe { blst_scalar_from_fr(&mut scalar, &rhs.0); + // BLS12-381 scalar field order has 255 significant bits. blst_p1_mult(&mut out, &self.0, scalar.b.as_ptr(), 255); } G1Projective(out) diff --git a/crates/frost/src/frost_core.rs b/crates/frost/src/frost_core.rs index b01f230a..afe36e9b 100644 --- a/crates/frost/src/frost_core.rs +++ b/crates/frost/src/frost_core.rs @@ -4,29 +4,34 @@ //! Contains the key material types (identifiers, shares, packages) and the //! polynomial evaluation functions needed by the kryptology-compatible DKG. -#![allow(clippy::arithmetic_side_effects)] - use std::{ cmp::Ordering, collections::{BTreeMap, BTreeSet}, }; use super::*; +use zeroize::ZeroizeOnDrop; /// Errors from key operations. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum FrostCoreError { /// Participant ID is zero. + #[error("participant ID is zero")] InvalidZeroScalar, /// Invalid number of minimum signers (must be >= 2 and <= max_signers). + #[error("invalid minimum signer count")] InvalidMinSigners, /// Invalid number of maximum signers (must be >= 2). + #[error("invalid maximum signer count")] InvalidMaxSigners, /// The secret share verification (Feldman VSS) failed. + #[error("invalid secret share")] InvalidSecretShare, /// Commitment count mismatch during aggregation. + #[error("incorrect number of commitments")] IncorrectNumberOfCommitments, /// The commitment has no coefficients. + #[error("incorrect commitment")] IncorrectCommitment, } @@ -34,7 +39,10 @@ pub enum FrostCoreError { /// /// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/identifier.rs#L14-L26 #[derive(Copy, Clone, Debug)] -pub struct Identifier(Scalar); +pub struct Identifier { + id: u32, + scalar: Scalar, +} impl Identifier { /// Create a new identifier from a non-zero u32. @@ -43,19 +51,24 @@ impl Identifier { if scalar == Scalar::ZERO { Err(FrostCoreError::InvalidZeroScalar) } else { - Ok(Self(scalar)) + Ok(Self { id, scalar }) } } + /// Return the raw participant ID. + pub fn to_u32(&self) -> u32 { + self.id + } + /// Return the underlying scalar. pub fn to_scalar(&self) -> Scalar { - self.0 + self.scalar } } impl PartialEq for Identifier { fn eq(&self, other: &Self) -> bool { - self.0 == other.0 + self.id == other.id } } @@ -69,18 +82,9 @@ impl PartialOrd for Identifier { // See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/identifier.rs#L121-L137 impl Ord for Identifier { - /// Compare identifiers by their numeric scalar value, using big-endian byte - /// order. Serializes to little-endian, and compares in reverse order. + /// Compare identifiers by their original participant ID. fn cmp(&self, other: &Self) -> Ordering { - let a = self.0.to_bytes(); - let b = other.0.to_bytes(); - for i in (0..32).rev() { - match a[i].cmp(&b[i]) { - Ordering::Equal => continue, - other => return other, - } - } - Ordering::Equal + self.id.cmp(&other.id) } } @@ -134,7 +138,7 @@ impl VerifiableSecretSharingCommitment { /// A secret scalar value representing a signer's share of the group secret. /// /// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/keys.rs#L82-L87 -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug, ZeroizeOnDrop)] pub struct SigningShare(Scalar); impl SigningShare { @@ -153,6 +157,7 @@ impl SigningShare { Self::new(evaluate_polynomial(peer, coefficients)) } } + /// A public group element that represents a single signer's public /// verification share. /// @@ -242,6 +247,7 @@ impl SecretShare { /// Checks that `G * signing_share == evaluate_vss(identifier, commitment)`. /// /// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/keys.rs#L431-L468 + #[allow(clippy::arithmetic_side_effects)] pub fn verify(&self) -> Result<(), FrostCoreError> { let f_result = G1Projective::generator() * self.signing_share.to_scalar(); let result = evaluate_vss(self.identifier, &self.commitment); @@ -257,12 +263,16 @@ impl SecretShare { /// A key package containing all key material for a participant. /// /// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/keys.rs#L617-L643 -#[derive(Debug)] +#[derive(Debug, ZeroizeOnDrop)] pub struct KeyPackage { + #[zeroize(skip)] identifier: Identifier, signing_share: SigningShare, + #[zeroize(skip)] verifying_share: VerifyingShare, + #[zeroize(skip)] verifying_key: VerifyingKey, + #[zeroize(skip)] min_signers: u16, } @@ -378,6 +388,7 @@ impl PublicKeyPackage { /// `a_0 + a_1 * x + a_2 * x^2 + ... + a_{t-1} * x^{t-1}`. /// /// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/keys.rs#L573-L595 +#[allow(clippy::arithmetic_side_effects)] fn evaluate_polynomial(identifier: Identifier, coefficients: &[Scalar]) -> Scalar { let mut value = Scalar::ZERO; let x = identifier.to_scalar(); @@ -398,6 +409,7 @@ fn evaluate_polynomial(identifier: Identifier, coefficients: &[Scalar]) -> Scala /// Computes `sum_{k=0}^{t-1} commitment[k] * identifier^k`. /// /// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/keys.rs#L597-L615 +#[allow(clippy::arithmetic_side_effects)] fn evaluate_vss( identifier: Identifier, commitment: &VerifiableSecretSharingCommitment, @@ -420,6 +432,7 @@ fn evaluate_vss( /// elements across all participants. /// /// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/keys.rs#L35-L62 +#[allow(clippy::arithmetic_side_effects)] fn sum_commitments( commitments: &[&VerifiableSecretSharingCommitment], ) -> Result { diff --git a/crates/frost/src/kryptology.rs b/crates/frost/src/kryptology.rs index 865a2414..80932859 100644 --- a/crates/frost/src/kryptology.rs +++ b/crates/frost/src/kryptology.rs @@ -9,49 +9,59 @@ //! The output types ([`KeyPackage`], [`PublicKeyPackage`]) are standard //! frost-core types usable with frost-core's signing protocol. -#![allow(clippy::arithmetic_side_effects)] - use std::collections::BTreeMap; use blst::*; use rand_core::{CryptoRng, RngCore}; use sha2::{Digest, Sha256}; +use zeroize::ZeroizeOnDrop; use super::*; /// Errors from the kryptology-compatible DKG. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum DkgError { /// Participant ID is zero or out of range. + #[error("invalid participant ID {0}")] InvalidParticipantId(u32), /// Two or more partial signatures share the same identifier. + #[error("duplicate participant identifier {0}")] DuplicateIdentifier(u32), /// Fewer partial signatures than the threshold were provided. + #[error("insufficient signers")] InsufficientSigners, /// Invalid number of signers. + #[error("invalid signer count")] InvalidSignerCount, /// Invalid proof of knowledge from a specific participant. + #[error("invalid proof from participant {culprit}")] InvalidProof { /// The 1-indexed ID of the participant whose proof failed. culprit: u32, }, /// Invalid Feldman share from a specific participant. + #[error("invalid share from participant {culprit}")] InvalidShare { /// The 1-indexed ID of the participant whose share failed. culprit: u32, }, /// Wrong number of received packages. + #[error("incorrect package count")] IncorrectPackageCount, /// Failed to deserialize a scalar from wire format bytes. + #[error("invalid scalar encoding")] InvalidScalar, /// Failed to deserialize a G1 point from wire format bytes. + #[error("invalid point encoding")] InvalidPoint, /// Commitment count does not match threshold. + #[error("invalid commitment count from participant {participant}")] InvalidCommitmentCount { /// The participant whose commitment count was wrong. participant: u32, }, /// An error from frost-core. + #[error(transparent)] FrostCoreError(FrostCoreError), } @@ -87,9 +97,10 @@ pub struct Round2Bcast { /// A Shamir secret share matching Go's `sharing.ShamirShare`. /// /// The `value` field is in **big-endian** byte order. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, ZeroizeOnDrop)] pub struct ShamirShare { /// The share identifier (1-indexed participant ID). + #[zeroize(skip)] pub id: u32, /// The share value as big-endian scalar bytes. pub value: [u8; 32], @@ -100,24 +111,30 @@ pub struct ShamirShare { /// # Security /// /// This MUST NOT be sent to other participants. +#[derive(ZeroizeOnDrop)] pub struct Round1Secret { + #[zeroize(skip)] id: u32, + #[zeroize(skip)] ctx: u8, coefficients: Vec, + #[zeroize(skip)] commitment: VerifiableSecretSharingCommitment, + #[zeroize(skip)] threshold: u16, + #[zeroize(skip)] max_signers: u16, } impl Round1Secret { - /// Reconstruct a [`Round1Secret`] from wire-format data (e.g. a test - /// fixture) so that the standard [`round2`] function can be called. + /// Reconstruct a [`Round1Secret`] from wire-format test fixture data so + /// that the standard [`round2`] function can be called. /// - /// `own_share` is the big-endian scalar the participant computed for - /// itself. It is stored as the constant term of a zero polynomial so - /// that [`round2`]'s `from_coefficients` evaluation returns it - /// unchanged. - pub fn from_raw( + /// Testing-only helper: `own_share` is stored as the constant term of a + /// synthetic zero polynomial so that [`round2`]'s `from_coefficients` + /// evaluation returns it unchanged. + #[cfg(test)] + pub(crate) fn from_raw( id: u32, ctx: u8, threshold: u16, @@ -125,6 +142,8 @@ impl Round1Secret { own_share: &[u8; 32], commitment_bytes: &[[u8; 48]], ) -> Result { + validate_round_parameters(id, threshold, max_signers)?; + let own_share_scalar = scalar_from_be(own_share)?; let commitment = deserialize_commitment(id, threshold, commitment_bytes)?; @@ -157,7 +176,8 @@ pub fn scalar_from_be(bytes: &[u8; 32]) -> Result { } /// RFC 9380 Section 5.3.1 using SHA-256 -pub fn expand_msg_xmd(msg: &[u8], dst: &[u8], len_in_bytes: usize) -> Vec { +#[allow(clippy::arithmetic_side_effects)] +pub(crate) fn expand_msg_xmd(msg: &[u8], dst: &[u8], len_in_bytes: usize) -> Vec { const B_IN_BYTES: usize = 32; // SHA-256 output const S_IN_BYTES: usize = 64; // SHA-256 block size @@ -215,6 +235,21 @@ pub fn expand_msg_xmd(msg: &[u8], dst: &[u8], len_in_bytes: usize) -> Vec { out } +fn validate_round_parameters(id: u32, threshold: u16, max_signers: u16) -> Result<(), DkgError> { + // Kryptology encodes participant identifiers into a single byte. + if max_signers > u16::from(u8::MAX) { + return Err(DkgError::InvalidSignerCount); + } + + validate_num_of_signers(threshold, max_signers)?; + + if id == 0 || id > u32::from(max_signers) { + return Err(DkgError::InvalidParticipantId(id)); + } + + Ok(()) +} + /// Kryptology hash-to-scalar. /// /// See: https://github.com/coinbase/kryptology/blob/1dcc062313d99f2e56ce6abc2003ef63c52dd4a5/pkg/core/curves/bls12381_curve.go#L50 @@ -222,16 +257,10 @@ const KRYPTOLOGY_DST: &[u8] = b"BLS12381_XMD:SHA-256_SSWU_RO_"; /// Hash to scalar using kryptology's ExpandMsgXmd construction. /// -/// `ExpandMsgXmd(SHA-256, msg, DST, 48)` -> reverse bytes -> pad to 64 -> -/// `Scalar::from_bytes_wide`. +/// `ExpandMsgXmd(SHA-256, msg, DST, 48)` -> `Scalar::from_be_bytes_wide`. fn kryptology_hash_to_scalar(msg: &[u8]) -> Scalar { let xmd = expand_msg_xmd(msg, KRYPTOLOGY_DST, 48); - let mut reversed = [0u8; 48]; - reversed.copy_from_slice(&xmd); - reversed.reverse(); - let mut wide = [0u8; 64]; - wide[..48].copy_from_slice(&reversed); - Scalar::from_bytes_wide(&wide) + Scalar::from_be_bytes_wide(&xmd) } /// Compute the DKG challenge matching kryptology's format. @@ -271,6 +300,7 @@ fn deserialize_commitment( /// - `max_signers`: Total number of signers (n). /// - `ctx`: DKG context byte (typically 0). /// - `rng`: Cryptographic RNG. +#[allow(clippy::arithmetic_side_effects)] pub fn round1( id: u32, threshold: u16, @@ -278,16 +308,7 @@ pub fn round1( ctx: u8, rng: &mut R, ) -> Result<(Round1Bcast, BTreeMap, Round1Secret), DkgError> { - // Kryptology encodes participant identifiers into a single byte. - if max_signers > u16::from(u8::MAX) { - return Err(DkgError::InvalidSignerCount); - } - - validate_num_of_signers(threshold, max_signers)?; - - if id == 0 || id > u32::from(max_signers) { - return Err(DkgError::InvalidParticipantId(id)); - } + validate_round_parameters(id, threshold, max_signers)?; // Generate random polynomial coefficients [a_0, ..., a_{t-1}] let coefficients: Vec = (0..threshold).map(|_| Scalar::random(&mut *rng)).collect(); @@ -368,13 +389,19 @@ pub fn round1( /// [`Round1Bcast`]. /// - `received_shares`: Map from source participant ID to the [`ShamirShare`] /// they sent us. +#[allow(clippy::arithmetic_side_effects)] pub fn round2( secret: Round1Secret, received_bcasts: &BTreeMap, received_shares: &BTreeMap, ) -> Result<(Round2Bcast, KeyPackage, PublicKeyPackage), DkgError> { - let expected = (secret.max_signers - 1) as usize; - if received_bcasts.len() != expected || received_shares.len() != expected { + let min_received = (secret.threshold - 1) as usize; + let max_received = (secret.max_signers - 1) as usize; + if received_bcasts.len() < min_received + || received_bcasts.len() > max_received + || received_shares.len() < min_received + || received_shares.len() > max_received + { return Err(DkgError::IncorrectPackageCount); } @@ -387,6 +414,10 @@ pub fn round2( let mut share_sum = Scalar::ZERO; for (&sender_id, bcast) in received_bcasts { + if sender_id == secret.id { + return Err(DkgError::InvalidParticipantId(sender_id)); + } + let sender_commitment = deserialize_commitment(sender_id, secret.threshold, &bcast.commitments)?; let a0 = sender_commitment.coefficients()[0].value(); @@ -394,20 +425,23 @@ pub fn round2( // Verify proof of knowledge let wi = scalar_from_be(&bcast.wi)?; let ci = scalar_from_be(&bcast.ci)?; + if ci == Scalar::ZERO { + return Err(DkgError::InvalidProof { culprit: sender_id }); + } // Reconstruct R' = Wi*G - Ci*A_{j,0} let r_reconstructed = G1Projective::generator() * wi - a0 * ci; let sender_id_u8 = u8::try_from(sender_id).map_err(|_| DkgError::InvalidParticipantId(sender_id))?; let ci_check = kryptology_challenge(sender_id_u8, secret.ctx, &a0, &r_reconstructed); - if ci_check != ci { + if !ci_check.constant_time_eq(&ci) { return Err(DkgError::InvalidProof { culprit: sender_id }); } // Verify Feldman share let share = received_shares .get(&sender_id) - .ok_or(DkgError::IncorrectPackageCount)?; + .ok_or(DkgError::InvalidShare { culprit: sender_id })?; if share.id != secret.id { return Err(DkgError::InvalidShare { culprit: sender_id }); } @@ -433,7 +467,7 @@ pub fn round2( let verifying_share = VerifyingShare::new(verifying_share_element); // Build PublicKeyPackage from all participants' commitments - peer_commitments.insert(own_identifier, secret.commitment); + peer_commitments.insert(own_identifier, secret.commitment.clone()); let commitment_refs: BTreeMap = peer_commitments.iter().map(|(id, c)| (*id, c)).collect(); let public_key_package = PublicKeyPackage::from_dkg_commitments(&commitment_refs)?; @@ -478,17 +512,12 @@ impl BlsPartialSignature { /// /// Computes `partial_sig = (key_package.signing_share) * H(msg)` where H /// hashes the message to a G2 point using the Ethereum 2.0 DST. - /// - /// The `id` must be the original 1-indexed kryptology participant ID. - pub fn from_key_package(id: u32, key_package: &KeyPackage, msg: &[u8]) -> BlsPartialSignature { + pub fn from_key_package(key_package: &KeyPackage, msg: &[u8]) -> BlsPartialSignature { let scalar = key_package.signing_share().to_scalar(); - { - let signing_share: &Scalar = &scalar; - let h_msg = hash_to_g2(msg); - BlsPartialSignature { - identifier: id, - point: p2_mult(&h_msg, signing_share), - } + let h_msg = hash_to_g2(msg); + BlsPartialSignature { + identifier: key_package.identifier().to_u32(), + point: p2_mult(&h_msg, &scalar), } } } @@ -518,6 +547,7 @@ impl BlsSignature { /// /// Returns [`DkgError::InsufficientSigners`] if `min_signers < 2` or /// fewer than `min_signers` partial signatures are provided. + #[allow(clippy::arithmetic_side_effects)] pub fn from_partial_signatures( min_signers: u16, partial_sigs: &[BlsPartialSignature], @@ -609,6 +639,7 @@ fn p2_mult(point: &blst_p2, scalar: &Scalar) -> blst_p2 { let mut out = blst_p2::default(); unsafe { blst_scalar_from_fr(&mut s, &scalar.0); + // BLS12-381 scalar field order has 255 significant bits. blst_p2_mult(&mut out, point, s.b.as_ptr(), 255); } out diff --git a/crates/frost/tests/kryptology_interop.rs b/crates/frost/src/kryptology_interop_tests.rs similarity index 94% rename from crates/frost/tests/kryptology_interop.rs rename to crates/frost/src/kryptology_interop_tests.rs index 432e8ebd..9e066119 100644 --- a/crates/frost/tests/kryptology_interop.rs +++ b/crates/frost/src/kryptology_interop_tests.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; -use pluto_frost::kryptology; +use crate::kryptology; use serde::Deserialize; #[derive(Clone, Deserialize)] @@ -73,7 +73,7 @@ impl From<&FixtureRound1Bcast> for kryptology::Round1Bcast { #[test] fn kryptology_fixture_round2_interop_2_of_3_ctx_0() { replay_fixture( - include_str!("./kryptology_fixtures/2-of-3-ctx-0.json"), + include_str!("../tests/kryptology_fixtures/2-of-3-ctx-0.json"), true, ); } @@ -81,7 +81,7 @@ fn kryptology_fixture_round2_interop_2_of_3_ctx_0() { #[test] fn kryptology_fixture_round2_interop_3_of_3_ctx_0() { replay_fixture( - include_str!("./kryptology_fixtures/3-of-3-ctx-0.json"), + include_str!("../tests/kryptology_fixtures/3-of-3-ctx-0.json"), true, ); } @@ -89,7 +89,7 @@ fn kryptology_fixture_round2_interop_3_of_3_ctx_0() { #[test] fn kryptology_fixture_round2_interop_malformed_share_id() { replay_fixture( - include_str!("./kryptology_fixtures/malformed-share-id.json"), + include_str!("../tests/kryptology_fixtures/malformed-share-id.json"), false, ); } @@ -97,7 +97,7 @@ fn kryptology_fixture_round2_interop_malformed_share_id() { #[test] fn kryptology_fixture_round2_interop_invalid_proof() { replay_fixture( - include_str!("./kryptology_fixtures/invalid-proof.json"), + include_str!("../tests/kryptology_fixtures/invalid-proof.json"), false, ); } @@ -200,8 +200,8 @@ fn replay_fixture(json: &str, require_group_signature: bool) { let message = b"kryptology fixture signing"; let partial_sigs: Vec<_> = key_packages - .iter() - .map(|(&id, kp)| kryptology::BlsPartialSignature::from_key_package(id, kp, message)) + .values() + .map(|kp| kryptology::BlsPartialSignature::from_key_package(kp, message)) .collect(); let signature = diff --git a/crates/frost/src/lib.rs b/crates/frost/src/lib.rs index 1197bf98..b4ad21cf 100644 --- a/crates/frost/src/lib.rs +++ b/crates/frost/src/lib.rs @@ -3,7 +3,6 @@ //! Go's Coinbase Kryptology FROST DKG, and BLS threshold signing (Ethereum 2.0 //! compatible). -#![allow(non_snake_case)] #![doc = include_str!("../dkg.md")] pub mod curve; @@ -12,7 +11,9 @@ pub mod kryptology; pub use curve::*; pub use frost_core::*; -pub use rand_core; +pub use rand_core::{CryptoRng, RngCore}; +#[cfg(test)] +mod kryptology_interop_tests; #[cfg(test)] mod tests; diff --git a/crates/frost/src/tests.rs b/crates/frost/src/tests.rs index 8e8013a2..b3ee9a3d 100644 --- a/crates/frost/src/tests.rs +++ b/crates/frost/src/tests.rs @@ -76,6 +76,28 @@ fn kryptology_accepts_255_signers_boundary() { assert!(shares.contains_key(&255)); } +#[test] +fn kryptology_rejects_invalid_signer_counts() { + let mut rng = StdRng::seed_from_u64(7); + + assert!(matches!( + kryptology::round1(1, 1, 3, 0, &mut rng), + Err(kryptology::DkgError::FrostCoreError( + crate::FrostCoreError::InvalidMinSigners + )) + )); + assert!(matches!( + kryptology::round1(1, 3, 2, 0, &mut rng), + Err(kryptology::DkgError::FrostCoreError( + crate::FrostCoreError::InvalidMinSigners + )) + )); + assert!(matches!( + kryptology::round1(0, 2, 3, 0, &mut rng), + Err(kryptology::DkgError::InvalidParticipantId(0)) + )); +} + /// Full DKG round-trip: 3-of-3 DKG, then BLS threshold sign and verify. #[test] fn kryptology_bls_round_trip_3_of_3() { @@ -151,9 +173,7 @@ fn kryptology_bls_round_trip_3_of_3() { let partial_sigs: Vec<_> = key_packages .keys() - .map(|&id| { - kryptology::BlsPartialSignature::from_key_package(id, &key_packages[&id], message) - }) + .map(|&id| kryptology::BlsPartialSignature::from_key_package(&key_packages[&id], message)) .collect(); let signature = kryptology::BlsSignature::from_partial_signatures(threshold, &partial_sigs) @@ -216,9 +236,7 @@ fn kryptology_bls_round_trip_2_of_3() { let partial_sigs: Vec<_> = signers .iter() - .map(|&id| { - kryptology::BlsPartialSignature::from_key_package(id, &key_packages[&id], message) - }) + .map(|&id| kryptology::BlsPartialSignature::from_key_package(&key_packages[&id], message)) .collect(); let signature = kryptology::BlsSignature::from_partial_signatures(threshold, &partial_sigs) @@ -299,3 +317,106 @@ fn kryptology_share_id_mismatch_rejected() { other => panic!("expected InvalidShare, got {other:?}"), } } + +#[test] +fn kryptology_round2_accepts_threshold_subset() { + let mut rng = StdRng::seed_from_u64(321); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (bcast1, shares1, _secret1) = + kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast2, _shares2, secret2) = + kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast3, _shares3, _secret3) = + kryptology::round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + + let received_bcasts: BTreeMap = [(1, bcast1)].into(); + let received_shares: BTreeMap = [(1, shares1[&2].clone())].into(); + + kryptology::round2(secret2, &received_bcasts, &received_shares) + .expect("threshold-1 peer packages should be enough"); +} + +#[test] +fn kryptology_round2_rejects_missing_share_with_culprit() { + let mut rng = StdRng::seed_from_u64(322); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (bcast1, _shares1, _secret1) = + kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast2, _shares2, secret2) = + kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast3, shares3, _secret3) = + kryptology::round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + + let received_bcasts: BTreeMap = [(1, bcast1), (3, bcast3)].into(); + let received_shares: BTreeMap = [(3, shares3[&2].clone())].into(); + + let result = kryptology::round2(secret2, &received_bcasts, &received_shares); + assert!(matches!( + result, + Err(kryptology::DkgError::InvalidShare { culprit: 1 }) + )); +} + +#[test] +fn kryptology_round2_rejects_self_broadcast() { + let mut rng = StdRng::seed_from_u64(323); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (_bcast1, shares1, _secret1) = + kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast2, _shares2, secret2) = + kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + + let received_bcasts: BTreeMap = [(2, bcast2)].into(); + let received_shares: BTreeMap = [(2, shares1[&2].clone())].into(); + + let result = kryptology::round2(secret2, &received_bcasts, &received_shares); + assert!(matches!( + result, + Err(kryptology::DkgError::InvalidParticipantId(2)) + )); +} + +#[test] +fn kryptology_duplicate_partial_signatures_rejected() { + let mut rng = StdRng::seed_from_u64(324); + let threshold = 2u16; + let max_signers = 2u16; + let ctx = 0u8; + let message = b"duplicate signer"; + + let (bcast1, shares1, secret1) = + kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast2, shares2, secret2) = + kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + + let (_round2_bcast1, key_package1, _public_key_package1) = kryptology::round2( + secret1, + &[(2, bcast2.clone())].into(), + &[(2, shares2[&1].clone())].into(), + ) + .unwrap(); + let (_round2_bcast2, _key_package2, _public_key_package2) = kryptology::round2( + secret2, + &[(1, bcast1)].into(), + &[(1, shares1[&2].clone())].into(), + ) + .unwrap(); + + let partial = kryptology::BlsPartialSignature::from_key_package(&key_package1, message); + let result = + kryptology::BlsSignature::from_partial_signatures(threshold, &[partial.clone(), partial]); + + assert!(matches!( + result, + Err(kryptology::DkgError::DuplicateIdentifier(1)) + )); +} diff --git a/crates/frost/tests/kryptology_round_trip.rs b/crates/frost/tests/kryptology_round_trip.rs index e0335f46..96d16126 100644 --- a/crates/frost/tests/kryptology_round_trip.rs +++ b/crates/frost/tests/kryptology_round_trip.rs @@ -96,9 +96,7 @@ fn kryptology_bls_round_trip_2_of_4_ctx_0() { let partial_sigs: Vec<_> = signing_participants .iter() - .map(|&id| { - kryptology::BlsPartialSignature::from_key_package(id, &key_packages[&id], message) - }) + .map(|&id| kryptology::BlsPartialSignature::from_key_package(&key_packages[&id], message)) .collect(); assert_eq!(partial_sigs.len(), threshold as usize); From e209512656493360d2eb04c8abf93add55d79295 Mon Sep 17 00:00:00 2001 From: Quang Le Date: Fri, 1 May 2026 15:41:16 +0700 Subject: [PATCH 2/7] fix: refine error --- crates/frost/src/kryptology.rs | 66 +++++++++---------- crates/frost/src/kryptology_interop_tests.rs | 4 +- .../kryptology_round_trip_tests.rs} | 3 +- crates/frost/src/lib.rs | 2 + crates/frost/src/tests.rs | 18 ++--- 5 files changed, 48 insertions(+), 45 deletions(-) rename crates/frost/{tests/kryptology_round_trip.rs => src/kryptology_round_trip_tests.rs} (99%) diff --git a/crates/frost/src/kryptology.rs b/crates/frost/src/kryptology.rs index 80932859..5db8c19d 100644 --- a/crates/frost/src/kryptology.rs +++ b/crates/frost/src/kryptology.rs @@ -18,9 +18,9 @@ use zeroize::ZeroizeOnDrop; use super::*; -/// Errors from the kryptology-compatible DKG. +/// Errors from the kryptology-compatible FROST protocol. #[derive(Debug, thiserror::Error)] -pub enum DkgError { +pub enum KryptologyError { /// Participant ID is zero or out of range. #[error("invalid participant ID {0}")] InvalidParticipantId(u32), @@ -62,13 +62,7 @@ pub enum DkgError { }, /// An error from frost-core. #[error(transparent)] - FrostCoreError(FrostCoreError), -} - -impl From for DkgError { - fn from(e: FrostCoreError) -> Self { - DkgError::FrostCoreError(e) - } + FrostCoreError(#[from] FrostCoreError), } /// Kryptology Round 1 broadcast data matching Go's `frost.Round1Bcast`. @@ -141,7 +135,7 @@ impl Round1Secret { max_signers: u16, own_share: &[u8; 32], commitment_bytes: &[[u8; 48]], - ) -> Result { + ) -> Result { validate_round_parameters(id, threshold, max_signers)?; let own_share_scalar = scalar_from_be(own_share)?; @@ -169,10 +163,10 @@ pub fn scalar_to_be(s: &Scalar) -> [u8; 32] { } /// Convert big-endian 32 bytes to a `Scalar`. -pub fn scalar_from_be(bytes: &[u8; 32]) -> Result { +pub fn scalar_from_be(bytes: &[u8; 32]) -> Result { let mut le = *bytes; le.reverse(); - Scalar::from_bytes(&le).ok_or(DkgError::InvalidScalar) + Scalar::from_bytes(&le).ok_or(KryptologyError::InvalidScalar) } /// RFC 9380 Section 5.3.1 using SHA-256 @@ -235,16 +229,20 @@ pub(crate) fn expand_msg_xmd(msg: &[u8], dst: &[u8], len_in_bytes: usize) -> Vec out } -fn validate_round_parameters(id: u32, threshold: u16, max_signers: u16) -> Result<(), DkgError> { +fn validate_round_parameters( + id: u32, + threshold: u16, + max_signers: u16, +) -> Result<(), KryptologyError> { // Kryptology encodes participant identifiers into a single byte. if max_signers > u16::from(u8::MAX) { - return Err(DkgError::InvalidSignerCount); + return Err(KryptologyError::InvalidSignerCount); } validate_num_of_signers(threshold, max_signers)?; if id == 0 || id > u32::from(max_signers) { - return Err(DkgError::InvalidParticipantId(id)); + return Err(KryptologyError::InvalidParticipantId(id)); } Ok(()) @@ -280,12 +278,13 @@ fn deserialize_commitment( participant: u32, threshold: u16, commitments: &[[u8; 48]], -) -> Result { +) -> Result { if commitments.len() != threshold as usize { - return Err(DkgError::InvalidCommitmentCount { participant }); + return Err(KryptologyError::InvalidCommitmentCount { participant }); } - VerifiableSecretSharingCommitment::from_commitments(commitments).ok_or(DkgError::InvalidPoint) + VerifiableSecretSharingCommitment::from_commitments(commitments) + .ok_or(KryptologyError::InvalidPoint) } /// Perform Round 1 of the kryptology-compatible DKG. @@ -307,7 +306,7 @@ pub fn round1( max_signers: u16, ctx: u8, rng: &mut R, -) -> Result<(Round1Bcast, BTreeMap, Round1Secret), DkgError> { +) -> Result<(Round1Bcast, BTreeMap, Round1Secret), KryptologyError> { validate_round_parameters(id, threshold, max_signers)?; // Generate random polynomial coefficients [a_0, ..., a_{t-1}] @@ -394,7 +393,7 @@ pub fn round2( secret: Round1Secret, received_bcasts: &BTreeMap, received_shares: &BTreeMap, -) -> Result<(Round2Bcast, KeyPackage, PublicKeyPackage), DkgError> { +) -> Result<(Round2Bcast, KeyPackage, PublicKeyPackage), KryptologyError> { let min_received = (secret.threshold - 1) as usize; let max_received = (secret.max_signers - 1) as usize; if received_bcasts.len() < min_received @@ -402,7 +401,7 @@ pub fn round2( || received_shares.len() < min_received || received_shares.len() > max_received { - return Err(DkgError::IncorrectPackageCount); + return Err(KryptologyError::IncorrectPackageCount); } let own_identifier = Identifier::from_u32(secret.id)?; @@ -415,7 +414,7 @@ pub fn round2( for (&sender_id, bcast) in received_bcasts { if sender_id == secret.id { - return Err(DkgError::InvalidParticipantId(sender_id)); + return Err(KryptologyError::InvalidParticipantId(sender_id)); } let sender_commitment = @@ -426,24 +425,25 @@ pub fn round2( let wi = scalar_from_be(&bcast.wi)?; let ci = scalar_from_be(&bcast.ci)?; if ci == Scalar::ZERO { - return Err(DkgError::InvalidProof { culprit: sender_id }); + return Err(KryptologyError::InvalidProof { culprit: sender_id }); } // Reconstruct R' = Wi*G - Ci*A_{j,0} let r_reconstructed = G1Projective::generator() * wi - a0 * ci; let sender_id_u8 = - u8::try_from(sender_id).map_err(|_| DkgError::InvalidParticipantId(sender_id))?; + u8::try_from(sender_id) + .map_err(|_| KryptologyError::InvalidParticipantId(sender_id))?; let ci_check = kryptology_challenge(sender_id_u8, secret.ctx, &a0, &r_reconstructed); if !ci_check.constant_time_eq(&ci) { - return Err(DkgError::InvalidProof { culprit: sender_id }); + return Err(KryptologyError::InvalidProof { culprit: sender_id }); } // Verify Feldman share let share = received_shares .get(&sender_id) - .ok_or(DkgError::InvalidShare { culprit: sender_id })?; + .ok_or(KryptologyError::InvalidShare { culprit: sender_id })?; if share.id != secret.id { - return Err(DkgError::InvalidShare { culprit: sender_id }); + return Err(KryptologyError::InvalidShare { culprit: sender_id }); } let share_scalar = scalar_from_be(&share.value)?; @@ -452,7 +452,7 @@ pub fn round2( SecretShare::new(own_identifier, signing_share, sender_commitment.clone()); secret_share .verify() - .map_err(|_| DkgError::InvalidShare { culprit: sender_id })?; + .map_err(|_| KryptologyError::InvalidShare { culprit: sender_id })?; share_sum = share_sum + share_scalar; @@ -545,22 +545,22 @@ impl BlsSignature { /// Matches Go's `combineSigs` in /// `kryptology/pkg/signatures/bls/bls_sig/usual_bls_sig.go`. /// - /// Returns [`DkgError::InsufficientSigners`] if `min_signers < 2` or + /// Returns [`KryptologyError::InsufficientSigners`] if `min_signers < 2` or /// fewer than `min_signers` partial signatures are provided. #[allow(clippy::arithmetic_side_effects)] pub fn from_partial_signatures( min_signers: u16, partial_sigs: &[BlsPartialSignature], - ) -> Result { + ) -> Result { if min_signers < 2 || partial_sigs.len() < min_signers as usize { - return Err(DkgError::InsufficientSigners); + return Err(KryptologyError::InsufficientSigners); } // Check for duplicate identifiers let mut seen = std::collections::BTreeSet::new(); for ps in partial_sigs { if !seen.insert(ps.identifier) { - return Err(DkgError::DuplicateIdentifier(ps.identifier)); + return Err(KryptologyError::DuplicateIdentifier(ps.identifier)); } } @@ -581,7 +581,7 @@ impl BlsSignature { } let num = x_vals[j]; let den = x_vals[j] - x_vals[i]; - let den_inv = den.invert().ok_or(DkgError::InvalidSignerCount)?; + let den_inv = den.invert().ok_or(KryptologyError::InvalidSignerCount)?; lambda = lambda * num * den_inv; } diff --git a/crates/frost/src/kryptology_interop_tests.rs b/crates/frost/src/kryptology_interop_tests.rs index 9e066119..8107912d 100644 --- a/crates/frost/src/kryptology_interop_tests.rs +++ b/crates/frost/src/kryptology_interop_tests.rs @@ -174,14 +174,14 @@ fn replay_fixture(json: &str, require_group_signature: bool) { ExpectedRound2::InvalidShare { culprit } => { let err = result.expect_err("round2 should fail"); assert!( - matches!(err, kryptology::DkgError::InvalidShare { culprit: c } if c == *culprit), + matches!(err, kryptology::KryptologyError::InvalidShare { culprit: c } if c == *culprit), "expected InvalidShare(culprit={culprit}), got {err:?}" ); } ExpectedRound2::InvalidProof { culprit } => { let err = result.expect_err("round2 should fail"); assert!( - matches!(err, kryptology::DkgError::InvalidProof { culprit: c } if c == *culprit), + matches!(err, kryptology::KryptologyError::InvalidProof { culprit: c } if c == *culprit), "expected InvalidProof(culprit={culprit}), got {err:?}" ); } diff --git a/crates/frost/tests/kryptology_round_trip.rs b/crates/frost/src/kryptology_round_trip_tests.rs similarity index 99% rename from crates/frost/tests/kryptology_round_trip.rs rename to crates/frost/src/kryptology_round_trip_tests.rs index 96d16126..bf994eed 100644 --- a/crates/frost/tests/kryptology_round_trip.rs +++ b/crates/frost/src/kryptology_round_trip_tests.rs @@ -2,9 +2,10 @@ use std::collections::BTreeMap; -use pluto_frost::kryptology; use rand::{SeedableRng, rngs::StdRng}; +use crate::kryptology; + /// FROST DKG + BLS threshold signing (Ethereum 2.0 compatible). /// This matches Go's signing flow: non-interactive BLS partial signatures /// combined via Lagrange interpolation, verified with standard BLS pairings. diff --git a/crates/frost/src/lib.rs b/crates/frost/src/lib.rs index b4ad21cf..1bb2a571 100644 --- a/crates/frost/src/lib.rs +++ b/crates/frost/src/lib.rs @@ -16,4 +16,6 @@ pub use rand_core::{CryptoRng, RngCore}; #[cfg(test)] mod kryptology_interop_tests; #[cfg(test)] +mod kryptology_round_trip_tests; +#[cfg(test)] mod tests; diff --git a/crates/frost/src/tests.rs b/crates/frost/src/tests.rs index b3ee9a3d..79e19580 100644 --- a/crates/frost/src/tests.rs +++ b/crates/frost/src/tests.rs @@ -62,7 +62,7 @@ fn kryptology_rejects_more_than_255_signers() { assert!(matches!( result, - Err(kryptology::DkgError::InvalidSignerCount) + Err(kryptology::KryptologyError::InvalidSignerCount) )); } @@ -82,19 +82,19 @@ fn kryptology_rejects_invalid_signer_counts() { assert!(matches!( kryptology::round1(1, 1, 3, 0, &mut rng), - Err(kryptology::DkgError::FrostCoreError( + Err(kryptology::KryptologyError::FrostCoreError( crate::FrostCoreError::InvalidMinSigners )) )); assert!(matches!( kryptology::round1(1, 3, 2, 0, &mut rng), - Err(kryptology::DkgError::FrostCoreError( + Err(kryptology::KryptologyError::FrostCoreError( crate::FrostCoreError::InvalidMinSigners )) )); assert!(matches!( kryptology::round1(0, 2, 3, 0, &mut rng), - Err(kryptology::DkgError::InvalidParticipantId(0)) + Err(kryptology::KryptologyError::InvalidParticipantId(0)) )); } @@ -282,7 +282,7 @@ fn kryptology_invalid_proof_rejected() { let result = kryptology::round2(secret2, &received_bcasts, &received_shares); assert!(result.is_err()); match result.unwrap_err() { - kryptology::DkgError::InvalidProof { culprit } => assert_eq!(culprit, 1), + kryptology::KryptologyError::InvalidProof { culprit } => assert_eq!(culprit, 1), other => panic!("expected InvalidProof, got {other:?}"), } } @@ -313,7 +313,7 @@ fn kryptology_share_id_mismatch_rejected() { let result = kryptology::round2(secret2, &received_bcasts, &received_shares); assert!(result.is_err()); match result.unwrap_err() { - kryptology::DkgError::InvalidShare { culprit } => assert_eq!(culprit, 1), + kryptology::KryptologyError::InvalidShare { culprit } => assert_eq!(culprit, 1), other => panic!("expected InvalidShare, got {other:?}"), } } @@ -359,7 +359,7 @@ fn kryptology_round2_rejects_missing_share_with_culprit() { let result = kryptology::round2(secret2, &received_bcasts, &received_shares); assert!(matches!( result, - Err(kryptology::DkgError::InvalidShare { culprit: 1 }) + Err(kryptology::KryptologyError::InvalidShare { culprit: 1 }) )); } @@ -381,7 +381,7 @@ fn kryptology_round2_rejects_self_broadcast() { let result = kryptology::round2(secret2, &received_bcasts, &received_shares); assert!(matches!( result, - Err(kryptology::DkgError::InvalidParticipantId(2)) + Err(kryptology::KryptologyError::InvalidParticipantId(2)) )); } @@ -417,6 +417,6 @@ fn kryptology_duplicate_partial_signatures_rejected() { assert!(matches!( result, - Err(kryptology::DkgError::DuplicateIdentifier(1)) + Err(kryptology::KryptologyError::DuplicateIdentifier(1)) )); } From e1e80869e4d4f99808db41538ff74f5d4aa739d6 Mon Sep 17 00:00:00 2001 From: Quang Le Date: Fri, 1 May 2026 15:44:50 +0700 Subject: [PATCH 3/7] fix: more tests on frost core --- crates/frost/src/frost_core.rs | 61 ++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/crates/frost/src/frost_core.rs b/crates/frost/src/frost_core.rs index afe36e9b..07585f12 100644 --- a/crates/frost/src/frost_core.rs +++ b/crates/frost/src/frost_core.rs @@ -474,3 +474,64 @@ pub fn validate_num_of_signers(min_signers: u16, max_signers: u16) -> Result<(), } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn identifier_from_u32_rejects_zero() { + assert!(matches!( + Identifier::from_u32(0), + Err(FrostCoreError::InvalidZeroScalar) + )); + } + + #[test] + fn validate_num_of_signers_rejects_invalid_bounds() { + assert!(matches!( + validate_num_of_signers(1, 3), + Err(FrostCoreError::InvalidMinSigners) + )); + assert!(matches!( + validate_num_of_signers(2, 1), + Err(FrostCoreError::InvalidMaxSigners) + )); + assert!(matches!( + validate_num_of_signers(3, 2), + Err(FrostCoreError::InvalidMinSigners) + )); + } + + #[test] + fn secret_share_verify_rejects_invalid_share() { + let id = Identifier::from_u32(1).unwrap(); + let commitment = VerifiableSecretSharingCommitment::new(vec![CoefficientCommitment::new( + G1Projective::generator(), + )]); + let invalid_share = + SecretShare::new(id, SigningShare::new(Scalar::ZERO), commitment.clone()); + assert!(matches!( + invalid_share.verify(), + Err(FrostCoreError::InvalidSecretShare) + )); + } + + #[test] + fn verifying_key_from_commitment_rejects_empty_commitment() { + let empty_commitment = VerifiableSecretSharingCommitment::new(vec![]); + assert!(matches!( + VerifyingKey::from_commitment(&empty_commitment), + Err(FrostCoreError::IncorrectCommitment) + )); + } + + #[test] + fn public_key_package_from_dkg_commitments_rejects_empty_commitments() { + let empty_commitments = BTreeMap::new(); + assert!(matches!( + PublicKeyPackage::from_dkg_commitments(&empty_commitments), + Err(FrostCoreError::IncorrectNumberOfCommitments) + )); + } +} From adb534642b1a30dabe27e6cbd2251f12f32d2f9d Mon Sep 17 00:00:00 2001 From: Quang Le Date: Fri, 1 May 2026 15:46:43 +0700 Subject: [PATCH 4/7] fix: add tests for curve --- crates/frost/src/curve.rs | 84 +++++++++++++++++++++++++++++++++++++++ crates/frost/src/tests.rs | 7 ---- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/crates/frost/src/curve.rs b/crates/frost/src/curve.rs index 6e8f2da9..b91224ff 100644 --- a/crates/frost/src/curve.rs +++ b/crates/frost/src/curve.rs @@ -302,3 +302,87 @@ impl From for G1Projective { G1Projective(p) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn scalar_one_matches_blst_conversion() { + assert_eq!(Scalar::ONE, Scalar::from(1u64)); + } + + #[test] + fn scalar_round_trips_little_endian_bytes() { + let scalar = Scalar::from(42); + let bytes = scalar.to_bytes(); + + assert_eq!(Scalar::from_bytes(&bytes), Some(scalar)); + } + + #[test] + fn scalar_rejects_out_of_range_bytes() { + assert_eq!(Scalar::from_bytes(&[0xff; 32]), None); + } + + #[test] + fn scalar_from_be_bytes_wide_matches_reversed_le_wide() { + let be = [7u8; 48]; + let from_be = Scalar::from_be_bytes_wide(&be); + + let mut reversed = be; + reversed.reverse(); + let mut wide = [0u8; 64]; + wide[..48].copy_from_slice(&reversed); + + assert_eq!(from_be, Scalar::from_bytes_wide(&wide)); + } + + #[test] + fn scalar_constant_time_eq_matches_equality() { + let a = Scalar::from(42); + let b = Scalar::from(42); + let c = Scalar::from(43); + + assert!(a.constant_time_eq(&b)); + assert!(!a.constant_time_eq(&c)); + } + + #[test] + fn scalar_zeroize_clears_limbs() { + let mut scalar = Scalar::from(42); + + scalar.zeroize(); + + assert_eq!(scalar, Scalar::ZERO); + } + + #[test] + fn scalar_invert_returns_none_for_zero() { + assert_eq!(Scalar::ZERO.invert(), None); + } + + #[test] + fn scalar_invert_returns_multiplicative_inverse() { + let scalar = Scalar::from(42); + let inverse = scalar.invert().expect("non-zero scalar should invert"); + + assert_eq!(scalar * inverse, Scalar::ONE); + } + + #[test] + fn g1_projective_rejects_identity_compressed_point() { + let identity = G1Affine::from(G1Projective::identity()).to_compressed(); + + assert_eq!(G1Projective::from_compressed(&identity), None); + } + + #[test] + fn g1_affine_round_trips_generator_compressed_point() { + let generator = G1Projective::generator(); + let compressed = G1Affine::from(generator).to_compressed(); + let affine = G1Affine::from_compressed(&compressed).expect("generator should deserialize"); + + assert_eq!(G1Projective::from(affine), generator); + } +} diff --git a/crates/frost/src/tests.rs b/crates/frost/src/tests.rs index 79e19580..dc149b49 100644 --- a/crates/frost/src/tests.rs +++ b/crates/frost/src/tests.rs @@ -4,13 +4,6 @@ use rand::{SeedableRng, rngs::StdRng}; use crate::kryptology; -#[test] -fn scalar_one_precomputed() { - let constant = crate::Scalar::ONE; - let computed = crate::Scalar::from(1u64); - assert_eq!(constant, computed); -} - /// RFC 9380 Section 5.3.1 test vector for expand_msg_xmd with SHA-256. /// DST = "QUUX-V01-CS02-with-expander-SHA256-128" /// msg = "" (empty), len_in_bytes = 0x20 (32) From 55320a3e9469ede5e486fcaa56d6687890812c6e Mon Sep 17 00:00:00 2001 From: Quang Le Date: Fri, 1 May 2026 15:53:37 +0700 Subject: [PATCH 5/7] fix: add and refactor tests on kryptology --- crates/frost/src/kryptology.rs | 452 +++++++++++++++++++++++++++++++++ crates/frost/src/lib.rs | 2 - crates/frost/src/tests.rs | 415 ------------------------------ 3 files changed, 452 insertions(+), 417 deletions(-) delete mode 100644 crates/frost/src/tests.rs diff --git a/crates/frost/src/kryptology.rs b/crates/frost/src/kryptology.rs index 5db8c19d..7a861091 100644 --- a/crates/frost/src/kryptology.rs +++ b/crates/frost/src/kryptology.rs @@ -644,3 +644,455 @@ fn p2_mult(point: &blst_p2, scalar: &Scalar) -> blst_p2 { } out } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use rand::{SeedableRng, rngs::StdRng}; + + use super::*; + + #[test] + fn scalar_from_be_rejects_invalid_scalar_encoding() { + assert!(matches!( + scalar_from_be(&[0xff; 32]), + Err(KryptologyError::InvalidScalar) + )); + } + + #[test] + fn deserialize_commitment_rejects_wrong_commitment_count() { + let commitments = []; + + assert!(matches!( + deserialize_commitment(2, 1, &commitments), + Err(KryptologyError::InvalidCommitmentCount { participant: 2 }) + )); + } + + #[test] + fn deserialize_commitment_rejects_invalid_point() { + let commitments = [[0u8; 48]]; + + assert!(matches!( + deserialize_commitment(2, 1, &commitments), + Err(KryptologyError::InvalidPoint) + )); + } + + #[test] + fn round2_rejects_insufficient_package_count() { + let mut rng = StdRng::seed_from_u64(11); + let (_bcast, _shares, secret) = round1(1, 2, 3, 0, &mut rng).unwrap(); + + assert!(matches!( + round2(secret, &BTreeMap::new(), &BTreeMap::new()), + Err(KryptologyError::IncorrectPackageCount) + )); + } + + #[test] + fn from_partial_signatures_rejects_insufficient_signers() { + assert!(matches!( + BlsSignature::from_partial_signatures(2, &[]), + Err(KryptologyError::InsufficientSigners) + )); + } + + /// RFC 9380 Section 5.3.1 test vector for expand_msg_xmd with SHA-256. + /// DST = "QUUX-V01-CS02-with-expander-SHA256-128" + /// msg = "" (empty), len_in_bytes = 0x20 (32) + #[test] + fn expand_msg_xmd_rfc9380_vector() { + let dst = b"QUUX-V01-CS02-with-expander-SHA256-128"; + let msg = b""; + let expected = + hex::decode("68a985b87eb6b46952128911f2a4412bbc302a9d759667f87f7a21d803f07235") + .unwrap(); + + let result = expand_msg_xmd(msg, dst, 32); + assert_eq!(result, expected, "expand_msg_xmd empty message vector"); + } + + /// RFC 9380 test vector: msg = "abc", len = 32 + #[test] + fn expand_msg_xmd_rfc9380_abc() { + let dst = b"QUUX-V01-CS02-with-expander-SHA256-128"; + let msg = b"abc"; + let expected = + hex::decode("d8ccab23b5985ccea865c6c97b6e5b8350e794e603b4b97902f53a8a0d605615") + .unwrap(); + + let result = expand_msg_xmd(msg, dst, 32); + assert_eq!(result, expected, "expand_msg_xmd abc vector"); + } + + /// RFC 9380 test vector: msg = "", len = 0x80 (128 bytes) + #[test] + fn expand_msg_xmd_rfc9380_long_output() { + let dst = b"QUUX-V01-CS02-with-expander-SHA256-128"; + let msg = b""; + let expected = hex::decode( + "af84c27ccfd45d41914fdff5df25293e221afc53d8ad2ac06d5e3e2948\ + 5dadbee0d121587713a3e0dd4d5e69e93eb7cd4f5df4cd103e188cf60c\ + b02edc3edf18eda8576c412b18ffb658e3dd6ec849469b979d444cf7b2\ + 6911a08e63cf31f9dcc541708d3491184472c2c29bb749d4286b004ceb\ + 5ee6b9a7fa5b646c993f0ced", + ) + .unwrap(); + + let result = expand_msg_xmd(msg, dst, 128); + assert_eq!(result, expected, "expand_msg_xmd 128-byte output vector"); + } + + #[test] + fn round1_rejects_more_than_255_signers() { + let mut rng = StdRng::seed_from_u64(42); + let result = round1(1, 2, 256, 0, &mut rng); + + assert!(matches!(result, Err(KryptologyError::InvalidSignerCount))); + } + + #[test] + fn round1_accepts_255_signers_boundary() { + let mut rng = StdRng::seed_from_u64(4242); + let (_bcast, shares, _secret) = round1(1, 2, 255, 9, &mut rng) + .expect("255 signers should remain within kryptology's u8 transport limit"); + + assert_eq!(shares.len(), 254); + assert!(shares.contains_key(&255)); + } + + #[test] + fn round1_rejects_invalid_signer_counts() { + let mut rng = StdRng::seed_from_u64(7); + + assert!(matches!( + round1(1, 1, 3, 0, &mut rng), + Err(KryptologyError::FrostCoreError( + FrostCoreError::InvalidMinSigners + )) + )); + assert!(matches!( + round1(1, 3, 2, 0, &mut rng), + Err(KryptologyError::FrostCoreError( + FrostCoreError::InvalidMinSigners + )) + )); + assert!(matches!( + round1(0, 2, 3, 0, &mut rng), + Err(KryptologyError::InvalidParticipantId(0)) + )); + } + + /// Full DKG round-trip: 3-of-3 DKG, then BLS threshold sign and verify. + #[test] + fn bls_round_trip_3_of_3() { + let mut rng = StdRng::seed_from_u64(42); + let threshold = 3u16; + let max_signers = 3u16; + let ctx = 0u8; + + let mut bcasts: BTreeMap = BTreeMap::new(); + let mut all_shares: BTreeMap> = BTreeMap::new(); + let mut secrets: BTreeMap = BTreeMap::new(); + + for id in 1..=u32::from(max_signers) { + let (bcast, shares, secret) = + round1(id, threshold, max_signers, ctx, &mut rng).expect("round1 should succeed"); + bcasts.insert(id, bcast); + secrets.insert(id, secret); + + for (&target_id, share) in &shares { + all_shares + .entry(target_id) + .or_default() + .insert(id, share.clone()); + } + } + + let mut key_packages = BTreeMap::new(); + let mut public_key_packages = Vec::new(); + let mut round2_bcasts = BTreeMap::new(); + + for id in 1..=u32::from(max_signers) { + let received_bcasts: BTreeMap = bcasts + .iter() + .filter(|(k, _)| **k != id) + .map(|(k, v)| (*k, v.clone())) + .collect(); + + let received_shares = all_shares.remove(&id).unwrap(); + let secret = secrets.remove(&id).unwrap(); + + let (r2_bcast, key_package, pub_package) = + round2(secret, &received_bcasts, &received_shares).expect("round2 should succeed"); + + round2_bcasts.insert(id, r2_bcast); + key_packages.insert(id, key_package); + public_key_packages.push(pub_package); + } + + let vk = public_key_packages[0].verifying_key(); + for pkg in &public_key_packages[1..] { + assert_eq!( + vk, + pkg.verifying_key(), + "all participants must agree on the group key" + ); + } + + let vk_bytes = round2_bcasts[&1].verification_key; + for (&id, bcast) in &round2_bcasts { + assert_eq!( + bcast.verification_key, vk_bytes, + "participant {id} round2 broadcast has different group key" + ); + } + + let message = b"test message"; + + let partial_sigs: Vec<_> = key_packages + .keys() + .map(|&id| BlsPartialSignature::from_key_package(&key_packages[&id], message)) + .collect(); + + let signature = BlsSignature::from_partial_signatures(threshold, &partial_sigs) + .expect("BLS signature combination should succeed"); + + assert!( + signature.verify(vk, message), + "3-of-3 BLS threshold signature should verify" + ); + } + + /// 2-of-3 DKG then BLS threshold signing (Ethereum 2.0 compatible). + #[test] + fn bls_round_trip_2_of_3() { + let mut rng = StdRng::seed_from_u64(123); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let mut bcasts: BTreeMap = BTreeMap::new(); + let mut all_shares: BTreeMap> = BTreeMap::new(); + let mut secrets: BTreeMap = BTreeMap::new(); + + for id in 1..=u32::from(max_signers) { + let (bcast, shares, secret) = round1(id, threshold, max_signers, ctx, &mut rng).unwrap(); + bcasts.insert(id, bcast); + secrets.insert(id, secret); + for (&target_id, share) in &shares { + all_shares + .entry(target_id) + .or_default() + .insert(id, share.clone()); + } + } + + let mut key_packages = BTreeMap::new(); + let mut public_key_packages = Vec::new(); + + for id in 1..=u32::from(max_signers) { + let received_bcasts: BTreeMap<_, _> = bcasts + .iter() + .filter(|(k, _)| **k != id) + .map(|(k, v)| (*k, v.clone())) + .collect(); + let received_shares = all_shares.remove(&id).unwrap(); + let secret = secrets.remove(&id).unwrap(); + + let (_r2_bcast, key_package, pub_package) = + round2(secret, &received_bcasts, &received_shares).unwrap(); + key_packages.insert(id, key_package); + public_key_packages.push(pub_package); + } + + let message = b"threshold signing"; + let signers: [u32; 2] = [1, 2]; + + let partial_sigs: Vec<_> = signers + .iter() + .map(|&id| BlsPartialSignature::from_key_package(&key_packages[&id], message)) + .collect(); + + let signature = BlsSignature::from_partial_signatures(threshold, &partial_sigs) + .expect("BLS signature combination should succeed"); + + let vk = public_key_packages[0].verifying_key(); + assert!( + signature.verify(vk, message), + "BLS threshold signature should verify" + ); + + assert!( + !signature.verify(vk, b"wrong message"), + "BLS signature should not verify against a different message" + ); + } + + /// Verify that an invalid proof is caught in round2. + #[test] + fn round2_rejects_invalid_proof() { + let mut rng = StdRng::seed_from_u64(99); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (mut bcast1, shares1, _secret1) = + round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast2, _shares2, secret2) = + round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast3, shares3, _secret3) = + round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + + bcast1.ci[31] ^= 0x01; + + let received_bcasts: BTreeMap = + [(1, bcast1.clone()), (3, bcast3.clone())].into(); + let received_shares: BTreeMap = + [(1, shares1[&2].clone()), (3, shares3[&2].clone())].into(); + + let result = round2(secret2, &received_bcasts, &received_shares); + assert!(result.is_err()); + match result.unwrap_err() { + KryptologyError::InvalidProof { culprit } => assert_eq!(culprit, 1), + other => panic!("expected InvalidProof, got {other:?}"), + } + } + + /// Verify that a share addressed to the wrong participant is rejected in + /// round2. + #[test] + fn round2_rejects_share_id_mismatch() { + let mut rng = StdRng::seed_from_u64(42); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (bcast1, shares1, _secret1) = + round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast2, _shares2, secret2) = + round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast3, shares3, _secret3) = + round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + + let received_bcasts: BTreeMap = [(1, bcast1), (3, bcast3)].into(); + + let mut wrong_share = shares1[&2].clone(); + wrong_share.id = 3; + let received_shares: BTreeMap = + [(1, wrong_share), (3, shares3[&2].clone())].into(); + + let result = round2(secret2, &received_bcasts, &received_shares); + assert!(result.is_err()); + match result.unwrap_err() { + KryptologyError::InvalidShare { culprit } => assert_eq!(culprit, 1), + other => panic!("expected InvalidShare, got {other:?}"), + } + } + + #[test] + fn round2_accepts_threshold_subset() { + let mut rng = StdRng::seed_from_u64(321); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (bcast1, shares1, _secret1) = + round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast2, _shares2, secret2) = + round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast3, _shares3, _secret3) = + round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + + let received_bcasts: BTreeMap = [(1, bcast1)].into(); + let received_shares: BTreeMap = [(1, shares1[&2].clone())].into(); + + round2(secret2, &received_bcasts, &received_shares) + .expect("threshold-1 peer packages should be enough"); + } + + #[test] + fn round2_rejects_missing_share_with_culprit() { + let mut rng = StdRng::seed_from_u64(322); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (bcast1, _shares1, _secret1) = + round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast2, _shares2, secret2) = + round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast3, shares3, _secret3) = + round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + + let received_bcasts: BTreeMap = [(1, bcast1), (3, bcast3)].into(); + let received_shares: BTreeMap = [(3, shares3[&2].clone())].into(); + + let result = round2(secret2, &received_bcasts, &received_shares); + assert!(matches!( + result, + Err(KryptologyError::InvalidShare { culprit: 1 }) + )); + } + + #[test] + fn round2_rejects_self_broadcast() { + let mut rng = StdRng::seed_from_u64(323); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (_bcast1, shares1, _secret1) = + round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast2, _shares2, secret2) = + round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + + let received_bcasts: BTreeMap = [(2, bcast2)].into(); + let received_shares: BTreeMap = [(2, shares1[&2].clone())].into(); + + let result = round2(secret2, &received_bcasts, &received_shares); + assert!(matches!( + result, + Err(KryptologyError::InvalidParticipantId(2)) + )); + } + + #[test] + fn from_partial_signatures_rejects_duplicate_signers() { + let mut rng = StdRng::seed_from_u64(324); + let threshold = 2u16; + let max_signers = 2u16; + let ctx = 0u8; + let message = b"duplicate signer"; + + let (bcast1, shares1, secret1) = + round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast2, shares2, secret2) = + round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + + let (_round2_bcast1, key_package1, _public_key_package1) = round2( + secret1, + &[(2, bcast2.clone())].into(), + &[(2, shares2[&1].clone())].into(), + ) + .unwrap(); + let (_round2_bcast2, _key_package2, _public_key_package2) = round2( + secret2, + &[(1, bcast1)].into(), + &[(1, shares1[&2].clone())].into(), + ) + .unwrap(); + + let partial = BlsPartialSignature::from_key_package(&key_package1, message); + let result = + BlsSignature::from_partial_signatures(threshold, &[partial.clone(), partial]); + + assert!(matches!( + result, + Err(KryptologyError::DuplicateIdentifier(1)) + )); + } +} diff --git a/crates/frost/src/lib.rs b/crates/frost/src/lib.rs index 1bb2a571..0884466d 100644 --- a/crates/frost/src/lib.rs +++ b/crates/frost/src/lib.rs @@ -17,5 +17,3 @@ pub use rand_core::{CryptoRng, RngCore}; mod kryptology_interop_tests; #[cfg(test)] mod kryptology_round_trip_tests; -#[cfg(test)] -mod tests; diff --git a/crates/frost/src/tests.rs b/crates/frost/src/tests.rs deleted file mode 100644 index dc149b49..00000000 --- a/crates/frost/src/tests.rs +++ /dev/null @@ -1,415 +0,0 @@ -use std::collections::BTreeMap; - -use rand::{SeedableRng, rngs::StdRng}; - -use crate::kryptology; - -/// RFC 9380 Section 5.3.1 test vector for expand_msg_xmd with SHA-256. -/// DST = "QUUX-V01-CS02-with-expander-SHA256-128" -/// msg = "" (empty), len_in_bytes = 0x20 (32) -#[test] -fn expand_msg_xmd_rfc9380_vector() { - let dst = b"QUUX-V01-CS02-with-expander-SHA256-128"; - let msg = b""; - let expected = - hex::decode("68a985b87eb6b46952128911f2a4412bbc302a9d759667f87f7a21d803f07235").unwrap(); - - let result = kryptology::expand_msg_xmd(msg, dst, 32); - assert_eq!(result, expected, "expand_msg_xmd empty message vector"); -} - -/// RFC 9380 test vector: msg = "abc", len = 32 -#[test] -fn expand_msg_xmd_rfc9380_abc() { - let dst = b"QUUX-V01-CS02-with-expander-SHA256-128"; - let msg = b"abc"; - let expected = - hex::decode("d8ccab23b5985ccea865c6c97b6e5b8350e794e603b4b97902f53a8a0d605615").unwrap(); - - let result = kryptology::expand_msg_xmd(msg, dst, 32); - assert_eq!(result, expected, "expand_msg_xmd abc vector"); -} - -/// RFC 9380 test vector: msg = "", len = 0x80 (128 bytes) -#[test] -fn expand_msg_xmd_rfc9380_long_output() { - let dst = b"QUUX-V01-CS02-with-expander-SHA256-128"; - let msg = b""; - let expected = hex::decode( - "af84c27ccfd45d41914fdff5df25293e221afc53d8ad2ac06d5e3e2948\ - 5dadbee0d121587713a3e0dd4d5e69e93eb7cd4f5df4cd103e188cf60c\ - b02edc3edf18eda8576c412b18ffb658e3dd6ec849469b979d444cf7b2\ - 6911a08e63cf31f9dcc541708d3491184472c2c29bb749d4286b004ceb\ - 5ee6b9a7fa5b646c993f0ced", - ) - .unwrap(); - - let result = kryptology::expand_msg_xmd(msg, dst, 128); - assert_eq!(result, expected, "expand_msg_xmd 128-byte output vector"); -} - -#[test] -fn kryptology_rejects_more_than_255_signers() { - let mut rng = StdRng::seed_from_u64(42); - let result = kryptology::round1(1, 2, 256, 0, &mut rng); - - assert!(matches!( - result, - Err(kryptology::KryptologyError::InvalidSignerCount) - )); -} - -#[test] -fn kryptology_accepts_255_signers_boundary() { - let mut rng = StdRng::seed_from_u64(4242); - let (_bcast, shares, _secret) = kryptology::round1(1, 2, 255, 9, &mut rng) - .expect("255 signers should remain within kryptology's u8 transport limit"); - - assert_eq!(shares.len(), 254); - assert!(shares.contains_key(&255)); -} - -#[test] -fn kryptology_rejects_invalid_signer_counts() { - let mut rng = StdRng::seed_from_u64(7); - - assert!(matches!( - kryptology::round1(1, 1, 3, 0, &mut rng), - Err(kryptology::KryptologyError::FrostCoreError( - crate::FrostCoreError::InvalidMinSigners - )) - )); - assert!(matches!( - kryptology::round1(1, 3, 2, 0, &mut rng), - Err(kryptology::KryptologyError::FrostCoreError( - crate::FrostCoreError::InvalidMinSigners - )) - )); - assert!(matches!( - kryptology::round1(0, 2, 3, 0, &mut rng), - Err(kryptology::KryptologyError::InvalidParticipantId(0)) - )); -} - -/// Full DKG round-trip: 3-of-3 DKG, then BLS threshold sign and verify. -#[test] -fn kryptology_bls_round_trip_3_of_3() { - let mut rng = StdRng::seed_from_u64(42); - let threshold = 3u16; - let max_signers = 3u16; - let ctx = 0u8; - - let mut bcasts: BTreeMap = BTreeMap::new(); - let mut all_shares: BTreeMap> = BTreeMap::new(); - let mut secrets: BTreeMap = BTreeMap::new(); - - for id in 1..=u32::from(max_signers) { - let (bcast, shares, secret) = kryptology::round1(id, threshold, max_signers, ctx, &mut rng) - .expect("round1 should succeed"); - bcasts.insert(id, bcast); - secrets.insert(id, secret); - - for (&target_id, share) in &shares { - all_shares - .entry(target_id) - .or_default() - .insert(id, share.clone()); - } - } - - // --- Round 2: each participant verifies + aggregates --- - let mut key_packages = BTreeMap::new(); - let mut public_key_packages = Vec::new(); - let mut round2_bcasts = BTreeMap::new(); - - for id in 1..=u32::from(max_signers) { - // Collect broadcasts from everyone except ourselves - let received_bcasts: BTreeMap = bcasts - .iter() - .filter(|(k, _)| **k != id) - .map(|(k, v)| (*k, v.clone())) - .collect(); - - let received_shares = all_shares.remove(&id).unwrap(); - let secret = secrets.remove(&id).unwrap(); - - let (r2_bcast, key_package, pub_package) = - kryptology::round2(secret, &received_bcasts, &received_shares) - .expect("round2 should succeed"); - - round2_bcasts.insert(id, r2_bcast); - key_packages.insert(id, key_package); - public_key_packages.push(pub_package); - } - - // All participants should agree on the group verification key - let vk = public_key_packages[0].verifying_key(); - for pkg in &public_key_packages[1..] { - assert_eq!( - vk, - pkg.verifying_key(), - "all participants must agree on the group key" - ); - } - - // All Round2Bcast should carry the same verification_key - let vk_bytes = round2_bcasts[&1].verification_key; - for (&id, bcast) in &round2_bcasts { - assert_eq!( - bcast.verification_key, vk_bytes, - "participant {id} round2 broadcast has different group key" - ); - } - - // BLS sign with all signers (t-of-t) - let message = b"test message"; - - let partial_sigs: Vec<_> = key_packages - .keys() - .map(|&id| kryptology::BlsPartialSignature::from_key_package(&key_packages[&id], message)) - .collect(); - - let signature = kryptology::BlsSignature::from_partial_signatures(threshold, &partial_sigs) - .expect("BLS signature combination should succeed"); - - assert!( - signature.verify(vk, message), - "3-of-3 BLS threshold signature should verify" - ); -} - -/// 2-of-3 DKG then BLS threshold signing (Ethereum 2.0 compatible). -#[test] -fn kryptology_bls_round_trip_2_of_3() { - let mut rng = StdRng::seed_from_u64(123); - let threshold = 2u16; - let max_signers = 3u16; - let ctx = 0u8; - - // Round 1 - let mut bcasts: BTreeMap = BTreeMap::new(); - let mut all_shares: BTreeMap> = BTreeMap::new(); - let mut secrets: BTreeMap = BTreeMap::new(); - - for id in 1..=u32::from(max_signers) { - let (bcast, shares, secret) = - kryptology::round1(id, threshold, max_signers, ctx, &mut rng).unwrap(); - bcasts.insert(id, bcast); - secrets.insert(id, secret); - for (&target_id, share) in &shares { - all_shares - .entry(target_id) - .or_default() - .insert(id, share.clone()); - } - } - - // Round 2 - let mut key_packages = BTreeMap::new(); - let mut public_key_packages = Vec::new(); - - for id in 1..=u32::from(max_signers) { - let received_bcasts: BTreeMap<_, _> = bcasts - .iter() - .filter(|(k, _)| **k != id) - .map(|(k, v)| (*k, v.clone())) - .collect(); - let received_shares = all_shares.remove(&id).unwrap(); - let secret = secrets.remove(&id).unwrap(); - - let (_r2_bcast, key_package, pub_package) = - kryptology::round2(secret, &received_bcasts, &received_shares).unwrap(); - key_packages.insert(id, key_package); - public_key_packages.push(pub_package); - } - - // BLS sign with only participants 1 and 2 (threshold = 2) - let message = b"threshold signing"; - let signers: [u32; 2] = [1, 2]; - - let partial_sigs: Vec<_> = signers - .iter() - .map(|&id| kryptology::BlsPartialSignature::from_key_package(&key_packages[&id], message)) - .collect(); - - let signature = kryptology::BlsSignature::from_partial_signatures(threshold, &partial_sigs) - .expect("BLS signature combination should succeed"); - - let vk = public_key_packages[0].verifying_key(); - assert!( - signature.verify(vk, message), - "BLS threshold signature should verify" - ); - - // Verify wrong message fails - assert!( - !signature.verify(vk, b"wrong message"), - "BLS signature should not verify against a different message" - ); -} - -/// Verify that an invalid proof is caught in round2. -#[test] -fn kryptology_invalid_proof_rejected() { - let mut rng = StdRng::seed_from_u64(99); - let threshold = 2u16; - let max_signers = 3u16; - let ctx = 0u8; - - let (mut bcast1, shares1, _secret1) = - kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (_bcast2, _shares2, secret2) = - kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast3, shares3, _secret3) = - kryptology::round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); - - // Corrupt participant 1's proof (flip LSB of ci, keeping it a valid scalar) - bcast1.ci[31] ^= 0x01; - - // Participant 2 should reject participant 1's proof - let received_bcasts: BTreeMap = - [(1, bcast1.clone()), (3, bcast3.clone())].into(); - let received_shares: BTreeMap = - [(1, shares1[&2].clone()), (3, shares3[&2].clone())].into(); - - let result = kryptology::round2(secret2, &received_bcasts, &received_shares); - assert!(result.is_err()); - match result.unwrap_err() { - kryptology::KryptologyError::InvalidProof { culprit } => assert_eq!(culprit, 1), - other => panic!("expected InvalidProof, got {other:?}"), - } -} - -/// Verify that a share addressed to the wrong participant is rejected in -/// round2. -#[test] -fn kryptology_share_id_mismatch_rejected() { - let mut rng = StdRng::seed_from_u64(42); - let threshold = 2u16; - let max_signers = 3u16; - let ctx = 0u8; - - let (bcast1, shares1, _secret1) = - kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (_bcast2, _shares2, secret2) = - kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast3, shares3, _secret3) = - kryptology::round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); - - let received_bcasts: BTreeMap = [(1, bcast1), (3, bcast3)].into(); - - let mut wrong_share = shares1[&2].clone(); - wrong_share.id = 3; - let received_shares: BTreeMap = - [(1, wrong_share), (3, shares3[&2].clone())].into(); - - let result = kryptology::round2(secret2, &received_bcasts, &received_shares); - assert!(result.is_err()); - match result.unwrap_err() { - kryptology::KryptologyError::InvalidShare { culprit } => assert_eq!(culprit, 1), - other => panic!("expected InvalidShare, got {other:?}"), - } -} - -#[test] -fn kryptology_round2_accepts_threshold_subset() { - let mut rng = StdRng::seed_from_u64(321); - let threshold = 2u16; - let max_signers = 3u16; - let ctx = 0u8; - - let (bcast1, shares1, _secret1) = - kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (_bcast2, _shares2, secret2) = - kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - let (_bcast3, _shares3, _secret3) = - kryptology::round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); - - let received_bcasts: BTreeMap = [(1, bcast1)].into(); - let received_shares: BTreeMap = [(1, shares1[&2].clone())].into(); - - kryptology::round2(secret2, &received_bcasts, &received_shares) - .expect("threshold-1 peer packages should be enough"); -} - -#[test] -fn kryptology_round2_rejects_missing_share_with_culprit() { - let mut rng = StdRng::seed_from_u64(322); - let threshold = 2u16; - let max_signers = 3u16; - let ctx = 0u8; - - let (bcast1, _shares1, _secret1) = - kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (_bcast2, _shares2, secret2) = - kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast3, shares3, _secret3) = - kryptology::round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); - - let received_bcasts: BTreeMap = [(1, bcast1), (3, bcast3)].into(); - let received_shares: BTreeMap = [(3, shares3[&2].clone())].into(); - - let result = kryptology::round2(secret2, &received_bcasts, &received_shares); - assert!(matches!( - result, - Err(kryptology::KryptologyError::InvalidShare { culprit: 1 }) - )); -} - -#[test] -fn kryptology_round2_rejects_self_broadcast() { - let mut rng = StdRng::seed_from_u64(323); - let threshold = 2u16; - let max_signers = 3u16; - let ctx = 0u8; - - let (_bcast1, shares1, _secret1) = - kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast2, _shares2, secret2) = - kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - - let received_bcasts: BTreeMap = [(2, bcast2)].into(); - let received_shares: BTreeMap = [(2, shares1[&2].clone())].into(); - - let result = kryptology::round2(secret2, &received_bcasts, &received_shares); - assert!(matches!( - result, - Err(kryptology::KryptologyError::InvalidParticipantId(2)) - )); -} - -#[test] -fn kryptology_duplicate_partial_signatures_rejected() { - let mut rng = StdRng::seed_from_u64(324); - let threshold = 2u16; - let max_signers = 2u16; - let ctx = 0u8; - let message = b"duplicate signer"; - - let (bcast1, shares1, secret1) = - kryptology::round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast2, shares2, secret2) = - kryptology::round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - - let (_round2_bcast1, key_package1, _public_key_package1) = kryptology::round2( - secret1, - &[(2, bcast2.clone())].into(), - &[(2, shares2[&1].clone())].into(), - ) - .unwrap(); - let (_round2_bcast2, _key_package2, _public_key_package2) = kryptology::round2( - secret2, - &[(1, bcast1)].into(), - &[(1, shares1[&2].clone())].into(), - ) - .unwrap(); - - let partial = kryptology::BlsPartialSignature::from_key_package(&key_package1, message); - let result = - kryptology::BlsSignature::from_partial_signatures(threshold, &[partial.clone(), partial]); - - assert!(matches!( - result, - Err(kryptology::KryptologyError::DuplicateIdentifier(1)) - )); -} From 3ce9ada38895ec04135671640add6bb66613be86 Mon Sep 17 00:00:00 2001 From: Quang Le Date: Fri, 1 May 2026 16:34:43 +0700 Subject: [PATCH 6/7] fix: simplify tests --- crates/frost/src/curve.rs | 6 ++++ crates/frost/src/kryptology.rs | 65 ++++++++++++++++++++++------------ 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/crates/frost/src/curve.rs b/crates/frost/src/curve.rs index b91224ff..2c690628 100644 --- a/crates/frost/src/curve.rs +++ b/crates/frost/src/curve.rs @@ -370,6 +370,12 @@ mod tests { assert_eq!(scalar * inverse, Scalar::ONE); } + #[test] + fn g1_projective_identity_reports_identity() { + assert!(G1Projective::identity().is_identity()); + assert!(!G1Projective::generator().is_identity()); + } + #[test] fn g1_projective_rejects_identity_compressed_point() { let identity = G1Affine::from(G1Projective::identity()).to_compressed(); diff --git a/crates/frost/src/kryptology.rs b/crates/frost/src/kryptology.rs index 7a861091..4a9a3aa8 100644 --- a/crates/frost/src/kryptology.rs +++ b/crates/frost/src/kryptology.rs @@ -430,9 +430,8 @@ pub fn round2( // Reconstruct R' = Wi*G - Ci*A_{j,0} let r_reconstructed = G1Projective::generator() * wi - a0 * ci; - let sender_id_u8 = - u8::try_from(sender_id) - .map_err(|_| KryptologyError::InvalidParticipantId(sender_id))?; + let sender_id_u8 = u8::try_from(sender_id) + .map_err(|_| KryptologyError::InvalidParticipantId(sender_id))?; let ci_check = kryptology_challenge(sender_id_u8, secret.ctx, &a0, &r_reconstructed); if !ci_check.constant_time_eq(&ci) { return Err(KryptologyError::InvalidProof { culprit: sender_id }); @@ -880,7 +879,8 @@ mod tests { let mut secrets: BTreeMap = BTreeMap::new(); for id in 1..=u32::from(max_signers) { - let (bcast, shares, secret) = round1(id, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast, shares, secret) = + round1(id, threshold, max_signers, ctx, &mut rng).unwrap(); bcasts.insert(id, bcast); secrets.insert(id, secret); for (&target_id, share) in &shares { @@ -925,6 +925,10 @@ mod tests { signature.verify(vk, message), "BLS threshold signature should verify" ); + let signature_bytes = signature.to_bytes(); + let parsed_signature = blst::min_pk::Signature::from_bytes(&signature_bytes) + .expect("combined signature should serialize to compressed bytes"); + assert_eq!(parsed_signature.to_bytes(), signature_bytes); assert!( !signature.verify(vk, b"wrong message"), @@ -944,8 +948,7 @@ mod tests { round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); let (_bcast2, _shares2, secret2) = round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast3, shares3, _secret3) = - round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast3, shares3, _secret3) = round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); bcast1.ci[31] ^= 0x01; @@ -962,6 +965,32 @@ mod tests { } } + #[test] + fn round2_rejects_zero_challenge() { + let mut rng = StdRng::seed_from_u64(98); + let threshold = 2u16; + let max_signers = 3u16; + let ctx = 0u8; + + let (mut bcast1, shares1, _secret1) = + round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (_bcast2, _shares2, secret2) = + round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + + bcast1.ci = [0; 32]; + + let result = round2( + secret2, + &[(1, bcast1)].into(), + &[(1, shares1[&2].clone())].into(), + ); + + assert!(matches!( + result, + Err(KryptologyError::InvalidProof { culprit: 1 }) + )); + } + /// Verify that a share addressed to the wrong participant is rejected in /// round2. #[test] @@ -971,12 +1000,10 @@ mod tests { let max_signers = 3u16; let ctx = 0u8; - let (bcast1, shares1, _secret1) = - round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast1, shares1, _secret1) = round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); let (_bcast2, _shares2, secret2) = round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast3, shares3, _secret3) = - round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast3, shares3, _secret3) = round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); let received_bcasts: BTreeMap = [(1, bcast1), (3, bcast3)].into(); @@ -1000,8 +1027,7 @@ mod tests { let max_signers = 3u16; let ctx = 0u8; - let (bcast1, shares1, _secret1) = - round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast1, shares1, _secret1) = round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); let (_bcast2, _shares2, secret2) = round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); let (_bcast3, _shares3, _secret3) = @@ -1025,8 +1051,7 @@ mod tests { round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); let (_bcast2, _shares2, secret2) = round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast3, shares3, _secret3) = - round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast3, shares3, _secret3) = round1(3, threshold, max_signers, ctx, &mut rng).unwrap(); let received_bcasts: BTreeMap = [(1, bcast1), (3, bcast3)].into(); let received_shares: BTreeMap = [(3, shares3[&2].clone())].into(); @@ -1047,8 +1072,7 @@ mod tests { let (_bcast1, shares1, _secret1) = round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast2, _shares2, secret2) = - round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast2, _shares2, secret2) = round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); let received_bcasts: BTreeMap = [(2, bcast2)].into(); let received_shares: BTreeMap = [(2, shares1[&2].clone())].into(); @@ -1068,10 +1092,8 @@ mod tests { let ctx = 0u8; let message = b"duplicate signer"; - let (bcast1, shares1, secret1) = - round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); - let (bcast2, shares2, secret2) = - round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast1, shares1, secret1) = round1(1, threshold, max_signers, ctx, &mut rng).unwrap(); + let (bcast2, shares2, secret2) = round1(2, threshold, max_signers, ctx, &mut rng).unwrap(); let (_round2_bcast1, key_package1, _public_key_package1) = round2( secret1, @@ -1087,8 +1109,7 @@ mod tests { .unwrap(); let partial = BlsPartialSignature::from_key_package(&key_package1, message); - let result = - BlsSignature::from_partial_signatures(threshold, &[partial.clone(), partial]); + let result = BlsSignature::from_partial_signatures(threshold, &[partial.clone(), partial]); assert!(matches!( result, From 87c692c0eaa7e168c4b0329edfd0b12281ea1c3a Mon Sep 17 00:00:00 2001 From: Quang Le Date: Sat, 2 May 2026 18:26:45 +0700 Subject: [PATCH 7/7] fix: address comments --- crates/frost/src/kryptology.rs | 42 +++++++++++++++----- crates/frost/src/kryptology_interop_tests.rs | 3 ++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/crates/frost/src/kryptology.rs b/crates/frost/src/kryptology.rs index 4a9a3aa8..08f94dba 100644 --- a/crates/frost/src/kryptology.rs +++ b/crates/frost/src/kryptology.rs @@ -9,7 +9,7 @@ //! The output types ([`KeyPackage`], [`PublicKeyPackage`]) are standard //! frost-core types usable with frost-core's signing protocol. -use std::collections::BTreeMap; +use std::{collections::BTreeMap, fmt}; use blst::*; use rand_core::{CryptoRng, RngCore}; @@ -171,7 +171,7 @@ pub fn scalar_from_be(bytes: &[u8; 32]) -> Result { /// RFC 9380 Section 5.3.1 using SHA-256 #[allow(clippy::arithmetic_side_effects)] -pub(crate) fn expand_msg_xmd(msg: &[u8], dst: &[u8], len_in_bytes: usize) -> Vec { +fn expand_msg_xmd(msg: &[u8], dst: &[u8], len_in_bytes: usize) -> Vec { const B_IN_BYTES: usize = 32; // SHA-256 output const S_IN_BYTES: usize = 64; // SHA-256 block size @@ -505,6 +505,22 @@ pub struct BlsPartialSignature { point: blst_p2, } +impl fmt::Debug for BlsPartialSignature { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut affine = blst_p2_affine::default(); + let mut bytes = [0u8; 96]; + unsafe { + blst_p2_to_affine(&mut affine, &self.point); + blst_p2_affine_compress(bytes.as_mut_ptr(), &affine); + } + + f.debug_struct("BlsPartialSignature") + .field("identifier", &self.identifier) + .field("point", &bytes) + .finish() + } +} + impl BlsPartialSignature { /// Produce a BLS partial signature from a [`KeyPackage`] produced by /// kryptology DKG. @@ -527,6 +543,14 @@ pub struct BlsSignature { point: blst_p2, } +impl fmt::Debug for BlsSignature { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("BlsSignature") + .field(&self.to_bytes()) + .finish() + } +} + impl BlsSignature { /// Serialize to 96-byte compressed G2 point. pub fn to_bytes(&self) -> [u8; 96] { @@ -569,7 +593,6 @@ impl BlsSignature { .collect(); let mut combined = blst_p2::default(); - let mut first = true; for (i, ps) in partial_sigs.iter().enumerate() { // Lagrange coefficient: L_i(0) = prod_{j!=i} ( x_j / (x_j - x_i) ) @@ -580,20 +603,17 @@ impl BlsSignature { } let num = x_vals[j]; let den = x_vals[j] - x_vals[i]; + // Duplicate identifiers are rejected above, so this should + // only fail if the invariant is broken. let den_inv = den.invert().ok_or(KryptologyError::InvalidSignerCount)?; lambda = lambda * num * den_inv; } let weighted = p2_mult(&ps.point, &lambda); - if first { - combined = weighted; - first = false; - } else { - let mut tmp = blst_p2::default(); - unsafe { blst_p2_add_or_double(&mut tmp, &combined, &weighted) }; - combined = tmp; - } + let mut tmp = blst_p2::default(); + unsafe { blst_p2_add_or_double(&mut tmp, &combined, &weighted) }; + combined = tmp; } Ok(BlsSignature { point: combined }) diff --git a/crates/frost/src/kryptology_interop_tests.rs b/crates/frost/src/kryptology_interop_tests.rs index 8107912d..d62a30a3 100644 --- a/crates/frost/src/kryptology_interop_tests.rs +++ b/crates/frost/src/kryptology_interop_tests.rs @@ -189,6 +189,9 @@ fn replay_fixture(json: &str, require_group_signature: bool) { } if !require_group_signature { + // Error fixtures assert each participant's expected round2 outcome + // above; they intentionally do not produce enough key packages for a + // group signature check. return; }