feat(dkg): FROST implementation

[emlautarom1]

Partially resolves #254.

---

Based on https://github.com/ZcashFoundation/frost, we build a library implementing FROST compatible with Coinbase's Kryptology (https://github.com/coinbase/kryptology/blob/v1.8.0/pkg/dkg/frost/README.md). No new dependencies are added to the workspace.

I've created some custom Go test cases in Charon that are not part of this PR that generate some fixtures with intermediate values which gives us some initial confidence that we can interop with Go nodes. Still, full compatibility requires further attention.

This PR requires nuanced review due to the challenging cryptography aspects of the code.

[emlautarom1]

@claude Review

[claude]

Claude finished @emlautarom1's task in 7m 8s —— View job

---

Code Review — feat(dkg): FROST implementation

Todo List

• Gather context and understand the request
• Read crates/frost/Cargo.toml and src/lib.rs
• Review crates/frost/src/curve.rs (elliptic curve abstractions)
• Review crates/frost/src/frost_core.rs (core FROST protocol)
• Review crates/frost/src/kryptology.rs (Kryptology compatibility layer)
• Review crates/frost/src/tests.rs and test fixtures
• Review crates/frost/tests/kryptology_interop.rs and kryptology_round_trip.rs
• Review crates/frost/dkg.md documentation
• Compile and post final review

---

This is a well-structured implementation. The protocol math is correct, the test coverage is solid, and the fixture-based cross-language interop is a nice confidence booster. Below are findings by severity.

---

🔴 High — Secret Material Not Zeroized

Round1Secret.coefficients (Vec<Scalar>), SigningShare (wrapping a Scalar), and KeyPackage.signing_share all hold secret key material but have no Drop implementation that clears the memory. Process memory dumps, swap files, or memory-safety bugs elsewhere in the process can recover key material.

// crates/frost/src/kryptology.rs:103-110
pub struct Round1Secret {
    id: u32,
    ctx: u8,
    coefficients: Vec<Scalar>,   // ← secret polynomial coefficients
    ...
}

// crates/frost/src/frost_core.rs:138
pub struct SigningShare(Scalar);  // ← secret scalar, no ZeroizeOnDrop

Recommendation: Add zeroize as a dependency and derive ZeroizeOnDrop for Round1Secret, SigningShare, and any other types that own secret scalars. Fix this →

---

🔴 High — Scalar::from_bytes_wide Does Not Actually Reduce 64 Bytes

The docstring says "Reduce 64 little-endian bytes modulo the scalar field order", but blst_scalar_from_le_bytes internally reads at most 32 bytes (the size of blst_scalar). The upper 32 bytes of the 64-byte input are silently dropped:

// crates/frost/src/curve.rs:71-79
pub fn from_bytes_wide(bytes: &[u8; 64]) -> Self {
    let mut scalar = blst_scalar::default();
    let mut fr = blst_fr::default();
    unsafe {
        blst_scalar_from_le_bytes(&mut scalar, bytes.as_ptr(), 64); // reads only [0..32]
        blst_fr_from_scalar(&mut fr, &scalar);
    }
    Scalar(fr)
}

Effect on Scalar::random (curve.rs:82-86): 64 random bytes are generated but only the first 32 are used. These 32 bytes represent a uniform draw from [0, 2^256). Since floor(2^256 / r) = 2 (BLS12-381 r ≈ 0.453 × 2^256), values in [0, 2^256 mod r) have ~50% higher probability — a statistical distance from uniform of ≈ 9%. For secret polynomial coefficients and Schnorr nonces, this is a meaningful bias.

Effect on kryptology_hash_to_scalar: Only 32 of the 48-byte XMD output are actually processed. The fixture tests passing strongly suggests kryptology's Go blst bindings exhibit the same truncation behavior, so the hash-to-scalar IS compatible — but the function is doing less work than advertised.

Recommendation: Either use rejection sampling (generate 32 bytes, call blst_scalar_fr_check, retry if out of range), or confirm and document that this matches kryptology's exact blst behavior (accepting the minor statistical bias). Also update the docstring to reflect reality. Fix this →

---

🟡 Medium — Missing std::error::Error + Display Impls

Neither DkgError nor FrostCoreError implements std::error::Error or std::fmt::Display. This prevents ergonomic use with ? into Box<dyn Error>, anyhow, or any crate that wraps errors.

