[kwsantiago]

What

A checked draw now hashes every fill it hands out, after the health criteria have judged the raw bytes, together with a process-wide fill counter, the process id and the clock. An RNG health failure also becomes its own KeepError::Entropy variant instead of being flattened into KeepError::Encryption(String).

Why

CheckedRng builds its judge per call, so a source that replays the same sequence from one call to the next (A, B in one commitment, A, B again in the next) passes every comparison inside each call. frost-core derives a nonce as H3(random_bytes || signing_share), so that source committed to the same nonce pair in every session, and two signatures over different messages give up the share. Comparing across calls would mean keeping drawn bytes for the life of the process, which was ruled out earlier. Hashing in a counter that never repeats within a process closes the case without keeping anything drawn.

The judge still sees the source's own bytes. Hashing first would hide a stuck source, since its hash looks healthy to every criterion. A mutation check confirms the existing stuck, cycling and partial-fill tests fail if the order is reversed.

Scope and limits

• Within one process, no two fills hash the same input, whatever the source does. Across process starts (each run of a one-shot command, a restart, a snapshot restore) only the pid and the clock tell fills apart, which is best effort. The docs say so.
• This does not add entropy. A source that is weak but never repeats is outside what any check at the draw can see.
• The file is also compiled into the enclave, so the enclave's nonce draws get the same separation.
• No process-wide sticky latch in keep-core on a refused draw. Callers that hold signing state already latch, and the new error variant lets others branch on Degraded without matching a message.

Tests

• New: a replaying source across two calls never repeats a nonce (secp256k1 and Ed25519); every path out of the judged source is separated (next_u32, next_u64, try_fill_bytes); identical fills are separated at several widths.
• Mutation: removing the separation fails the replay tests; separating before judging fails 12 existing refusal tests.
• cargo test -p keep-core --features ed25519, the enclave suite, keep-nip46 and keep-frost-net, and workspace clippy with the CI features all pass.

Summary by CodeRabbit

• Bug Fixes
  • Entropy health failures now retain their specific error details in core processing, improving error reporting in mobile apps.
  • Client-facing cryptographic errors continue to use a generic message to avoid exposing sensitive details.
  • Repeated random-number fills within a process now produce separated outputs, including when the source replays the same bytes.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4744da45-2dd9-42f0-8da4-30514863039c

Commits

Reviewing files that changed from the base of the PR and between 5476b26 and 81e2dd3.

Files selected for processing: 7

• keep-core/Cargo.toml
• keep-core/src/crypto.rs
• keep-core/src/error.rs
• keep-core/src/frost/checked_rng.rs
• keep-frost-net/src/node/signing.rs
• keep-mobile/src/lib.rs
• keep-nip46/src/error.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

CheckedRng now separates judged entropy fills, with tests for replayed sources and multiple fill methods. KeepError preserves entropy health errors as a distinct variant, and mobile and NIP-46 error mappings now handle that variant.

Changes

Checked RNG fill separation

Layer / File(s)	Summary
Separate checked fills keep-core/Cargo.toml, keep-core/src/frost/checked_rng.rs	CheckedRng judges source bytes before separating fills with per-fill process data and hashing. Tests cover replayed sources, several fill widths, and multiple fill methods. The sha2 dependency enables zeroize.

Entropy error preservation

Layer / File(s)	Summary
Preserve entropy health errors keep-core/src/error.rs, keep-core/src/crypto.rs	KeepError adds a transparent Entropy variant and removes the conversion to Encryption. Tests check that the error kind and display text are preserved.
Map entropy errors in consumers keep-mobile/src/lib.rs, keep-nip46/src/error.rs, keep-frost-net/src/node/signing.rs	The mobile adapter maps entropy errors to EncryptionError with the error text. NIP-46 maps them to "Cryptographic operation failed". The signing comment identifies the entropy refusal path.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant EntropySource
  participant CheckedRng
  participant RngConsumer
  RngConsumer->>CheckedRng: request fill
  CheckedRng->>EntropySource: draw bytes
  CheckedRng->>CheckedRng: judge original bytes
  CheckedRng->>CheckedRng: separate destination bytes
  CheckedRng-->>RngConsumer: return separated bytes

Loading

Suggested reviewers: wksantiago

Merge Risk: Minimal · up to 81e2d

The change keeps a replayed RNG sequence from producing repeated nonces within a process and keeps RNG health failures distinguishable in keep-core. No actionable merge-blocking issue remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 6 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: preventing a replayed RNG sequence from producing a repeated FROST nonce.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 6 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• 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

_{A rabbit checks each byte with care,
Then hashes fills that travel there.
Old streams return, but outputs part,
While errors keep their proper heart.
I nibble greens and hop away.}

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