-
Notifications
You must be signed in to change notification settings - Fork 87
Add VRF support and update documentation #719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis PR adds Verifiable Random Function (VRF) support to pallas-crypto by integrating the cardano-crypto library. A new vrf module is exposed (gated by feature flag) with helpers for draft-03 and draft-13 operations, alongside comprehensive golden tests and documentation updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces Verifiable Random Function (VRF) support to the pallas-crypto library, enabling users to generate proofs and verify outputs using both draft-03 (the Cardano standard) and draft-13 VRF variants. The implementation wraps the cardano-crypto crate's VRF primitives and provides a clean API for downstream consumers.
Key Changes:
- Adds a new
vrfmodule with support for both draft-03 and draft-13 VRF variants - Introduces comprehensive test coverage including IETF test vectors and Cardano-specific message tests
- Fixes proptest edge cases in KES tests by adding proper assumptions
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pallas/Cargo.toml | Adds vrf feature flag to expose VRF functionality at the top-level crate |
| pallas-crypto/Cargo.toml | Adds cardano-crypto dependency with VRF feature and defines vrf feature flag |
| pallas-crypto/src/lib.rs | Conditionally exports the new vrf module when feature flag is enabled |
| pallas-crypto/src/vrf/mod.rs | New module providing VRF API with functions for keypair generation, proving, and verification for both draft variants |
| pallas-crypto/tests/vrf_golden.rs | Comprehensive golden tests validating VRF implementation against IETF test vectors and Cardano use cases |
| pallas-crypto/tests/kes_golden.rs | New golden tests for KES Sum6 algorithm covering key generation, signing, verification, and determinism |
| pallas-crypto/src/kes/summed_kes_tests.rs | Fixes proptest edge cases by adding assumptions to prevent false failures when randomly generated messages are equal |
| pallas-crypto/proptest-regressions/kes/summed_kes_tests.txt | Proptest regression file capturing previously failing test cases |
| pallas-crypto/README.md | Updates VRF documentation to clarify that both draft-03 and draft-13 are supported behind the vrf feature |
| pallas-crypto/src/kes/README.md | Documents that VRF support now lives alongside KES and mentions Cardano's use of KES Sum6 and VRF draft-03 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let mut seed1 = seed.clone(); | ||
| let (sk1, vk1) = Sum6Kes::keygen(&mut key_bytes1, &mut seed1); | ||
|
|
||
| let mut key_bytes2 = [0u8; Sum6Kes::SIZE + 4]; | ||
| let mut seed2 = seed.clone(); |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary clone of a Copy type. Arrays implement Copy, so cloning is not needed. Simply assign the value directly.
| let mut seed1 = seed.clone(); | |
| let (sk1, vk1) = Sum6Kes::keygen(&mut key_bytes1, &mut seed1); | |
| let mut key_bytes2 = [0u8; Sum6Kes::SIZE + 4]; | |
| let mut seed2 = seed.clone(); | |
| let mut seed1 = seed; | |
| let (sk1, vk1) = Sum6Kes::keygen(&mut key_bytes1, &mut seed1); | |
| let mut key_bytes2 = [0u8; Sum6Kes::SIZE + 4]; | |
| let mut seed2 = seed; |
| /// Produce a draft-13 VRF proof and output for the given message. | ||
| pub fn prove_draft13( | ||
| sk: &VrfSigningKey, | ||
| message: &[u8], | ||
| ) -> CryptoResult<([u8; cardano_crypto::vrf::draft13::PROOF_SIZE], Output)> { | ||
| let proof = VrfDraft13::prove(sk, message)?; | ||
| let output = VrfDraft13::proof_to_hash(&proof)?; | ||
| Ok((proof, Output::new(output))) | ||
| } |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent return types between draft-03 and draft-13 prove functions. The prove_draft03 function returns a VrfProof, while prove_draft13 returns a raw array [u8; PROOF_SIZE]. Consider making these consistent by either having both return typed wrappers (like VrfProof) or both return raw arrays for consistency in the API.
| /// Error type alias for VRF operations. | ||
| pub type Error = CryptoError; | ||
| /// Result type alias for VRF operations. | ||
| pub type Result<T> = CryptoResult<T>; |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type aliases should be declared near the top of the module, not at the end. According to Rust conventions, type aliases should appear early in the module, typically after imports and before function definitions, to improve readability and discoverability.
| /// Convenience wrapper that runs draft-03 end-to-end and returns a certified output. | ||
| pub fn certify_draft03(sk: &VrfSigningKey, message: &[u8]) -> CryptoResult<CertifiedOutput> { | ||
| CertifiedVrf::eval(sk, message) | ||
| } | ||
|
|
||
| /// Verify a certified draft-03 output. | ||
| pub fn verify_certified( | ||
| vk: &VrfVerificationKey, | ||
| certified: &CertifiedOutput, | ||
| message: &[u8], | ||
| ) -> CryptoResult<()> { | ||
| certified.verify(vk, message) | ||
| } |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for convenience wrapper functions. The module includes certify_draft03 and verify_certified functions that lack test coverage. Consider adding tests for these functions to ensure they work correctly, especially since they're part of the public API.
| /// Error type alias for VRF operations. | ||
| pub type Error = CryptoError; | ||
| /// Result type alias for VRF operations. |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type aliases lack documentation comments. The Error and Result type aliases should include doc comments explaining that they're aliases for CryptoError and CryptoResult from the cardano-crypto crate, to help users understand the error types they'll encounter when using this API.
| /// Error type alias for VRF operations. | |
| pub type Error = CryptoError; | |
| /// Result type alias for VRF operations. | |
| /// Error type alias for VRF operations. | |
| /// | |
| /// This is an alias for [`CryptoError`] from the [`cardano-crypto`] crate, | |
| /// representing errors that can occur during VRF operations in this API. | |
| pub type Error = CryptoError; | |
| /// Result type alias for VRF operations. | |
| /// | |
| /// This is an alias for [`CryptoResult`] from the [`cardano-crypto`] crate, | |
| /// representing the result of VRF operations in this API. |
| let (sk1, vk1) = Sum6Kes::keygen(&mut key_bytes1, &mut seed1); | ||
|
|
||
| let mut key_bytes2 = [0u8; Sum6Kes::SIZE + 4]; | ||
| let mut seed2 = seed.clone(); |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary clone of a Copy type. Arrays implement Copy, so cloning is not needed. Simply assign the value directly.
| let mut seed2 = seed.clone(); | |
| let mut seed2 = seed; |
| /// Generate a draft-03 keypair from a 32-byte seed. | ||
| pub fn keypair_from_seed(seed: &[u8; SEED_SIZE]) -> VrfKeyPair { | ||
| VrfKeyPair::generate(seed) | ||
| } |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name 'keypair_from_seed' is specific to draft-03. Consider renaming to 'keypair_from_seed_draft03' or documenting that it specifically generates draft-03 keypairs, especially since this module supports both draft-03 and draft-13 variants.
| SECRET_KEY_SIZE, SEED_SIZE, | ||
| }; | ||
|
|
||
| /// Generate a draft-03 keypair from a 32-byte seed. |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing documentation about draft variant compatibility. Since the module supports both draft-03 and draft-13, it would be helpful to document that keypairs generated by this function can be used with both variants, or if they're specific to draft-03 only.
| /// Generate a draft-03 keypair from a 32-byte seed. | |
| /// Generate a VRF keypair from a 32-byte seed. | |
| /// | |
| /// The generated keypair is compatible with both draft-03 (Cardano standard) and draft-13 VRF variants. |
| rand_core = "0.9" | ||
| serde = { version = "1.0.143", features = ["derive"] } | ||
| pallas-codec = { version = "=1.0.0-alpha.3", path = "../pallas-codec" } | ||
| cardano-crypto = { version = "=1.0.6", optional = true, features = ["vrf"] } |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using exact version match for dependency. The cardano-crypto dependency uses '=1.0.6' which requires an exact version match. While this ensures reproducibility, it may cause issues when other crates in the dependency tree need a different patch version of the same major.minor version. Consider using '^1.0.6' (caret requirements) or '~1.0.6' (tilde requirements) to allow compatible patch updates while maintaining stability.
| cardano-crypto = { version = "=1.0.6", optional = true, features = ["vrf"] } | |
| cardano-crypto = { version = "^1.0.6", optional = true, features = ["vrf"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
pallas-crypto/src/kes/summed_kes_tests.rs (2)
49-51: Precondition is logically sound.Adding
prop_assume!(msg1 != msg2)is appropriate for this test, as it validates that different messages produce different signatures. The blank line at Line 51 seems unnecessary but is a minor formatting preference.
74-76: Precondition is logically sound.Adding
prop_assume!(msg1 != msg2)correctly ensures the test validates signature verification failure for a different message. The blank line at Line 76 is unnecessary but minor.pallas-crypto/tests/kes_golden.rs (1)
50-68: LGTM! Validates deterministic key generation.This test correctly validates that identical seeds produce identical keypairs and signatures, which is crucial for deterministic key derivation. Note that
.clone()on lines 55 and 59 is unnecessary since[u8; 32]implementsCopy, but this is harmless.pallas-crypto/src/vrf/mod.rs (2)
54-72: Consider using the re-exported constant for consistency.The draft-13 functions use the full path
cardano_crypto::vrf::draft13::PROOF_SIZEinstead of the re-exportedDRAFT13_PROOF_SIZE(line 14). While functionally equivalent, using the re-exported constant would be more consistent with the module's design pattern.Apply this diff to improve consistency:
pub fn prove_draft13( sk: &VrfSigningKey, message: &[u8], -) -> CryptoResult<([u8; cardano_crypto::vrf::draft13::PROOF_SIZE], Output)> { +) -> CryptoResult<([u8; DRAFT13_PROOF_SIZE], Output)> { let proof = VrfDraft13::prove(sk, message)?; let output = VrfDraft13::proof_to_hash(&proof)?; Ok((proof, Output::new(output))) } /// Verify a draft-13 VRF proof and return the output if valid. pub fn verify_draft13( vk: &VrfVerificationKey, - proof: &[u8; cardano_crypto::vrf::draft13::PROOF_SIZE], + proof: &[u8; DRAFT13_PROOF_SIZE], message: &[u8], ) -> CryptoResult<Output> { let output = VrfDraft13::verify(vk, proof, message)?; Ok(Output::new(output)) }
98-101: Consider relocating type aliases before the test module.While functionally correct, placing type aliases after the test module is unconventional. Moving them closer to the imports or before the test module would improve code organization.
Consider moving these aliases to after line 16 (after the re-exports):
pub use cardano_crypto::vrf::{ // ... existing re-exports }; /// Error type alias for VRF operations. pub type Error = CryptoError; /// Result type alias for VRF operations. pub type Result<T> = CryptoResult<T>; /// Generate a draft-03 keypair from a 32-byte seed. pub fn keypair_from_seed(seed: &[u8; SEED_SIZE]) -> VrfKeyPair { // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
pallas-crypto/Cargo.toml(2 hunks)pallas-crypto/README.md(1 hunks)pallas-crypto/proptest-regressions/kes/summed_kes_tests.txt(1 hunks)pallas-crypto/src/kes/README.md(1 hunks)pallas-crypto/src/kes/summed_kes_tests.rs(2 hunks)pallas-crypto/src/lib.rs(1 hunks)pallas-crypto/src/vrf/mod.rs(1 hunks)pallas-crypto/tests/kes_golden.rs(1 hunks)pallas-crypto/tests/vrf_golden.rs(1 hunks)pallas/Cargo.toml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pallas-crypto/tests/vrf_golden.rs (2)
pallas-crypto/src/vrf/mod.rs (3)
keypair_from_seed(19-21)verify_draft03(31-38)prove_draft03(24-28)examples/wallet/src/lib.rs (1)
len(49-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (18)
pallas-crypto/README.md (1)
11-11: LGTM! Clear documentation of VRF support.The update accurately describes the VRF support (draft-03 and draft-13) and correctly indicates it's behind the
vrffeature flag, aligning with the implementation.pallas-crypto/src/kes/README.md (1)
14-15: LGTM! Helpful context for users.The documentation clearly indicates VRF support is available under the
vrffeature and provides useful context that Cardano currently uses KES Sum6 and VRF draft-03.pallas-crypto/Cargo.toml (1)
46-46: LGTM! Feature flag is properly defined.The
vrffeature correctly enables thecardano-crypto/vrfdependency, following the same pattern as the existingkesfeature.pallas/Cargo.toml (1)
33-33: LGTM! Clean feature passthrough.The
vrffeature properly maps topallas-crypto/vrf, allowing users to enable VRF support at the workspace level.pallas-crypto/proptest-regressions/kes/summed_kes_tests.txt (1)
1-7: LGTM! Standard proptest regression artifact.This is a properly formatted proptest regression file that captures previously generated failure cases for deterministic re-testing. Including it in source control is the recommended practice.
pallas-crypto/src/lib.rs (1)
8-9: LGTM! Proper feature-gated module export.The VRF module is correctly exposed behind the
vrffeature flag using standard Rust conditional compilation, consistent with the crate's architecture.pallas-crypto/tests/kes_golden.rs (2)
6-29: LGTM! Comprehensive period evolution test.This test thoroughly validates KES key evolution across all 64 periods (0-63), including signing, verification at correct periods, and proper failure for adjacent periods. The test coverage is excellent.
31-48: LGTM! Important verification key stability test.This test validates a critical property: that the verification key remains constant across key evolution periods, which is essential for KES correctness. The test is well-structured and clearly documents the expected behavior.
pallas-crypto/tests/vrf_golden.rs (5)
1-10: LGTM! Test setup and helper function are appropriate.The feature gate, imports, and
hex_decodehelper are correctly implemented. The use ofunwrap()in the helper is acceptable for test code where input validity is guaranteed.
12-41: LGTM! Thorough validation against IETF reference vectors.The test correctly validates keypair derivation, proof generation, verification, and the important invariant that
proof_to_hashmatches the verification output.
43-70: LGTM! Good addition of size constant validation.This test extends coverage to non-empty messages and includes explicit validation of proof and output sizes against the module's constants, which strengthens the test suite.
72-98: LGTM! Comprehensive IETF vector coverage.The third reference vector test provides additional validation with a different message length, completing the golden test suite for draft-03.
100-117: LGTM! Excellent coverage of Cardano-specific scenarios.This test validates the public wrapper API with realistic Cardano use cases and important edge cases (empty messages, zero-filled buffers), ensuring the wrappers correctly round-trip for varied inputs.
pallas-crypto/src/vrf/mod.rs (5)
1-16: LGTM! Clear documentation and comprehensive re-exports.The module documentation clearly states its purpose, and all necessary VRF types and constants are properly re-exported for downstream use.
18-28: LGTM! Well-designed wrapper functions.The functions provide a clean API that combines proof generation with output computation, which is the expected pattern for VRF usage.
30-52: LGTM! Consistent verification API.The verification functions complement the proving functions well, and the certified output helpers provide a convenient end-to-end workflow for users who don't need separate proof/output handling.
74-96: LGTM! Effective round-trip tests.The internal tests provide good smoke tests for both draft-03 and draft-13 variants, validating the fundamental prove/verify consistency.
7-16: The cardano-crypto 1.0.6 dependency is a published and available version with no known security advisories as of December 2025. The cryptographic library concern is valid, but no security issues were identified for this version.
| rand_core = "0.9" | ||
| serde = { version = "1.0.143", features = ["derive"] } | ||
| pallas-codec = { version = "=1.0.0-alpha.3", path = "../pallas-codec" } | ||
| cardano-crypto = { version = "=1.0.6", optional = true, features = ["vrf"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check cardano-crypto versions on crates.io
curl -s https://crates.io/api/v1/crates/cardano-crypto | jq '.versions[] | select(.num | test("1\\.0")) | {num, created_at, yanked}' | head -50Repository: txpipe/pallas
Length of output: 665
🏁 Script executed:
#!/bin/bash
# Get summary info about cardano-crypto crate
curl -s https://crates.io/api/v1/crates/cardano-crypto | jq '.crate | {name, max_version, documentation}'Repository: txpipe/pallas
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Search for any security advisories or issues with cardano-crypto
curl -s https://rustsec.org/advisory.json | jq '.[] | select(.crate.name == "cardano-crypto") | {id, crate, title, date}'Repository: txpipe/pallas
Length of output: 114
🌐 Web query:
cardano-crypto 1.0.6 security vulnerability advisory
💡 Result:
I couldn't find any published security advisory or CVE specifically for cardano-crypto 1.0.6. The package pages I checked (Hackage and docs.rs) show the cardano-crypto project but contain no vulnerability advisory for 1.0.6. [1][2]
If you want, I can:
- run deeper CVE/NVD and GitHub issue/commit searches for that exact version, or
- monitor vendor channels (Cardano release notes, Hackage package page, GitHub) and alert you if an advisory appears.
Sources:
- Hackage — cardano-crypto package page. [1]
- docs.rs — cardano-crypto crate pages (examples: 1.0.1, 1.0.5). [2]
Review the rationale for exact version pinning on cardano-crypto.
Version 1.0.6 exists, is the latest available, and has no known security advisories. However, the exact version constraint =1.0.6 will prevent adopting future patch releases. For a cryptographic library, confirm that this strict pinning is intentional and document the reason—whether for reproducibility, compatibility, or other constraints. If intentional, consider documenting it; otherwise, consider using ^1.0.6 to allow patch-level updates.
🤖 Prompt for AI Agents
In pallas-crypto/Cargo.toml around line 23 the dependency is pinned exact to
cardano-crypto = "=1.0.6", which prevents automatic adoption of future patch
fixes; either (A) if strict pinning is intentional for reproducibility or
compatibility, add a clear comment immediately above that line documenting the
rationale (e.g., ABI/behavior guarantees, tested release, or security/regulatory
requirement), or (B) if strict pinning is not required, relax the constraint to
allow patch updates by changing the requirement to ^1.0.6 (or use "1.0.6"
without =) so cargo will accept compatible patch releases; after changing, run
cargo update and test to ensure no regressions.
Introduce support for Verifiable Random Functions (VRF) alongside KES, including necessary documentation updates. This enhancement allows users to generate proofs and verify outputs for both draft-03 and draft-13 variants of VRF.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.