// crates/frost/src/kryptology.rs:24
pub enum DkgError { ... }  // no Error, no Display

// crates/frost/src/frost_core.rs:18
pub enum FrostCoreError { ... }  // same

thiserror is already in the workspace. Fix this →

---

🟡 Medium — Round1Secret::from_raw Should Be Test-Only

from_raw exists solely to reconstruct a Round1Secret from fixture data — it has no valid production use case (in real DKG, the secret is always created by round1). It is currently public, which exposes a subtle escape hatch where the polynomial coefficients are faked ([own_share_scalar, 0, 0, ...]). If a caller uses it incorrectly, round2 will silently produce wrong key material.

// crates/frost/src/kryptology.rs:112-143
pub fn from_raw(...) -> Result<Self, DkgError> {
    let mut coefficients = vec![Scalar::ZERO; threshold as usize];
    coefficients[0] = own_share_scalar; // polynomial trick for test replay
    ...
}

Recommendation: Gate behind #[cfg(any(test, feature = "testing"))] or move to the test module. If kept public, document prominently that this is for testing only and that the coefficients stored are synthetic (not the real polynomial). Fix this →

---

🟡 Medium — BlsPartialSignature::from_key_package — Mismatched id Risk

The id parameter is separate from the KeyPackage, and there is no internal check that they agree:

// crates/frost/src/kryptology.rs:483
pub fn from_key_package(id: u32, key_package: &KeyPackage, msg: &[u8]) -> BlsPartialSignature {
    // id is used as the Lagrange x-coordinate but KeyPackage.identifier() is never consulted

If a caller passes the wrong id, Lagrange interpolation will silently produce an incorrect combined signature. The signature won't verify, but there is no runtime diagnostic.

Recommendation: Read id from key_package.identifier() instead of taking it as a parameter, or assert agreement: debug_assert_eq!(Identifier::from_u32(id).map(|i| i.to_scalar()), Ok(key_package.identifier().to_scalar())). Fix this →

---

🟢 Low — expand_msg_xmd Panics Instead of Returning Errors

// crates/frost/src/kryptology.rs:165-168
assert!(ell <= 255, "RFC 9380: ell must be at most 255");
assert!(len_in_bytes <= 65535, ...);
assert!(dst.len() <= 255, ...);

These assertions will panic! if called with oversized inputs. Since the DST is a hardcoded constant and len_in_bytes is always 48, this is never reachable in production — but it makes expand_msg_xmd a footgun for any future caller. Consider returning a Result or making the function pub(crate) and documenting the preconditions.

---

🟢 Low — Blanket #![allow(non_snake_case)] in lib.rs

// crates/frost/src/lib.rs:6
#![allow(non_snake_case)]

This covers the entire crate. The FROST naming convention (R, A0, G) only applies to a handful of local variables in kryptology.rs. Prefer per-function #[allow(non_snake_case)] attributes. Fix this →

---

🟢 Low — blst_p1_mult / blst_p2_mult nbits=255 Should Have a Comment

// crates/frost/src/curve.rs:217
blst_p1_mult(&mut out, &self.0, scalar.b.as_ptr(), 255);
// crates/frost/src/kryptology.rs:613
blst_p2_mult(&mut out, point, s.b.as_ptr(), 255);

This is correct (BLS12-381 r has its highest bit at position 254, so 255 bits suffices), but a one-line comment would save the next reviewer from having to verify it independently.

---

🟢 Low — Glob Re-exports Obscure the Public API Surface

// crates/frost/src/lib.rs
pub use curve::*;
pub use frost_core::*;

This makes it hard to determine the intended public API without reading each module. Explicit re-exports would be clearer for semver stability and documentation. Consider using explicit pub use curve::{Scalar, G1Affine, G1Projective}; etc.

---

🟢 Low — Missing Test Coverage

• validate_num_of_signers with min_signers < 2 and min_signers > max_signers
• from_partial_signatures with duplicate identifiers
• BLS signing with a non-consecutive subset of signers (e.g., participants 1 and 3 out of 1-2-3-4)
• Identifier::from_u32(0) — error case

---

✅ Positive Observations

• Protocol math is correct: The Schnorr proof wi = k + a0·ci with verification R' = wi·G − ci·A0 is consistent throughout.
• RFC 9380 test vectors: expand_msg_xmd is validated against the spec.
• Cross-language fixtures: Fixture interop gives real confidence in wire format compatibility.
• Subgroup check on deserialization: blst_p1_affine_in_g1 + identity rejection in G1Projective::from_compressed prevents small-subgroup attacks.
• Horner's method in evaluate_polynomial: efficient and correct for threshold-degree polynomials.
• Culprit identification in error variants (InvalidProof { culprit }, InvalidShare { culprit }) — important for operational diagnostics.
• Round1Secret not Clone/Debug: prevents accidental log leakage of the secret polynomial.
• missing_docs = "deny" and strict clippy lints (unwrap_used, cast_lossless, etc.) in Cargo.toml — good hygiene.

---

[mskrzypkows]

It can be done without manually reversing:

fn kryptology_hash_to_scalar(msg: &[u8]) -> Scalar {
    let xmd = expand_msg_xmd(msg, KRYPTOLOGY_DST, 48);
    Scalar::from_be_bytes_wide(&xmd)
}

and new Scalar function needed:

    /// 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)
    }

