feat(dkg): FROST implementation by emlautarom1 · Pull Request #328 · NethermindEth/pluto

emlautarom1 · 2026-04-14T22:28:47Z

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.

- Adjust `&` handling in patterns

- Replaces custom deserialization code

- Use vec directly

- Infallible conversions - Disable arithmetic checks

- Use `assert!` over `debug_assert!` - Reference RFC when required

emlautarom1 · 2026-04-14T22:29:58Z

@claude Review

claude · 2026-04-14T22:30:11Z

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 · 2026-04-20T08:32:37Z

+///
+/// `ExpandMsgXmd(SHA-256, msg, DST, 48)` -> reverse bytes -> pad to 64 ->
+/// `Scalar::from_bytes_wide`.
+fn kryptology_hash_to_scalar(msg: &[u8]) -> Scalar {


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

mskrzypkows · 2026-04-20T09:01:22Z

+    rng: &mut R,
+) -> Result<(Round1Bcast, BTreeMap<u32, ShamirShare>, Round1Secret), DkgError> {
+    // Kryptology encodes participant identifiers into a single byte.
+    if max_signers > u16::from(u8::MAX) {


Round1Secret::from_raw misses the validation logic (lines 282 - 290). What do you think about extracting it to the separate function and using in both places?

mskrzypkows · 2026-04-20T09:26:42Z

+        BTreeMap::new();
+    let mut share_sum = Scalar::ZERO;
+
+    for (&sender_id, bcast) in received_bcasts {


We could check secret.id == sender_id and return an error to prevent some wrong usage of the round2 function.

agree on this

mskrzypkows · 2026-04-20T10:49:57Z

+    /// The `id` must be the original 1-indexed kryptology participant ID.
+    pub fn from_key_package(id: u32, key_package: &KeyPackage, msg: &[u8]) -> BlsPartialSignature {
+        let scalar = key_package.signing_share().to_scalar();
+        {


Why is it nested ?

mskrzypkows · 2026-04-20T11:21:29Z

+//! Contains the key material types (identifiers, shares, packages) and the
+//! polynomial evaluation functions needed by the kryptology-compatible DKG.
+
+#![allow(clippy::arithmetic_side_effects)]


Would be better to limit it to some specific scopes.

mskrzypkows · 2026-04-20T11:21:47Z

+//! The output types ([`KeyPackage`], [`PublicKeyPackage`]) are standard
+//! frost-core types usable with frost-core's signing protocol.
+
+#![allow(clippy::arithmetic_side_effects)]


Would be better to limit it to some specific scopes.

mskrzypkows · 2026-04-20T11:51:46Z

+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.
+    fn cmp(&self, other: &Self) -> Ordering {


This can be possibly optimized if we would keep u32 value at the construction time:

#[derive(Copy, Clone, Debug)] pub struct Identifier { id: u32, scalar: Scalar, } impl Identifier { pub fn from_u32(id: u32) -> Result<Self, FrostCoreError> { let scalar = Scalar::from(u64::from(id)); if scalar == Scalar::ZERO { Err(FrostCoreError::InvalidZeroScalar) } else { Ok(Self { id, scalar }) } } pub fn to_scalar(&self) -> Scalar { self.scalar } } impl Ord for Identifier { fn cmp(&self, other: &Self) -> Ordering { self.id.cmp(&other.id) } }

mskrzypkows · 2026-04-20T12:10:58Z

+
+/// Errors from the kryptology-compatible DKG.
+#[derive(Debug)]
+pub enum DkgError {


Would be nice to cover all errors with tests.

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:

Secret key types (SigningShare, KeyPackage, Round1Secret) have no Zeroize/Drop implementation — signing key material lingers in process memory.
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.
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.
Round1Secret::from_raw performs no input validation (id, threshold, max_signers), enabling underflow in round2 and bypassed peer verification when max_signers=1.
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 · 2026-04-23T12:20:33Z

+                Ordering::Equal => continue,
+                other => return other,
+            }
+        }


major: No Zeroize/Drop on secret key types — signing material lingers in memory

SigningShare, KeyPackage, Round1Secret (and Round1Secret::coefficients) all hold secret scalar material (blst_fr = [u64; 4]) but implement neither Zeroize nor a custom Drop. When these values are dropped the heap/stack bytes are never overwritten, leaving long-term signing shares and DKG polynomial coefficients recoverable from memory dumps or swap for the lifetime of the process.

The workspace already depends on zeroize = "1.8.2". Add zeroize.workspace = true to [dependencies] in Cargo.toml, implement Zeroize for Scalar (zero the four u64 limbs), then derive ZeroizeOnDrop for SigningShare, KeyPackage, and Round1Secret (clearing coefficients: Vec<Scalar> on drop).

varex83agent · 2026-04-23T12:20:33Z

+    /// 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(


major: Round1Secret::from_raw skips all input validation

Unlike round1, from_raw never calls validate_num_of_signers or checks id. Consequences:

max_signers = 0: round2 computes (secret.max_signers - 1) as usize; in release builds the u16 subtraction wraps to 65535, making the package-count check demand 65535 entries and silently return Err(IncorrectPackageCount).

max_signers = 1 / threshold = 1: expected = 0, both length checks pass on empty maps, the peer-verification loop is skipped entirely, and the caller receives a KeyPackage with no proof-of-knowledge or Feldman VSS verification performed.

id = 0: surfaces as opaque DkgError::FrostCoreError(InvalidZeroScalar) deep in round2 instead of DkgError::InvalidParticipantId(0).

Fix: add the same guards as round1 at the top of from_raw:

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

Intended to be used for tests only, probably better to gate it behind a cfg.

It's better to have the #[cfg(test)]

Also add comment on the behavior

varex83agent · 2026-04-23T12:20:34Z

+    };
+
+    Ok((bcast, shares, secret))
+}


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.

varex83agent · 2026-04-23T12:20:34Z

+}
+
+/// RFC 9380 Section 5.3.1 using SHA-256
+pub fn expand_msg_xmd(msg: &[u8], dst: &[u8], len_in_bytes: usize) -> Vec<u8> {


major: expand_msg_xmd is pub with assert!() — panics on invalid caller input

Three assert!() calls will terminate the process if a downstream caller passes invalid parameters (ell > 255, len_in_bytes > 65535, dst.len() > 255). While current internal callers use hardcoded safe values, the function is part of the public API. A networked validator node integrating this crate could be crashed by triggering a code path that indirectly calls this with attacker-influenced lengths.

Options:

Change the signature to fn expand_msg_xmd(...) -> Result<Vec<u8>, ExpandMsgError> and replace assert! with if ... { return Err(...) }.

Make it pub(crate) since no external caller needs it directly (the Kryptology hash-to-scalar is what callers actually need).

Option 2 is simpler if no external party needs to call expand_msg_xmd directly.

I think this comment is legit

No external usage so pub(crate) sounds more reasonable.

varex83agent · 2026-04-23T12:20:34Z

+#[derive(Debug)]
+pub enum FrostCoreError {
+    /// Participant ID is zero.
+    InvalidZeroScalar,


major: FrostCoreError and DkgError do not implement std::error::Error

Both error types derive Debug manually but do not implement std::error::Error. This means they:

Cannot be used with the ? operator in downstream crates

Cannot be wrapped via #[from]

Cannot be boxed as Box<dyn std::error::Error>

The project uses thiserror consistently for library error types. Add thiserror.workspace = true to [dependencies] and:

#[derive(Debug, thiserror::Error)] pub enum FrostCoreError { #[error("participant ID is zero")] InvalidZeroScalar, // ... }

Also, FrostCoreError is an implementation detail — it should be pub(crate) and only DkgError should be part of the public API surface.

varex83agent · 2026-04-23T12:20:34Z

+        // Verify Feldman share
+        let share = received_shares
+            .get(&sender_id)
+            .ok_or(DkgError::IncorrectPackageCount)?;


minor: Misleading IncorrectPackageCount error when a share is missing for a specific sender

let share = received_shares .get(&sender_id) .ok_or(DkgError::IncorrectPackageCount)?;

IncorrectPackageCount is already used above for the wrong map length. When a sender is present in received_bcasts but absent from received_shares, the correct error is one that identifies the culprit. Use DkgError::InvalidShare { culprit: sender_id } here (or add a dedicated MissingShare { culprit } variant) so the error message attributes the failure to the correct participant.

varex83agent · 2026-04-23T12:20:34Z

+//! Go's Coinbase Kryptology FROST DKG, and BLS threshold signing (Ethereum 2.0
+//! compatible).
+
+#![allow(non_snake_case)]


minor: #![allow(non_snake_case)] at crate root is too broad

wi and ci are already valid snake_case identifiers (all lowercase). If this allow was added for mathematical variable names from the FROST paper (e.g., Ri, Wi) those usages should be individually annotated:

#[allow(non_snake_case)] fn kryptology_challenge(...) { let Ri = ...; }

A crate-level suppression will hide real naming violations in any code added to the crate in the future.

varex83agent · 2026-04-23T12:20:34Z

+    /// 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 {


nit: Unnecessary inner scope and reference alias in from_key_package — looks like an unfinished zeroization attempt:

let scalar = key_package.signing_share().to_scalar(); { let signing_share: &Scalar = &scalar; // redundant reborrow let h_msg = hash_to_g2(msg); BlsPartialSignature { identifier: id, point: p2_mult(&h_msg, signing_share) } }

The inner block drops signing_share but scalar remains live and unzeroized in the outer scope. Simplify:

let scalar = key_package.signing_share().to_scalar(); let h_msg = hash_to_g2(msg); BlsPartialSignature { identifier: id, point: p2_mult(&h_msg, &scalar) }

(A real zeroization should use Zeroizing<Scalar> once Scalar: Zeroize is implemented.)

varex83agent · 2026-04-23T12:20:34Z

+
+pub use curve::*;
+pub use frost_core::*;
+pub use rand_core;


nit: pub use rand_core re-exports the entire rand_core crate as stable public API, creating hard version coupling — any semver bump to rand_core in the workspace becomes a breaking change for downstream users of this crate. If the intent is to let callers use RngCore + CryptoRng without adding rand_core as a direct dep, re-export only the specific traits: pub use rand_core::{CryptoRng, RngCore};. Otherwise remove the pub.

varex83agent · 2026-04-23T12:20:34Z

+    pub fn verify(&self, verifying_key: &VerifyingKey, msg: &[u8]) -> bool {
+        let pk_affine = G1Affine::from(verifying_key.to_element());
+        let pk = blst::min_pk::PublicKey::from(pk_affine.0);
+


nit: blst_p1_mult and blst_p2_mult are called with nbits=255 — this is correct (the BLS12-381 scalar field order r has exactly 255 significant bits), but future readers will wonder why 255 and not 256. Add a brief comment:

// BLS12-381 scalar field order has 255 significant bits; blst skips the MSB. blst_p1_mult(&mut out, &self.0, scalar.b.as_ptr(), 255);

ai agent review

iamquang95 · 2026-04-24T09:27:49Z

+
+/// BLS12-381 scalar field element. Wrapper around `blst_fr` in Montgomery form.
+#[derive(Copy, Clone, Default, PartialEq, Eq)]
+pub struct Scalar(pub(crate) blst_fr);


Should impl Zeroize for Scalar

iamquang95 · 2026-04-24T09:28:24Z

+/// # Security
+///
+/// This MUST NOT be sent to other participants.
+pub struct Round1Secret {


should derive ZeroizeOnDrop

iamquang95 · 2026-04-24T09:29:08Z

+///
+/// See: https://github.com/ZcashFoundation/frost/blob/3ffc19d8f473d5bc4e07ed41bc884bdb42d6c29f/frost-core/src/keys.rs#L82-L87
+#[derive(Copy, Clone, Debug)]
+pub struct SigningShare(Scalar);


should derive ZeroizeOnDrop

iamquang95 · 2026-04-24T09:29:34Z

+/// 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)]


Should derive ZeroizeOnDrop

iamquang95 · 2026-04-24T09:29:50Z

+/// A Shamir secret share matching Go's `sharing.ShamirShare`.
+///
+/// The `value` field is in **big-endian** byte order.
+#[derive(Clone, Debug, PartialEq, Eq)]


Should derive ZeroizeOnDrop

iamquang95 · 2026-04-24T09:33:23Z

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


This seem stricter than golang implementation of round2 check, they are checking with threshold

https://github.com/coinbase/kryptology/blob/v1.8.0/pkg/dkg/frost/dkg_round2.go#L39-L49

iamquang95 · 2026-04-24T09:34:19Z

+        BTreeMap::new();
+    let mut share_sum = Scalar::ZERO;
+
+    for (&sender_id, bcast) in received_bcasts {


agree on this

iamquang95 · 2026-04-24T09:35:54Z

+    /// 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(


It's better to have the #[cfg(test)]

iamquang95 · 2026-04-24T09:36:11Z

+    /// 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(


Also add comment on the behavior

emlautarom1 added 20 commits April 14, 2026 12:33

Move code as-is from private repo

f5aa965

Rename dir

cb9e16a

Add crate

1e03f39

Fix crate toml

cbfb2ee

Fix imports and compiler issues

f116695

- Adjust `&` handling in patterns

Apply formatting

9b989ea

Apply clippy suggestions

5aa73eb

Adjust docs

c58b00a

Inline readme

0569fcd

Remove bench-specific configs

6810436

Reduce attribute usage

944a6bc

Use std imports

8de0c79

Add proper permalinks

bc49b22

Adjust test names

9b0b81b

Enable lints

eb7e70e

Use derive macros when possible

1832c1c

- Replaces custom deserialization code

Inline const

24fb183

Simplify fixture parsing

e83c631

- Use vec directly

Fix clippy lints

44c11a4

- Infallible conversions - Disable arithmetic checks

Assert conversions

3952fb1

- Use `assert!` over `debug_assert!` - Reference RFC when required

emlautarom1 requested review from frozenspider, iamquang95 and varex83 April 14, 2026 22:28

emlautarom1 and others added 2 commits April 15, 2026 11:01

Update lockfile

9830515

Merge branch 'main' into emlautarom1/frost_kryptology

627b8f7

mskrzypkows reviewed Apr 20, 2026

View reviewed changes

varex83agent previously requested changes Apr 23, 2026

View reviewed changes

iamquang95 reviewed Apr 24, 2026

View reviewed changes

Conversation

emlautarom1 commented Apr 14, 2026

Uh oh!

emlautarom1 commented Apr 14, 2026

Uh oh!

claude Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — feat(dkg): FROST implementation

🔴 High — Secret Material Not Zeroized

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

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

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

🟡 Medium — BlsPartialSignature::from_key_package — Mismatched id Risk

🟢 Low — expand_msg_xmd Panics Instead of Returning Errors

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

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

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

🟢 Low — Missing Test Coverage

✅ Positive Observations

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

varex83agent left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

claude Bot commented Apr 14, 2026 •

edited

Loading

Code Review — `feat(dkg): FROST implementation`

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

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

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

🟡 Medium — `BlsPartialSignature::from_key_package` — Mismatched `id` Risk

🟢 Low — `expand_msg_xmd` Panics Instead of Returning Errors

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

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