[kwsantiago]

#1064 routed FROST nonce commitments through an RNG that is checked at the point of the draw. The draws behind the long-lived secrets were still unchecked: DKG round one, the trusted dealer, the ed25519 dealer, and share refresh. Those draws produce the signing shares and the group key, so a degraded source there is worse than a degraded nonce: a nonce leak exposes one share, a predictable dealer polynomial exposes the key the shares were derived from.

Five call sites now draw through CheckedRng, and the module moves from frost/nonce.rs to frost/checked_rng.rs since it is no longer specific to nonces.

What the criteria now apply to

Each 32-byte block is compared against every block already judged in the same call, not only against the block before it. Retaining one block makes the comparisons adjacent-only, which a source alternating two far-apart values passes: a threshold-3 polynomial draws A, B, A, both adjacent pairs look healthy, and the group key is accepted from a source holding one bit of entropy. Comparing against all of them catches the repeat, and a third block additionally allows the one criterion a pair cannot carry, a byte position the source never moves. Across two draws that position holds the same extreme value about once in a thousand tries, which would refuse healthy draws at that rate, so the pairwise criterion drops it and this one restores it.

Fills are accumulated into whole blocks rather than judged per fill. curve25519-dalek's Scalar::random fills 64 bytes in one call where k256 fills 32 in a rejection loop, so a rule written for a single width judges nothing at all on the other curve. Accumulating also means a remainder joins the next fill instead of being dropped.

A call that judged no whole block, or that ends with bytes left over, is refused as Unavailable rather than passed. Those bytes became a secret and no criterion examined them, and certifying them with blocks drawn afterwards would read as assurance over material nothing looked at. Unavailable rather than Degraded because this is the absence of a verdict, not a bad one: the caller refuses the draw without concluding the machine is broken, which is what Degraded latches a co-signer for.

Calls that make a single draw

A share refresh at a threshold of two draws one coefficient, and that coefficient is the refresh delta, so leaving it unjudged means a refresh that heals nothing while reporting success. When a call has judged fewer blocks than the criteria need, the checked source draws from the same RNG until it has them. That costs at most two extra 32-byte draws, needs no process-wide state, and catches a stuck source on the first affected call rather than the second.

Worth being exact about the reach: at such a site the only comparison is the secret block against bytes drawn after it. That catches a source stuck, cycling or barely moving, because such a source is still that way when probed. It says nothing about a source that emits one chosen value and then healthy bytes, and no check at the draw can.

Supporting changes

• refresh_shares refuses a threshold below two. The value comes from stored share metadata, which carries no floor of its own, and is handed to frost-core as min_signers; frost-core validates before it draws, so the refusal arrived from inside the draw rather than from keep-core's own guard.
• clippy.toml disallows the raw draw functions in both ciphersuite spellings, including the refresh and repair entries that take an RNG but have no caller here yet. Each sanctioned site marks the lint expect rather than allow: an entry that stops resolving after a dependency bump would otherwise go quiet, where an unfulfilled expectation is a warning and CI runs with -D warnings.
• CI's clippy covers --workspace --all-targets with the features named. Test code was previously unlinted, and whether the ed25519 module was linted at all depended on workspace feature unification.
• CI now runs cargo test -p keep-core --features ed25519 --lib. No job enabled that feature, so the ed25519 tests were type-checked and never executed, for the one ciphersuite whose fill width the block rule exists for.

Coverage: stuck, creeping, all-zero, alternating and frozen-position sources; a source that looks healthy between fills but repeats within one, which isolates the width rule; a narrow draw and a ragged draw; a closure that never draws, which is an ordinary validation error and not a fault; group key generation on both curves; and the probe accounted for by byte count.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6a4d2518-cf51-4d09-acaf-d4d44ac57556

📥 Commits

Reviewing files that changed from the base of the PR and between 9a293a5 and 91b1b43.

📒 Files selected for processing: 16

• .github/workflows/ci.yml
• clippy.toml
• keep-core/src/entropy.rs
• keep-core/src/frost/bip32_signing.rs
• keep-core/src/frost/checked_rng.rs
• keep-core/src/frost/coordinator.rs
• keep-core/src/frost/dealer.rs
• keep-core/src/frost/dkg.rs
• keep-core/src/frost/ed25519/dealer.rs
• keep-core/src/frost/ed25519/signer.rs
• keep-core/src/frost/mod.rs
• keep-core/src/frost/nonce.rs
• keep-core/src/frost/refresh.rs
• keep-core/src/frost/signing.rs
• keep-core/src/frost/taproot.rs
• keep-frost-net/src/nonce_pool.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}