[varex83agent]

This PR adds the pluto-frost crate implementing a FROST-compatible DKG and BLS threshold signing over BLS12-381, targeting interoperability with Go's Coinbase Kryptology FROST DKG. The cryptographic math has been verified correct: the Horner polynomial evaluation, challenge preimage format, expand_msg_xmd construction (matching RFC 9380 and confirmed against test vectors), blst_p1_mult bit-width (255 is correct for the BLS12-381 scalar field), and the Scalar::ONE Montgomery constant all check out.

Major issues requiring changes before merge:

1. Secret key types (SigningShare, KeyPackage, Round1Secret) have no Zeroize/Drop implementation — signing key material lingers in process memory.
2. round2 enforces exactly max_signers-1 received packages; Go's implementation accepts any count in [threshold-1, max_signers] for fault-tolerance, meaning a single offline peer aborts every honest participant in the Rust version.
3. expand_msg_xmd is pub with three assert!() calls that will panic the process if a downstream caller passes e.g. a DST > 255 bytes.
4. Round1Secret::from_raw performs no input validation (id, threshold, max_signers), enabling underflow in round2 and bypassed peer verification when max_signers=1.
5. FrostCoreError and DkgError do not implement std::error::Error, breaking usage with ? in downstream crates.

Minor/nit findings cover test coverage gaps (no ctx≠0 test, no tampered-share test), API surface issues (glob re-exports, public fields on wire types), and a non-constant-time scalar equality used in proof verification.

[varex83agent]

major: round2 requires exactly max_signers-1 packages; Go allows threshold-1..max_signers

Go's Round2 (dkg_round2.go) accepts threshold-1 <= len(bcast) <= max_signers, allowing fault-tolerant DKG where an offline or slow peer does not abort the round for everyone else — as long as at least threshold-1 other peers responded, the participant can complete. The Rust implementation requires exactly max_signers - 1:

let expected = (secret.max_signers - 1) as usize;
if received_bcasts.len() != expected || received_shares.len() != expected {
    return Err(DkgError::IncorrectPackageCount);
}

This is a functional deviation: a single missing broadcast fails round2 for every honest participant even when enough shares exist to complete DKG. This should either be relaxed to >= (secret.threshold - 1) as usize, or explicitly documented as a deliberate stricter constraint.

ai agent review

[varex83]

Since it already works, let's merge this and make all fixes in #364

[iamquang95]

@varex83 The #364: I addressed the comments, I think it can be merged into this PR. So we have a "full" pr related to FROST implementation, also check the addressed comments which related to this PR.

[iamquang95]

@claude

[claude]

Claude finished @iamquang95's task in 2m 55s —— View job

---

Todo List

• Gather context and read current code state
• Review what issues from previous reviews remain open vs addressed
• Post findings

---

Review — Current State of feat(dkg): FROST implementation

I've read the current branch HEAD (crates/frost/src/{lib,curve,frost_core,kryptology}.rs). The vast majority of concerns from the previous review rounds have been addressed. Here is a full accounting.

---

✅ Addressed

