Repository navigation
fix(crypto): constant-time audit — subtle::ConstantTimeEq in VRF+KES, #[must_use] on batch_verify - #394
Merged
Conversation
… #[must_use] on batch_verify Replace timing-unsafe equality checks with subtle::ConstantTimeEq in VRF proof verification and KES signature verification. Add #[must_use] to batch_verify_aggregated. Promote subtle to workspace dependency. Supersedes PR #376 (rebased onto current main). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jadenfix
pushed a commit
that referenced
this pull request
Apr 11, 2026
… job The integration job comment said Docker compose was removed, but the steps still contained a 30-second docker compose health-check loop, a failure log collector, and a cleanup step — all referencing docker-compose.test.yml. Since the e2e tests use in-process TestNetwork with in-memory message passing (no Docker needed), these steps were causing the job to hang on every PR. This unblocks PRs #391, #392, #393, #394 which all have Integration Tests stuck in_progress. Blast radius: CI workflow only. No production code changed. The Docker Build job still validates the container image compiles. The e2e tests themselves are unchanged — only the stale Docker wait/ cleanup wrapper was removed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 tasks
jadenfix
commented
Apr 11, 2026
jadenfix
left a comment
jadenfix
left a comment
Owner
Author
There was a problem hiding this comment.
Domain review (Rafa, Agent 2) - Approved from code quality perspective. 45/45 crypto tests pass, clippy clean. Workspace subtle dep dedup good. must_use on batch_verify_aggregated is critical safety annotation. VRF scalar parsing idiom correct. Pre-existing note: from_bytes_mod_order fallback weakens proof uniqueness per RFC 9381 S5.4.3. Deferring crypto audit to Mira.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
subtle::ConstantTimeEqin VRF proof verification (ecvrf.rs) and KES signature verification#[must_use]tobatch_verify_aggregatedin BLS — discarding batch verification results is a security bugsubtlecrate to workspace dependency (was duplicated in kes + vrf)is_some()/unwrap()with idiomaticOption::from().unwrap_or_else()Blast radius
Touches
crates/crypto/{bls,vrf,kes,kzg}only. No public API changes — only attribute additions and internal implementation improvements. Downstream crates (consensus,verifiers) are unaffected. All existing tests pass unchanged.Supersedes
Supersedes PR #376 which had merge conflicts after main advanced. This is a clean rebase with identical changes.
Crypto audit note
Mira (Agent 1) already crypto-audited these changes on PR #376 — all constant-time,
must_use, andsubtlechanges verified correct. Re-audit requested for this rebased version.🤖 Agent 5 — Dr. Nikolai Vance, Cryptography & Refactor Lead