Issue	Where fixed
Zeroize on secret types	Round1Secret, SigningShare, KeyPackage, ShamirShare all derive ZeroizeOnDrop; Scalar implements Zeroize
FrostCoreError / DkgError missing std::error::Error	Both now use thiserror::Error; error renamed KryptologyError (more appropriate)
Round1Secret::from_raw public + no validation	Gated behind #[cfg(test)]/pub(crate); calls validate_round_parameters
from_key_package id mismatch	Now reads id from key_package.identifier().to_u32()
round2 too strict (required exactly max_signers-1)	Now accepts threshold-1 ..= max_signers-1 (received_bcasts.len() < min_received || > max_received)
No zero-ci guard	Added explicit rejection at kryptology.rs:427-429
Missing-share error doesn't identify culprit	Changed to KryptologyError::InvalidShare { culprit: sender_id }
Scalar PartialEq not constant-time	Proof check uses ci_check.constant_time_eq(&ci)
kryptology_hash_to_scalar byte-reversal	Now uses Scalar::from_be_bytes_wide directly
Identifier recomputes scalar on every call	Now stores both u32 and Scalar
#![allow(non_snake_case)] crate-level	Removed from lib.rs and not present in any module
expand_msg_xmd public with panics	Now private (fn, not pub fn)
blst_p1_mult/blst_p2_mult nbits=255 unexplained	Comments added in curve.rs:241 and kryptology.rs:661
pub use rand_core re-exports entire crate	Changed to pub use rand_core::{CryptoRng, RngCore}
Unnecessary inner scope in from_key_package	Simplified
sender_id == secret.id not checked in round2	Added check at kryptology.rs:416-418
Test coverage gaps	New tests: round2_accepts_threshold_subset, round2_rejects_missing_share_with_culprit, round2_rejects_self_broadcast, round2_rejects_zero_challenge, from_partial_signatures_rejects_duplicate_signers

---

⚠️ Still Outstanding

1. Scalar::random uses biased 32-byte draw — Medium

Scalar::random generates 64 random bytes then calls from_bytes_wide, but blst_scalar_from_le_bytes internally only reads the first 32 bytes regardless of the nbytes argument (the scalar struct holds 4×u64 = 32 bytes). The 64-byte rng.fill_bytes call thus generates 64 bytes but discards the upper half. The resulting 32-byte uniform draw in [0, 2^256) has a ~9% statistical bias toward values in [0, 2^256 mod r) since floor(2^256/r) = 2 and the BLS12-381 scalar order r ≈ 0.453 × 2^256. This affects polynomial coefficients and Schnorr nonces. Rejection sampling (generate 32 bytes, call blst_scalar_fr_check, retry if ≥ r) would be unbiased. Fix this →

2. from_bytes_wide docstring is misleading — Low

// curve.rs:72
/// Reduce 64 little-endian bytes modulo the scalar field order.
pub fn from_bytes_wide(bytes: &[u8; 64]) -> Self {
    blst_scalar_from_le_bytes(&mut scalar, bytes.as_ptr(), 64); // reads only [0..32]

The docstring promises 64-byte wide reduction but blst only materialises 32 bytes. Should say: "Deserialize from 64 LE bytes; only the first 32 bytes are used due to blst internals." Fix this →

3. Glob re-exports — Low

lib.rs:12-13 still has pub use curve::*; pub use frost_core::*;. Every pub item (including SecretShare, CoefficientCommitment, VerifiableSecretSharingCommitment) is part of the crate's public API. Enumerating explicit re-exports would give cleaner semver stability.

4. Missing non-consecutive signer subset test — Low

Both existing BLS round-trip tests use signers {1,2} or {1,2,3}. A test with {1,3} from a {1,2,3} DKG would confirm that Lagrange interpolation works correctly when participants are non-contiguous. Fix this →

---

Summary

This PR is in good shape. All high and medium issues from both previous review rounds have been resolved. The two items worth addressing before merge are the Scalar::random bias (#1 above) and the misleading docstring (#2); both are small, self-contained fixes. Items #3 and #4 are quality-of-life and can follow in #364.

[iamquang95]

Seem good, let's merge

[iamquang95]

Currently ignore RUSTSEC-2026-0118 and RUSTSEC-2026-0119 since there are some libp2p crates are depedning on that. Not a blocker. Issue is tracked here: #365

cc @varex83 good to merge